SpiffeUtil.loadTrustBundleFromFile accepts invalid trust domain names (e.g. `UPPER.example.com`, `invalid#character`) as bundle map keys

Offen Anfängerfreundlich
#13,154 1 Kommentar 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Maintainer antworten meist innerhalb von 1 Tag

Dieses Issue hat noch niemand übernommen.

Bewertung

Schwierigkeit
2/5
Geschätzter Aufwand
1-3 Stunden
Anfängerfreundlichkeit
82/100
Issue-Typ
Bug
Klarheit
Klar beschrieben
Aktivitätsstatus
Aktiv
Tech-Stack
java
Bereich
security

Rechercherichtung

Die Lücke liegt in core/src/main/java/io/grpc/internal/SpiffeUtil.java, in loadTrustBundleFromFile, wo die Schlüssel von trust_domains ohne Validierung durchlaufen werden. Der vorhandene Helfer validateTrustDomain in derselben Datei setzt die Regel bereits durch. Führe SpiffeUtilTest in core/src/test/java/io/grpc/internal/ aus und füge dann einen Test für einen ungültigen Schlüssel wie UPPER.example.com hinzu. Fertig ist es, wenn das Laden für ungültige Schlüssel IllegalArgumentException wirft und die bestehenden Tests weiterhin bestehen.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

bug
What version of gRPC-Java are you using?

master at 15d64a4 (1.86.0-SNAPSHOT); SpiffeUtil is unchanged on current master.

What is your environment?

Linux x86_64, OpenJDK 21.0.2, Gradle 8.14.5 (wrapper).

What did you expect to see?

Loading a SPIFFE bundle map whose trust_domains keys are not valid SPIFFE trust domain names fails with IllegalArgumentException. The javadoc says: "If any element of the JSON content is invalid or unsupported, an IllegalArgumentException is thrown and the entire Bundle is considered invalid." SpiffeUtil already has validateTrustDomain() ([a-z0-9._-]+, max 255 chars), which SpiffeUtil.parse() uses.

What did you see instead?

The file loads and the invalid names are kept as keys of the bundle map.

Location (grpc/grpc-java @ 15d64a4):

  public static SpiffeBundle loadTrustBundleFromFile(String trustBundleFile) throws IOException {
    Map<String, ?> trustDomainsNode = readTrustDomainsFromFile(trustBundleFile);
    Map<String, List<X509Certificate>> trustBundleMap = new HashMap<>();
    Map<String, Long> sequenceNumbers = new HashMap<>();
    for (String trustDomainName : trustDomainsNode.keySet()) {
      Map<String, ?> domainNode = JsonUtil.getObject(trustDomainsNode, trustDomainName);

Problem

The map is used by the file-watcher certificate provider (spiffe_trust_bundle_map_file, xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProvider.java:141-144). A peer's trust domain always comes from SpiffeUtil.parse(), so it is always valid and lower-case. A key like Example.com or invalid#character can therefore never match a peer. That entry is silently dead: the bundle loads without error, and handshakes from that domain later fail with Spiffe Trust Map doesn't contain trust domain 'example.com' from peer leaf certificate, which points away from the real problem (a malformed key in the file). This fails closed, not open; the issue is that a malformed bundle file is accepted instead of being rejected when it is loaded, as the javadoc promises.

Steps to reproduce the bug

Reproduction

Regression test added to SpiffeUtilTest.CertificateApiTest:

diff --git a/core/src/test/java/io/grpc/internal/SpiffeUtilTest.java b/core/src/test/java/io/grpc/internal/SpiffeUtilTest.java
--- a/core/src/test/java/io/grpc/internal/SpiffeUtilTest.java
+++ b/core/src/test/java/io/grpc/internal/SpiffeUtilTest.java
@@ -32,6 +32,7 @@ import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
 import java.security.cert.X509Certificate;
 import java.util.Arrays;
 import java.util.Collection;
@@ -419,6 +420,21 @@ public class SpiffeUtilTest {
           + DOMAIN_ERROR_MESSAGE, iae.getMessage());
     }
 
+    @Test
+    public void loadTrustBundleFromFileInvalidTrustDomainTest() throws Exception {
+      for (String trustDomain : new String[] {"invalid#character", "UPPER.example.com"}) {
+        File bundleFile = tempFolder.newFile();
+        try (OutputStream fileStream = new FileOutputStream(bundleFile)) {
+          fileStream.write(("{\"trust_domains\": {\"" + trustDomain + "\": {}}}")
+              .getBytes(StandardCharsets.UTF_8));
+        }
+        IllegalArgumentException iae = assertThrows(IllegalArgumentException.class,
+            () -> SpiffeUtil.loadTrustBundleFromFile(bundleFile.toString()));
+        assertEquals("Trust Domain must contain only letters, numbers, dots, dashes, and "
+            + "underscores ([a-z0-9.-_])", iae.getMessage());
+      }
+    }
+
     @Test
     public void loadTrustBundleFromFileParameterValidityTest() {
       NullPointerException npe = assertThrows(NullPointerException.class, () -> SpiffeUtil

./gradlew :grpc-core:test --tests 'io.grpc.internal.SpiffeUtilTest*' -PskipCodegen=true -PskipAndroid=true

SpiffeUtilTest > io.grpc.internal.SpiffeUtilTest$CertificateApiTest.loadTrustBundleFromFileInvalidTrustDomainTest FAILED
    java.lang.AssertionError: expected java.lang.IllegalArgumentException to be thrown, but nothing was thrown
        at org.junit.Assert.assertThrows(Assert.java:1028)
        at org.junit.Assert.assertThrows(Assert.java:981)
        at io.grpc.internal.SpiffeUtilTest$CertificateApiTest.loadTrustBundleFromFileInvalidTrustDomainTest(SpiffeUtilTest.java:431)

1583 tests completed, 1 failed, 17 skipped

Environment:

Already fixed in grpc/grpc-go

PR: https://github.com/grpc/grpc-go/pull/8124 ("[Security] Add support for parsing SPIFFE Bundle Maps", merged 2025-03-12T17:32:01Z, merge commit 75d4a606397). grpc-go's bundle map loader parses every key with spiffeid.TrustDomainFromString and fails the whole file on an invalid trust domain; the PR adds testdata/spiffe/spiffebundle_invalid_trustdomain.json (key invalid#character) to its failure cases.

How grpc-go fixed it (relevant hunks, verbatim):

--- a/internal/credentials/spiffe/spiffe.go
+++ b/internal/credentials/spiffe/spiffe.go
@@ -0,0 +1,68 @@
...
+	bundleMap := map[string]*spiffebundle.Bundle{}
+	for td, jsonBundle := range result.Bundles {
+		trustDomain, err := spiffeid.TrustDomainFromString(td)
+		if err != nil {
+			return nil, fmt.Errorf("spiffe: invalid trust domain (%v) found when parsing map file (%v): %v", td, filePath, err)
+		}
...

Suggested fix

Validate each trust_domains key with the existing validateTrustDomain() before parsing its keys:

diff --git a/core/src/main/java/io/grpc/internal/SpiffeUtil.java b/core/src/main/java/io/grpc/internal/SpiffeUtil.java
--- a/core/src/main/java/io/grpc/internal/SpiffeUtil.java
+++ b/core/src/main/java/io/grpc/internal/SpiffeUtil.java
@@ -170,6 +170,7 @@ public final class SpiffeUtil {
     Map<String, List<X509Certificate>> trustBundleMap = new HashMap<>();
     Map<String, Long> sequenceNumbers = new HashMap<>();
     for (String trustDomainName : trustDomainsNode.keySet()) {
+      validateTrustDomain(trustDomainName);
       Map<String, ?> domainNode = JsonUtil.getObject(trustDomainsNode, trustDomainName);
       if (domainNode.size() == 0) {
         trustBundleMap.put(trustDomainName, Collections.emptyList());

Verified on 15d64a4 (OpenJDK 21.0.2, Gradle 8.14.5): the test above fails without this patch and passes with it; no other test results change (:grpc-core:test; :grpc-xds:test, which uses the bundle map, also unchanged).

I can submit a PR for this.

Checked: #11575 and #11522 (initial SpiffeUtil / trust bundle loading) and #12911 (JWK entry missing x5c) are related but none covers trust domain name validation in the bundle map; searches for SpiffeUtil trust domain validation found nothing else.

Vorherrschende Sprache
Java
Sterne
12.1k
Forks
4k
Ø Merge
1 T. 13 Std.
Gemergte PRs (30 T.)
42

Entwicklungsumgebung

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus grpc/grpc-java

Alle Issues in grpc/grpc-java

Ähnliche Issues

Weitere Issues zu Java

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.