Skip to content

Commit

Permalink
pkcs7: remove default cipher from PKCS7.encrypt
Browse files Browse the repository at this point in the history
Require that users explicitly specify the desired algorithm. In my
opinion, we are not in a position to specify the default cipher.

When OpenSSL::PKCS7.encrypt is given only two arguments, it uses
"RC2-40-CBC" as the symmetric cipher algorithm. 40-bit RC2 is a US
export-grade cipher and considered insecure.

Although this is technically a breaking change, the impact should be
minimal. Even when OpenSSL is compiled with RC2 support and the macro
OPENSSL_NO_RC2 is not defined, it will not actually work on modern
systems because RC2 is part of the legacy provider.
  • Loading branch information
rhenium committed Sep 5, 2024
1 parent ade5076 commit 439f456
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 15 deletions.
28 changes: 13 additions & 15 deletions ext/openssl/ossl_pkcs7.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,14 @@ ossl_pkcs7_s_sign(int argc, VALUE *argv, VALUE klass)

/*
* call-seq:
* PKCS7.encrypt(certs, data, [, cipher [, flags]]) => pkcs7
* PKCS7.encrypt(certs, data, cipher, flags = 0) => pkcs7
*
* Creates a PKCS #7 enveloped-data structure.
*
* Before version 3.3.0, +cipher+ was optional and defaulted to
* <tt>"RC2-40-CBC"</tt>.
*
* See also the man page PKCS7_encrypt(3).
*/
static VALUE
ossl_pkcs7_s_encrypt(int argc, VALUE *argv, VALUE klass)
Expand All @@ -273,21 +280,12 @@ ossl_pkcs7_s_encrypt(int argc, VALUE *argv, VALUE klass)
PKCS7 *p7;

rb_scan_args(argc, argv, "22", &certs, &data, &cipher, &flags);
if(NIL_P(cipher)){
#if !defined(OPENSSL_NO_RC2)
ciph = EVP_rc2_40_cbc();
#elif !defined(OPENSSL_NO_DES)
ciph = EVP_des_ede3_cbc();
#elif !defined(OPENSSL_NO_RC2)
ciph = EVP_rc2_40_cbc();
#elif !defined(OPENSSL_NO_AES)
ciph = EVP_EVP_aes_128_cbc();
#else
ossl_raise(ePKCS7Error, "Must specify cipher");
#endif

if (NIL_P(cipher)) {
rb_raise(rb_eArgError,
"cipher must be specified. Before version 3.3, " \
"the default cipher was RC2-40-CBC.");
}
else ciph = ossl_evp_get_cipherbyname(cipher);
ciph = ossl_evp_get_cipherbyname(cipher);
flg = NIL_P(flags) ? 0 : NUM2INT(flags);
ret = NewPKCS7(cPKCS7);
in = ossl_obj2bio(&data);
Expand Down
5 changes: 5 additions & 0 deletions test/openssl/test_pkcs7.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ def test_enveloped
assert_equal(data, p7.decrypt(@rsa1024, @ee2_cert))

assert_equal(data, p7.decrypt(@rsa1024))

# Default cipher has been removed in v3.3
assert_raise_with_message(ArgumentError, /RC2-40-CBC/) {
OpenSSL::PKCS7.encrypt(certs, data)
}
end

def test_empty_signed_data_ruby_bug_19974
Expand Down

0 comments on commit 439f456

Please sign in to comment.