diff --git a/AGENTS.md b/AGENTS.md index 0def818..ffa217b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,7 +18,7 @@ All functionality lives in package `main`, split across six source files: - `readInteractive()` - Prompts for a token interactively using `chzyer/readline` - `decodeAndPrint()` / `printParsedJWT()` - `decodeAndPrint` parses the JWT with `parseUnverifiedJWT` and hands the result to `printParsedJWT`, which orchestrates output and verifies the signature when a key is provided. `printParsedJWT` runs `formatTimestamps` on a `maps.Clone` of the claims, so the display rewrite never touches the authoritative parse and the same `parsedJWT` can still be validated - `parsedJWT` / `parseUnverifiedJWT()` / `splitCompactJWT()` / `decodeJSON()` / `isJSONWhitespace()` - Strictly decode the header, claims, and other displayed JSON with exact `json.Number` values and reject malformed or trailing JSON data. `decodeJSON` decides the ordinary no-trailing-data case with `isJSONWhitespace` over what follows the value, and only hands the input back to the decoder — which is what phrases the rejection — when something else is there. That set is JSON's own four whitespace characters, not Unicode's: a vertical tab after a value is trailing data and must still reach the decoder that says so. `parseUnverifiedJWT` returns a `parsedJWT` (the segments, decoded header and claims, signing method, and decoded signature bytes) that is threaded through the decode, signature, and claim steps, so one run parses the token once instead of once per step. It does the segment work itself rather than calling `jwt.ParseUnverified`, whose plain `json.Unmarshal` of the header and claims loses number precision and accepts trailing data — every one of its decodes would be discarded and redone strictly here. The remaining steps mirror `ParseUnverified` exactly, `splitCompactJWT`'s rejection of extra delimiters and the refusal of a missing or unknown `alg` included, so the set of tokens jwtd decodes is unchanged -- `verifyJWTSignature()` / `validMethodsForKey()` / `printSignatureVerdict()` - `verifyJWTSignature` takes a `parsedJWT` and does the cryptographic check without printing, calling the parsed token's `SigningMethod.Verify` over its own segments rather than re-parsing the compact string through `jwt.Parse`; the claims are never consulted, so the result reflects only the signature, not expiry. **It must check the header `alg` against `validMethodsForKey` before verifying.** That allowlist is what `jwt.WithValidMethods` applied while verification went through `jwt.Parse`; verifying from an already-parsed token makes it jwtd's own check, and without it an HS256 token signed with the bytes of a published public key verifies against that key. `TestVerifyJWTSignature_RejectsAlgOutsideKeyTypeAllowlist` holds it down across every key type. `verifyJWTSignature` returns `valid`, an invalid-signature `reason`, and a separate hard `err` (unusable key). `printSignatureVerdict` renders `Signature: VALID`/`INVALID` from it and returns the `errInvalidSignature` sentinel on failure so the CLI exits nonzero. The `--json` path reuses the same core. Production code always holds a `parsedJWT` at this point, so there is no string-taking wrapper in the package; the tests keep one (`verifySignature` in `helpers_test.go`) for the cases that start from a compact string +- `verifyJWTSignature()` / `validMethodsForKey()` / `printSignatureVerdict()` - `verifyJWTSignature` takes a `parsedJWT` and does the cryptographic check without printing, calling the parsed token's `SigningMethod.Verify` over its own segments rather than re-parsing the compact string through `jwt.Parse`; the claims are never consulted, so the result reflects only the signature, not expiry. **It must check the header `alg` against `validMethodsForKey` before verifying.** That allowlist is what `jwt.WithValidMethods` applied while verification went through `jwt.Parse`; verifying from an already-parsed token makes it jwtd's own check, and without it an HS256 token signed with the bytes of a published public key verifies against that key. `TestVerifyJWTSignature_RejectsAlgOutsideKeyTypeAllowlist` holds it down across every key type. The allowlist fails closed: `validMethodsForKey` returns an empty list for a key type it does not know (an X25519 key, which parses as a valid JWE key but can verify no JWS), and `verifyJWTSignature` rejects such a key outright instead of skipping the check and leaving the refusal to each `Verify` implementation's type assertion; `TestVerifyJWTSignature_RejectsKeyTypeWithoutJWSAlgorithms` pins that. `verifyJWTSignature` returns `valid`, an invalid-signature `reason`, and a separate hard `err` (unusable key). `printSignatureVerdict` renders `Signature: VALID`/`INVALID` from it and returns the `errInvalidSignature` sentinel on failure so the CLI exits nonzero. The `--json` path reuses the same core. Production code always holds a `parsedJWT` at this point, so there is no string-taking wrapper in the package; the tests keep one (`verifySignature` in `helpers_test.go`) for the cases that start from a compact string - `publicKeyForVerification()` - Extracts the public key from RSA/ECDSA/Ed25519 private keys ### `formatter.go` - Colored JSON rendering diff --git a/jsonout_test.go b/jsonout_test.go index 265912a..44e99d6 100644 --- a/jsonout_test.go +++ b/jsonout_test.go @@ -164,6 +164,7 @@ func TestApplyColorMode(t *testing.T) { {name: "never disables color", mode: "never", wantNoColor: true}, {name: "json forces color off regardless", mode: "always", jsonOut: true, wantNoColor: true}, {name: "invalid value errors", mode: "bogus", wantErr: true}, + {name: "invalid value errors under json too", mode: "bogus", jsonOut: true, wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/jwe.go b/jwe.go index 202d5de..4a82de9 100644 --- a/jwe.go +++ b/jwe.go @@ -83,36 +83,37 @@ func decodeAndPrintJWE(w io.Writer, tokenStr, keyStr string) error { return fmt.Errorf("parsing JWE: %w", err) } - f := newFormatter() - header, err := jweProtectedHeaderMap(tokenStr) if err != nil { return err } + + // The key is resolved before anything is printed, so an unusable key + // fails with the error alone instead of a partial section ahead of it. + var key any + if keyStr != "" { + key, err = loadKeyForKID(keyStr, headerKID(header)) + if err != nil { + return fmt.Errorf("loading decryption key: %w", err) + } + } + + f := newFormatter() if err := printSection(w, f, "Protected Header", header); err != nil { return err } + if _, err := fmt.Fprintln(w); err != nil { + return err + } if keyStr == "" { - if _, err := fmt.Fprintln(w); err != nil { - return err - } return printEncryptedParts(w, tokenStr) } - key, err := loadKeyForKID(keyStr, headerKID(header)) - if err != nil { - return fmt.Errorf("loading decryption key: %w", err) - } - plaintext, err := jwe.Decrypt(key) if err != nil { return fmt.Errorf("decrypting JWE: %w", err) } - - if _, err := fmt.Fprintln(w); err != nil { - return err - } return printDecryptedPayload(w, f, plaintext) } diff --git a/jwe_test.go b/jwe_test.go index 87991b1..ca0071c 100644 --- a/jwe_test.go +++ b/jwe_test.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "encoding/json" "os" + "path/filepath" "strings" "testing" "time" @@ -206,6 +207,23 @@ func TestDecodeAndPrintJWE_WrongKey(t *testing.T) { } } +// TestDecodeAndPrintJWE_UnusableKeyPrintsNothing pins that the key is resolved +// before any section is written, so an unusable key yields the error alone +// rather than a partial Protected Header ahead of it. +func TestDecodeAndPrintJWE_UnusableKeyPrintsNothing(t *testing.T) { + key := generateRSAKey(t) + token := encryptJWE(t, key, []byte(`{"sub":"user1"}`)) + + var buf bytes.Buffer + err := decodeAndPrintJWE(&buf, token, filepath.Join(t.TempDir(), "missing.pem")) + if err == nil || !strings.Contains(err.Error(), "loading decryption key") { + t.Fatalf("expected key loading error, got: %v", err) + } + if buf.Len() != 0 { + t.Errorf("expected no output before the key error, got:\n%s", buf.String()) + } +} + func TestDecodeAndPrintJWE_NonJSONPayload(t *testing.T) { key := generateRSAKey(t) token := encryptJWE(t, key, []byte("plain text content, not JSON")) diff --git a/main.go b/main.go index 512d8f2..da880b3 100644 --- a/main.go +++ b/main.go @@ -23,6 +23,11 @@ import ( var errInvalidSignature = errors.New("invalid signature") +// errNoToken is returned when stdin or the interactive prompt supplied nothing +// but whitespace, so an empty pipe is reported as such rather than as a +// malformed token. +var errNoToken = errors.New("no token provided") + // version is set at build time via -ldflags. var version = "dev" @@ -145,19 +150,19 @@ func decodeJWTHuman(w io.Writer, tokenStr, keyStr string, checks claimChecks) er // color even when piped; "never" disables it. --json output is plain JSON, so // color is always off there regardless of the flag. func applyColorMode(mode string, jsonOut bool) error { - if jsonOut { - color.NoColor = true - return nil - } + // The value is validated before --json is consulted, so a typo is + // reported the same way whether or not JSON output is on. switch mode { - case "auto": - case "always": - color.NoColor = false - case "never": - color.NoColor = true + case "auto", "always", "never": default: return fmt.Errorf("invalid --color value %q: use auto, always, or never", mode) } + switch { + case jsonOut, mode == "never": + color.NoColor = true + case mode == "always": + color.NoColor = false + } return nil } @@ -208,7 +213,11 @@ func readToken(args []string) (string, error) { if err != nil { return "", fmt.Errorf("reading stdin: %w", err) } - return sanitizeToken(string(data)), nil + token := sanitizeToken(string(data)) + if token == "" { + return "", errNoToken + } + return token, nil } return readInteractive() @@ -240,7 +249,7 @@ func readInteractive() (token string, err error) { token = sanitizeToken(line) if token == "" { - return "", fmt.Errorf("no token provided") + return "", errNoToken } return token, nil } @@ -422,8 +431,16 @@ func verifyJWTSignature(p *parsedJWT, keyStr string) (valid bool, reason error, // it has to be spelled out here, and it must stay ahead of the Verify call // below — without it an HS256 token signed with a published public key // verifies against that key. + // + // A key type with no JWS algorithms at all (an X25519 key, which is a + // valid JWE key but can sign nothing) is rejected here as well, rather + // than relying on every Verify implementation to refuse the Go type. alg := p.method.Alg() - if methods := validMethodsForKey(key); methods != nil && !slices.Contains(methods, alg) { + methods := validMethodsForKey(key) + if len(methods) == 0 { + return false, fmt.Errorf("%w: key type %T cannot verify a JWS", jwt.ErrTokenSignatureInvalid, key), nil + } + if !slices.Contains(methods, alg) { return false, fmt.Errorf("%w: signing method %v is invalid", jwt.ErrTokenSignatureInvalid, alg), nil } @@ -446,7 +463,9 @@ func headerKID(header map[string]any) string { } // validMethodsForKey returns the JWS algorithm names compatible with the -// given verification key type, or nil for unknown key types. +// given verification key type. An unknown key type gets an empty list, which +// verifyJWTSignature treats as "verifies nothing": the allowlist fails closed +// instead of being skipped. func validMethodsForKey(key any) []string { switch key.(type) { case *rsa.PublicKey: diff --git a/main_test.go b/main_test.go index bdcb425..503a098 100644 --- a/main_test.go +++ b/main_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "crypto/ecdh" "crypto/ed25519" "crypto/rand" "crypto/x509" @@ -11,6 +12,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "strings" "testing" "time" @@ -354,6 +356,31 @@ func TestReadToken_FromStdinPipe_WrappedToken(t *testing.T) { } } +// TestReadToken_EmptyStdinPipe pins that a pipe carrying only whitespace is +// reported as "no token provided", matching the interactive prompt, instead of +// surfacing as a malformed-token parse error. +func TestReadToken_EmptyStdinPipe(t *testing.T) { + origStdin := os.Stdin + + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("creating pipe: %v", err) + } + + go func() { + fmt.Fprint(w, " \n") + w.Close() + }() + + os.Stdin = r + defer func() { os.Stdin = origStdin }() + + _, err = readToken([]string{}) + if !errors.Is(err, errNoToken) { + t.Fatalf("expected errNoToken, got: %v", err) + } +} + func TestReadToken_FromStdinPipe(t *testing.T) { origStdin := os.Stdin @@ -1199,6 +1226,49 @@ func TestVerifyJWTSignature_RejectsAlgOutsideKeyTypeAllowlist(t *testing.T) { } } +// TestVerifyJWTSignature_RejectsKeyTypeWithoutJWSAlgorithms pins that the +// allowlist fails closed for a key type it does not know. An X25519 key parses +// (it is a valid JWE key) but can verify no JWS algorithm, so the allowlist +// itself must refuse it — before any Verify implementation is asked to. +func TestVerifyJWTSignature_RejectsKeyTypeWithoutJWSAlgorithms(t *testing.T) { + x25519Key, err := ecdh.X25519().GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("generating X25519 key: %v", err) + } + der, err := x509.MarshalPKCS8PrivateKey(x25519Key) + if err != nil { + t.Fatalf("marshalling X25519 key: %v", err) + } + keyPath := filepath.Join(t.TempDir(), "x25519.pem") + if err := os.WriteFile(keyPath, pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}), 0o600); err != nil { + t.Fatalf("writing key file: %v", err) + } + + if methods := validMethodsForKey(x25519Key); len(methods) != 0 { + t.Fatalf("validMethodsForKey(X25519) = %v, want none", methods) + } + + for _, token := range []string{ + signJWTWithHMAC(t, []byte("a-shared-secret-of-sufficient-len"), jwt.MapClaims{"sub": "test"}), + signJWT(t, generateRSAKey(t), jwt.MapClaims{"sub": "test"}), + } { + p, err := parseUnverifiedJWT(token) + if err != nil { + t.Fatalf("unexpected parse error: %v", err) + } + valid, reason, err := verifyJWTSignature(p, keyPath) + if err != nil { + t.Fatalf("unexpected hard error: %v", err) + } + if valid { + t.Fatal("signature accepted against a key type that can verify no JWS") + } + if reason == nil || !strings.Contains(reason.Error(), "cannot verify a JWS") { + t.Errorf("expected key-type rejection in reason, got: %v", reason) + } + } +} + // TestParseUnverifiedJWT_RejectsMalformedTokens pins the token shapes the // decoder refuses. parseUnverifiedJWT splits and decodes the segments itself // instead of calling jwt.ParseUnverified, so these rejections are jwtd's own diff --git a/output.go b/output.go index 98a7d01..4bd1b37 100644 --- a/output.go +++ b/output.go @@ -494,11 +494,10 @@ func printSection(w io.Writer, f *jsonFormatter, label string, data any) error { // megabytes — goes straight to the writer instead of through a second copy of // itself. func writeFormattedJSON(w io.Writer, pretty []byte) error { - if !isBelowDEL(pretty) { - _, err := fmt.Fprintln(w, escapeFormattedJSONControls(pretty)) - return err - } - if _, err := w.Write(pretty); err != nil { + // escapeFormattedJSONControls decides the fast path itself, so the buffer + // is scanned once here rather than once per function. + escaped := escapeFormattedJSONControls(pretty) + if _, err := io.WriteString(w, escaped); err != nil { return err } _, err := io.WriteString(w, "\n")