[PATCH 1/7] bcrypt: Introduce export_gnutls_datum() helper and use it in export_gnutls_pubkey_rsa().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
    Gnutls datum size might appear shorter than expected due to higher bits being zero
    in the random numbers. Gnutls export functions don't usually pad the data when exporting
    keys. bcrypt does not expect that currently. This is reproducible as random failures
    in some of the existing tests. Here is the testing patch which reliably reproduces
    this issue: https://gist.github.com/gofman/b15f91e2fb5246d13edcaf30eb6196f1.

dlls/bcrypt/gnutls.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/dlls/bcrypt/gnutls.c b/dlls/bcrypt/gnutls.c
index c065ac31fba..9e6dd4cc9a6 100644
--- a/dlls/bcrypt/gnutls.c
+++ b/dlls/bcrypt/gnutls.c
@@ -28,6 +28,7 @@
 #ifdef HAVE_GNUTLS_CIPHER_INIT

#include <stdarg.h>
+#include <assert.h>
 #include <gnutls/gnutls.h>
 #include <gnutls/crypto.h>
 #include <gnutls/abstract.h>
@@ -665,11 +666,27 @@ static void CDECL key_symmetric_destroy( struct key *key )
     if (key_data(key)->cipher) pgnutls_cipher_deinit( key_data(key)->cipher );
 }

+static void export_gnutls_datum( UCHAR *buffer, ULONG length, gnutls_datum_t *d, ULONG *actual_length )
+{
+    ULONG size = d->size;
+    UCHAR *src = d->data;
+
+    assert( size <= length + 1 );
+    if (size == length + 1)
+    {
+        assert(!src[0]);
+        ++src;
+        --size;
+    }
+    *actual_length = size;
+    memcpy( buffer, src, size );
+}
+
 static NTSTATUS export_gnutls_pubkey_rsa( gnutls_privkey_t gnutls_key, ULONG bitlen, UCHAR **pubkey, ULONG *pubkey_len )
 {
     BCRYPT_RSAKEY_BLOB *rsa_blob;
     gnutls_datum_t m, e;
-    UCHAR *dst, *src;
+    UCHAR *dst;
     int ret;

if ((ret = pgnutls_privkey_export_rsa_raw( gnutls_key, &m, &e, NULL, NULL, NULL, NULL, NULL, NULL )))
@@ -686,32 +703,18 @@ static NTSTATUS export_gnutls_pubkey_rsa( gnutls_privkey_t gnutls_key, ULONG bit
     }

dst = (UCHAR *)(rsa_blob + 1);
-    if (e.size == bitlen / 8 + 1 && !e.data[0])
-    {
-        src = e.data + 1;
-        e.size--;
-    }
-    else src = e.data;
-    memcpy( dst, src, e.size );
+    export_gnutls_datum( dst, bitlen / 8, &e, &rsa_blob->cbPublicExp );

-    dst += e.size;
-    if (m.size == bitlen / 8 + 1 && !m.data[0])
-    {
-        src = m.data + 1;
-        m.size--;
-    }
-    else src = m.data;
-    memcpy( dst, src, m.size );
+    dst += rsa_blob->cbPublicExp;
+    export_gnutls_datum( dst, bitlen / 8, &m, &rsa_blob->cbModulus );

rsa_blob->Magic       = BCRYPT_RSAPUBLIC_MAGIC;
     rsa_blob->BitLength   = bitlen;
-    rsa_blob->cbPublicExp = e.size;
-    rsa_blob->cbModulus   = m.size;
     rsa_blob->cbPrime1    = 0;
     rsa_blob->cbPrime2    = 0;

*pubkey = (UCHAR *)rsa_blob;
-    *pubkey_len = sizeof(*rsa_blob) + e.size + m.size;
+    *pubkey_len = sizeof(*rsa_blob) + rsa_blob->cbPublicExp + rsa_blob->cbModulus;

free( e.data ); free( m.data );
     return STATUS_SUCCESS;

[PATCH 2/7] bcrypt: Pad exported datums in export_gnutls_pubkey_ecc().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
 dlls/bcrypt/gnutls.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/dlls/bcrypt/gnutls.c b/dlls/bcrypt/gnutls.c
index 9e6dd4cc9a6..86a8fed0bab 100644
--- a/dlls/bcrypt/gnutls.c
+++ b/dlls/bcrypt/gnutls.c
@@ -670,6 +670,7 @@ static void export_gnutls_datum( UCHAR *buffer, ULONG length, gnutls_datum_t *d,
 {
     ULONG size = d->size;
     UCHAR *src = d->data;
+    ULONG offset;

assert( size <= length + 1 );
     if (size == length + 1)
@@ -678,8 +679,17 @@ static void export_gnutls_datum( UCHAR *buffer, ULONG length, gnutls_datum_t *d,
         ++src;
         --size;
     }
-    *actual_length = size;
-    memcpy( buffer, src, size );
+    if (actual_length)
+    {
+        offset = 0;
+        *actual_length = size;
+    }
+    else
+    {
+        offset = length - size;
+        memset( buffer, 0, offset );
+    }
+    memcpy( buffer + offset, src, size );
 }

static NTSTATUS export_gnutls_pubkey_rsa( gnutls_privkey_t gnutls_key, ULONG bitlen, UCHAR **pubkey, ULONG *pubkey_len )
@@ -727,7 +737,7 @@ static NTSTATUS export_gnutls_pubkey_ecc( gnutls_privkey_t gnutls_key, enum alg_
     gnutls_ecc_curve_t curve;
     gnutls_datum_t x, y;
     DWORD magic, size;
-    UCHAR *src, *dst;
+    UCHAR *dst;
     int ret;

switch (alg_id)
@@ -758,7 +768,7 @@ static NTSTATUS export_gnutls_pubkey_ecc( gnutls_privkey_t gnutls_key, enum alg_
         return STATUS_NOT_IMPLEMENTED;
     }

-    if (!(ecc_blob = RtlAllocateHeap( GetProcessHeap(), 0, sizeof(*ecc_blob) + x.size + y.size )))
+    if (!(ecc_blob = RtlAllocateHeap( GetProcessHeap(), 0, sizeof(*ecc_blob) + size * 2 )))
     {
         pgnutls_perror( ret );
         free( x.data ); free( y.data );
@@ -769,14 +779,10 @@ static NTSTATUS export_gnutls_pubkey_ecc( gnutls_privkey_t gnutls_key, enum alg_
     ecc_blob->cbKey   = size;

dst = (UCHAR *)(ecc_blob + 1);
-    if (x.size == size + 1) src = x.data + 1;
-    else src = x.data;
-    memcpy( dst, src, size );
+    export_gnutls_datum( dst, size, &x, NULL );

dst += size;
-    if (y.size == size + 1) src = y.data + 1;
-    else src = y.data;
-    memcpy( dst, src, size );
+    export_gnutls_datum( dst, size, &y, NULL );

*pubkey = (UCHAR *)ecc_blob;
     *pubkey_len = sizeof(*ecc_blob) + ecc_blob->cbKey * 2;

[PATCH 3/7] bcrypt: Pad exported datums in export_gnutls_pubkey_dsa().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
 dlls/bcrypt/gnutls.c | 42 +++++++++---------------------------------
 1 file changed, 9 insertions(+), 33 deletions(-)

diff --git a/dlls/bcrypt/gnutls.c b/dlls/bcrypt/gnutls.c
index 86a8fed0bab..abbbb1ee89f 100644
--- a/dlls/bcrypt/gnutls.c
+++ b/dlls/bcrypt/gnutls.c
@@ -795,7 +795,7 @@ static NTSTATUS export_gnutls_pubkey_dsa( gnutls_privkey_t gnutls_key, ULONG bit
 {
     BCRYPT_DSA_KEY_BLOB *dsa_blob;
     gnutls_datum_t p, q, g, y;
-    UCHAR *dst, *src;
+    UCHAR *dst;
     int ret;

if ((ret = pgnutls_privkey_export_dsa_raw( gnutls_key, &p, &q, &g, &y, NULL )))
@@ -810,7 +810,7 @@ static NTSTATUS export_gnutls_pubkey_dsa( gnutls_privkey_t gnutls_key, ULONG bit
         return STATUS_NOT_IMPLEMENTED;
     }

-    if (!(dsa_blob = RtlAllocateHeap( GetProcessHeap(), 0, sizeof(*dsa_blob) + p.size + g.size + y.size )))
+    if (!(dsa_blob = RtlAllocateHeap( GetProcessHeap(), 0, sizeof(*dsa_blob) + bitlen / 8 * 3 )))
     {
         pgnutls_perror( ret );
         free( p.data ); free( q.data ); free( g.data ); free( y.data );
@@ -818,40 +818,16 @@ static NTSTATUS export_gnutls_pubkey_dsa( gnutls_privkey_t gnutls_key, ULONG bit
     }

dst = (UCHAR *)(dsa_blob + 1);
-    if (p.size == bitlen / 8 + 1 && !p.data[0])
-    {
-        src = p.data + 1;
-        p.size--;
-    }
-    else src = p.data;
-    memcpy( dst, src, p.size );
+    export_gnutls_datum( dst, bitlen / 8, &p, NULL );

-    dst += p.size;
-    if (g.size == bitlen / 8 + 1 && !g.data[0])
-    {
-        src = g.data + 1;
-        g.size--;
-    }
-    else src = g.data;
-    memcpy( dst, src, g.size );
+    dst += bitlen / 8;
+    export_gnutls_datum( dst, bitlen / 8, &g, NULL );

-    dst += g.size;
-    if (y.size == bitlen / 8 + 1 && !y.data[0])
-    {
-        src = y.data + 1;
-        y.size--;
-    }
-    else src = y.data;
-    memcpy( dst, src, y.size );
+    dst += bitlen / 8;
+    export_gnutls_datum( dst, bitlen / 8, &y, NULL );

dst = dsa_blob->q;
-    if (q.size == sizeof(dsa_blob->q) + 1 && !q.data[0])
-    {
-        src = q.data + 1;
-        q.size--;
-    }
-    else src = q.data;
-    memcpy( dst, src, sizeof(dsa_blob->q) );
+    export_gnutls_datum( dst, sizeof(dsa_blob->q), &q, NULL );

dsa_blob->dwMagic = BCRYPT_DSA_PUBLIC_MAGIC;
     dsa_blob->cbKey   = bitlen / 8;
@@ -859,7 +835,7 @@ static NTSTATUS export_gnutls_pubkey_dsa( gnutls_privkey_t gnutls_key, ULONG bit
     memset( dsa_blob->Seed, 0, sizeof(dsa_blob->Seed) ); /* FIXME */

*pubkey = (UCHAR *)dsa_blob;
-    *pubkey_len = sizeof(*dsa_blob) + p.size + g.size + y.size;
+    *pubkey_len = sizeof(*dsa_blob) + dsa_blob->cbKey * 3;

free( p.data ); free( q.data ); free( g.data ); free( y.data );
     return STATUS_SUCCESS;

[PATCH 4/7] bcrypt: Pad exported datums in export_gnutls_pubkey_dsa_capi().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
 dlls/bcrypt/gnutls.c | 68 +++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 36 deletions(-)

diff --git a/dlls/bcrypt/gnutls.c b/dlls/bcrypt/gnutls.c
index abbbb1ee89f..f5e4651a03a 100644
--- a/dlls/bcrypt/gnutls.c
+++ b/dlls/bcrypt/gnutls.c
@@ -841,14 +841,28 @@ static NTSTATUS export_gnutls_pubkey_dsa( gnutls_privkey_t gnutls_key, ULONG bit
     return STATUS_SUCCESS;
 }

+static void revert_byte_string( UCHAR *d, ULONG length )
+{
+    unsigned int i;
+    UCHAR tmp;
+
+    for (i = 0; i < length / 2; ++i )
+    {
+        tmp = d[i];
+        d[i] = d[length - i - 1];
+        d[length - i - 1] = tmp;
+    }
+}
+
 static NTSTATUS export_gnutls_pubkey_dsa_capi( gnutls_privkey_t gnutls_key, const DSSSEED *seed, ULONG bitlen,
                                                UCHAR **pubkey, ULONG *pubkey_len )
 {
     BLOBHEADER *hdr;
     DSSPUBKEY *dsskey;
     gnutls_datum_t p, q, g, y;
-    UCHAR *dst, *src;
-    int i, ret, size = sizeof(*hdr) + sizeof(*dsskey) + sizeof(*seed);
+    UCHAR *dst;
+    int ret, size = sizeof(*hdr) + sizeof(*dsskey) + sizeof(*seed);
+    ULONG q_size;

if (bitlen > 1024)
     {
@@ -862,7 +876,9 @@ static NTSTATUS export_gnutls_pubkey_dsa_capi( gnutls_privkey_t gnutls_key, cons
         return STATUS_INTERNAL_ERROR;
     }

-    if (!(hdr = RtlAllocateHeap( GetProcessHeap(), 0, size + p.size + q.size + g.size + y.size )))
+    q_size = sizeof(((BCRYPT_DSA_KEY_BLOB *)NULL)->q);
+
+    if (!(hdr = RtlAllocateHeap( GetProcessHeap(), 0, size + bitlen / 8 * 3 + q_size )))
     {
         pgnutls_perror( ret );
         free( p.data ); free( q.data ); free( g.data ); free( y.data );
@@ -879,46 +895,26 @@ static NTSTATUS export_gnutls_pubkey_dsa_capi( gnutls_privkey_t gnutls_key, cons
     dsskey->bitlen = bitlen;

dst = (UCHAR *)(dsskey + 1);
-    if (p.size % 2)
-    {
-        src = p.data + 1;
-        p.size--;
-    }
-    else src = p.data;
-    for (i = 0; i < p.size; i++) dst[i] = src[p.size - i - 1];
+    export_gnutls_datum( dst, bitlen / 8, &p, NULL );
+    revert_byte_string( dst, bitlen / 8 );
+    dst += bitlen / 8;

-    dst += p.size;
-    if (q.size % 2)
-    {
-        src = q.data + 1;
-        q.size--;
-    }
-    else src = q.data;
-    for (i = 0; i < q.size; i++) dst[i] = src[q.size - i - 1];
+    export_gnutls_datum( dst, q_size, &q, NULL );
+    revert_byte_string( dst, q_size );
+    dst += q_size;

-    dst += q.size;
-    if (g.size % 2)
-    {
-        src = g.data + 1;
-        g.size--;
-    }
-    else src = g.data;
-    for (i = 0; i < g.size; i++) dst[i] = src[g.size - i - 1];
+    export_gnutls_datum( dst, bitlen / 8, &g, NULL );
+    revert_byte_string( dst, bitlen / 8 );
+    dst += bitlen / 8;

-    dst += g.size;
-    if (y.size % 2)
-    {
-        src = y.data + 1;
-        y.size--;
-    }
-    else src = y.data;
-    for (i = 0; i < y.size; i++) dst[i] = src[y.size - i - 1];
+    export_gnutls_datum( dst, bitlen / 8, &y, NULL );
+    revert_byte_string( dst, bitlen / 8 );
+    dst += bitlen / 8;

-    dst += y.size;
     memcpy( dst, seed, sizeof(*seed) );

*pubkey = (UCHAR *)hdr;
-    *pubkey_len = size + p.size + q.size + g.size + y.size;
+    *pubkey_len = size + bitlen / 8 * 3 + q_size;

free( p.data ); free( q.data ); free( g.data ); free( y.data );
     return STATUS_SUCCESS;

[PATCH 5/7] bcrypt: Pad exported datums in key_export_ecc().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
 dlls/bcrypt/gnutls.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/dlls/bcrypt/gnutls.c b/dlls/bcrypt/gnutls.c
index f5e4651a03a..85fd998f061 100644
--- a/dlls/bcrypt/gnutls.c
+++ b/dlls/bcrypt/gnutls.c
@@ -1002,7 +1002,7 @@ static NTSTATUS CDECL key_export_ecc( struct key *key, UCHAR *buf, ULONG len, UL
     gnutls_ecc_curve_t curve;
     gnutls_datum_t x, y, d;
     DWORD magic, size;
-    UCHAR *src, *dst;
+    UCHAR *dst;
     int ret;

switch (key->alg_id)
@@ -1042,19 +1042,13 @@ static NTSTATUS CDECL key_export_ecc( struct key *key, UCHAR *buf, ULONG len, UL
         ecc_blob->cbKey   = size;

dst = (UCHAR *)(ecc_blob + 1);
-        if (x.size == size + 1) src = x.data + 1;
-        else src = x.data;
-        memcpy( dst, src, size );
-
+        export_gnutls_datum( dst, size, &x, NULL );
         dst += size;
-        if (y.size == size + 1) src = y.data + 1;
-        else src = y.data;
-        memcpy( dst, src, size );

+        export_gnutls_datum( dst, size, &y, NULL );
         dst += size;
-        if (d.size == size + 1) src = d.data + 1;
-        else src = d.data;
-        memcpy( dst, src, size );
+
+        export_gnutls_datum( dst, size, &d, NULL );
     }

free( x.data ); free( y.data ); free( d.data );

[PATCH 6/7] bcrypt: Pad exported datums in key_export_dsa_capi().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
 dlls/bcrypt/gnutls.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/dlls/bcrypt/gnutls.c b/dlls/bcrypt/gnutls.c
index 85fd998f061..b68718ba534 100644
--- a/dlls/bcrypt/gnutls.c
+++ b/dlls/bcrypt/gnutls.c
@@ -1145,8 +1145,8 @@ static NTSTATUS CDECL key_export_dsa_capi( struct key *key, UCHAR *buf, ULONG le
     BLOBHEADER *hdr;
     DSSPUBKEY *pubkey;
     gnutls_datum_t p, q, g, y, x;
-    UCHAR *src, *dst;
-    int i, ret, size;
+    UCHAR *dst;
+    int ret, size;

if ((ret = pgnutls_privkey_export_dsa_raw( key_data(key)->privkey, &p, &q, &g, &y, &x )))
     {
@@ -1154,9 +1154,9 @@ static NTSTATUS CDECL key_export_dsa_capi( struct key *key, UCHAR *buf, ULONG le
         return STATUS_INTERNAL_ERROR;
     }

-    if ((q.size != 20 && q.size != 21) || (x.size != 20 && x.size != 21))
+    if (q.size > 21 || x.size > 21)
     {
-        ERR( "can't export key in this format\n" );
+        ERR( "can't export key in this format, \n" );
         free( p.data ); free( q.data ); free( g.data ); free( y.data ); free( x.data );
         return STATUS_NOT_SUPPORTED;
     }
@@ -1176,26 +1176,22 @@ static NTSTATUS CDECL key_export_dsa_capi( struct key *key, UCHAR *buf, ULONG le
         pubkey->bitlen = key->u.a.bitlen;

dst = (UCHAR *)(pubkey + 1);
-        if (p.size % 2) src = p.data + 1;
-        else src = p.data;
-        for (i = 0; i < size; i++) dst[i] = src[size - i - 1];
-
+        export_gnutls_datum( dst, size, &p, NULL );
+        revert_byte_string( dst, size );
         dst += size;
-        if (q.size % 2) src = q.data + 1;
-        else src = q.data;
-        for (i = 0; i < 20; i++) dst[i] = src[20 - i - 1];

+        export_gnutls_datum( dst, 20, &q, NULL );
+        revert_byte_string( dst, 20 );
         dst += 20;
-        if (g.size % 2) src = g.data + 1;
-        else src = g.data;
-        for (i = 0; i < size; i++) dst[i] = src[size - i - 1];

+        export_gnutls_datum( dst, size, &g, NULL );
+        revert_byte_string( dst, size );
         dst += size;
-        if (x.size % 2) src = x.data + 1;
-        else src = x.data;
-        for (i = 0; i < 20; i++) dst[i] = src[20 - i - 1];

+        export_gnutls_datum( dst, 20, &x, NULL );
+        revert_byte_string( dst, 20 );
         dst += 20;
+
         memcpy( dst, &key->u.a.dss_seed, sizeof(key->u.a.dss_seed) );
     }

[PATCH 7/7] bcrypt: Fix padding in format_gnutls_signature().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
 dlls/bcrypt/gnutls.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/dlls/bcrypt/gnutls.c b/dlls/bcrypt/gnutls.c
index b68718ba534..13e126ad276 100644
--- a/dlls/bcrypt/gnutls.c
+++ b/dlls/bcrypt/gnutls.c
@@ -1643,9 +1643,8 @@ static NTSTATUS format_gnutls_signature( enum alg_id type, gnutls_datum_t signat
     case ALG_ID_DSA:
     {
         int err;
-        unsigned int pad_size_r, pad_size_s, sig_len = get_signature_length( type );
+        unsigned int sig_len = get_signature_length( type );
         gnutls_datum_t r, s; /* format as r||s */
-        unsigned char *r_data, *s_data;

if ((err = pgnutls_decode_rs_value( &signature, &r, &s )))
         {
@@ -1656,21 +1655,7 @@ static NTSTATUS format_gnutls_signature( enum alg_id type, gnutls_datum_t signat
         *ret_len = sig_len;
         if (output_len < sig_len) return STATUS_BUFFER_TOO_SMALL;

-        if (r.size % 2) /* remove prepended zero byte */
-        {
-            r.size--;
-            r_data = r.data + 1;
-        }
-        else r_data = r.data;
-
-        if (s.size % 2)
-        {
-            s.size--;
-            s_data = s.data + 1;
-        }
-        else s_data = s.data;
-
-        if (r.size + s.size > sig_len)
+        if (r.size > sig_len / 2 + 1 || s.size > sig_len / 2 + 1)
         {
             ERR( "we didn't get a correct signature\n" );
             return STATUS_INTERNAL_ERROR;
@@ -1678,12 +1663,8 @@ static NTSTATUS format_gnutls_signature( enum alg_id type, gnutls_datum_t signat

if (output)
         {
-            pad_size_r = (sig_len / 2) - r.size;
-            pad_size_s = (sig_len / 2) - s.size;
-            memset( output, 0, sig_len );
-
-            memcpy( output + pad_size_r, r_data, r.size );
-            memcpy( output + (sig_len / 2) + pad_size_s, s_data, s.size );
+            export_gnutls_datum( output, sig_len / 2, &r, NULL );
+            export_gnutls_datum( output + sig_len / 2, sig_len / 2, &s, NULL );
         }

free( r.data ); free( s.data );