[PATCH v2 0/1] MR10222: bcrypt: Improve RSA OAEP parameter handling.
Handle OAEP parameter setup failures when setting SPKI parameters and propagate GNUTLS_E_UNKNOWN_PK_ALGORITHM as STATUS_NOT_SUPPORTED For OAEP encrypt/decrypt, accept empty labels by only setting OAEP SPKI parameters when a non-empty label is provided Match native Windows OAEP(NULL) behavior for encryption by allowing size-only queries and returning STATUS_INVALID_PARAMETER for actual encryption when pPaddingInfo is NULL Add tests covering OAEP empty-label roundtrip and OAEP(NULL) size-query vs. encrypt behavior ##### Validation Built bcrypt_test from this branch(branch: bcrypt-rsa) Ran that same branch built test binary on: * Windows 11 (24H2): pass * Wine master: fail (OAEP cases) * Wine (bcrypt-rsa): pass -- v2: bcrypt: Improve RSA OAEP parameter handling. https://gitlab.winehq.org/wine/wine/-/merge_requests/10222
From: Sreehari Anil <sreehari7102008@gmail.com> Handle OAEP parameter setup failures when setting SPKI parameters and propagate GNUTLS_E_UNKNOWN_PK_ALGORITHM as STATUS_NOT_SUPPORTED. For OAEP encrypt/decrypt, accept empty labels by only setting OAEP SPKI parameters when a non-empty label is provided. Match native Windows OAEP(NULL) behavior for encryption by allowing size-only queries and returning STATUS_INVALID_PARAMETER for actual encryption when pPaddingInfo is NULL. Add tests covering OAEP empty-label roundtrip and OAEP(NULL) size-query vs. encrypt behavior. Signed-off-by: Sreehari Anil <sreehari7102008@gmail.com> --- dlls/bcrypt/gnutls.c | 68 +++++++++++++++++++++++++++++--------- dlls/bcrypt/tests/bcrypt.c | 54 +++++++++++++++++++++++++++--- 2 files changed, 103 insertions(+), 19 deletions(-) diff --git a/dlls/bcrypt/gnutls.c b/dlls/bcrypt/gnutls.c index 3db02c75b9a..8aef6d0a75a 100644 --- a/dlls/bcrypt/gnutls.c +++ b/dlls/bcrypt/gnutls.c @@ -2313,7 +2313,7 @@ static NTSTATUS pubkey_set_rsa_pss_params( gnutls_pubkey_t key, gnutls_digest_al gnutls_x509_spki_t spki; int ret; - if (((ret = pgnutls_x509_spki_init( &spki ) < 0))) + if ((ret = pgnutls_x509_spki_init( &spki )) < 0) { pgnutls_perror( ret ); return STATUS_INTERNAL_ERROR; @@ -2509,7 +2509,7 @@ static NTSTATUS privkey_set_rsa_pss_params( gnutls_privkey_t key, gnutls_digest_ gnutls_x509_spki_t spki; int ret; - if (((ret = pgnutls_x509_spki_init( &spki ) < 0))) + if ((ret = pgnutls_x509_spki_init( &spki )) < 0) { pgnutls_perror( ret ); return STATUS_INTERNAL_ERROR; @@ -2841,12 +2841,18 @@ static NTSTATUS privkey_set_rsa_oaep_params( gnutls_privkey_t key, gnutls_digest gnutls_x509_spki_t spki; int ret; - if (((ret = pgnutls_x509_spki_init( &spki ) < 0))) + if ((ret = pgnutls_x509_spki_init( &spki )) < 0) { pgnutls_perror( ret ); return STATUS_INTERNAL_ERROR; } - pgnutls_x509_spki_set_rsa_oaep_params( spki, dig, label ); + if ((ret = pgnutls_x509_spki_set_rsa_oaep_params( spki, dig, label )) < 0) + { + pgnutls_x509_spki_deinit( spki ); + if (ret == GNUTLS_E_UNKNOWN_PK_ALGORITHM) return STATUS_NOT_SUPPORTED; + pgnutls_perror( ret ); + return STATUS_INTERNAL_ERROR; + } ret = pgnutls_privkey_set_spki( key, spki, 0 ); pgnutls_x509_spki_deinit( spki ); if (ret < 0) @@ -2864,26 +2870,36 @@ static NTSTATUS key_asymmetric_decrypt( void *args ) NTSTATUS status = STATUS_SUCCESS; int ret; - if (params->key->alg_id == ALG_ID_RSA && params->flags & BCRYPT_PAD_OAEP) + if (params->key->alg_id == ALG_ID_RSA && (params->flags & BCRYPT_PAD_OAEP)) { BCRYPT_OAEP_PADDING_INFO *pad = params->padding; gnutls_digest_algorithm_t dig; gnutls_datum_t label; + /* For OAEP decryption Windows requires a padding info structure with a hash + * algorithm; label may be empty (pbLabel == NULL and cbLabel == 0). */ if (!pad || !pad->pszAlgId) { WARN( "padding info not found\n" ); return STATUS_INVALID_PARAMETER; } + if ((dig = get_digest_from_id( pad->pszAlgId )) == GNUTLS_DIG_UNKNOWN) { FIXME( "hash algorithm %s not recognized\n", debugstr_w(pad->pszAlgId) ); return STATUS_NOT_SUPPORTED; } - label.data = pad->pbLabel; - label.size = pad->cbLabel; - if ((status = privkey_set_rsa_oaep_params( key_data(params->key)->a.privkey, dig, &label ))) return status; + /* Only set OAEP SPKI params for non-empty labels. + * For empty labels, use backend defaults for compatibility. */ + if (pad->pbLabel && pad->cbLabel) + { + label.data = pad->pbLabel; + label.size = pad->cbLabel; + status = privkey_set_rsa_oaep_params( key_data(params->key)->a.privkey, dig, &label ); + if (status == STATUS_NOT_SUPPORTED) status = STATUS_SUCCESS; /* Function not available, use defaults */ + if (status) return status; + } } e.data = params->input; @@ -2907,12 +2923,18 @@ static NTSTATUS pubkey_set_rsa_oaep_params( gnutls_pubkey_t key, gnutls_digest_a gnutls_x509_spki_t spki; int ret; - if (((ret = pgnutls_x509_spki_init( &spki ) < 0))) + if ((ret = pgnutls_x509_spki_init( &spki )) < 0) { pgnutls_perror( ret ); return STATUS_INTERNAL_ERROR; } - pgnutls_x509_spki_set_rsa_oaep_params( spki, dig, label ); + if ((ret = pgnutls_x509_spki_set_rsa_oaep_params( spki, dig, label )) < 0) + { + pgnutls_x509_spki_deinit( spki ); + if (ret == GNUTLS_E_UNKNOWN_PK_ALGORITHM) return STATUS_NOT_SUPPORTED; + pgnutls_perror( ret ); + return STATUS_INTERNAL_ERROR; + } ret = pgnutls_pubkey_set_spki( key, spki, 0 ); pgnutls_x509_spki_deinit( spki ); if (ret < 0) @@ -2939,26 +2961,42 @@ static NTSTATUS key_asymmetric_encrypt( void *args ) return !params->output ? STATUS_SUCCESS : STATUS_BUFFER_TOO_SMALL; } - if (params->key->alg_id == ALG_ID_RSA && params->flags & BCRYPT_PAD_OAEP) + if (params->key->alg_id == ALG_ID_RSA && (params->flags & BCRYPT_PAD_OAEP)) { BCRYPT_OAEP_PADDING_INFO *pad = params->padding; gnutls_digest_algorithm_t dig; gnutls_datum_t label; - if (!pad || !pad->pszAlgId || !pad->pbLabel) + /* Size-only OAEP queries can succeed without padding info, but an actual + * OAEP encryption call requires padding info on native Windows. */ + if (!pad) { WARN( "padding info not found\n" ); return STATUS_INVALID_PARAMETER; } + + if (!pad->pszAlgId) + { + WARN( "padding hash not found\n" ); + return STATUS_INVALID_PARAMETER; + } + if ((dig = get_digest_from_id( pad->pszAlgId )) == GNUTLS_DIG_UNKNOWN) { FIXME( "hash algorithm %s not recognized\n", debugstr_w(pad->pszAlgId) ); return STATUS_NOT_SUPPORTED; } - label.data = pad->pbLabel; - label.size = pad->cbLabel; - if ((status = pubkey_set_rsa_oaep_params( key_data(params->key)->a.pubkey, dig, &label ))) return status; + /* Only set OAEP SPKI params for non-empty labels. + * For empty labels, use backend defaults for compatibility. */ + if (pad->pbLabel && pad->cbLabel) + { + label.data = pad->pbLabel; + label.size = pad->cbLabel; + status = pubkey_set_rsa_oaep_params( key_data(params->key)->a.pubkey, dig, &label ); + if (status == STATUS_NOT_SUPPORTED) status = STATUS_SUCCESS; /* Function not available, use defaults */ + if (status) return status; + } } d.data = params->input; diff --git a/dlls/bcrypt/tests/bcrypt.c b/dlls/bcrypt/tests/bcrypt.c index 365e18d4d0e..a1eb766a03e 100644 --- a/dlls/bcrypt/tests/bcrypt.c +++ b/dlls/bcrypt/tests/bcrypt.c @@ -2776,10 +2776,6 @@ static void test_rsa_encrypt(void) encrypted_b = realloc(encrypted_b, encrypted_size); memset(encrypted_b, 0, encrypted_size); - ret = BCryptEncrypt(key, input, sizeof(input), NULL, NULL, 0, NULL, encrypted_size, &encrypted_size, BCRYPT_PAD_OAEP); - ok(ret == STATUS_SUCCESS, "got %lx\n", ret); - ok(encrypted_size == 80, "got size of %ld\n", encrypted_size); - encrypted_size = 0; ret = BCryptEncrypt(key, input, sizeof(input), NULL, NULL, 0, encrypted_a, 0, &encrypted_size, BCRYPT_PAD_OAEP); ok(ret == STATUS_BUFFER_TOO_SMALL, "got %lx\n", ret); @@ -2810,6 +2806,56 @@ static void test_rsa_encrypt(void) ok(decrypted_size == sizeof(input), "got %lu\n", decrypted_size); ok(!memcmp(decrypted, input, sizeof(input)), "unexpected output\n"); + /* Prove empty label (pbLabel NULL, cbLabel 0) works for OAEP. */ + { + BCRYPT_OAEP_PADDING_INFO sha1_empty_label; + UCHAR roundtrip_plain[sizeof(input)]; + DWORD roundtrip_size; + + sha1_empty_label.pszAlgId = BCRYPT_SHA1_ALGORITHM; + sha1_empty_label.pbLabel = NULL; + sha1_empty_label.cbLabel = 0; + + encrypted_size = 0; + ret = BCryptEncrypt(key, input, sizeof(input), &sha1_empty_label, NULL, 0, NULL, 0, &encrypted_size, BCRYPT_PAD_OAEP); + ok(ret == STATUS_SUCCESS, "encrypt with empty label failed: %lx\n", ret); + + encrypted_a = realloc(encrypted_a, encrypted_size); + ret = BCryptEncrypt(key, input, sizeof(input), &sha1_empty_label, NULL, 0, encrypted_a, encrypted_size, &encrypted_size, BCRYPT_PAD_OAEP); + ok(ret == STATUS_SUCCESS, "encrypt with empty label failed: %lx\n", ret); + + roundtrip_size = 0; + ret = BCryptDecrypt(key, encrypted_a, encrypted_size, &sha1_empty_label, NULL, 0, NULL, 0, &roundtrip_size, BCRYPT_PAD_OAEP); + ok(ret == STATUS_SUCCESS, "decrypt with empty label failed: %lx\n", ret); + ok(roundtrip_size == sizeof(input), "decrypted size %lu, expected %lu\n", roundtrip_size, (unsigned long)sizeof(input)); + + ret = BCryptDecrypt(key, encrypted_a, encrypted_size, &sha1_empty_label, NULL, 0, roundtrip_plain, roundtrip_size, &roundtrip_size, BCRYPT_PAD_OAEP); + ok(ret == STATUS_SUCCESS, "decrypt failed: %lx\n", ret); + ok(!memcmp(roundtrip_plain, input, sizeof(input)), "roundtrip plaintext mismatch\n"); + } + + /* OAEP(NULL) behavior observed on native Windows: + * - size query succeeds + * - actual encryption fails with STATUS_INVALID_PARAMETER */ + { + UCHAR *encrypted_null = NULL; + DWORD encrypted_null_size = 0; + + ret = BCryptEncrypt(key, input, sizeof(input), NULL, NULL, 0, NULL, 0, &encrypted_null_size, BCRYPT_PAD_OAEP); + ok(ret == STATUS_SUCCESS, "unexpected OAEP(NULL) size query status %lx\n", ret); + + if (ret == STATUS_SUCCESS) + { + encrypted_null = malloc(encrypted_null_size); + ret = BCryptEncrypt(key, input, sizeof(input), NULL, NULL, 0, encrypted_null, encrypted_null_size, + &encrypted_null_size, BCRYPT_PAD_OAEP); + ok(ret == STATUS_INVALID_PARAMETER || broken(ret == STATUS_SUCCESS), + "unexpected OAEP(NULL) encrypt status %lx\n", ret); + + free(encrypted_null); + } + } + free(encrypted_a); free(encrypted_b); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10222
participants (2)
-
Sreehari Anil -
Sreehari Anil (＠Sreehari425)