Skip to content

Commit b4ec4ff

Browse files
committed
rebase: apply review feedback on #159
* (`RSA.xs`, d/p/q/dmp1/dmq1/iqmp): `EVP_PKEY_get_bn_param()` returning 0 for absent parameters is expected behavior for public keys, not an error. The return values are now intentionally ignored — NULL pointers flow through `cor_bn2sv()` as `undef`, matching the pre-3.x behavior where `RSA_get0_key`/`RSA_get0_factors`/`RSA_get0_crt_params` simply set NULL for missing fields. - **Removed cascading inline cleanup and redundant `_is_private()` calls**: The fragile per-line cleanup chains and 6 redundant `_is_private()` calls (each performing an extra `EVP_PKEY_get_bn_param` round-trip) are eliminated. - **Added `ERR_clear_error()` after private component fetches**: Failed `EVP_PKEY_get_bn_param()` calls on public keys may push errors onto the OpenSSL error queue; draining it prevents stale errors from leaking into subsequent operations. - **Kept n/e mandatory checks**: `n` and `e` are present in every valid RSA key, so the croak-on-failure check is retained for these two parameters only.
1 parent b1400ea commit b4ec4ff

1 file changed

Lines changed: 12 additions & 26 deletions

File tree

RSA.xs

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -915,32 +915,18 @@ PPCODE:
915915
BN_free(n);
916916
croakSsl(__FILE__, __LINE__);
917917
}
918-
/* Private components are absent for public keys (return 0, leave ptr NULL).
919-
Only croak if the call fails while the key is marked private. */
920-
if (!EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_D, &d) && _is_private(p_rsa)) {
921-
BN_free(n); BN_free(e);
922-
croakSsl(__FILE__, __LINE__);
923-
}
924-
if (!EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_FACTOR1, &p) && _is_private(p_rsa)) {
925-
BN_free(n); BN_free(e); BN_clear_free(d);
926-
croakSsl(__FILE__, __LINE__);
927-
}
928-
if (!EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_FACTOR2, &q) && _is_private(p_rsa)) {
929-
BN_free(n); BN_free(e); BN_clear_free(d); BN_clear_free(p);
930-
croakSsl(__FILE__, __LINE__);
931-
}
932-
if (!EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_EXPONENT1, &dmp1) && _is_private(p_rsa)) {
933-
BN_free(n); BN_free(e); BN_clear_free(d); BN_clear_free(p); BN_clear_free(q);
934-
croakSsl(__FILE__, __LINE__);
935-
}
936-
if (!EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_EXPONENT2, &dmq1) && _is_private(p_rsa)) {
937-
BN_free(n); BN_free(e); BN_clear_free(d); BN_clear_free(p); BN_clear_free(q); BN_clear_free(dmp1);
938-
croakSsl(__FILE__, __LINE__);
939-
}
940-
if (!EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_COEFFICIENT1, &iqmp) && _is_private(p_rsa)) {
941-
BN_free(n); BN_free(e); BN_clear_free(d); BN_clear_free(p); BN_clear_free(q); BN_clear_free(dmp1); BN_clear_free(dmq1);
942-
croakSsl(__FILE__, __LINE__);
943-
}
918+
/* Private components are absent for public keys — EVP_PKEY_get_bn_param()
919+
returns 0 and may push errors onto the queue, but the pointer stays NULL
920+
so cor_bn2sv() will return undef. This matches the pre-3.x behaviour
921+
where RSA_get0_key/factors/crt_params simply set NULL for missing fields. */
922+
EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_D, &d);
923+
EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_FACTOR1, &p);
924+
EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_FACTOR2, &q);
925+
EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_EXPONENT1, &dmp1);
926+
EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_EXPONENT2, &dmq1);
927+
EVP_PKEY_get_bn_param(rsa, OSSL_PKEY_PARAM_RSA_COEFFICIENT1, &iqmp);
928+
/* Drain any errors pushed by expected failures on public keys. */
929+
ERR_clear_error();
944930
#else
945931
RSA_get0_key(rsa, &n, &e, &d);
946932
RSA_get0_factors(rsa, &p, &q);

0 commit comments

Comments
 (0)