google / webcrypto.dart

Cross-platform implementation of Web Cryptography APIs
https://pub.dev/packages/webcrypto
Apache License 2.0
71 stars 43 forks source link

Validate JWK for HmacSecretKey Class #109

Open HamdaanAliQuatil opened 1 month ago

HamdaanAliQuatil commented 1 month ago

Our goal is to make the API hard to use incorrectly. For the HmacSecretKey class we would want to validate the properties on importJsonWebKey against imported JWK to reduce the risk of accidentally using incorrect JWK. Below are a few options suggested by @jonasfj:

  1. Make properties on importJsonWebKey optional, and validate them when present.
  2. Make the properties required and validate with jwk.

We can try a mix for different parameters and we have a consensus of requiring the alg property to make it consistent with other JWK imports. However, note that the RFC 7517 states that the alg property is indeed optional and this should be kept into consideration while drafting a permanent solution for webcrypto.dart.

I will post my discovery here and would love to hear opinions on the same.

jonasfj commented 1 month ago

This probably doesn't just apply to HmacSecretKey, but all places where we do importJsonWebKey.

I think on the web window.crypto.subtle.importKey always requires that you explicitly specify:

We've largely opted not to do (c), see discussion in https://github.com/google/webcrypto.dart/issues/53.

But yes, in some cases parts of (b) could be omitted. In particular when talking about JWKs.

I guess the benefit of reading the hash from the JWK, is that it's easier to just download a JWK from somewhere and use it. Where as, the way the API currently looks you have to read the alg property of the JWKand switch onHS1,HS256, etc, to pass instances ofHash.sha1orHash.sha256in thehash` parameter. This isn't super convenient.

On the other hand, if you download a JWK from somewhere without checking to see what hash it's using, you might unintentionally be using a weaker hash than you intended. And on most use cases you probably know what kind of algorithm you expect to be using, and you shouldn't want your code to dynamically use a different hash.

So perhaps it's best to say that you can import a JWK, but you must specify what you expect the JWK to be.

And if you want to support importing any kind if JWK, then it's in convenient, and you'll have read the JWK alg, etc...

I'm actually leaning towards the current API, just because we don't necessarily want this package to provide something that is possibly too smart. And maybe it's better to be simple and a little inconvenient. And if someone really wants a smart crypto library that an import any JWK by just parsing the JWK, then maybe that someone should make a high-level package with a high-level API for this purpose.

jonasfj commented 1 month ago

As such maybe the only take away from this is:

But I'm also interested in hearing other perspectives.