Skip to content

Conversation

nikic
Copy link
Member

@nikic nikic commented Aug 10, 2021

Switch default cipher for openssl_pkcs7_encrypt() and
openssl_cms_encrypt() from RC2-40 to 3DES.

The RC2-40 cipher is considered insecure and is not loaded by
default in OpenSSL 3, which means that these functions will
always fail with default arguments.

As the used algorithm is embedded in the result (which makes this
different from the openssl_encrypt() case) changing the default
algorithm should be safe.

@nikic nikic requested a review from bukka August 10, 2021 14:57
@nikic
Copy link
Member Author

nikic commented Aug 10, 2021

Ah, looks like the OpenSSL docs recommend using 3DES:

EVP_des_ede3_cbc() (triple DES) is the algorithm of choice for S/MIME use because most clients will support it.

I'll switch to using that as the default then...

Switch default cipher for openssl_pkcs7_encrypt() and
openssl_cms_encrypt() from RC2-40 to 3DES.

The RC2-40 cipher is considered insecure and is not loaded by
default in OpenSSL 3, which means that these functions will
always fail with default arguments.

As the used algorithm is embedded in the result (which makes this
different from the openssl_encrypt() case) changing the default
algorithm should be safe.
@nikic nikic changed the title Switch default cipher to AES-128-CBC Switch default cipher to 3DES Aug 10, 2021
@NattyNarwhal
Copy link
Member

I'll admit I'm not an S/MIME or cryptography expert (so I don't know if there's any compatibility constraints going on here), but 3DES sounds really, really bad considering it's been very vulnerable for a while. Is there any reason why AES couldn't be used instead? (also, when have the docs been last updated? docs recommending in 2021 to use 3DES made me do a double take)

@nikic
Copy link
Member Author

nikic commented Aug 12, 2021

The recommendation seems to be 15 years old, so it may well be outdated: https://github.com/openssl/openssl/blame/1c0eede9827b0962f1d752fa4ab5d436fa039da4/doc/man3/PKCS7_encrypt.pod

Though this is also the default the OpenSSL command line utilities, so it's not just the docs.

@nikic
Copy link
Member Author

nikic commented Aug 12, 2021

It looks like https://datatracker.ietf.org/doc/html/rfc5751 in "Changes since S/MIME v3.1" now has "AES-128 CBC" as MUST and tripleDES as SHOULD-.

@nikic
Copy link
Member Author

nikic commented Aug 12, 2021

Seems worth double checking this, so I opened an issue with the OpenSSL project: openssl/openssl#16302

@nikic
Copy link
Member Author

nikic commented Aug 13, 2021

Per openssl/openssl#16302 (comment) they seem to be open to changing the default in the future, so I'd also be fine with making the default AES_128_CBC rather than 3DES here.

@bukka
Copy link
Member

bukka commented Aug 15, 2021

Yeah I think it would be better to use AES-128-CBC. Please also add a note to UPGRADING as it's a BC break.

@bukka
Copy link
Member

bukka commented Aug 15, 2021

As you say, it shouldn't break that much code as it's part of the envelope but might be causing problems with interoperability potentially so note in UPGRADING should be done.

@nikic nikic closed this in 7b34db0 Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants