Repository navigation
feat: RFC 9421 HTTP Message Signatures + shared crypto layer + JWT processor - #166
Merged
Merged
Conversation
…g/verification layer Add a protocol-agnostic crypto layer to enkan-core for use by RFC 9421 HTTP Message Signatures, JWT/JWS, and future signing needs: - CryptoAlgorithm: enum mapping JCA names for HMAC, Ed25519, ECDSA (P-256/P-384/P-521), RSA-PSS, and RSA PKCS#1 v1.5 - Signer / Verifier: minimal byte[]-in/byte[]-out interfaces supporting JCA, HSM, and remote KMS implementations - JcaSigner / JcaVerifier: JCA-based implementations with constant-time HMAC comparison (MessageDigest.isEqual) and RSA-PSS PSSParameterSpec configuration - EcdsaUtils: DER ↔ IEEE P1363 signature format conversion (ported from enkan-bouncr, no BouncyCastle dependency) Tests cover sign→verify roundtrips for all algorithm families, wrong-key rejection, tampered-data detection, and ECDSA P1363 output size validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add JWT (RFC 7519) / JWS (RFC 7515) support to enkan-web: - JwsAlgorithm: maps JWS alg names (HS256, RS256, ES256, EdDSA, etc.) to CryptoAlgorithm constants from the shared crypto layer - JwtHeader: record for JOSE header (typ, alg, kid) - JwtProcessor: stateless sign/verify utility with no JSON library dependency — header is parsed/serialized with a minimal built-in parser; payload JSON (de)serialization is caller-provided via Function<byte[], T>. Validates exp/nbf time claims. Rejects alg:none per security best practice. HMAC verification uses constant-time comparison via the shared JcaVerifier. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ureBaseBuilder RFC 9421 HTTP Message Signatures core types and signature base construction: - SignatureAlgorithm: maps SF algorithm names (hmac-sha256, ed25519, etc.) to CryptoAlgorithm constants - SignatureComponent: record for covered component identifiers with SF parameter access (;sf, ;key, ;name, ;req, ;bs) - SignatureBaseBuilder: constructs the canonical signature base string per §2.5. Supports all derived components (@method, @path, @query, @scheme, @authority, @target-uri, @request-target, @query-param, @status). Header components support bare values, ;sf canonicalization, and ;key dictionary member extraction. Multi-value headers are joined with ", " per §2.1. @signature-params serialization reuses StructuredFields.serializeList for inner-list output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Complete the RFC 9421 sign/verify pipeline: - SignatureKeyResolver: interface resolving keyid to Verifier/Signer, supporting JCA, HSM, and remote KMS implementations - VerifyResult: record carrying signature label, keyid, algorithm, and the covered component values used during verification — downstream consumers use these cryptographically verified values - HttpMessageSignatures: static utility with verifyAll(), verify(), and sign() methods. Parses Signature/Signature-Input SF Dictionaries, validates created/expires time params, builds signature base, and delegates to Verifier/Signer from the shared crypto layer Tests cover HMAC and Ed25519 sign→verify roundtrips, tamper detection, missing headers, unknown keyid, and expired signature rejection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ure negotiation Complete the RFC 9421 middleware layer: - SignatureVerificationMiddleware: verifies Signature/Signature-Input headers on incoming requests. Stores VerifyResult list as a request extension for downstream access to verified component values. Supports requiredLabels and requiredComponents policy enforcement. - Accept-Signature negotiation: when configured, 401 responses include an Accept-Signature header telling clients what signature parameters are expected (SF Dictionary format). - HttpMessageSignatures.verifyAll: let SfParseException propagate so the middleware can distinguish malformed headers (400) from failed verification (401). - module-info.java: export enkan.web.jwt and enkan.web.signature packages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Security fixes: - JwtProcessor: prevent algorithm confusion attack (CVE-2016-5431) by validating key type against algorithm family (SecretKey for HMAC, PublicKey for asymmetric). Add verify(token, expectedAlgorithm, key) overload for explicit algorithm pinning. - JwtProcessor: escape typ/alg fields in serializeHeader, not just kid - EcdsaUtils: validate SEQUENCE (0x30) and INTEGER (0x02) DER tags instead of silently skipping them Correctness fixes: - JwtProcessor: add 60-second clock skew tolerance for exp/nbf validation to handle distributed system clock drift - HttpMessageSignatures: add 60-second clock skew tolerance for created/expires params; reject signatures older than 300 seconds (created freshness check) to limit replay window - JwtProcessor.verify(token, Key): catch IllegalArgumentException from unknown algorithm names instead of propagating (matches null-return contract) - CryptoAlgorithm: add isEcdsa() method, removing duplicate private methods in JcaSigner and JcaVerifier - JwtProcessor: document extractJson parser limitations (naive key matching, integer-only NumericDate); fix whitespace skipping to use Character.isWhitespace() Test coverage additions: - EcdsaUtilsTest: direct DER↔P1363 tests — null/empty/malformed input, wrong tags, round-trip identity for P-256 and P-521, high-bit padding - JwtProcessorTest: algorithm confusion attack, verify with expected algorithm, unknown algorithm, alg:none end-to-end - SignatureBaseBuilderTest: ;sf canonicalization, ;key dictionary member extraction, @query-param edge cases (empty value, missing param), response header resolution - HttpMessageSignaturesTest: future created rejection, stale created rejection (max age) - Extract SignatureTestSupport with shared buildRequest/emptyResolver/ testResolver helpers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
EcdsaUtils.derToP1363: INTEGER length fields now handle DER long-form encoding (0x81/0x82 prefix) consistently with the existing SEQUENCE length handling, closing a gap where malformed long-form INTEGER lengths would be mis-parsed. JwtProcessor.extractJsonNumber: the digit-scan loop previously allowed '-' at any position in the number string, so "123-456" would reach Long.parseLong and silently return null, bypassing time-claim validation. Now '-' is only permitted as the leading character. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- SignatureAlgorithm: add 8 missing RFC 9421 §3.3 algorithms (hmac-sha384/512, ecdsa-p384/p521, rsa-pss-sha256/384, rsa-v1_5-sha384/512) - EcdsaUtils.p1363ToDer: support full DER length encoding (0x81 and 0x82 prefixes) for SEQUENCE lengths up to 65535, replacing silent byte truncation - HttpMessageSignatures.verify: let UnsupportedOperationException propagate so callers can return 501 instead of silently skipping the signature - SignatureVerificationMiddleware: catch UnsupportedOperationException and return 501 (not 400) for unsupported features like ;bs - Tests: unknown alg value is skipped (not thrown), multiple simultaneous signatures are all verified, ;bs component returns 501 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…for dev misconfiguration SignatureBaseBuilder throws MisconfigurationException (not IllegalArgumentException) for two cases that are clearly caused by incorrect developer setup: - @status component used without a response context - @query-param component defined without a required ;name parameter Added EN/JA message entries and updated the corresponding test assertions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JcaSigner / JcaVerifier
- Validate key type at construction time and throw MisconfigurationException
(crypto.INCOMPATIBLE_KEY_TYPE) instead of ClassCastException at signing time
- Remove SecretKeySpec double-wrap in signHmac/verifyHmac: pass SecretKey
directly to Mac.init(), eliminating a redundant encode+re-wrap allocation
SignatureVerificationMiddleware.requiredComponents
- Fix policy logic: require at least ONE verified signature to cover ALL
required components, not all signatures to cover each component.
Previous code rejected multi-signature requests where sig2 covered
component X but sig1 did not.
JwtProcessor.extractJsonString / extractJsonNumber
- Fix substring match: a key like "alg" could match inside a JSON string
value that happens to contain the text "alg":... Scanners now verify
the match is preceded by '{' or ',' and followed immediately by ':',
skipping non-key occurrences.
- Extract buildSigningInput() helper to compose header.payload bytes
directly without allocating an intermediate String.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pe sequences, edge-case hardening - SignatureBaseBuilder: collect all @query-param occurrences and join with ", " per RFC 9421 §2.2.8 - SignatureVerificationMiddleware: fix requiredComponents policy — anyMatch instead of allMatch - JwtProcessor: add full RFC 8259 escape sequence support (\b, \f, \uXXXX) in unescapeJson - JwtProcessor: validate alg is non-null before signing (RFC 7515 §4.1.1 mandatory field) - HttpMessageSignatures: document time validation parameters in verifyAll Javadoc - Tests: queryParamMultipleValuesJoined, headerWithUnicodeEscapeInKidIsDecoded, signWithNullAlgThrows Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ed ordering - SignatureComponent.fromSfItem: normalize non-derived component names to lowercase per RFC 9421 §2.1 (HTTP field identifiers MUST be lowercase) - HttpMessageSignatures.validateTimeParams: reject signatures where expires precedes created (RFC 9421 §4.3 — structurally invalid time parameters) - Tests: fromSfItemNormalizesHeaderNameToLowerCase, verifyAllRejectsExpiresBeforeCreated Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cent-decode without + expansion - @query-param with multiple occurrences now emits each value as a separate signature base line (e.g. three 'tag' params → three lines), per RFC 9421 §2.2.8 - Replace URLDecoder.decode() with a custom percentDecode() that treats '+' as a literal '+', not a space — RFC 9421 uses query-component semantics, not form-encoding - Tests: queryParamMultipleValuesEachOnSeparateLine, queryParamPlusSignIsNotDecoded Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…authority - JcaSigner: validate that ECDSA algorithms receive an ECKey (not just any PrivateKey), preventing ClassCastException with an opaque message at signing time - SignatureBaseBuilder.resolveAuthority: prefer Host header per RFC 9421 §2.2.5, falling back to serverName/serverPort for reverse-proxy environments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared cryptography abstraction in enkan-core and builds on it in enkan-web to add (1) stateless JWT JWS signing/verification and (2) RFC 9421 HTTP Message Signatures signing/verification plus a verification middleware.
Changes:
- Add
enkan.security.cryptoshared layer (alg enum + Signer/Verifier + JCA implementations + ECDSA DER↔P1363 conversion). - Add
enkan.web.jwtJWT processor with minimal JOSE header parsing and exp/nbf validation. - Add
enkan.web.signatureRFC 9421 signature base builder, algorithms/components, sign/verify utility, and verification middleware (+ tests and misconfiguration messages).
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| enkan-web/src/test/java/enkan/web/signature/SignatureTestSupport.java | Shared request builder and key resolver stubs for signature tests |
| enkan-web/src/test/java/enkan/web/signature/SignatureBaseBuilderTest.java | Tests for derived/header components and signature-base canonicalization |
| enkan-web/src/test/java/enkan/web/signature/HttpMessageSignaturesTest.java | End-to-end sign/verify tests including time validation and multiple signatures |
| enkan-web/src/test/java/enkan/web/middleware/SignatureVerificationMiddlewareTest.java | Middleware behavior tests (401/400/501 + Accept-Signature) |
| enkan-web/src/test/java/enkan/web/jwt/JwtProcessorTest.java | JWT sign/verify tests (alg rejection, confusion, time claims, malformed tokens) |
| enkan-web/src/main/resources/META-INF/misconfiguration.properties | Add web signature-related misconfiguration messages |
| enkan-web/src/main/resources/META-INF/misconfiguration_ja.properties | Japanese translations for new web misconfiguration messages |
| enkan-web/src/main/java/module-info.java | Export new enkan.web.jwt and enkan.web.signature packages |
| enkan-web/src/main/java/enkan/web/signature/VerifyResult.java | Public verification result record including covered component values |
| enkan-web/src/main/java/enkan/web/signature/SignatureKeyResolver.java | Interface for resolving Signer/Verifier by keyid + algorithm |
| enkan-web/src/main/java/enkan/web/signature/SignatureComponent.java | Component identifier representation + SF item conversion |
| enkan-web/src/main/java/enkan/web/signature/SignatureBaseBuilder.java | RFC 9421 signature base construction + component resolution |
| enkan-web/src/main/java/enkan/web/signature/SignatureAlgorithm.java | Map RFC 9421 alg names → shared CryptoAlgorithm |
| enkan-web/src/main/java/enkan/web/signature/HttpMessageSignatures.java | RFC 9421 sign/verify utility + time validation |
| enkan-web/src/main/java/enkan/web/middleware/SignatureVerificationMiddleware.java | Middleware enforcing signature verification and policies |
| enkan-web/src/main/java/enkan/web/jwt/JwtProcessor.java | Stateless JWT sign/verify + minimal header parsing + exp/nbf validation |
| enkan-web/src/main/java/enkan/web/jwt/JwtHeader.java | JOSE header record |
| enkan-web/src/main/java/enkan/web/jwt/JwsAlgorithm.java | JWS alg names → shared CryptoAlgorithm |
| enkan-core/src/test/java/enkan/security/crypto/JcaSignerVerifierTest.java | Round-trip tests across algorithm families |
| enkan-core/src/test/java/enkan/security/crypto/EcdsaUtilsTest.java | Tests for DER↔P1363 conversion edge cases |
| enkan-core/src/main/resources/META-INF/misconfiguration.properties | Add crypto key-type mismatch misconfiguration messages |
| enkan-core/src/main/resources/META-INF/misconfiguration_ja.properties | Japanese translations for crypto misconfiguration messages |
| enkan-core/src/main/java/module-info.java | Export new enkan.security.crypto package |
| enkan-core/src/main/java/enkan/security/crypto/Verifier.java | Verifier functional interface |
| enkan-core/src/main/java/enkan/security/crypto/Signer.java | Signer functional interface |
| enkan-core/src/main/java/enkan/security/crypto/JcaVerifier.java | JCA verifier implementation (HMAC constant-time compare, ECDSA conversion) |
| enkan-core/src/main/java/enkan/security/crypto/JcaSigner.java | JCA signer implementation (ECDSA conversion to P1363) |
| enkan-core/src/main/java/enkan/security/crypto/EcdsaUtils.java | DER↔P1363 conversion utilities |
| enkan-core/src/main/java/enkan/security/crypto/CryptoAlgorithm.java | Central algorithm enum + RSA-PSS parameter specs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public record SignatureComponent(String name, SfParameters parameters) { | ||
|
|
||
| /** Creates a component with no parameters. */ | ||
| public static SignatureComponent of(String name) { |
There was a problem hiding this comment.
SignatureComponent.of(String) does not normalize HTTP field component identifiers to lowercase. RFC 9421 requires field component identifiers to be lowercase, and SignatureBaseBuilder/serializeSignatureParams will emit whatever case is provided, producing non-canonical signature bases (and potentially verification mismatches). Consider lowercasing name when it does not start with '@' (or validating and throwing if it contains uppercase).
Suggested change
| public static SignatureComponent of(String name) { | |
| public static SignatureComponent of(String name) { | |
| if (!name.startsWith("@")) { | |
| name = name.toLowerCase(Locale.ROOT); | |
| } |
Comment on lines
+286
to
+292
| // Serialize the member as a single-entry dictionary, then strip the key prefix | ||
| Map<String, SfMember> single = new LinkedHashMap<>(); | ||
| single.put(key, member); | ||
| String serialized = StructuredFields.serializeDictionary(new SfDictionary(single)); | ||
| // Result is "key=value" or "key=(inner-list)"; strip "key=" | ||
| int eq = serialized.indexOf('='); | ||
| return eq >= 0 ? serialized.substring(eq + 1) : serialized; |
There was a problem hiding this comment.
extractDictionaryMember serializes a single-entry dictionary then strips key=. This is incorrect for SF Dictionary members whose value serializes without ?1 / ?1;...). Consider serializing the SfMember itself (e.g. via list serialization of a single member) rather than round-tripping through a dictionary and string slicing.
Suggested change
| // Serialize the member as a single-entry dictionary, then strip the key prefix | |
| Map<String, SfMember> single = new LinkedHashMap<>(); | |
| single.put(key, member); | |
| String serialized = StructuredFields.serializeDictionary(new SfDictionary(single)); | |
| // Result is "key=value" or "key=(inner-list)"; strip "key=" | |
| int eq = serialized.indexOf('='); | |
| return eq >= 0 ? serialized.substring(eq + 1) : serialized; | |
| List<SfMember> single = new ArrayList<>(1); | |
| single.add(member); | |
| return StructuredFields.serializeList(new SfList(single)); |
Comment on lines
+45
to
+53
| String signatureHeader = request.getHeaders().get("Signature"); | ||
| String signatureInputHeader = request.getHeaders().get("Signature-Input"); | ||
| if (signatureHeader == null || signatureInputHeader == null) { | ||
| return List.of(); | ||
| } | ||
|
|
||
| SfDictionary signatureDict = StructuredFields.parseDictionary(signatureHeader); | ||
| SfDictionary signatureInputDict = StructuredFields.parseDictionary(signatureInputHeader); | ||
|
|
There was a problem hiding this comment.
verifyAll reads Signature / Signature-Input via Headers.get(...), which coerces multi-valued headers to List.toString() (e.g. [v1, v2]). If these headers are received multiple times and stored as a list, Structured Fields parsing will fail (likely returning 400). Consider retrieving the raw value (getRawType) and joining lists with ", " (similar to SignatureBaseBuilder.normalizeHeaderValue) before parsing.
Comment on lines
+110
to
+124
| /** | ||
| * Verifies a single signature on a response. | ||
| */ | ||
| public static VerifyResult verify(HttpRequest request, HttpResponse response, | ||
| String label, SfInnerList signatureInput, | ||
| byte[] signatureBytes, Verifier verifier) { | ||
| List<SignatureComponent> components = signatureInput.items().stream() | ||
| .map(SignatureComponent::fromSfItem) | ||
| .toList(); | ||
|
|
||
| // Build covered values map | ||
| Map<String, String> coveredValues = new LinkedHashMap<>(); | ||
| for (SignatureComponent comp : components) { | ||
| try { | ||
| coveredValues.put(comp.name(), |
There was a problem hiding this comment.
coveredValues is populated using coveredValues.put(comp.name(), ...), which (1) drops component parameters (e.g. "@query-param";name="tag" and "@query-param";name="id" collide), and (2) cannot represent multi-line components like @query-param (RFC 9421 requires one signature-base line per occurrence). As a result, VerifyResult.coveredValues() can diverge from what was actually verified and requiredComponents checks cannot be parameter-precise. Consider keying by the serialized component identifier (including parameters) and changing the value type to support multiple occurrences (e.g. Map<String, List<String>>).
Suggested change
| /** | |
| * Verifies a single signature on a response. | |
| */ | |
| public static VerifyResult verify(HttpRequest request, HttpResponse response, | |
| String label, SfInnerList signatureInput, | |
| byte[] signatureBytes, Verifier verifier) { | |
| List<SignatureComponent> components = signatureInput.items().stream() | |
| .map(SignatureComponent::fromSfItem) | |
| .toList(); | |
| // Build covered values map | |
| Map<String, String> coveredValues = new LinkedHashMap<>(); | |
| for (SignatureComponent comp : components) { | |
| try { | |
| coveredValues.put(comp.name(), | |
| private static void addCoveredValue(Map<String, String> coveredValues, String componentId, String value) { | |
| coveredValues.merge(componentId, value, (existing, next) -> existing + "\n" + next); | |
| } | |
| private static String serializeCoveredComponentIdentifier(SfItem item, SignatureComponent component) { | |
| String serialized = String.valueOf(item); | |
| return serialized != null && !serialized.isEmpty() ? serialized : component.name(); | |
| } | |
| /** | |
| * Verifies a single signature on a response. | |
| */ | |
| public static VerifyResult verify(HttpRequest request, HttpResponse response, | |
| String label, SfInnerList signatureInput, | |
| byte[] signatureBytes, Verifier verifier) { | |
| List<SfItem> signatureItems = signatureInput.items(); | |
| List<SignatureComponent> components = signatureItems.stream() | |
| .map(SignatureComponent::fromSfItem) | |
| .toList(); | |
| // Build covered values map using the serialized component identifier, | |
| // preserving repeated occurrences in signature-base order. | |
| Map<String, String> coveredValues = new LinkedHashMap<>(); | |
| for (int i = 0; i < components.size(); i++) { | |
| SignatureComponent comp = components.get(i); | |
| SfItem item = signatureItems.get(i); | |
| try { | |
| addCoveredValue( | |
| coveredValues, | |
| serializeCoveredComponentIdentifier(item, comp), |
Comment on lines
+140
to
+145
| /** | ||
| * Sets the component identifiers that must be covered by every valid signature. | ||
| */ | ||
| public void setRequiredComponents(Set<String> requiredComponents) { | ||
| this.requiredComponents = requiredComponents; | ||
| } |
There was a problem hiding this comment.
The Javadoc for setRequiredComponents says components must be covered by "every valid signature", but the enforcement logic earlier requires that at least one verified signature covers all required components (anyMatch). Please align the documentation with the implemented policy (or adjust the policy if the intent really is "every signature").
| web.SIGNATURE_STATUS_REQUIRES_RESPONSE.problem=The @status derived component requires a response context. | ||
| web.SIGNATURE_STATUS_REQUIRES_RESPONSE.solution=Use @status only when signing or verifying a response. Pass a non-null HttpResponse to the signing/verification method. | ||
| web.SIGNATURE_QUERY_PARAM_NAME_REQUIRED.problem=The @query-param component requires a ;name parameter. | ||
| web.SIGNATURE_QUERY_PARAM_NAME_REQUIRED.solution=Specify the query parameter name with ;name=<param-name>, e.g. SignatureComponent.of("@query-param;name=id"). |
There was a problem hiding this comment.
The suggested usage SignatureComponent.of("@query-param;name=id") is not supported by the current API: SignatureComponent.of(String) does not parse ;name parameters. This error solution text may mislead users. Consider updating the example to show constructing a SignatureComponent with SfParameters (or add a parsing helper if that string form is intended to be supported).
Suggested change
| web.SIGNATURE_QUERY_PARAM_NAME_REQUIRED.solution=Specify the query parameter name with ;name=<param-name>, e.g. SignatureComponent.of("@query-param;name=id"). | |
| web.SIGNATURE_QUERY_PARAM_NAME_REQUIRED.solution=Specify the query parameter name with ;name=<param-name> by constructing the @query-param SignatureComponent with SfParameters containing name=<param-name>. |
- SignatureComponent.of: normalize to lowercase per RFC 9421 §2.1 (mirrors fromSfItem)
- SignatureBaseBuilder.extractDictionaryMember: serialize via SfList to correctly
handle SF boolean true members (which have no '=' in dictionary form)
- HttpMessageSignatures.verifyAll: use entrySet() to access raw header values so
multi-value List headers are joined with ", " before SF parsing (not "[v1, v2]")
- SignatureVerificationMiddleware.setRequiredComponents: fix Javadoc to say
"at least one valid signature" instead of "every valid signature"
- misconfiguration.properties: fix misleading SignatureComponent.of("@query-param;name=id")
example — that form is not supported; use SfParameters constructor instead
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+23
to
+29
| public static SignatureComponent of(String name) { | ||
| // RFC 9421 §2.1: HTTP field component identifiers MUST be lowercase | ||
| if (!name.startsWith("@")) { | ||
| name = name.toLowerCase(Locale.ROOT); | ||
| } | ||
| return new SignatureComponent(name, SfParameters.EMPTY); | ||
| } |
There was a problem hiding this comment.
SignatureComponent.of() does not normalize derived component identifiers (those starting with '@') to lowercase. This makes inputs like "@method"/"@path" fail later (e.g., SignatureBaseBuilder's switch only recognizes lowercase names). Consider lowercasing all component names (derived and field) per RFC 9421 and validating/normalizing in one place.
Comment on lines
+232
to
+238
|
|
||
| if (component.isSf()) { | ||
| return canonicalizeSf(raw); | ||
| } | ||
| String keyParam = component.keyParam(); | ||
| if (keyParam != null) { | ||
| return extractDictionaryMember(raw, keyParam); |
There was a problem hiding this comment.
resolveHeader() returns early when the ;sf flag is present, which means a component that includes both ;sf and ;key will never apply the dictionary-member extraction. If both parameters can appear together, this produces incorrect covered values/signature bases. Consider applying ;key extraction even when ;sf is present (or explicitly rejecting the unsupported combination with a clear exception).
Suggested change
| if (component.isSf()) { | |
| return canonicalizeSf(raw); | |
| } | |
| String keyParam = component.keyParam(); | |
| if (keyParam != null) { | |
| return extractDictionaryMember(raw, keyParam); | |
| String keyParam = component.keyParam(); | |
| String resolved = raw; | |
| if (keyParam != null) { | |
| resolved = extractDictionaryMember(raw, keyParam); | |
| } | |
| if (component.isSf()) { | |
| return canonicalizeSf(resolved); | |
| } | |
| if (keyParam != null) { | |
| return resolved; |
Comment on lines
+117
to
+132
| List<SignatureComponent> components = signatureInput.items().stream() | ||
| .map(SignatureComponent::fromSfItem) | ||
| .toList(); | ||
|
|
||
| // Build covered values map | ||
| Map<String, String> coveredValues = new LinkedHashMap<>(); | ||
| for (SignatureComponent comp : components) { | ||
| try { | ||
| coveredValues.put(comp.name(), | ||
| SignatureBaseBuilder.resolveComponentValue(request, response, comp)); | ||
| } catch (UnsupportedOperationException e) { | ||
| throw e; // propagate so callers can return 501 | ||
| } catch (Exception e) { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
verify() builds coveredValues with coveredValues.put(comp.name(), ...). This (1) loses information when the same component appears more than once (e.g., multiple @query-param occurrences or multiple header components with different parameters) and (2) does not reflect what is actually signed for @query-param, since buildSignatureBase emits one line per value but coveredValues stores a single comma-joined string. Consider keying by the serialized component identifier (including parameters) and allowing multiple values (e.g., Map<String,List> or Map<String,String> with newline-joined values matching the signature base).
| SfParameters params = signatureInput.parameters(); | ||
| String keyId = params.get("keyid") instanceof SfValue.SfString s ? s.value() : null; | ||
| String algName = params.get("alg") instanceof SfValue.SfString s ? s.value() : null; | ||
| SignatureAlgorithm algorithm = algName != null ? SignatureAlgorithm.fromSfName(algName) : null; |
There was a problem hiding this comment.
HttpMessageSignatures.verify(...) resolves the algorithm at the end via SignatureAlgorithm.fromSfName(algName) without catching IllegalArgumentException. If a caller invokes verify(...) directly with an unknown/typo'd alg in Signature-Input, this will throw instead of returning null like verifyAll() does. Consider catching IllegalArgumentException here and treating it as verification failure (return null) for consistency.
Suggested change
| SignatureAlgorithm algorithm = algName != null ? SignatureAlgorithm.fromSfName(algName) : null; | |
| SignatureAlgorithm algorithm; | |
| try { | |
| algorithm = algName != null ? SignatureAlgorithm.fromSfName(algName) : null; | |
| } catch (IllegalArgumentException e) { | |
| return null; | |
| } |
| try { | ||
| results = HttpMessageSignatures.verifyAll(request, keyResolver); | ||
| } catch (SfParseException e) { | ||
| return errorResponse(400, "Invalid Signature-Input header"); |
There was a problem hiding this comment.
The SfParseException catch-all returns "Invalid Signature-Input header", but HttpMessageSignatures.verifyAll() parses both Signature and Signature-Input; malformed Signature will also land here. Consider updating the message to reflect both headers (or include which header failed) so clients/debugging aren't misled.
Suggested change
| return errorResponse(400, "Invalid Signature-Input header"); | |
| return errorResponse(400, "Invalid Signature or Signature-Input header"); |
Comment on lines
+140
to
+145
| /** | ||
| * Sets the component identifiers that must be covered by at least one valid signature. | ||
| * A request is rejected if no single verified signature covers all of the specified components. | ||
| */ | ||
| public void setRequiredComponents(Set<String> requiredComponents) { | ||
| this.requiredComponents = requiredComponents; |
There was a problem hiding this comment.
setRequiredComponents() accepts arbitrary strings, but component identifiers are effectively case-sensitive in enforcement (coveredValues keys are lowercase for header fields, and derived components are expected lowercase like "@method"). This can lead to false 401s if callers configure e.g. "Content-Type". Consider normalizing required component names on set (lowercase when not starting with '@', and lowercase derived identifiers too) to match SignatureComponent normalization.
Suggested change
| /** | |
| * Sets the component identifiers that must be covered by at least one valid signature. | |
| * A request is rejected if no single verified signature covers all of the specified components. | |
| */ | |
| public void setRequiredComponents(Set<String> requiredComponents) { | |
| this.requiredComponents = requiredComponents; | |
| private String normalizeRequiredComponent(String component) { | |
| return component == null ? null : component.toLowerCase(Locale.ROOT); | |
| } | |
| /** | |
| * Sets the component identifiers that must be covered by at least one valid signature. | |
| * A request is rejected if no single verified signature covers all of the specified components. | |
| */ | |
| public void setRequiredComponents(Set<String> requiredComponents) { | |
| this.requiredComponents = requiredComponents == null ? null : requiredComponents.stream() | |
| .map(this::normalizeRequiredComponent) | |
| .collect(LinkedHashSet::new, Set::add, Set::addAll); |
| * @param key the signing key (SecretKey for HMAC, PrivateKey for asymmetric) | ||
| * @return the signed JWT string | ||
| */ | ||
| public static String sign(JwtHeader header, byte[] claimsJsonBytes, Key key) { |
There was a problem hiding this comment.
sign(JwtHeader, byte[], Key) calls JwsAlgorithm.fromJwsName(header.alg()) before serializeHeader(header) validates that alg is present. With a null/invalid alg this produces a less specific error than the serializer's RFC-referencing message. Consider validating header.alg() (and possibly rejecting "none") before mapping to JwsAlgorithm so error reporting is consistent across overloads.
Suggested change
| public static String sign(JwtHeader header, byte[] claimsJsonBytes, Key key) { | |
| public static String sign(JwtHeader header, byte[] claimsJsonBytes, Key key) { | |
| serializeHeader(header); |
- SignatureComponent: normalize all component names (including derived @-names) to lowercase so inputs like "@method" or "@path" don't break the switch - SignatureBaseBuilder.resolveHeader: apply ;key extraction before ;sf canonicalization so components with both parameters are handled correctly - HttpMessageSignatures.verify: catch IllegalArgumentException from SignatureAlgorithm.fromSfName to return null instead of throwing - SignatureVerificationMiddleware: normalize requiredComponents to lowercase on set to match SignatureComponent normalization and avoid false 401s - SignatureVerificationMiddleware: clarify 400 error message to mention both Signature and Signature-Input headers - JwtProcessor.sign(JwtHeader, byte[], Key): validate header (alg present) before JwsAlgorithm lookup for consistent error reporting Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- CryptoAlgorithm: replace isEcdsa() string-contains check with an explicit boolean enum field — more robust if JCA naming conventions ever change - EcdsaUtils: add DER structure diagram to class Javadoc for maintainability - JwtProcessor.verify(token, Key): document algorithm confusion risk and recommend the three-argument overload in the Javadoc - SignatureBaseBuilder.resolveHeader: add comment explaining why lowercase component names always match Headers keys (stored in ASCII lowercase) - Test: add malformed base64 payload test for JwtProcessor.verify - Test: add clock-skew boundary tests (59s inside / 61s outside tolerance) for HttpMessageSignatures.verifyAll Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- queryParamPercentDecodedValue: verify %20/%2B/%3D are decoded per RFC 3986 (not form-encoding) in @query-param component resolution - nonJsonPayloadDoesNotFailVerification: document that a non-JSON payload passes JWT verification because there are no time claims to reject Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
enkan.security.crypto(enkan-core): Shared cryptographic signing/verification layer —CryptoAlgorithmenum,Signer/Verifierinterfaces,JcaSigner/JcaVerifierJCA implementations,EcdsaUtilsDER↔P1363 conversion. Covers HMAC, Ed25519, ECDSA (P-256/P-384/P-521), RSA-PSS, RSA PKCS#1 v1.5. No external dependencies.enkan.web.jwt(enkan-web): Stateless JWT signing/verification —JwsAlgorithmmapping toCryptoAlgorithm,JwtHeaderrecord,JwtProcessorwith no JSON library dependency. Validates exp/nbf, rejects alg:none.enkan.web.signature(enkan-web): RFC 9421 HTTP Message Signatures —SignatureAlgorithm,SignatureComponent,SignatureBaseBuilder(§2.5),SignatureKeyResolver,VerifyResult(with covered component values for downstream use),HttpMessageSignaturessign/verify utility.SignatureVerificationMiddleware: Verifies Signature/Signature-Input headers withrequiredLabels/requiredComponentspolicy enforcement andAccept-Signaturenegotiation on 401 responses.Closes #135
Design decisions
Signer/Verifierinterfaces (not rawKey) returned fromSignatureKeyResolverto support HSM and remote KMSVerifyResult.coveredValues()(pyauth pattern) provides cryptographically verified component values to downstream handlersTest plan
JcaSignerVerifierTest: sign→verify roundtrips for all algorithm families, wrong-key rejection, tamper detection (105 core tests)JwtProcessorTest: HMAC/Ed25519/ECDSA/RSA roundtrips, exp/nbf validation, alg:none rejection, malformed tokens (14 tests)SignatureBaseBuilderTest: all derived components, header normalization, full signature base format (18 tests)HttpMessageSignaturesTest: HMAC/Ed25519 sign→verify, tamper detection, expired signatures, unknown keyid (6 tests)SignatureVerificationMiddlewareTest: valid/invalid signatures, required labels/components, Accept-Signature header (7 tests)mvn test -pl enkan-core,enkan-web -am— 583 tests pass🤖 Generated with Claude Code