[v2] cannot "sign" JWT with "none" about jwx HOT 14 CLOSED

@karelbilek For completeness' sake, when you say "This should work", what are you expecting it to do, and what is it doing instead?

from jwx.

Well, you can see how the other library does it.

In case you didn't notice, I wrote this library because I don't agree with many of the decisions that "other" libraries made (with all due respect to their efforts), so I need to think this over. In general I don't think that "none" should be allowed at all, but I'm trying to think of a compromise.

from jwx.

@karelbilek FYI In branch gh-888, I've implemented a jws.Sign() that allows generating JWS with alg=none. However, before merging or releasing, I need to think of how to handle it in jws.Verify() (or not) as well as how to document it.

Note to self: need to test for the case where jws.WithInsecureNoSignature() and jwt.WithKey are used at the same time

from jwx.

@karelbilek after a bit of tinkering, I think the correct way is:

• Allow signing with alg=none, when passed with an explicit option
• DO NOT allow alg=none in verification
• Ask the user to use jws.Parse if they really want to accept messages with alg=none

Please see #890 and let me know

from jwx.

Note, I haven't reviewed the code yet, but my immediate thought is probably this should just return an error.

if _, err := jwt.Sign(token, jwt.WithKey(jwa.NoSignature, nil)); err != nil {
   panic(err) // should get here
}

Is this what you want?

from jwx.

I want to create a JWT token with alg "none".

This should not return an error.

I will show an example later.

from jwx.

Oh, you actually want a "none"... Okay, I think I made it so that it refuses to do it. Anyways, an isolated test case would be nice just to make sure we're talking about exactly the same thing.

from jwx.

expected:

package main

import (
	"fmt"

	"github.com/lestrrat-go/jwx/v2/jwa"
	"github.com/lestrrat-go/jwx/v2/jwt"
)

func main() {
	token := jwt.New()
	token.Set(jwt.SubjectKey, "foo")
	token.Set(jwt.IssuerKey, "bar")

	signed, err := jwt.Sign(token, jwt.WithKey(jwa.NoSignature, nil))
	if err != nil {
		panic(err)
	}

	// output: eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJpc3MiOiJiYXIiLCJzdWIiOiJiYXIifQ.
	fmt.Printf("%s", signed)
}

Comparison with golang-jwt:

package main

import (
	"fmt"

	golangJWT "github.com/golang-jwt/jwt/v4"
)

func main() {
	token := golangJWT.NewWithClaims(golangJWT.SigningMethodNone, golangJWT.MapClaims{
		"sub": "bar",
		"iss": "bar",
	})
	signed, err := token.SignedString(golangJWT.UnsafeAllowNoneSignatureType)
	if err != nil {
		panic(err)
	}
	fmt.Printf("%s", signed)
}

from jwx.

Right, okay. We just don't allow "none". Let me get back to you in a few days, as I need to make up my mind on how/if I want to allow "none". Off the top of my head, I think if we allow the use of "none" it must be made a special case that requires an explicit opt-in like an extra option argument. But I need to think over what the best way is.

from jwx.

Well, you can see how the other library does it.

The main reason why I want this is to write some tests against code that it really doesn't allow "none", that's all.

from jwx.

Option 1: extra option

jwt.Sign(token, jwt.WithKey(jwa.NoSignature, nil), jwt.WithAllowNone(true))

pros: explicit, minimal change to API
cons: there's no way to check if the "none" algo slipped in by mistake

Option 2: special option

jwt.Sign(token, jwt.WithNoSignature())

pros: very explicit.
cons: new API

Option 3: New function

jwt.SignNone(token)

pros: very very explicit
cons: new API

from jwx.

(These are just my notes, please ignore)

• A new signer object for "none" is required.
• jws.Verify should mirror the behavior of jws.Sign wrt to "none" algorithm
• A separate, explicit option is probably better API wise.
• We should return an error when we find the use of "none" with jws.WithKey

Unfortunately, this probably doesn't work?

jws.Verify(jwsmsg, jws.WithInsecureNoneAlgorithm())

This breaks when alg = "none" and jws.WithKeyUsed is passed? Could be a documented behavior, but seems a bit confusing... error out when that happens? Also, what happens when jws.WithKey or jws.WithKeyProvider is passed along with jws.WithInsecureNoneAlgorithm? which one takes precedence?

Also, what should happen for JWS messages with multiple signatures, which include "none"?

from jwx.

minor note: the original test code said it expected eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJpc3MiOiJiYXIiLCJzdWIiOiJiYXIifQ., but that payload is {"sub": "bar", "iss": "bar"}, whereas the code produced {"sub":"foo", "iss":"bar"}

from jwx.

Perfect!

from jwx.