fix(spiffeid): tighten path validation and segment construction

java-spiffe-core/src/main/java/io/spiffe/spiffeid/SpiffeId.java

@@ -25,6 +25,7 @@ public final class SpiffeId {
     static final String DOT_SEGMENT = "Path cannot contain dot segments";
     static final String EMPTY_SEGMENT = "Path cannot contain empty segments";
     static final String TRAILING_SLASH = "Path cannot have a trailing slash";
+    static final String MISSING_LEADING_SLASH = "Path must start with '/'";
 
     private final TrustDomain trustDomain;
 
@@ -51,7 +52,7 @@ public static SpiffeId fromSegments(TrustDomain trustDomain, String... segments)
 
         StringBuilder path = new StringBuilder();
         for (String p : segments) {
-            validatePath(p);
+            validatePathSegment(p);
             path.append('/');
             path.append(p);
         }
@@ -137,37 +138,41 @@ public String toString() {
      */
     public static void validatePath(String path) {
         if (StringUtils.isBlank(path)) {
-            throw new IllegalArgumentException(EMPTY);
+            throw new InvalidSpiffeIdException(EMPTY);
+        }
+
+        if (!path.startsWith("/")) {
+            throw new InvalidSpiffeIdException(MISSING_LEADING_SLASH);
         }
 
-        int segmentStart = 0;
-        int segmentEnd = 0;
+        // Keep trailing empty segments so "/a/" is distinguished from "/a"
+        String[] segments = path.substring(1).split("/", -1);
+        for (int i = 0; i < segments.length; i++) {
+            String segment = segments[i];
+            boolean lastSegment = i == segments.length - 1;
 
-        for (; segmentEnd < path.length(); segmentEnd++) {
-            char c = path.charAt(segmentEnd);
-            if (c == '/') {
-                switch (path.substring(segmentStart, segmentEnd)) {
-                    case "/":
-                        throw new InvalidSpiffeIdException(EMPTY_SEGMENT);
-                    case "/.":
-                    case "/..":
-                        throw new InvalidSpiffeIdException(DOT_SEGMENT);
-                }
-                segmentStart = segmentEnd;
-                continue;
+            if (segment.isEmpty()) {
+                throw new InvalidSpiffeIdException(lastSegment ? TRAILING_SLASH : EMPTY_SEGMENT);
             }
+
+            validatePathSegment(segment);
+        }
+    }
+
+    private static void validatePathSegment(String segment) {
+        if (StringUtils.isEmpty(segment)) {
+            throw new InvalidSpiffeIdException(EMPTY);
+        }
+
+        if (".".equals(segment) || "..".equals(segment)) {
+            throw new InvalidSpiffeIdException(DOT_SEGMENT);
+        }
+
+        for (char c : segment.toCharArray()) {
             if (!isValidPathSegmentChar(c)) {
                 throw new InvalidSpiffeIdException(BAD_PATH_SEGMENT_CHAR);
             }
         }
-
-        switch (path.substring(segmentStart, segmentEnd)) {
-            case "/":
-                throw new InvalidSpiffeIdException(TRAILING_SLASH);
-            case "/.":
-            case "/..":
-                throw new InvalidSpiffeIdException(DOT_SEGMENT);
-        }
     }
 
     private static boolean isValidPathSegmentChar(char c) {

java-spiffe-core/src/test/java/io/spiffe/spiffeid/SpiffeIdTest.java

@@ -11,8 +11,11 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -61,15 +64,28 @@ void testParseValidSpiffeId(String input, TrustDomain expectedTrustDomain, Strin
 
     static Stream<Arguments> provideTestValidSpiffeIds() {
         return Stream.of(
+                Arguments.of("spiffe://trustdomain", TrustDomain.parse("trustdomain"), ""),
                 Arguments.of("spiffe://trustdomain/path", TrustDomain.parse("trustdomain"), "/path"),
                 Arguments.of("spiffe://trustdomain/path1/path2", TrustDomain.parse("trustdomain"), "/path1/path2"),
                 Arguments.of("spiffe://trustdomain/PATH1/PATH2", TrustDomain.parse("trustdomain"), "/PATH1/PATH2"),
                 Arguments.of("spiffe://trustdomain/9eebccd2-12bf-40a6-b262-65fe0487d453", TrustDomain.parse("trustdomain"), "/9eebccd2-12bf-40a6-b262-65fe0487d453"),
+                Arguments.of("spiffe://a_b.example/foo", TrustDomain.parse("a_b.example"), "/foo"),
+                Arguments.of("spiffe://1.2.3.4/service", TrustDomain.parse("1.2.3.4"), "/service"),
                 Arguments.of("SPIFFE://trustdomain/path", TrustDomain.parse("trustdomain"), "/path"),
                 Arguments.of("SpIfFe://TrUsTdOmAiN/Workload", TrustDomain.parse("trustdomain"), "/Workload")
         );
     }
 
+    static Stream<String> provideNonDnsShapedTrustDomains() {
+        return Stream.of(
+                "example..org",
+                ".example.org",
+                "example.org.",
+                "-example.org",
+                "example-.org"
+        );
+    }
+
     @ParameterizedTest
     @MethodSource("provideInvalidSpiffeIds")
     void testParseInvalidSpiffeId(String input, String expected) {
@@ -101,6 +117,8 @@ static Stream<Arguments> provideInvalidSpiffeIds() {
                 Arguments.of("spiffe://trustdomain/../other", "Path cannot contain dot segments"),
                 Arguments.of("spiffe://trustdomain/", "Path cannot have a trailing slash"),
                 Arguments.of("spiffe://trustdomain/path/", "Path cannot have a trailing slash"),
+                Arguments.of("spiffe://[::1]/service", "Trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores"),
+                Arguments.of("spiffe://[2001:db8::1]/service", "Trust domain characters are limited to lowercase letters, numbers, dots, dashes, and underscores"),
                 Arguments.of("xspiffe://trustdomain/path", "Scheme is missing or invalid")
         );
     }
@@ -140,13 +158,13 @@ void testOfInvalid(TrustDomain trustDomain, String[] inputPath, String expectedE
     static Stream<Arguments> provideInvalidArguments() {
         return Stream.of(
                 Arguments.of(null, new String[]{""}, "trustDomain must not be null"),
-                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"/ele%5ment"}, "Path segment characters are limited to letters, numbers, dots, dashes, and underscores"),
-                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"/path/"}, "Path cannot have a trailing slash"),
-                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"/ /"}, "Path segment characters are limited to letters, numbers, dots, dashes, and underscores"),
-                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"/"}, "Path cannot have a trailing slash"),
-                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"//"}, "Path cannot contain empty segments"),
-                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"/./"}, "Path cannot contain dot segments"),
-                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"/../"}, "Path cannot contain dot segments")
+                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{""}, "Cannot be empty"),
+                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"ele%5ment"}, "Path segment characters are limited to letters, numbers, dots, dashes, and underscores"),
+                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"/service"}, "Path segment characters are limited to letters, numbers, dots, dashes, and underscores"),
+                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"service/"}, "Path segment characters are limited to letters, numbers, dots, dashes, and underscores"),
+                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"foo/bar"}, "Path segment characters are limited to letters, numbers, dots, dashes, and underscores"),
+                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{"."}, "Path cannot contain dot segments"),
+                Arguments.of(TrustDomain.parse("trustdomain"), new String[]{".."}, "Path cannot contain dot segments")
         );
     }
 
@@ -239,4 +257,114 @@ void memberOf_aTrustDomainAndASpiffeIdWithDifferentTrustDomain_ReturnsFalse() {
 
         assertFalse(isMemberOf);
     }
+
+    @Test
+    void parseMixedCaseSchemeAndTrustDomain_toStringReturnsCanonicalFormAndPreservesPathCase() {
+        SpiffeId spiffeId = SpiffeId.parse("SPIFFE://EXAMPLE.ORG/MyService");
+
+        assertEquals("spiffe://example.org/MyService", spiffeId.toString());
+    }
+
+    @Test
+    void parseEquivalentIdsWithDifferentSchemeAndTrustDomainCase_areEqual() {
+        SpiffeId lowercase = SpiffeId.parse("spiffe://example.org/service");
+        SpiffeId uppercaseScheme = SpiffeId.parse("SPIFFE://example.org/service");
+        SpiffeId uppercaseTrustDomain = SpiffeId.parse("spiffe://EXAMPLE.ORG/service");
+        SpiffeId uppercaseBoth = SpiffeId.parse("SPIFFE://EXAMPLE.ORG/service");
+
+        assertEquals(lowercase, uppercaseScheme);
+        assertEquals(lowercase, uppercaseTrustDomain);
+        assertEquals(lowercase, uppercaseBoth);
+        assertEquals(lowercase.hashCode(), uppercaseBoth.hashCode());
+    }
+
+    @Test
+    void parseIdsWithDifferentPathCase_areNotEqual() {
+        SpiffeId lowercasePath = SpiffeId.parse("spiffe://example.org/service");
+        SpiffeId uppercasePath = SpiffeId.parse("spiffe://example.org/Service");
+
+        assertNotEquals(lowercasePath, uppercasePath);
+    }
+
+    @ParameterizedTest
+    @MethodSource("provideNonDnsShapedTrustDomains")
+    void parseSpiffeIdWithNonDnsShapedTrustDomain_isAccepted(String trustDomainName) {
+        SpiffeId spiffeId = SpiffeId.parse("spiffe://" + trustDomainName + "/service");
+
+        assertEquals(TrustDomain.parse(trustDomainName), spiffeId.getTrustDomain());
+        assertEquals("/service", spiffeId.getPath());
+        assertEquals("spiffe://" + trustDomainName + "/service", spiffeId.toString());
+    }
+
+    @ParameterizedTest
+    @MethodSource("provideNonDnsShapedTrustDomains")
+    void fromSegmentsWithNonDnsShapedTrustDomain_isAccepted(String trustDomainName) {
+        TrustDomain trustDomain = TrustDomain.parse(trustDomainName);
+
+        SpiffeId spiffeId = SpiffeId.fromSegments(trustDomain, "service");
+
+        assertEquals(trustDomain, spiffeId.getTrustDomain());
+        assertEquals("/service", spiffeId.getPath());
+        assertEquals("spiffe://" + trustDomainName + "/service", spiffeId.toString());
+    }
+
+    @ParameterizedTest
+    @MethodSource("provideInvalidSegmentsForFromSegments")
+    void fromSegments_invalidSegment_throwsInvalidSpiffeIdException(String segment, String expectedMessage) {
+        InvalidSpiffeIdException ex = assertThrows(
+                InvalidSpiffeIdException.class,
+                () -> SpiffeId.fromSegments(TrustDomain.parse("example.org"), segment));
+        assertEquals(expectedMessage, ex.getMessage());
+    }
+
+    static Stream<Arguments> provideInvalidSegmentsForFromSegments() {
+        return Stream.of(
+                Arguments.of(null, SpiffeId.EMPTY),
+                Arguments.of("", SpiffeId.EMPTY),
+                Arguments.of(" ", SpiffeId.BAD_PATH_SEGMENT_CHAR)
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("provideInvalidPathsForValidatePath")
+    void validatePath_invalidPath_throwsInvalidSpiffeIdException(String path, String expectedMessage) {
+        InvalidSpiffeIdException ex = assertThrows(
+                InvalidSpiffeIdException.class,
+                () -> SpiffeId.validatePath(path));
+        assertEquals(expectedMessage, ex.getMessage());
+    }
+
+    static Stream<Arguments> provideInvalidPathsForValidatePath() {
+        return Stream.of(
+                Arguments.of("   ", SpiffeId.EMPTY),
+                Arguments.of("foo", SpiffeId.MISSING_LEADING_SLASH),
+                Arguments.of("foo/bar", SpiffeId.MISSING_LEADING_SLASH),
+                Arguments.of("/foo//bar", SpiffeId.EMPTY_SEGMENT),
+                Arguments.of("/./other", SpiffeId.DOT_SEGMENT),
+                Arguments.of("/../other", SpiffeId.DOT_SEGMENT),
+                Arguments.of("/foo/.", SpiffeId.DOT_SEGMENT),
+                Arguments.of("/foo/..", SpiffeId.DOT_SEGMENT),
+                Arguments.of("/foo/", SpiffeId.TRAILING_SLASH),
+                Arguments.of("/", SpiffeId.TRAILING_SLASH),
+                Arguments.of("/ ", SpiffeId.BAD_PATH_SEGMENT_CHAR),
+                Arguments.of("/foo%5Cbar", SpiffeId.BAD_PATH_SEGMENT_CHAR),
+                Arguments.of("/foo bar", SpiffeId.BAD_PATH_SEGMENT_CHAR)
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("provideValidPathsForValidatePath")
+    void validatePath_validPath_doesNotThrow(String path) {
+        assertDoesNotThrow(() -> SpiffeId.validatePath(path));
+    }
+
+    static Stream<String> provideValidPathsForValidatePath() {
+        return Stream.of(
+                "/foo",
+                "/foo/bar",
+                "/PATH/path",
+                "/.../svc",
+                "/9eebccd2-12bf-40a6-b262-65fe0487d453"
+        );
+    }
 }

java-spiffe-core/src/test/java/io/spiffe/spiffeid/TrustDomainTest.java

@@ -26,6 +26,25 @@ void testTrustDomainFromNameMixedCase_isNormalizedToLowercase() {
         assertEquals("trustdomain", trustDomain.getName());
     }
 
+    @Test
+    void testTrustDomainFromNameWithUnderscore() {
+        TrustDomain trustDomain = TrustDomain.parse("trust_domain_name.example.com");
+        assertEquals("trust_domain_name.example.com", trustDomain.getName());
+    }
+
+    @Test
+    void testTrustDomainFromIpv4Name() {
+        TrustDomain trustDomain = TrustDomain.parse("1.2.3.4");
+        assertEquals("1.2.3.4", trustDomain.getName());
+    }
+
+    @ParameterizedTest
+    @MethodSource("provideNonDnsShapedTrustDomains")
+    void testTrustDomainFromNonDnsShapedName_isAccepted(String input) {
+        TrustDomain trustDomain = TrustDomain.parse(input);
+        assertEquals(input, trustDomain.getName());
+    }
+
     @Test
     void testFromIdStringWithoutPath() {
         TrustDomain trustDomain = TrustDomain.parse("spiffe://trustdomain");
@@ -90,6 +109,16 @@ static Stream<Arguments> provideInvalidTrustDomain() {
         );
     }
 
+    static Stream<String> provideNonDnsShapedTrustDomains() {
+        return Stream.of(
+                "example..org",
+                ".example.org",
+                "example.org.",
+                "-example.org",
+                "example-.org"
+        );
+    }
+
     @Test
     void testNewSpiffeId() {
         TrustDomain trustDomain = TrustDomain.parse("test.domain");
@@ -135,6 +164,12 @@ void testParseInvalidScheme_httpScheme_throwsInvalidScheme() {
                 () -> TrustDomain.parse("http://example.org"));
     }
 
+    @Test
+    void testParseIpv6TrustDomain_throwsInvalidTrustDomain() {
+        assertThrows(InvalidSpiffeIdException.class,
+                () -> TrustDomain.parse("[::1]"));
+    }
+
     @Test
     void testParseColonNotFollowedBySlash_validatesAsTrustDomain() {
         assertThrows(InvalidSpiffeIdException.class,

java-spiffe-core/src/test/java/io/spiffe/svid/jwtsvid/JwtSvidParseInsecureTest.java

@@ -84,7 +84,7 @@ void testParseInsecure_nullAudience_throwsNullPointerException() throws JwtSvidE
         try {
             KeyPair key1 = TestUtils.generateECKeyPair(Curve.P_521);
             TrustDomain trustDomain = TrustDomain.parse("test.domain");
-            SpiffeId spiffeId = trustDomain.newSpiffeId("/host");
+            SpiffeId spiffeId = trustDomain.newSpiffeId("host");
             Set<String> audience = Collections.singleton("audience");
             Date expiration = new Date(System.currentTimeMillis() + 3600000);
             JWTClaimsSet claims = TestUtils.buildJWTClaimSet(audience, spiffeId.toString(), expiration);