[PATCH 1/7] bcrypt: Introduce export_gnutls_datum() helper and use it in export_gnutls_pubkey_rsa().
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- 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; -- 2.29.2
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- 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; -- 2.29.2
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- 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; -- 2.29.2
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- 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; -- 2.29.2
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- 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 ); -- 2.29.2
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- 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) ); } -- 2.29.2
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- 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 ); -- 2.29.2
participants (1)
-
Paul Gofman