Do not overide the user provided padding croak instead

[timlegge]

@Koan-Bot @Koan-Bot Please review

[Koan-Bot]

Good change — croaking is the right call. Silently overriding user padding to OAEP was a hidden footgun: the code "worked" but didn't do what the user asked. Explicit failure is better.

The crypt_pad variable removal is clean. In the !is_encrypt branch it was a pure copy of p_rsa->padding, and in the is_encrypt branch it was the silent override being removed. Using p_rsa->padding directly simplifies the code.

One issue:

Bug in croak message — the new croak suggests calling use_pkcs1_padding():

croak("Only OAEP padding or no padding is supported for encrypt/decrypt. "
      "Call \"use_pkcs1_padding()\" or \"use_no_padding()\" first.");

But use_pkcs1_padding() for encryption will immediately hit the Marvin attack croak at line 449:

croak("PKCS#1 v1.5 padding for encryption is vulnerable to the Marvin attack. ...")

The message should suggest use_pkcs1_oaep_padding() instead of use_pkcs1_padding():

croak("Only OAEP padding or no padding is supported for encrypt/decrypt. "
      "Call \"use_pkcs1_oaep_padding()\" or \"use_no_padding()\" first.");

Note: on OpenSSL 3.x this croak is effectively a safety net that can't trigger under normal usage — use_sslv23_padding() is #if'd out on 3.x, and PKCS1/PSS are already caught earlier. But the message should still be correct for anyone reading the code or if a future padding mode is added.

[timlegge]

@Koan-Bot review again

[Koan-Bot]

PR Review — Do not overide the user provided padding croak instead

The previous review's blocking issue (incorrect croak message suggesting use_pkcs1_padding() instead of use_pkcs1_oaep_padding()) has been fixed. The croak message at line 469 now correctly suggests use_pkcs1_oaep_padding(). The crypt_pad variable removal is clean — it was either a redundant copy of p_rsa->padding (in the !is_encrypt branch) or the silent override being removed (in the is_encrypt branch). Using p_rsa->padding directly at line 490 is correct since all invalid padding values are now caught by croaks before reaching that line. The validation logic at line 467 correctly allows only RSA_NO_PADDING and RSA_PKCS1_OAEP_PADDING for encryption, and PKCS1/PSS are already caught by the earlier checks at lines 453 and 458. This is a net improvement: explicit failure over silent behavior override.

---

---

Checklist

• No hardcoded secrets
• Error messages do not expose internals
• Resource cleanup on error paths
• Input validation at boundaries

---

Summary

The previous review's blocking issue (incorrect croak message suggesting use_pkcs1_padding() instead of use_pkcs1_oaep_padding()) has been fixed. The croak message at line 469 now correctly suggests use_pkcs1_oaep_padding(). The crypt_pad variable removal is clean — it was either a redundant copy of p_rsa->padding (in the !is_encrypt branch) or the silent override being removed (in the is_encrypt branch). Using p_rsa->padding directly at line 490 is correct since all invalid padding values are now caught by croaks before reaching that line. The validation logic at line 467 correctly allows only RSA_NO_PADDING and RSA_PKCS1_OAEP_PADDING for encryption, and PKCS1/PSS are already caught by the earlier checks at lines 453 and 458. This is a net improvement: explicit failure over silent behavior override.

---

Automated review by Kōan5c5b50f
cb2baa6

[Koan-Bot]

@timlegge: @Koan-Bot review again

The previous review's only blocking issue — the croak message incorrectly suggesting use_pkcs1_padding() — has been fixed. The message now correctly says use_pkcs1_oaep_padding(). LGTM, no remaining issues.