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

Abierto Apto para principiantes
#13,154 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
82/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
java
Área
security

Línea de trabajo

La brecha está en core/src/main/java/io/grpc/internal/SpiffeUtil.java, en loadTrustBundleFromFile, donde las claves de trust_domains se recorren sin validación. El helper existente validateTrustDomain en el mismo archivo ya aplica la regla. Ejecuta SpiffeUtilTest en core/src/test/java/io/grpc/internal/ y luego añade una prueba para una clave inválida como UPPER.example.com. Está terminado cuando la carga lanza IllegalArgumentException para claves inválidas y las pruebas existentes siguen pasando.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.

Lenguaje dominante
Java
Estrellas
12.1k
Forks
4k
Merge medio
1 d 13 h
PR fusionados (30 d)
42

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de grpc/grpc-java

Todos los issues de grpc/grpc-java

Issues similares

Más issues de Java

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.