Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

export-p12: use OpenSSLv3-compatible ciphers (AES-256-CBC & SHA256) with OpenSSL 1.1 #1081

Closed
wants to merge 1 commit into from

Conversation

adaugherity
Copy link

This is my stab at fixing #966. It makes the exported .p12 file readable by OpenSSL v3 without needing -legacy.

I think it's better to have this on by default, but allow opting out where backwards compatibility is needed. To provide compatibility, I decide to make the "export-p12 CN legacy" option suppress this behavior and use the default ciphers.

(That option is no longer OpenSSL v3-only: it now provides "legacy" behavior on all versions of OpenSSL. For v3, it uses -legacy; for non-v3, it uses the default ciphers, omitting the keypbe/certpbe/macalg options.)

This makes the exported .p12 file readable by OpenSSLv3 without needing
-legacy.  Using "export-p12 <CN> legacy" will suppress this behavior.

Fixes OpenVPN#966
@TinCanTech
Copy link
Collaborator

This is my stab at

I think it's better to

Do you have anything better than a stab at ?

For example, you could document your use case and easyrsa output, when using -legacy your way.

@TinCanTech
Copy link
Collaborator

From your code comment:
-This makes the .p12 files readable by OpenSSL 3.0 without needing '-legacy'.

Easy-RSA does not use PKCS files, it only exports them. What ever application does use PKCS files must be able to read those files, regardless of format.

Example: OpenVPN must have built-in support to read both varieties of .p12 files.

Easy-RSA does not need to force OpenSSL 1x to behave as OpenSSL 3x.

I cannot find a reason to make the change suggested here.

@adaugherity
Copy link
Author

The scenario is where Easy-RSA is running on a server with OpenSSL 1.1 (as many LTS distros still are). The PKCS#12 files generated by easyrsa export-p12 from that system cannot be used by clients with OpenVPN linked against OpenSSL v3 (including the 2.6 builds for Windows). This is the issue reported in #966, which was closed with "PRs welcome", so I'm attempting to solve that.

I believe this problem is better solved by producing p12 files which are compliant with the cryptography policies of modern OpenSSL, rather than adding more options/shims to OpenVPN to support legacy ciphers (which they probably wouldn't want to do anyway). Also, it's possible for OpenSSL v3 to be installed without legacy support, or to have it disabled via system policy, and producing compliant files avoids both those issues.

As I understand, it's actually OpenSSL itself erroring out when OpenVPN attempts to load the "legacy" p12 files:

2024-02-20 13:26:41 OpenSSL: error:0308010C:digital envelope routines::unsupported:Global default library context, Algorithm (RC2-40-CBC : 0), Properties ()
2024-02-20 13:26:41 OpenSSL: error:11800071:PKCS12 routines::mac verify failure:
2024-02-20 13:26:41 Decoding PKCS12 failed. Probably wrong password or unsupported/legacy encryption
2024-02-20 13:26:41 SIGUSR1[soft,private-key-password-failure] received, process restarting
2024-02-20 13:26:41 MANAGEMENT: >STATE:1708457201,RECONNECTING,private-key-password-failure,,,,,
2024-02-20 13:26:41 Restart pause, 1 second(s)
2024-02-20 13:26:44 MANAGEMENT: Client disconnected
2024-02-20 13:26:44 ERROR: could not read Private Key username/password/ok/string from management interface
2024-02-20 13:26:44 Exiting due to fatal error

In most GUI frontends, this is presented as a prompt for the .p12 bundle's password (even if it doesn't have one), which then refuses your password, as reported in OpenVPN/openvpn#238. In that issue they tell the reporter to recreate the p12 file without legacy ciphers, which this PR will do for easy-rsa. The p12 file produced with my changes is loaded properly by OpenVPN 2.6 + OpenSSL 3.x.

I'm happy to add more documentation wherever is appropriate, but I can't find the export-p12 command documented anywhere in the codebase, except for a couple recent changelog entries and the help text (which I've modified accordingly... didn't think this should be too verbose, but do you want more?).

@adaugherity
Copy link
Author

adaugherity commented Feb 20, 2024

More details: there are two types of PKCS#12 files produced, which I will call "modern" and "legacy".

These are the ciphers reported by openssl pkcs12 -noout -info -in pki/private/adaugherity.p12:

Modern

MAC: sha256, Iteration 2048
MAC length: 32, salt length: 8
PKCS7 Encrypted data: PBES2, PBKDF2, AES-256-CBC, Iteration 2048, PRF hmacWithSHA256
Certificate bag
Certificate bag
PKCS7 Data
Shrouded Keybag: PBES2, PBKDF2, AES-256-CBC, Iteration 2048, PRF hmacWithSHA256

Legacy

MAC: sha1, Iteration 2048
MAC length: 20, salt length: 8
PKCS7 Encrypted data: pbeWithSHA1And40BitRC2-CBC, Iteration 2048
Certificate bag
Certificate bag
PKCS7 Data
Shrouded Keybag: pbeWithSHA1And3-KeyTripleDES-CBC, Iteration 2048

OpenSSL 3.x will not load this file unless you add -legacy. Without it you get this error after the first PKCS7 line:

Error outputting keys and certificates
40B76E0EFC7E0000:error:0308010C:digital envelope routines:inner_evp_generic_fetch:unsupported:crypto/evp/evp_fetch.c:373:Global default library context, Algorithm (RC2-40-CBC : 0), Properties ()

Easy-RSA export-p12 output

These tables describe the types of PKCS#12 files produced by easyrsa export-p12 CN vs. easyrsa export-p12 CN legacy, on different OpenSSL versions:

Current state (git HEAD 70bb7ec)

Version export-p12 legacy? Output type
OpenSSL 3 N modern
OpenSSL 3 Y legacy
OpenSSL 1.1 N legacy
OpenSSL 1.1 Y error

With this PR

Version export-p12 legacy? Output type
OpenSSL 3 N modern
OpenSSL 3 Y legacy
OpenSSL 1.1 N modern
OpenSSL 1.1 Y legacy

As you can see, this makes the output more consistent.

@TinCanTech
Copy link
Collaborator

TinCanTech commented Feb 21, 2024

do you want more?

I think you have covered everything. If there are further questions, I'm sure that you will have suitable answers.

Thank you for these details, they make all the difference.

@TinCanTech
Copy link
Collaborator

TinCanTech commented Feb 21, 2024

Before I proceed further with this, I would like to hear some feedback from more senior members of OpenVPN.

This is an unusual change in that, EasyRSA is taking a different default position with regard to using OpenSSL. Otherwise, EasyRSA follows OpenSSL defaults and then allows options to modify those defaults.

This is the only example, that I can think of, where EasyRSA is taking an active role in what we expect of OpenSSL. Namely, forcing OpenSSL 1x to behave as 3x.

This is a decision regarding EasyRSA policy toward the OpenSSL 1x to 3x transition.

I think this change is a good idea but I have been wrong before.

@TinCanTech
Copy link
Collaborator

I believe that the issue being addressed here is directly related to OpenVPN/openvpn#505

Awaiting feedback from Openvpn..

@cron2
Copy link

cron2 commented Feb 23, 2024

When creating "crypto stuff", it makes sense to always use reasonably-recent algorithms - so generally, going for modern-format .p12 is the right thing to do. In this case, with OpenVPN as consumer in mind, doubly so ;-) - because those "I have upgraded OpenVPN and all of a sudden my trusty old .p12 file does not load anymore" complaints land on our plate.

So, from OpenVPN upstream, this is a welcome change (not having looked at the code, but the conceptual change).

@selvanair
Copy link

I believe that the issue being addressed here is directly related to OpenVPN/openvpn#505

[OpenVPN/openvpn#505] is about tpm2-pkcs11 engine not working with OpenSSL 3. Its not up to OpenVPN to make it work. Instead tpm2-pkcs11 has to be made compatible with OpenSSL 3.

For the end user, a more practical option is to use tpm2-openssl which should work with OpenVPN+OpenSSL3 at least in theory. Or we can help to make it work.

That said, the legacy pkcs12 issue is unrelated. If easy-rsa is outputting pkcs12 files it looks useful to provide an option to use a more modern encryption instead of sticking to the OpenSSL default. I suppose that's the intent of this PR.

This is the only example, that I can think of, where EasyRSA is taking an active role in what we expect of OpenSSL. Namely, forcing OpenSSL 1x to behave as 3x.

I wouldn't characterize it that way. Its more like not using default settings for pkcs12 creation on OpenSSL 1 as that is no longer considered secure.

That said this is not a show-stopper as OpenVPN linked to OpenSSL3 can be made to read legacy pkcs12 files by loading the "legacy" provider.

@schwabe
Copy link

schwabe commented Feb 24, 2024

In case of pkcs12, macOS and Android will not import those AES256 ones. For those you actually still need the 3DES variants. PKCS12 support is currently an absolute minefield.

@TinCanTech TinCanTech added enhancement Full-Approval Merge is imminent Community reveiwed Genuine community participation Version 3.2.0-Release and removed discussion labels Feb 24, 2024
@TinCanTech TinCanTech added this to the v3.2.0 milestone Feb 24, 2024
@TinCanTech
Copy link
Collaborator

TinCanTech commented Feb 24, 2024

@cron2

from OpenVPN upstream, this is a welcome change

Good news!

@selvanair

Its more like not using default settings for pkcs12 creation on OpenSSL 1 as that is no longer considered secure

Good point!

@schwabe

macOS and Android will not import those AES256 ones

Thank you. EasyRSA retains 'legacy' option for this landmine!

@adaugherity

I am happy to merge this as-is.

However, with your permission, I will make some subsequent, minor changes to "style".

Thank you everyone, for your help.

@adaugherity
Copy link
Author

Yes, you have my permission to make style changes. Thanks!

@TinCanTech
Copy link
Collaborator

@adaugherity This PR cannot be merged because the commit is not signed.

I have an alternative PR prepared, which refers to this as the originating code.

The second option above would save us all some time. ;-)

@TinCanTech
Copy link
Collaborator

Superseded-by: #1083

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community reveiwed Genuine community participation enhancement Full-Approval Merge is imminent PKCS Signature Signature Required Version 3.2.0-Release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants