Add clone_key() method to private key types

[djc]

Right now it's possible to clone a PrivateKeyDer, but it's pretty unergonomic. I added this in a work repo:

// `PrivateKeyDer` doesn't implement `Clone` to help avoid accental leaking of the secret key.
// Unfortunately working around this is a bit involved.
fn clone_key(key: &PrivateKeyDer<'_>) -> PrivateKeyDer<'static> {
    match &key {
        PrivateKeyDer::Pkcs1(key) => {
            PrivateKeyDer::from(PrivatePkcs1KeyDer::from(key.secret_pkcs1_der().to_vec()))
        }
        PrivateKeyDer::Sec1(key) => {
            PrivateKeyDer::from(PrivateSec1KeyDer::from(key.secret_sec1_der().to_vec()))
        }
        PrivateKeyDer::Pkcs8(key) => {
            PrivateKeyDer::from(PrivatePkcs8KeyDer::from(key.secret_pkcs8_der().to_vec()))
        }
        _ => unreachable!(),
    }
}

Pave this path, while still avoiding a Clone impl that would make it easy to leak private key material.

Fixes #25.

[ctz]

Is the theory that making people write clone_key() (instead of clone() via std::clone::Clone) will give them more pause to consider if it is really needed? I can be down with that, just want to ensure I'm not missing anything.

[djc]

Is the theory that making people write clone_key() (instead of clone() via std::clone::Clone) will give them more pause to consider if it is really needed? I can be down with that, just want to ensure I'm not missing anything.

Yes, that is the theory. In particular, while .clone() might still be something to pay attention to especially when applied to a binding called key or private_key, it can be quite easy to slap a derive(Clone) on a type which might include the key, which wouldn't work in this case and require a manual Clone implementation. This is analogous to how we don't have an AsRef<[u8]> implementation for the private key types but we do have a secret_*der() method.

ring AFAICT doesn't expose any way of getting at the internals of private keys except for directly after generating one, so it is more strict. I'm hoping to strike a balance that does make it possible but that triggers enough thinking to consider whether it's the right thing to do in context.

[djc]

For crates that have a non-zero major version, ideally we should bump the minor version when new API is added. This is not as strict of a cultural norm in the crates ecosystem as the semver-compatibility rules, but it can help to distinguish when you can downgrade (I'm a little fuzzy on the details right now).

[cpu]

Thanks, this seems reasonable to me. If both you and the OP have needed this it's fair to assume others will too.

[eric-seppanen]

In my case, I didn't want access to the internals of the key, I just had a few layers of abstraction between the place where I acquired the private key and where it gets used. Those intermediate layers generated borrow check errors (because they were async fns that needed to be 'static), so I needed a way to jump from maybe-borrowed to owned.