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

Add support for PKCS#12 #41

Closed
aidanhs opened this issue Mar 5, 2017 · 7 comments
Closed

Add support for PKCS#12 #41

aidanhs opened this issue Mar 5, 2017 · 7 comments

Comments

@aidanhs
Copy link

aidanhs commented Mar 5, 2017

As briefly discussed in briansmith/ring#224.

@briansmith
Copy link
Owner

I'm using https://tools.ietf.org/html/rfc7292 as the spec reference.

It seems like private keys are stored in PKCS#8 format. So we'd need to add PKCS#8 to ring in order to do this. PKCS#8 is already coming to ring soon, so that's OK.

You mentioned in the ring thread that you want to support the password-encrypted format, specifically. IIUC, the recommended mode is PBES2 from PKCS #5, for which we can use https://tools.ietf.org/html/rfc8018 as the reference. This in turn requires an AES-CBC implementation, which ring doesn't have. But actually many implementations don't actually support PBES2 and don't support AES, so to work with them, one would need to implement PBES1 (instead of, or in addition to, PBES2), presumably using 3DES-CBC. So, no matter what, we'd be adding weaker and obsolete crypto to ring to support this. Not sure that that's a dealbreaker. But, it seems bad that there's absolutely no way to use AES-SIV or AES-GCM-SIV or similar algorithms instead of AES-CBC-MAC or 3DES-CBC-MAC. Am I overlooking something?

Maybe a simpler solution would be to change the native-tls API to accept a pair (unencrypted PKCS#8 private key, CMS-wrapped certificate chain) as an alternative to PKCS#12.

Another potential solution would be to use PKCS#15 as the native-tls standard format. This would require rust-openssl to implement PKCS#15, but that's no big deal. It's no harder than implementing PKCS#12 in webpki + ring, at least.

(See https://unmitigatedrisk.com/?p=543 for background on the current state of badness for PKCS#12 and PKCS#15 and the interop problems.)

@briansmith
Copy link
Owner

See http://toroid.org/pemtrans.

@briansmith
Copy link
Owner

@briansmith
Copy link
Owner

I think we don't need to do this. Check out https://github.com/quininer/tokio-rustls. That replaces tokio-tls and AFAICT it avoids the need to deal with rust-native-tls at all.

@briansmith
Copy link
Owner

I'm going to close this. Maybe at some point in the future we'll need PKCS#12 support but it is far from the most urgent thing we could be doing so I don't even want to review a PR that somebody else would write. Also, I think it makes much more sense, at least in theory, to use tokio-rustls, rather than try to get Rustls working with both tokio-tls + rust-native-tls.

Thanks for submitting the issue though! Feel free to ping me if you need some help with PRs for tokio-rustls.

@aidanhs
Copy link
Author

aidanhs commented Mar 30, 2017

Understood, your position is fair.

However, I will continue to pursue this across the rest of the tls 'ecosystem' - I think it is important that users are able to swap out their tls implementation at any level of the stack (e.g. if a binary like cargo used reqwest for doing https requests, the person compiling cargo should be able to pick rustls without having to make code changes N levels down in the dependency tree).

However, there's a lot of peripheral work that needs to be done before it's viable to do this in a way that works well for crate consumers (the way I'm currently thinking of requires some changes to cargo). As part of the ecosystem changes that would be required, maybe native-tls could be changed to not use PKCS#12 and this problem goes away. We'll see.

@briansmith
Copy link
Owner

As far as your swappability concern goes, I suggest working on PKCS#8 support for the other implementations, rather than PKCS#12 support. Also, I suggest that insofar as you want to have a uniform interface across implementations, it would be better to focus on giving Rustls the ideal API, and then modifying the other implementations to copy Rustls's API. That way, Rustls and the stack below it doesn't have to spend effort on the swappability stuff.

This issue was closed.
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

No branches or pull requests

2 participants