From 868e5c551b40671180e849133a7d1824cedd7e88 Mon Sep 17 00:00:00 2001
From: Bart de Water <496367+bdewater@users.noreply.github.com>
Date: Wed, 5 Oct 2022 15:40:15 -0400
Subject: [PATCH] Change RSA padding default to OAEP

PKCS#1 v1.5 has been unsafe since 1998's Bleichenbacher attack. Even if these PKey::RSA methods are deprecated in favor of the new PKey::PKey#{encrypt,decrypt} interface, they can and are still used as https://shopify.engineering/building-flex-comp shows - in which case this library is recommending unsafe defaults to people who are not intimitely familiar with cryptography.
---
 lib/openssl/pkey.rb           | 16 ++++++++--------
 test/openssl/test_pkey_rsa.rb | 15 +++++++--------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/lib/openssl/pkey.rb b/lib/openssl/pkey.rb
index d51f066b8..03241401f 100644
--- a/lib/openssl/pkey.rb
+++ b/lib/openssl/pkey.rb
@@ -363,13 +363,13 @@ def new(*args, &blk) # :nodoc:
     #    rsa.private_encrypt(string, padding) -> String
     #
     # Encrypt +string+ with the private key.  +padding+ defaults to
-    # PKCS1_PADDING. The encrypted string output can be decrypted using
+    # PKCS1_OAEP_PADDING. The encrypted string output can be decrypted using
     # #public_decrypt.
     #
     # <b>Deprecated in version 3.0</b>.
     # Consider using PKey::PKey#sign_raw and PKey::PKey#verify_raw, and
     # PKey::PKey#verify_recover instead.
-    def private_encrypt(string, padding = PKCS1_PADDING)
+    def private_encrypt(string, padding = PKCS1_OAEP_PADDING)
       n or raise OpenSSL::PKey::RSAError, "incomplete RSA"
       private? or raise OpenSSL::PKey::RSAError, "private key needed."
       begin
@@ -386,12 +386,12 @@ def private_encrypt(string, padding = PKCS1_PADDING)
     #    rsa.public_decrypt(string, padding) -> String
     #
     # Decrypt +string+, which has been encrypted with the private key, with the
-    # public key.  +padding+ defaults to PKCS1_PADDING.
+    # public key.  +padding+ defaults to PKCS1_OAEP_PADDING.
     #
     # <b>Deprecated in version 3.0</b>.
     # Consider using PKey::PKey#sign_raw and PKey::PKey#verify_raw, and
     # PKey::PKey#verify_recover instead.
-    def public_decrypt(string, padding = PKCS1_PADDING)
+    def public_decrypt(string, padding = PKCS1_OAEP_PADDING)
       n or raise OpenSSL::PKey::RSAError, "incomplete RSA"
       begin
         verify_recover(nil, string, {
@@ -407,12 +407,12 @@ def public_decrypt(string, padding = PKCS1_PADDING)
     #    rsa.public_encrypt(string, padding) -> String
     #
     # Encrypt +string+ with the public key.  +padding+ defaults to
-    # PKCS1_PADDING. The encrypted string output can be decrypted using
+    # PKCS1_OAEP_PADDING. The encrypted string output can be decrypted using
     # #private_decrypt.
     #
     # <b>Deprecated in version 3.0</b>.
     # Consider using PKey::PKey#encrypt and PKey::PKey#decrypt instead.
-    def public_encrypt(data, padding = PKCS1_PADDING)
+    def public_encrypt(data, padding = PKCS1_OAEP_PADDING)
       n or raise OpenSSL::PKey::RSAError, "incomplete RSA"
       begin
         encrypt(data, {
@@ -428,11 +428,11 @@ def public_encrypt(data, padding = PKCS1_PADDING)
     #    rsa.private_decrypt(string, padding) -> String
     #
     # Decrypt +string+, which has been encrypted with the public key, with the
-    # private key. +padding+ defaults to PKCS1_PADDING.
+    # private key. +padding+ defaults to PKCS1_OAEP_PADDING.
     #
     # <b>Deprecated in version 3.0</b>.
     # Consider using PKey::PKey#encrypt and PKey::PKey#decrypt instead.
-    def private_decrypt(data, padding = PKCS1_PADDING)
+    def private_decrypt(data, padding = PKCS1_OAEP_PADDING)
       n or raise OpenSSL::PKey::RSAError, "incomplete RSA"
       private? or raise OpenSSL::PKey::RSAError, "private key needed."
       begin
diff --git a/test/openssl/test_pkey_rsa.rb b/test/openssl/test_pkey_rsa.rb
index fa84b76f4..194dda2bd 100644
--- a/test/openssl/test_pkey_rsa.rb
+++ b/test/openssl/test_pkey_rsa.rb
@@ -155,8 +155,8 @@ def test_sign_verify_raw_legacy
     plain1 = key.public_decrypt(cipher1, OpenSSL::PKey::RSA::PKCS1_PADDING)
     assert_equal(plain0, plain1)
 
-    cipherdef = key.private_encrypt(plain0) # PKCS1_PADDING is default
-    plain1 = key.public_decrypt(cipherdef)
+    cipherdef = key.private_encrypt(plain0, OpenSSL::PKey::RSA::PKCS1_PADDING)
+    plain1 = key.public_decrypt(cipherdef, OpenSSL::PKey::RSA::PKCS1_PADDING)
     assert_equal(plain0, plain1)
     assert_equal(cipher1, cipherdef)
 
@@ -226,19 +226,18 @@ def test_encrypt_decrypt_legacy
     rsapriv = Fixtures.pkey("rsa-1")
     rsapub = OpenSSL::PKey.read(rsapriv.public_to_der)
 
-    # Defaults to PKCS #1 v1.5
     raw = "data"
-    enc_legacy = rsapub.public_encrypt(raw)
+    enc_legacy = rsapub.public_encrypt(raw, OpenSSL::PKey::RSA::PKCS1_PADDING)
     assert_equal raw, rsapriv.decrypt(enc_legacy)
     enc_new = rsapub.encrypt(raw)
-    assert_equal raw, rsapriv.private_decrypt(enc_new)
+    assert_equal raw, rsapriv.private_decrypt(enc_new, OpenSSL::PKey::RSA::PKCS1_PADDING)
 
-    # OAEP with default parameters
+    # Defaults to OAEP with default parameters
     raw = "data"
-    enc_legacy = rsapub.public_encrypt(raw, OpenSSL::PKey::RSA::PKCS1_OAEP_PADDING)
+    enc_legacy = rsapub.public_encrypt(raw)
     assert_equal raw, rsapriv.decrypt(enc_legacy, { "rsa_padding_mode" => "oaep" })
     enc_new = rsapub.encrypt(raw, { "rsa_padding_mode" => "oaep" })
-    assert_equal raw, rsapriv.private_decrypt(enc_legacy, OpenSSL::PKey::RSA::PKCS1_OAEP_PADDING)
+    assert_equal raw, rsapriv.private_decrypt(enc_legacy)
   end
 
   def test_export