[PATCH v4 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 -- v4: 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 | 54 +++++++++++++++++++++++++++----------- dlls/bcrypt/tests/bcrypt.c | 54 +++++++++++++++++++++++++++++++++++--- 2 files changed, 89 insertions(+), 19 deletions(-) diff --git a/dlls/bcrypt/gnutls.c b/dlls/bcrypt/gnutls.c index 3db02c75b9a..c087c6ecfce 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,7 +2870,7 @@ 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; @@ -2875,15 +2881,21 @@ static NTSTATUS key_asymmetric_decrypt( void *args ) 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; + 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; + if (status) return status; + } } e.data = params->input; @@ -2907,12 +2919,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 +2957,32 @@ 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) + 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 = pubkey_set_rsa_oaep_params( key_data(params->key)->a.pubkey, dig, &label ))) return status; + 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; + 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
Thank you for the review. I’ve merged the checks and removed the redundant comments. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10222#note_130911
On Mon Mar 2 12:10:28 2026 +0000, Hans Leidekker wrote:
Thanks, I pointed out a few minor issues. Otherwise this looks good. Thank you for the review. I’ve merged the checks and removed the redundant comments.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10222#note_130915
This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10222
participants (3)
-
Hans Leidekker (＠hans) -
Sreehari Anil
-
Sreehari Anil (＠Sreehari425)