Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add explicit HMAC key type #84

Open
jsha opened this issue Dec 14, 2023 · 1 comment
Open

Add explicit HMAC key type #84

jsha opened this issue Dec 14, 2023 · 1 comment

Comments

@jsha
Copy link
Collaborator

jsha commented Dec 14, 2023

Right now HMAC keys are []byte. That's nice and simple, but it means that they do not carry type information about which algorithm they are meant to be used with. So when verifying a JWS / JWT signature using HMAC, the decision to use HS256, HS384, or HS512 is determined by the submitted JWS "alg" header instead of the application developer.

To solve that, we should wrap HMAC keys in a type that indicates the appropriate algorithm, e.g. type HS256Key struct { key: []byte }.

@jsha
Copy link
Collaborator Author

jsha commented Dec 14, 2023

On thinking about it some more, I realized public key verification has the same problem: the key is trusted to provide the general key type (RSA, ECDSA, Ed25519), but specifics of signing like which hash and which RSA signing algorithm to use are specified by user-supplied data (the "alg" header).

I think the solution is this:

Verify currently takes a big list of types behind an interface{}:

// The verificationKey argument must have one of these types:
//   - ed25519.PublicKey
//   - *ecdsa.PublicKey
//   - *rsa.PublicKey
//   - *JSONWebKey
//   - JSONWebKey
//   - []byte
//   - Any type that implements the OpaqueVerifier interface.

Instead I think it should take exactly one concrete type, *JSONWebKey . That carriers its own Key interface{} , with a similar list of types:

	// Key is the Go in-memory representation of this key. It must have one
	// of these types:
	//  - ed25519.PublicKey
	//  - ed25519.PrivateKey
	//  - *ecdsa.PublicKey
	//  - *ecdsa.PrivateKey
	//  - *rsa.PublicKey
	//  - *rsa.PrivateKey
	//  - []byte (a symmetric key)

But it also carries its own Algorithm field, which allows disambiguating HS256 vs HS384 etc.

For convenience there would be constructors that build a JSONWebKey from parts, e.g. HMACKey(key []byte, alg SigningAlgorithm) *JSONWebKey

But also this is significantly more churn, it's not strictly needed to allow correct applications (it just encourages more correct applications) and we don't have to target everything for the v4 release. So putting this on hold for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant