Skip to content

CompactSerialize (and FullSerialize) can invalidate the signature of a parsed JWS by re-ordering protected header keys #228

Description

@AdrianBannister

When a compact JWS is parsed and then serialized again, go-jose can change the exact protected-header bytes and return a token whose signature no longer matches the serialized output.

This surfaced downstream in cloudflare/cloudflared, where a cached access token is read from disk, parsed, and then re-serialized via CompactSerialize(). The workaround PR is here: cloudflare/cloudflared#1630

That PR includes a unit test (TestGetTokenIfExists_PreservesOriginalTokenString) that directly demonstrates the bug: it crafts a JWS with non-alphabetical header key order ({"typ":"JWT","alg":"RS256"}), shows that CompactSerialize() produces a different string than what was parsed, and confirms the signature is invalidated by the round-trip.

What happens

ParseSigned preserves the original protected-header bytes in signature.original.Protected, and verification already uses those original bytes in computeAuthData.

However, the serialization paths ignore those preserved bytes and instead re-marshal the protected header map:

  • CompactSerialize
  • DetachedCompactSerialize
  • FullSerialize

In jws.go, those paths call mustSerializeJSON(...) on signature.protected. But rawHeader is a map[HeaderKey]*json.RawMessage, and marshaling a Go map sorts keys alphabetically. That changes the exact JSON byte sequence in the protected header, which changes its base64url encoding, which means the signature is no longer valid for the re-serialized token.

Minimal repro

Start with a valid compact JWS whose protected header is:

{"typ":"JWT","alg":"HS256"}

After ParseSigned(...).CompactSerialize(), the protected header gets re-serialized as:

{"alg":"HS256","typ":"JWT"}

Those are semantically equivalent JSON objects, but they are different bytes, and JWS signatures cover the exact base64url-encoded protected header bytes.

A concrete token that reproduces this:

eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjMifQ.avABcH6uR9MnyP0k-uJCIdNCG8-mVd3G0FxWXxkWFOo

Its protected header decodes to {"typ":"JWT","alg":"HS256"}.

A round trip changes the token:

parsed, err := jose.ParseSigned(token, []jose.SignatureAlgorithm{jose.HS256})
if err != nil {
    // ...
}

roundTripped, err := parsed.CompactSerialize()
if err != nil {
    // ...
}

fmt.Println(token == roundTripped) // false

And the new token no longer verifies, because the signature was created over the original protected-header bytes.

Why this seems unintended

There is already code in ParseSigned/computeAuthData that explicitly preserves and uses the original protected-header bytes so parsed JWSs can be handled losslessly, including unknown header fields. The serialization paths bypass that preserved data and re-marshal from the parsed map instead.

Expected behavior

If a JWS was parsed from an existing token and the protected header bytes are available, serializing it again should reuse those original protected-header bytes rather than re-marshal the parsed map.

At minimum, parse -> serialize should not change a valid token into one whose signature no longer matches the serialized output.

Possible fix

When signature.original != nil and signature.original.Protected != nil, use those original protected-header bytes in the serialization paths instead of mustSerializeJSON(signature.protected).

It would also be useful to add regression tests covering:

  • compact serialization round-trips
  • detached compact serialization round-trips
  • full JSON serialization round-trips
  • protected headers with non-alphabetical key order and/or unknown fields

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions