feat: derive from Clone on PrivateKeyDer enum

[ologbonowiwi]

No description provided.

[ctz]

I think we specifically want to avoid making private key material types Clone, so it is possible to minimise the number of places they exist in memory. What problem are you trying to solve?

[cpu]

Hi @ologbonowiwi

Can you speak to your motivation for this change?

In the past we've explicitly avoided adding Clone to private key types as it can lead to leaking private key bytes.

Edit: oops, sorry for the double post :-) Looks like Ctz and I were racing.

[cpu]

Some previous discussion in #8, #26, rustls/pemfile#19

[ologbonowiwi]

I have a project where I depend on these keys, and I'd like to have the ability to Clone a struct that is an array of PrivateKeyDer.

Due to an architectural decision on the project I'm working on (tailcallhq/tailcall), we have restrictions on where to do IO operations, and we wish to evaluate these keys early and use them later on.

If I can't clone, what I'll end up having to do is holding the bytes/string (which is clonable, can be leaked, and will not have the explicitly of knowing it is a private key), and later on (right before using it), call rustls_pemfile::read_all with the bytes that hold the private key in plain text

[ologbonowiwi]

And also, thanks for the quick response! 😄

[ctz]

These objects are immutable, so having multiple independent copies is not really useful. Have you considered wrapping them in Rc/Arc? This gives you (~free/cheap) clone without any underlying copies happening.

[ologbonowiwi]

It makes sense @ctz! I'm quite new to the Rust ecosystem, so I didn't knew about this "workaround". Thanks for the hand!

[cpu]

I think this is a place where we could help users with more documentation on the rationale for not including Clone: #30

[ologbonowiwi]

Hey guys, update on my use case: early evaluate the Item worked to move it around the application, but it did not work to load the file, as TlsAcceptor::builder().with_single_cert needs ownership of the key, which can't be given since it can't be cloned and is inside an Arc.

What I'm planning to do to avoid such issue is hold the plain file (content) on the application and late evaluate it to an Item right before the creation of TlsAcceptor. Anyway, thanks for your kindness and velocity on giving me feedback around this issue.

[djc]

You should be able to use clone_key()?

https://docs.rs/rustls-pki-types/latest/rustls_pki_types/enum.PrivateKeyDer.html#method.clone_key

[ologbonowiwi]

It worked @djc, thanks 🙌🏽 was able to achieve a clone-like behavior by using Arc + clone_key.

I apologize for my ignorance, but what's the difference between deriving explicitly from Clone and implementing clone_key, and why clone_key is safe to use?

[djc]

The idea was that clone_key() forces an explicit usage, whereas it would be trivial to oversee deriving Clone of some type that includes a PrivateKeyDer, leading to the key potentially leaking into places it shouldn't go.

It's admittedly a nuanced topic -- this is the line we've ended up at for now.