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

Đang mở Phù hợp với người mới
#13,154 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức phù hợp với người mới
82/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
java
Lĩnh vực
security

Hướng nghiên cứu

Chỗ thiếu nằm trong core/src/main/java/io/grpc/internal/SpiffeUtil.java, tại loadTrustBundleFromFile, nơi các khóa của trust_domains được duyệt mà không kiểm tra. Hàm hỗ trợ có sẵn validateTrustDomain trong cùng tệp đã thực thi quy tắc này. Hãy chạy SpiffeUtilTest trong core/src/test/java/io/grpc/internal/, sau đó thêm một kiểm thử cho khóa không hợp lệ như UPPER.example.com. Hoàn thành khi việc tải ném ra IllegalArgumentException với khóa không hợp lệ và các kiểm thử hiện có vẫn đạt.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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.

Ngôn ngữ chính
Java
Star
12.1k
Fork
4k
Merge trung bình
1 ngày 13 giờ
Pull request đã merge (30 ngày)
42

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của grpc/grpc-java

Tất cả issue của grpc/grpc-java

Issue tương tự

Thêm issue về Java

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.