DecodingKey is difficult to re-use

[tjwilson90]

impl<'a> DecodingKey<'a> {
    pub fn from_rsa_pem(key: &'a [u8]) -> Result<Self> {
        let pem_key = PemEncodedKey::new(key)?;
        let content = pem_key.as_rsa_key()?;
        Ok(DecodingKey {
            family: AlgorithmFamily::Rsa,
            kind: DecodingKeyKind::SecretOrDer(Cow::Owned(content.to_vec())),
        })
    }
}

This constructs a DecodingKey that owns all of its state, but the signature suggests that it borrows the input key. As a result, the only way to construct a DecodingKey<'static> for lazy_static or once_cell is to leak the input key (or unsafely transmute the lifetime of the input key and hope that DecodingKey never changes its implementation to actually.borrow the input).

[Keats]

The intent was indeed to use it in a lazy_static way, something like (pseudo-code)

lazy_static! {
  let decoding_key = {
      let bytes = include_bytes!("public.pem");
      DecodingKey::from_rsa_pem(bytes).unwrap()
  }
}

What would you change?

[tjwilson90]

That works, but only if I embed my public key in my program (which would require me to recompile and redeploy if I change my key, and would make it impossible for a user of my pre-compiled program to generate and use their own keys).

Since the resulting DecodingKey doesn't need to maintain a borrow on the key, the key should have a distinct lifetime, i.e., the type of key should be &[u8], not &'a [u8].

[MayaRainer]

Just ran into this as well. It'd be way more versatile if DecodingKey's methods just borrowed or consumed the key, depending on internal implementation, rather than requiring an external guarantee about the lifetime.

[Keats]

I've pushed a stop-gap change in 7.1.0, more changes can be done later if necessary as breaking change

[dakom]

Right now my setup is something like this:

lazy_static! {
    pub static ref JWT_ENCODING_KEY:EncodingKey = {
        let secret = std::env::var("JWT_SECRET").expect("must have JWT_SECRET set");
        EncodingKey::from_secret(secret.as_ref())
    };

    pub static ref JWT_DECODING_KEY:DecodingKey<'static> = {
        let secret = std::env::var("JWT_SECRET").expect("must have JWT_SECRET set");
        DecodingKey::from_secret(secret.as_ref())
    };

It fails to compile of course.

EncodingKey is fine, but DecodingKey fails since secret is dropped right away.

imho DecodingKey should really just own its data. There's really only two scenarios iirc:

1. Totally new keys. Anyway it has to be re-created, so holding references doesn't help
2. Total re-use, in which case a reference to the DecodingKey is what's needed, and internal references of the DecodingKey don't help.
3. Partial re-use. Here is where it would make sense for the DecodingKey to hold references, so, say, part of its data could be swapped out. I don't think that applies here?

Am I missing something?

[Keats]

#128 should make it easier but that's a breaking change.

[dakom]

How about adding this to that breaking change as well?

impl DecodingKey<'static> {
    pub fn from_secret_clone(secret: &[u8]) -> Self {
        DecodingKey {
            family: AlgorithmFamily::Hmac,
            kind: DecodingKeyKind::SecretOrDer(Cow::Owned(secret.to_vec())),
        }
    }
}

[Keats]

Can we work on a PR with breaking changes for the future 8.0? You can go wild

cc @neoeinstein

As well as any other issues for EncodingKey or really anything else.

[Keats]

Anyone up to do a PR on #160 ?