RustCrypto / formats

Cryptography-related format encoders/decoders: DER, PEM, PKCS, PKIX
235 stars 125 forks source link

PKCS#12 / PFX support #3

Closed jklong closed 2 weeks ago

jklong commented 2 years ago

Thoughts on adding PKCS12/RFC7292 and (eventually) complete or near complete PFX support? It's a monster and it sucks but it's in wide use thanks to Windows' use of the format.

I'm happy to take ownership of the feature and start work but can't guarantee any timelines on it

tarcieri commented 2 years ago

This is definitely something we'd love to have, thanks for volunteering! We also happen to be the owners of the pkcs12 crate.

(Also FYI, we have a brand new "formats" repo, and I've just transferred the issue there)

It's a monster and it sucks

My main recommendation would be to keep the scope of the initial implementation small rather than try to provide a comprehensive implementation of the full specification.

I would suggest supporting a single private key and its associated certificate chain in an initial implementation.

Also I believe someone has already implemented the PKCS#12 KDF and wanted to upstream it to this project, but offhand I don't remember where I discussed that. I can attempt to track that work down.

jklong commented 2 years ago

My main recommendation would be to keep the scope of the initial implementation small rather than try to provide a comprehensive implementation of the full specification.

Absolutely. I was just gauging appetite overall before starting.

Also I believe someone has already implemented the PKCS#12 KDF and wanted to upstream it to this project, but offhand I don't remember where I discussed that. I can attempt to track that work down.

If you do have something that would be great.

xemwebe commented 1 year ago

Also I believe someone has already implemented the PKCS#12 KDF and wanted to upstream it to this project, but offhand I don't remember where I discussed that. I can attempt to track that work down.

I require exactly the PKCS#12 KDF to support communication with some legacy system. Is this already somewhere else implemented or still an open issue?

tarcieri commented 1 year ago

Unfortunately I wasn't able to find where I saw that. Perhaps it can be found with GitHub Code Search

xemwebe commented 1 year ago

I searched GitHub already, but without success. Meanwhile, I implemented the KDF in rust myself. It is not that complicated. If you think this is interesting enough, I could prepare a PR for that. Should it go here or would the KDF repository the better place for that.

tarcieri commented 1 year ago

Could go either way on that. Maybe start here and we can factor it out if it makes sense.

xemwebe commented 1 year ago

The POC for that can be found here: https://github.com/xemwebe/formats/tree/pkcs12_kdf/pkcs12 The code needs to be generalized for accepting arbitrary digest methods, and I probably need some time to understand how to fit this in the existing framework.

The KDF accepts in general passwords (which needs to be converted to unicode) and IVs of arbitrary lengths and returns a key of arbitrary length. This seems to require feature "alloc" to be enabled. Is this acceptable, or would you prefer some variant with predefined arrays of fixed length (returning an error if the size does not fit)?

carl-wallace commented 1 year ago

FYI, I am going to try to move #836 from the pkcs7 crate to the cms crate and generally align it with current. Can give a look at the KDF in context once that is done. It may be next week before I get to that though.

xemwebe commented 1 year ago

The KDF works now with various digest methods and I have prepared a PR #1154. All test cases have been checked against openssl. If you have any suggestions or improvements, please let me know.

xemwebe commented 1 year ago

I have posted an update: zeroizing intermediate secrets, removing the unused error type and add test case for very long output keys that check some code regions not covered in the other tests.

xemwebe commented 1 year ago

I believe to have addressed all reviewer's remarks regarding PR #1154 so far and will wait for further reviewer suggestions, if any.

tarcieri commented 2 weeks ago

We've merged #1154 and published a pkcs12 crate, so I'd say we can close this.