From 8b8a9f649724f281fcacc52a93a15cf6c1da2186 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 26 Sep 2023 14:09:57 -0400 Subject: [PATCH] WI: add test to verify we don't allow empty signatures for JWT (#18586) Encoded JWTs include an `alg` header key that tells the verifier which signature algorithm to use. Bafflingly, the JWT standard allows a value of `"none"` here which bypasses signature verification. In all shipped versions of Nomad, we explicitly configure verification to a specific algorithm and ignore the header value entirely to avoid this protocol flaw. But in #18123 we updated our JWT library to `go-jose`, which rightfully doesn't support `"none"` but this detail isn't encoded anywhere in our code base. Add a test that ensures we catch any regressions in the library. --- nomad/encrypter_test.go | 53 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/nomad/encrypter_test.go b/nomad/encrypter_test.go index d0c07b1da..ddbfa35dc 100644 --- a/nomad/encrypter_test.go +++ b/nomad/encrypter_test.go @@ -6,6 +6,9 @@ package nomad import ( "bytes" "context" + "encoding/base64" + "encoding/json" + "fmt" "os" "path/filepath" "testing" @@ -375,3 +378,53 @@ func TestEncrypter_SignVerify(t *testing.T) { require.Equal(t, alloc.JobID, got.JobID) require.Equal(t, "web", got.TaskName) } + +func TestEncrypter_SignVerify_AlgNone(t *testing.T) { + + ci.Parallel(t) + srv, shutdown := TestServer(t, func(c *Config) { + c.NumSchedulers = 0 // Prevent automatic dequeue + }) + defer shutdown() + testutil.WaitForLeader(t, srv.RPC) + + alloc := mock.Alloc() + claims := structs.NewIdentityClaims(alloc.Job, alloc, "web", alloc.LookupTask("web").Identity, time.Now()) + e := srv.encrypter + + keyset, err := e.activeKeySet() + must.NoError(t, err) + keyID := keyset.rootKey.Meta.KeyID + + // the go-jose library rightfully doesn't acccept alg=none, so we'll forge a + // JWT with alg=none and some attempted claims + + bodyData, err := json.Marshal(claims) + must.NoError(t, err) + body := make([]byte, base64.StdEncoding.EncodedLen(len(bodyData))) + base64.StdEncoding.Encode(body, bodyData) + + // Try without a key ID + headerData := []byte(`{"alg":"none","typ":"JWT"}`) + header := make([]byte, base64.StdEncoding.EncodedLen(len(headerData))) + base64.StdEncoding.Encode(header, headerData) + + badJWT := fmt.Sprintf("%s.%s.", string(header), string(body)) + + got, err := e.VerifyClaim(badJWT) + must.Error(t, err) + must.ErrorContains(t, err, "missing key ID header") + must.Nil(t, got) + + // Try with a valid key ID + headerData = []byte(fmt.Sprintf(`{"alg":"none","kid":"%s","typ":"JWT"}`, keyID)) + header = make([]byte, base64.StdEncoding.EncodedLen(len(headerData))) + base64.StdEncoding.Encode(header, headerData) + + badJWT = fmt.Sprintf("%s.%s.", string(header), string(body)) + + got, err = e.VerifyClaim(badJWT) + must.Error(t, err) + must.ErrorContains(t, err, "invalid signature") + must.Nil(t, got) +}