diff --git a/workcontext/decode.go b/workcontext/decode.go index a644e301..96b8ccd2 100644 --- a/workcontext/decode.go +++ b/workcontext/decode.go @@ -39,11 +39,11 @@ func decodeClaims(encoded string) (*basev0.WorkContextV1, []byte, []byte, error) if !found { return nil, nil, nil, fmt.Errorf("%w: token is not .", ErrInvalid) } - claims, err := base64.RawURLEncoding.DecodeString(payload) + claims, err := decodeSegment(payload) if err != nil { return nil, nil, nil, fmt.Errorf("%w: payload is not base64url: %v", ErrInvalid, err) } - sig, err := base64.RawURLEncoding.DecodeString(signature) + sig, err := decodeSegment(signature) if err != nil { return nil, nil, nil, fmt.Errorf("%w: signature is not base64url: %v", ErrInvalid, err) } @@ -106,6 +106,20 @@ func decodeClaims(encoded string) (*basev0.WorkContextV1, []byte, []byte, error) return wc, claims, sig, nil } +// decodeSegment requires the exact unpadded base64url spelling. DecodeString +// alone ignores CR/LF and accepts nonzero unused bits. Re-encoding catches both +// while preserving empty signatures for the authentication check to refuse. +func decodeSegment(segment string) ([]byte, error) { + decoded, err := base64.RawURLEncoding.DecodeString(segment) + if err != nil { + return nil, err + } + if base64.RawURLEncoding.EncodeToString(decoded) != segment { + return nil, fmt.Errorf("noncanonical base64url encoding") + } + return decoded, nil +} + // unknownFieldIn reports the path of the first unknown field it finds, // recursively, or "" when there is none. Recursion is the point: an unknown // field nested inside a seal or an actor hop is as foreign as one at the top, diff --git a/workcontext/decode_test.go b/workcontext/decode_test.go index 2a91754e..43f3a781 100644 --- a/workcontext/decode_test.go +++ b/workcontext/decode_test.go @@ -564,3 +564,66 @@ func TestRecheckReVerifiesUnderTheKeyTheVerifierHoldsNow(t *testing.T) { require.ErrorIs(t, err, workcontext.ErrInvalid) require.ErrorContains(t, err, "no longer holds, the key") } + +// Equivalent base64 bytes must not create alternate spellings of a credential. +func TestTokenSegmentsRequireCanonicalBase64URL(t *testing.T) { + h := newHarness(t) + aud := audience + token, _ := h.ownerSession(aud) + payload, signature, _ := strings.Cut(token, ".") + if len(payload)%4 == 0 { + aud += "x" + token, _ = h.ownerSession(aud) + payload, signature, _ = strings.Cut(token, ".") + } + require.NotZero(t, len(payload)%4, "payload must exercise unused bits") + checks := map[string]func(string) error{ + "Decode": func(token string) error { _, err := workcontext.Decode(token); return err }, + "Verify": func(token string) error { _, err := h.verify(aud, token); return err }, + "Authenticate": func(token string) error { + _, err := h.authenticator(aud).Authenticate(context.Background(), token) + return err + }, + "Inspect": func(token string) error { _, err := h.inspector().Inspect(token, aud); return err }, + } + for name, check := range checks { + t.Run(name, func(t *testing.T) { + require.NoError(t, check(token), "canonical control") + for _, segment := range []struct{ name, text string }{{"payload", payload}, {"signature", signature}} { + const alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_" + last := len(segment.text) - 1 + unusedBit := segment.text[:last] + string(alphabet[strings.IndexByte(alphabet, segment.text[last])^1]) + original, err := base64.RawURLEncoding.DecodeString(segment.text) + require.NoError(t, err) + equivalent, err := base64.RawURLEncoding.DecodeString(unusedBit) + require.NoError(t, err) + require.Equal(t, original, equivalent, "only unused bits changed") + for variant, text := range map[string]string{ + "unused-bit": unusedBit, + "padding": segment.text + "=", + "double-padding": segment.text + "==", + "CR": segment.text[:1] + "\r" + segment.text[1:], + "LF": segment.text[:1] + "\n" + segment.text[1:], + "CRLF": segment.text[:1] + "\r\n" + segment.text[1:], + } { + t.Run(segment.name+"/"+variant, func(t *testing.T) { + altered := text + "." + signature + if segment.name == "signature" { + altered = payload + "." + text + } + err := check(altered) + require.ErrorIs(t, err, workcontext.ErrInvalid) + require.ErrorContains(t, err, segment.name) + require.ErrorContains(t, err, "base64url") + }) + } + } + }) + } + // Empty signatures are structurally decodable; authentication still refuses + // them as signatures, rather than changing their established classification. + require.NoError(t, checks["Decode"](payload+".")) + for _, name := range []string{"Verify", "Authenticate", "Inspect"} { + require.ErrorContains(t, checks[name](payload+"."), "signature") + } +}