fix: prevent JWT attacks by spoofed keyIDs (#4186)

* fix: prevent JWT attacks by spoofed keyIDs

* DEPENDENCIES

* use constant

* DEPENDENCIES

DEPENDENCIES

@@ -125,7 +125,7 @@ maven/mavencentral/io.netty/netty-tcnative-classes/2.0.56.Final, Apache-2.0, app
 maven/mavencentral/io.netty/netty-transport-native-unix-common/4.1.86.Final, Apache-2.0 AND BSD-3-Clause AND MIT, approved, CQ20926
 maven/mavencentral/io.netty/netty-transport/4.1.86.Final, Apache-2.0 AND BSD-3-Clause AND MIT, approved, CQ20926
 maven/mavencentral/io.opentelemetry.instrumentation/opentelemetry-instrumentation-annotations/1.32.0, Apache-2.0, approved, #11684
-maven/mavencentral/io.opentelemetry.proto/opentelemetry-proto/1.3.1-alpha, None, restricted, #14688
+maven/mavencentral/io.opentelemetry.proto/opentelemetry-proto/1.3.1-alpha, Apache-2.0, approved, #14688
 maven/mavencentral/io.opentelemetry/opentelemetry-api/1.32.0, Apache-2.0, approved, #11682
 maven/mavencentral/io.opentelemetry/opentelemetry-context/1.32.0, Apache-2.0, approved, #11683
 maven/mavencentral/io.prometheus/simpleclient/0.16.0, Apache-2.0, approved, clearlydefined

extensions/common/crypto/jwt-verifiable-credentials/src/main/java/org/eclipse/edc/verifiablecredentials/jwt/JwtPresentationVerifier.java

@@ -26,6 +26,7 @@
 import org.eclipse.edc.token.spi.TokenValidationRule;
 import org.eclipse.edc.token.spi.TokenValidationRulesRegistry;
 import org.eclipse.edc.token.spi.TokenValidationService;
+import org.eclipse.edc.verifiablecredentials.jwt.rules.IssuerKeyIdValidationRule;
 
 import java.text.ParseException;
 import java.util.ArrayList;
@@ -110,8 +111,11 @@ public Result<Void> verify(String serializedJwt, VerifierContext context) {
         try {
             // obtain the actual JSON structure
             var signedJwt = SignedJWT.parse(serializedJwt);
+            var keyId = signedJwt.getHeader().getKeyID();
             if (isCredential(signedJwt)) {
-                return tokenValidationService.validate(serializedJwt, publicKeyResolver, tokenValidationRulesRegistry.getRules(JWT_VC_TOKEN_CONTEXT))
+                var rules = new ArrayList<>(tokenValidationRulesRegistry.getRules(JWT_VC_TOKEN_CONTEXT));
+                rules.add(new IssuerKeyIdValidationRule(keyId));
+                return tokenValidationService.validate(serializedJwt, publicKeyResolver, rules)
                         .mapTo();
             }
 
@@ -120,7 +124,7 @@ public Result<Void> verify(String serializedJwt, VerifierContext context) {
             }
 
             //we can be sure to have a presentation token
-            verificationResult = tokenValidationService.validate(serializedJwt, publicKeyResolver, vpValidationRules(context.getAudience()));
+            verificationResult = tokenValidationService.validate(serializedJwt, publicKeyResolver, vpValidationRules(context.getAudience(), keyId));
 
             var vpClaim = (Map<String, Object>) signedJwt.getJWTClaimsSet().getClaim(VP_CLAIM);
 
@@ -145,16 +149,18 @@ public Result<Void> verify(String serializedJwt, VerifierContext context) {
         return verificationResult.mapTo();
     }
 
-    private List<TokenValidationRule> vpValidationRules(String audience) {
+    private List<TokenValidationRule> vpValidationRules(String audience, String keyId) {
 
+        var rules = new ArrayList<>(tokenValidationRulesRegistry.getRules(JWT_VP_TOKEN_CONTEXT));
+        rules.add(new IssuerKeyIdValidationRule(keyId));
         return Optional.ofNullable(audience)
                 .map(aud -> {
-                    List<TokenValidationRule> r = new ArrayList<>(tokenValidationRulesRegistry.getRules(JWT_VP_TOKEN_CONTEXT));
+                    List<TokenValidationRule> r = new ArrayList<>(rules);
                     var audRule = new AudienceValidationRule(audience);
                     r.add(audRule);
                     return r;
                 })
-                .orElse(tokenValidationRulesRegistry.getRules(JWT_VP_TOKEN_CONTEXT));
+                .orElse(rules);
 
     }
 

extensions/common/crypto/jwt-verifiable-credentials/src/main/java/org/eclipse/edc/verifiablecredentials/jwt/rules/IssuerKeyIdValidationRule.java

@@ -0,0 +1,46 @@
+/*
+ *  Copyright (c) 2024 Bayerische Motoren Werke Aktiengesellschaft (BMW AG)
+ *
+ *  This program and the accompanying materials are made available under the
+ *  terms of the Apache License, Version 2.0 which is available at
+ *  https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  SPDX-License-Identifier: Apache-2.0
+ *
+ *  Contributors:
+ *       Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation
+ *
+ */
+
+package org.eclipse.edc.verifiablecredentials.jwt.rules;
+
+import org.eclipse.edc.jwt.spi.JwtRegisteredClaimNames;
+import org.eclipse.edc.spi.iam.ClaimToken;
+import org.eclipse.edc.spi.result.Result;
+import org.eclipse.edc.token.spi.TokenValidationRule;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.Map;
+
+/**
+ * Asserts that the {@code kid} header of a JWT contains the {@code iss}:
+ * <pre>
+ *     kid := iss # key-id
+ * </pre>
+ * where {@code iss} usually are DIDs, and the {@code key-id} is an arbitrary string.
+ */
+public class IssuerKeyIdValidationRule implements TokenValidationRule {
+    private final String keyId;
+
+    public IssuerKeyIdValidationRule(String tokenKeyIdHeader) {
+        this.keyId = tokenKeyIdHeader;
+    }
+
+    @Override
+    public Result<Void> checkRule(@NotNull ClaimToken toVerify, @Nullable Map<String, Object> additional) {
+        var iss = toVerify.getStringClaim(JwtRegisteredClaimNames.ISSUER);
+        // keyID MUST be a composite of the issuer and the key-id in the form <ISSUER>#<keyID>
+        return keyId.matches("%s#.*".formatted(iss)) ? Result.success() : Result.failure("kid header '%s' expected to correlate to 'iss' claim ('%s'), but it did not.".formatted(keyId, iss));
+    }
+}

extensions/common/crypto/jwt-verifiable-credentials/src/test/java/org/eclipse/edc/verifiablecredentials/jwt/JwtPresentationVerifierTest.java

@@ -18,9 +18,14 @@
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.JWSAlgorithm;
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.crypto.ECDSASigner;
 import com.nimbusds.jose.jwk.Curve;
 import com.nimbusds.jose.jwk.ECKey;
 import com.nimbusds.jose.jwk.gen.ECKeyGenerator;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
 import org.eclipse.edc.iam.identitytrust.spi.verification.VerifierContext;
 import org.eclipse.edc.jsonld.util.JacksonJsonLd;
 import org.eclipse.edc.junit.annotations.ComponentTest;
@@ -34,7 +39,10 @@
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
+import java.sql.Date;
+import java.time.Instant;
 import java.util.Map;
+import java.util.UUID;
 
 import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat;
 import static org.eclipse.edc.spi.result.Result.success;
@@ -48,6 +56,7 @@
 import static org.eclipse.edc.verifiablecredentials.jwt.TestConstants.VP_CONTENT_TEMPLATE;
 import static org.eclipse.edc.verifiablecredentials.jwt.TestConstants.VP_HOLDER_ID;
 import static org.eclipse.edc.verifiablecredentials.jwt.TestFunctions.createPublicKey;
+import static org.mockito.ArgumentMatchers.endsWith;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -241,6 +250,105 @@ void verifyPresentation_wrongAudience() {
         assertThat(result).isFailed().detail().contains("Token audience claim (aud -> [invalid-vp-audience]) did not contain expected audience: did:web:myself");
     }
 
+    @Test
+    void verifyPresentation_jwtVpHasSpoofedKidClaim() throws JOSEException {
+
+        var spoofedKey = new ECKeyGenerator(Curve.P_256).keyID("did:web:attacker#violating-key").generate();
+
+        when(publicKeyResolverMock.resolveKey(endsWith("violating-key"))).thenReturn(success(createPublicKey(spoofedKey.toPublicJWK())));
+
+        // create first VC-JWT (signed by the central issuer)
+        var vcJwt1 = JwtCreationUtils.createJwt(vcSigningKey, CENTRAL_ISSUER_DID, "degreeSub", VP_HOLDER_ID, Map.of("vc", VC_CONTENT_DEGREE_EXAMPLE));
+
+        var vpContent = VP_CONTENT_TEMPLATE.formatted("\"" + vcJwt1 + "\"");
+
+
+        String vpJwt;
+        try {
+            var signer = new ECDSASigner(spoofedKey.toECPrivateKey());
+
+            // Prepare JWT with claims set
+            var now = Date.from(Instant.now());
+            var claimsSet = new JWTClaimsSet.Builder()
+                    .issuer(VP_HOLDER_ID)
+                    .subject("testSub")
+                    .issueTime(now)
+                    .audience(MY_OWN_DID)
+                    .notBeforeTime(now)
+                    .claim("jti", UUID.randomUUID().toString())
+                    .expirationTime(Date.from(Instant.now().plusSeconds(60)));
+
+            Map.of("vp", asMap(vpContent)).forEach(claimsSet::claim);
+
+            var signedJwt = new SignedJWT(new JWSHeader.Builder(JWSAlgorithm.ES256).keyID(spoofedKey.getKeyID()).build(), claimsSet.build());
+
+            signedJwt.sign(signer);
+
+            vpJwt = signedJwt.serialize();
+
+        } catch (JOSEException e) {
+            throw new RuntimeException(e);
+        }
+        var context = VerifierContext.Builder.newInstance()
+                .verifier(verifier)
+                .audience(MY_OWN_DID)
+                .build();
+        var result = verifier.verify(vpJwt, context);
+
+        assertThat(result)
+                .isFailed()
+                .detail()
+                .isEqualTo("kid header 'did:web:attacker#violating-key' expected to correlate to 'iss' claim ('did:web:test-issuer'), but it did not.");
+    }
+
+    @Test
+    void verifyCredential_jwtVpHasSpoofedKidClaim() throws JOSEException {
+
+        var spoofedKey = new ECKeyGenerator(Curve.P_256).keyID("did:web:attacker#violating-key").generate();
+
+        when(publicKeyResolverMock.resolveKey(endsWith("violating-key"))).thenReturn(success(createPublicKey(spoofedKey.toPublicJWK())));
+
+        // create first VC-JWT (signed by the central issuer)
+
+        String vcJwt1;
+        try {
+            var signer = new ECDSASigner(spoofedKey.toECPrivateKey());
+
+            // Prepare JWT with claims set
+            var now = Date.from(Instant.now());
+            var claimsSet = new JWTClaimsSet.Builder()
+                    .issuer(CENTRAL_ISSUER_DID)
+                    .subject("degreeSub")
+                    .issueTime(now)
+                    .audience(VP_HOLDER_ID)
+                    .notBeforeTime(now)
+                    .claim("jti", UUID.randomUUID().toString())
+                    .expirationTime(Date.from(Instant.now().plusSeconds(60)));
+
+            Map.of("vc", VC_CONTENT_DEGREE_EXAMPLE).forEach(claimsSet::claim);
+
+            var signedJwt = new SignedJWT(new JWSHeader.Builder(JWSAlgorithm.ES256).keyID(spoofedKey.getKeyID()).build(), claimsSet.build());
+
+            signedJwt.sign(signer);
+
+            vcJwt1 = signedJwt.serialize();
+
+        } catch (JOSEException e) {
+            throw new RuntimeException(e);
+        }
+        var context = VerifierContext.Builder.newInstance()
+                .verifier(verifier)
+                .audience(MY_OWN_DID)
+                .build();
+        var result = verifier.verify(vcJwt1, context);
+
+        assertThat(result)
+                .isFailed()
+                .detail()
+                .isEqualTo("kid header 'did:web:attacker#violating-key' expected to correlate to 'iss' claim ('did:web:some-official-issuer'), but it did not.");
+    }
+
+
     private Map<String, Object> asMap(String rawContent) {
         try {
             return mapper.readValue(rawContent, new TypeReference<>() {