From 919285cfa7feea35185075a644c6cc291c16c27c Mon Sep 17 00:00:00 2001 From: vimanikag Date: Tue, 9 Sep 2025 11:52:35 +0000 Subject: [PATCH 01/12] 12326 :: DNS SAN matching does not handle wildcards in split patterns that Envoy does --- .../certs/bad_wildcard_dns_certificate.pem | 24 ++++ .../certs/wildcard_dns_certificate.pem | 25 ++++ .../security/trust/XdsX509TrustManager.java | 87 ++++++++--- .../security/CommonTlsContextTestsUtil.java | 2 + .../trust/XdsX509TrustManagerTest.java | 135 ++++++++++++++++++ 5 files changed, 254 insertions(+), 19 deletions(-) create mode 100644 testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem create mode 100644 testing/src/main/resources/certs/wildcard_dns_certificate.pem diff --git a/testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem b/testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem new file mode 100644 index 00000000000..040b07773cb --- /dev/null +++ b/testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem @@ -0,0 +1,24 @@ +-----BEGIN CERTIFICATE----- +MIID7zCCAtegAwIBAgIUCs5j4C2KXgCRVFa48kc5TYRS1JswDQYJKoZIhvcNAQEL +BQAwGTEXMBUGA1UEAwwOTXkgSW50ZXJuYWwgQ0EwHhcNMjUwOTA4MTI0NTQyWhcN +MjYwOTA4MTI0NTQyWjBlMQswCQYDVQQGEwJVUzERMA8GA1UECAwISWxsaW5vaXMx +EDAOBgNVBAcMB0NoaWNhZ28xFTATBgNVBAoMDEV4YW1wbGUsIENvLjEaMBgGA1UE +AwwRKi50ZXN0Lmdvb2dsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK +AoIBAQCqKnJzYfTFd/Rh+iYwuSlTDejDtSKE8OsByTdCSf5xjcesnCLASvEoCXmE +UdgRKU+lmW2vn9OCgoXAeGfbYjyEjb1AaZhL++qwLHbXAGcEqRdOquMMR1RORa1+ +pjU/IZOOPJgwQxh1FzQE+oP3v1ZbelNF0crru9d4G2atV+iR9vRRuxCdy1Md+Yer +BJL05WWd5ujSa+82KKq2If4EZD4oLT8WjXKF6NIFZuCBHXtLGM9u0lsjR+L/6Ntz +cp0rpTsMeA8BIQTl3pC2+UCRwasDEz8p2jJ3AUCFxfj13rsTfWt80eg0p/oxsINN +PLUtLZ9hbgLyQwZdKWhMpHTq9qzTAgMBAAGjgeIwgd8wCwYDVR0PBAQDAgXgMBMG +A1UdJQQMMAoGCCsGAQUFBwMBMHsGA1UdEQR0MHKCCioqbHlmdC5jb22CCmx5ZnQq +Ki5jb22CCmx5KipmdC5jb22CCGx5ZnQuYyptgggqeWZ0LmMqbYIKKi5seWZ0LmNv +bYIHbCpmdC5jb4IIbHk/dC5jb22CCGxmKnQuY29tggkqbHlmdC5jb22HBMCoAQMw +HQYDVR0OBBYEFGaC9jswbSvwVM0mH4Fw8d4g43CEMB8GA1UdIwQYMBaAFB5bLRTe +Vki0qsiYFA8ugQdM9Aa4MA0GCSqGSIb3DQEBCwUAA4IBAQA4KAJD17VZqzS59mKw +k5mZAmQoY5LbTIusbUuHKvVMJig6bDFwbbeTwcSE492sZQQN/ZP0OlAQGBK/pxl9 +ynrTlh95SqhLgWgVfh//EmVKbMq+tJKlixz7fTgpjMxka4iCzzQtyYUIy3XhrqKY +B8TBt4M2O52clG/xp/2zMvs4zkjXxuHVSHpMWQV4wGqb+/Rk5oPUCqklOfqQHQcf +3EqqVVArk0AzG0tHiXiQUNggioMZfL/pqsLqOsnSVKSCg5avy4sVXDoB5YHtBpx2 +VL77nfG49WbSg5yGqrPAzIeAu6+ffhTt0XhegxvaV/F/ZnvSMSI59ntGYfsoEqtc +O8w2 +-----END CERTIFICATE----- \ No newline at end of file diff --git a/testing/src/main/resources/certs/wildcard_dns_certificate.pem b/testing/src/main/resources/certs/wildcard_dns_certificate.pem new file mode 100644 index 00000000000..ab4b21c2c10 --- /dev/null +++ b/testing/src/main/resources/certs/wildcard_dns_certificate.pem @@ -0,0 +1,25 @@ +-----BEGIN CERTIFICATE----- +MIIEIjCCAwqgAwIBAgIUCs5j4C2KXgCRVFa48kc5TYRS1JkwDQYJKoZIhvcNAQEL +BQAwGTEXMBUGA1UEAwwOTXkgSW50ZXJuYWwgQ0EwHhcNMjUwOTA4MTIzNTI4WhcN +MjYwOTA4MTIzNTI4WjBlMQswCQYDVQQGEwJVUzERMA8GA1UECAwISWxsaW5vaXMx +EDAOBgNVBAcMB0NoaWNhZ28xFTATBgNVBAoMDEV4YW1wbGUsIENvLjEaMBgGA1UE +AwwRKi50ZXN0Lmdvb2dsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK +AoIBAQCqKnJzYfTFd/Rh+iYwuSlTDejDtSKE8OsByTdCSf5xjcesnCLASvEoCXmE +UdgRKU+lmW2vn9OCgoXAeGfbYjyEjb1AaZhL++qwLHbXAGcEqRdOquMMR1RORa1+ +pjU/IZOOPJgwQxh1FzQE+oP3v1ZbelNF0crru9d4G2atV+iR9vRRuxCdy1Md+Yer +BJL05WWd5ujSa+82KKq2If4EZD4oLT8WjXKF6NIFZuCBHXtLGM9u0lsjR+L/6Ntz +cp0rpTsMeA8BIQTl3pC2+UCRwasDEz8p2jJ3AUCFxfj13rsTfWt80eg0p/oxsINN +PLUtLZ9hbgLyQwZdKWhMpHTq9qzTAgMBAAGjggEUMIIBEDALBgNVHQ8EBAMCBeAw +EwYDVR0lBAwwCgYIKwYBBQUHAwEwgasGA1UdEQSBozCBoIIQKi50ZXN0Lmdvb2ds +ZS5mcoIYd2F0ZXJ6b29pLnRlc3QuZ29vZ2xlLmJlghIqLnRlc3QueW91dHViZS5j +b22CCmEubHlmdC5jb22CCmEuTFlGVC5jb22CCWx5ZnQqLmNvbYIJKmx5ZnQuY29t +gghseWYqLmNvbYIJbHlmdCouY29tgghsKmZ0LmNvbYILdCoubHlmdC5jb22HBMCo +AQMwHQYDVR0OBBYEFGaC9jswbSvwVM0mH4Fw8d4g43CEMB8GA1UdIwQYMBaAFB5b +LRTeVki0qsiYFA8ugQdM9Aa4MA0GCSqGSIb3DQEBCwUAA4IBAQC5bu34wiKkck4z +aejXjh2PtW6YyzJS2eIi2MbRtF27WA7okM6ZYpz/Xf7dYygSitfsVgUyciZkkkf9 +I6Qi7M7cImBVpagB9w1HA6Fm30Flphgs+HhFdOB/VwDL1sU7YI4R88tPugnANeVq +cxxUbfkZvUxkbwnkgnA+ZoH6Orwjaz1I8I1mTJtZ6IotU42F2iwBBLv6r3xeiOq/ +gwmnPwO8T002OT5m8GyXd6O7cWMRH/Ys0K/hNpLmYWQxa86F3oWJi8RGF/h3ORnz +w7AAWS0PahXx0tmsaZNTZGOwyTnRL7thiXJajdCwpWWsClfgwhhZaZgUrqKbx3/r +2wZpTadR +-----END CERTIFICATE----- \ No newline at end of file diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index 1ecfe378d29..7324dce1ee4 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -111,45 +111,59 @@ private static boolean verifyDnsNameSafeRegex( private static boolean verifyDnsNamePrefix( String altNameFromCert, String sanToVerifyPrefix, boolean ignoreCase) { - if (Strings.isNullOrEmpty(sanToVerifyPrefix)) { + if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifyPrefix)) { return false; } - return ignoreCase - ? altNameFromCert.toLowerCase(Locale.ROOT).startsWith( - sanToVerifyPrefix.toLowerCase(Locale.ROOT)) - : altNameFromCert.startsWith(sanToVerifyPrefix); + if ((ignoreCase ? sanToVerifyPrefix.toLowerCase() : sanToVerifyPrefix).indexOf('*') == -1) { + return ignoreCase + ? altNameFromCert.toLowerCase(Locale.ROOT).startsWith( + sanToVerifyPrefix.toLowerCase(Locale.ROOT)) + : altNameFromCert.startsWith(sanToVerifyPrefix); + } + return verifyDnsNameWildcard(altNameFromCert, sanToVerifyPrefix , ignoreCase); } private static boolean verifyDnsNameSuffix( String altNameFromCert, String sanToVerifySuffix, boolean ignoreCase) { - if (Strings.isNullOrEmpty(sanToVerifySuffix)) { + if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifySuffix)) { return false; } - return ignoreCase - ? altNameFromCert.toLowerCase(Locale.ROOT).endsWith( - sanToVerifySuffix.toLowerCase(Locale.ROOT)) - : altNameFromCert.endsWith(sanToVerifySuffix); + if ((ignoreCase ? sanToVerifySuffix.toLowerCase() : sanToVerifySuffix).indexOf('*') == -1) { + return ignoreCase + ? altNameFromCert.toLowerCase(Locale.ROOT).endsWith( + sanToVerifySuffix.toLowerCase(Locale.ROOT)) + : altNameFromCert.endsWith(sanToVerifySuffix); + } + return verifyDnsNameWildcard(altNameFromCert, sanToVerifySuffix , ignoreCase); } private static boolean verifyDnsNameContains( String altNameFromCert, String sanToVerifySubstring, boolean ignoreCase) { - if (Strings.isNullOrEmpty(sanToVerifySubstring)) { + if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifySubstring)) { return false; } - return ignoreCase - ? altNameFromCert.toLowerCase(Locale.ROOT).contains( - sanToVerifySubstring.toLowerCase(Locale.ROOT)) - : altNameFromCert.contains(sanToVerifySubstring); + if ((ignoreCase + ? sanToVerifySubstring.toLowerCase() + : sanToVerifySubstring).indexOf('*') == -1) { + return ignoreCase + ? altNameFromCert.toLowerCase(Locale.ROOT).contains( + sanToVerifySubstring.toLowerCase(Locale.ROOT)) + : altNameFromCert.contains(sanToVerifySubstring); + } + return verifyDnsNameWildcard(altNameFromCert, sanToVerifySubstring , ignoreCase); } private static boolean verifyDnsNameExact( String altNameFromCert, String sanToVerifyExact, boolean ignoreCase) { - if (Strings.isNullOrEmpty(sanToVerifyExact)) { + if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifyExact)) { return false; } - return ignoreCase - ? sanToVerifyExact.equalsIgnoreCase(altNameFromCert) - : sanToVerifyExact.equals(altNameFromCert); + if ((ignoreCase ? sanToVerifyExact.toLowerCase() : sanToVerifyExact).indexOf('*') == -1) { + return ignoreCase + ? sanToVerifyExact.equalsIgnoreCase(altNameFromCert) + : sanToVerifyExact.equals(altNameFromCert); + } + return verifyDnsNameWildcard(altNameFromCert, sanToVerifyExact , ignoreCase); } private static boolean verifyDnsNameInSanList( @@ -303,4 +317,39 @@ public X509Certificate[] getAcceptedIssuers() { } return delegate.getAcceptedIssuers(); } + + private static boolean verifyDnsNameWildcard( + String altNameFromCert, String sanToVerify, boolean ignoreCase) { + if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerify)) { + return false; + } + String[] certLabels = (ignoreCase ? altNameFromCert.toLowerCase() : altNameFromCert) + .split("\\.", -1); + String[] sanLabels = (ignoreCase ? sanToVerify.toLowerCase() : sanToVerify) + .split("\\.", -1); + if (certLabels.length != sanLabels.length) { + return false; + } + if ((int) sanLabels[0].chars().filter(ch -> ch == '*').count() != 1 + || sanLabels[0].startsWith("xn--")) { + return false; + } + for (int i = 1; i < sanLabels.length; i++) { + if (!sanLabels[i].equals(certLabels[i])) { + return false; + } + } + return labelWildcardMatch(certLabels[0], sanLabels[0]); + } + + private static boolean labelWildcardMatch(String certLabel, String sanLabel) { + int starIndex = sanLabel.indexOf('*'); + String prefix = sanLabel.substring(0, starIndex); + String suffix = sanLabel.substring(starIndex + 1); + + if (certLabel.length() < (prefix).length() + suffix.length()) { + return false; + } + return certLabel.startsWith(prefix) && certLabel.endsWith(suffix); + } } diff --git a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java index 48814dece1d..c133ab04002 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java @@ -60,6 +60,8 @@ public class CommonTlsContextTestsUtil { public static final String BAD_SERVER_KEY_FILE = "badserver.key"; public static final String BAD_CLIENT_PEM_FILE = "badclient.pem"; public static final String BAD_CLIENT_KEY_FILE = "badclient.key"; + public static final String WILDCARD_DNS_PEM_FILE = "wildcard_dns_certificate.pem"; + public static final String BAD_WILDCARD_DNS_PEM_FILE = "bad_wildcard_dns_certificate.pem"; /** takes additional values and creates CombinedCertificateValidationContext as needed. */ private static CommonTlsContext buildCommonTlsContextWithAdditionalValues( diff --git a/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java b/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java index 6fa3d2e7d24..a6562fda970 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java @@ -18,11 +18,13 @@ import static com.google.common.truth.Truth.assertThat; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.BAD_SERVER_PEM_FILE; +import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.BAD_WILDCARD_DNS_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CA_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_SPIFFE_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_SPIFFE_PEM_FILE; +import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.WILDCARD_DNS_PEM_FILE; import static org.junit.Assert.fail; import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.doReturn; @@ -691,6 +693,139 @@ public void unsupportedAltNameType() throws CertificateException, IOException { } } + @Test + public void testVerifyDnsName_succeedsForValidWildcardSanNames() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setExact("*.lyft.com") + .setIgnoreCase(false) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); + trustManager.verifySubjectAltNameInChain(certs); + } + + @Test + public void testVerifyDnsName_succeedsForValidWildcardSanNames_ignoreCase() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setExact("*.lyft.com") + .setIgnoreCase(true) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); + trustManager.verifySubjectAltNameInChain(certs); + } + + @Test + public void testVerifyDnsName_failsForInvalidWildcard_SanNames() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setExact("lyft.com") + .setIgnoreCase(false) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(BAD_WILDCARD_DNS_PEM_FILE)); + try { + trustManager.verifySubjectAltNameInChain(certs); + fail("no exception thrown"); + } catch (CertificateException expected) { + assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); + } + } + + @Test + public void testVerifyDnsName_failsForInvalidWildcardSanNames_ignoreCase() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setExact("lyft.com") + .setIgnoreCase(true) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(BAD_WILDCARD_DNS_PEM_FILE)); + try { + trustManager.verifySubjectAltNameInChain(certs); + fail("no exception thrown"); + } catch (CertificateException expected) { + assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); + } + } + + @Test + public void testVerifyDnsName_failsForExtraLabel() throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setExact("test.lyft.com.extra") + .setIgnoreCase(false) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(BAD_WILDCARD_DNS_PEM_FILE)); + try { + trustManager.verifySubjectAltNameInChain(certs); + fail("no exception thrown"); + } catch (CertificateException expected) { + assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); + } + } + + @Test + public void testVerifyDnsName_failsForExtraLabel_ignoreCase() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setExact("test.lyft.com.extra") + .setIgnoreCase(true) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(BAD_WILDCARD_DNS_PEM_FILE)); + try { + trustManager.verifySubjectAltNameInChain(certs); + fail("no exception thrown"); + } catch (CertificateException expected) { + assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); + } + } + private TestSslEngine buildTrustManagerAndGetSslEngine() throws CertificateException, IOException, CertStoreException { SSLParameters sslParams = buildTrustManagerAndGetSslParameters(); From 6b0de5cd3f38cd155e54e02db1b9a9b3c7ff6d19 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Tue, 9 Sep 2025 12:54:07 +0000 Subject: [PATCH 02/12] 12326 :: DNS SAN matching does not handle wildcards in split patterns that Envoy does --- .../certs/bad_wildcard_dns_certificate.pem | 2 +- .../certs/wildcard_dns_certificate.pem | 2 +- .../security/trust/XdsX509TrustManager.java | 21 ++++++++++++------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem b/testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem index 040b07773cb..c82ae9b32c6 100644 --- a/testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem +++ b/testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem @@ -21,4 +21,4 @@ B8TBt4M2O52clG/xp/2zMvs4zkjXxuHVSHpMWQV4wGqb+/Rk5oPUCqklOfqQHQcf 3EqqVVArk0AzG0tHiXiQUNggioMZfL/pqsLqOsnSVKSCg5avy4sVXDoB5YHtBpx2 VL77nfG49WbSg5yGqrPAzIeAu6+ffhTt0XhegxvaV/F/ZnvSMSI59ntGYfsoEqtc O8w2 ------END CERTIFICATE----- \ No newline at end of file +-----END CERTIFICATE----- diff --git a/testing/src/main/resources/certs/wildcard_dns_certificate.pem b/testing/src/main/resources/certs/wildcard_dns_certificate.pem index ab4b21c2c10..3c9e62f2b71 100644 --- a/testing/src/main/resources/certs/wildcard_dns_certificate.pem +++ b/testing/src/main/resources/certs/wildcard_dns_certificate.pem @@ -22,4 +22,4 @@ cxxUbfkZvUxkbwnkgnA+ZoH6Orwjaz1I8I1mTJtZ6IotU42F2iwBBLv6r3xeiOq/ gwmnPwO8T002OT5m8GyXd6O7cWMRH/Ys0K/hNpLmYWQxa86F3oWJi8RGF/h3ORnz w7AAWS0PahXx0tmsaZNTZGOwyTnRL7thiXJajdCwpWWsClfgwhhZaZgUrqKbx3/r 2wZpTadR ------END CERTIFICATE----- \ No newline at end of file +-----END CERTIFICATE----- diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index 7324dce1ee4..4431842e6e0 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -114,7 +114,9 @@ private static boolean verifyDnsNamePrefix( if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifyPrefix)) { return false; } - if ((ignoreCase ? sanToVerifyPrefix.toLowerCase() : sanToVerifyPrefix).indexOf('*') == -1) { + if ((ignoreCase + ? sanToVerifyPrefix.toLowerCase(Locale.ROOT) + : sanToVerifyPrefix).indexOf('*') == -1) { return ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT).startsWith( sanToVerifyPrefix.toLowerCase(Locale.ROOT)) @@ -128,7 +130,9 @@ private static boolean verifyDnsNameSuffix( if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifySuffix)) { return false; } - if ((ignoreCase ? sanToVerifySuffix.toLowerCase() : sanToVerifySuffix).indexOf('*') == -1) { + if ((ignoreCase + ? sanToVerifySuffix.toLowerCase(Locale.ROOT) + : sanToVerifySuffix).indexOf('*') == -1) { return ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT).endsWith( sanToVerifySuffix.toLowerCase(Locale.ROOT)) @@ -143,7 +147,7 @@ private static boolean verifyDnsNameContains( return false; } if ((ignoreCase - ? sanToVerifySubstring.toLowerCase() + ? sanToVerifySubstring.toLowerCase(Locale.ROOT) : sanToVerifySubstring).indexOf('*') == -1) { return ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT).contains( @@ -158,7 +162,9 @@ private static boolean verifyDnsNameExact( if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifyExact)) { return false; } - if ((ignoreCase ? sanToVerifyExact.toLowerCase() : sanToVerifyExact).indexOf('*') == -1) { + if ((ignoreCase + ? sanToVerifyExact.toLowerCase(Locale.ROOT) + : sanToVerifyExact).indexOf('*') == -1) { return ignoreCase ? sanToVerifyExact.equalsIgnoreCase(altNameFromCert) : sanToVerifyExact.equals(altNameFromCert); @@ -323,9 +329,9 @@ private static boolean verifyDnsNameWildcard( if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerify)) { return false; } - String[] certLabels = (ignoreCase ? altNameFromCert.toLowerCase() : altNameFromCert) + String[] certLabels = (ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT) : altNameFromCert) .split("\\.", -1); - String[] sanLabels = (ignoreCase ? sanToVerify.toLowerCase() : sanToVerify) + String[] sanLabels = (ignoreCase ? sanToVerify.toLowerCase(Locale.ROOT) : sanToVerify) .split("\\.", -1); if (certLabels.length != sanLabels.length) { return false; @@ -346,8 +352,7 @@ private static boolean labelWildcardMatch(String certLabel, String sanLabel) { int starIndex = sanLabel.indexOf('*'); String prefix = sanLabel.substring(0, starIndex); String suffix = sanLabel.substring(starIndex + 1); - - if (certLabel.length() < (prefix).length() + suffix.length()) { + if (certLabel.length() < prefix.length() + suffix.length()) { return false; } return certLabel.startsWith(prefix) && certLabel.endsWith(suffix); From 3f45fa6c72ed351a9d26ff95e83573d4925dc476 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Tue, 9 Sep 2025 13:59:21 +0000 Subject: [PATCH 03/12] 12326 :: DNS SAN matching does not handle wildcards in split patterns that Envoy does --- .../security/trust/XdsX509TrustManager.java | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index 4431842e6e0..075ec63f076 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -111,65 +111,65 @@ private static boolean verifyDnsNameSafeRegex( private static boolean verifyDnsNamePrefix( String altNameFromCert, String sanToVerifyPrefix, boolean ignoreCase) { - if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifyPrefix)) { + if (Strings.isNullOrEmpty(sanToVerifyPrefix)) { return false; } if ((ignoreCase ? sanToVerifyPrefix.toLowerCase(Locale.ROOT) - : sanToVerifyPrefix).indexOf('*') == -1) { - return ignoreCase - ? altNameFromCert.toLowerCase(Locale.ROOT).startsWith( - sanToVerifyPrefix.toLowerCase(Locale.ROOT)) - : altNameFromCert.startsWith(sanToVerifyPrefix); + : sanToVerifyPrefix).contains("*")) { + return verifyDnsNameWildcard(altNameFromCert, sanToVerifyPrefix , ignoreCase); } - return verifyDnsNameWildcard(altNameFromCert, sanToVerifyPrefix , ignoreCase); + return ignoreCase + ? altNameFromCert.toLowerCase(Locale.ROOT).startsWith( + sanToVerifyPrefix.toLowerCase(Locale.ROOT)) + : altNameFromCert.startsWith(sanToVerifyPrefix); } private static boolean verifyDnsNameSuffix( String altNameFromCert, String sanToVerifySuffix, boolean ignoreCase) { - if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifySuffix)) { + if (Strings.isNullOrEmpty(sanToVerifySuffix)) { return false; } if ((ignoreCase ? sanToVerifySuffix.toLowerCase(Locale.ROOT) - : sanToVerifySuffix).indexOf('*') == -1) { - return ignoreCase - ? altNameFromCert.toLowerCase(Locale.ROOT).endsWith( - sanToVerifySuffix.toLowerCase(Locale.ROOT)) - : altNameFromCert.endsWith(sanToVerifySuffix); + : sanToVerifySuffix).contains("*")) { + return verifyDnsNameWildcard(altNameFromCert, sanToVerifySuffix , ignoreCase); } - return verifyDnsNameWildcard(altNameFromCert, sanToVerifySuffix , ignoreCase); + return ignoreCase + ? altNameFromCert.toLowerCase(Locale.ROOT).endsWith( + sanToVerifySuffix.toLowerCase(Locale.ROOT)) + : altNameFromCert.endsWith(sanToVerifySuffix); } private static boolean verifyDnsNameContains( String altNameFromCert, String sanToVerifySubstring, boolean ignoreCase) { - if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifySubstring)) { + if (Strings.isNullOrEmpty(sanToVerifySubstring)) { return false; } if ((ignoreCase ? sanToVerifySubstring.toLowerCase(Locale.ROOT) - : sanToVerifySubstring).indexOf('*') == -1) { - return ignoreCase - ? altNameFromCert.toLowerCase(Locale.ROOT).contains( - sanToVerifySubstring.toLowerCase(Locale.ROOT)) - : altNameFromCert.contains(sanToVerifySubstring); + : sanToVerifySubstring).contains("*")) { + return verifyDnsNameWildcard(altNameFromCert, sanToVerifySubstring , ignoreCase); } - return verifyDnsNameWildcard(altNameFromCert, sanToVerifySubstring , ignoreCase); + return ignoreCase + ? altNameFromCert.toLowerCase(Locale.ROOT).contains( + sanToVerifySubstring.toLowerCase(Locale.ROOT)) + : altNameFromCert.contains(sanToVerifySubstring); } private static boolean verifyDnsNameExact( String altNameFromCert, String sanToVerifyExact, boolean ignoreCase) { - if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerifyExact)) { + if (Strings.isNullOrEmpty(sanToVerifyExact)) { return false; } if ((ignoreCase ? sanToVerifyExact.toLowerCase(Locale.ROOT) - : sanToVerifyExact).indexOf('*') == -1) { - return ignoreCase - ? sanToVerifyExact.equalsIgnoreCase(altNameFromCert) - : sanToVerifyExact.equals(altNameFromCert); + : sanToVerifyExact).contains("*")) { + return verifyDnsNameWildcard(altNameFromCert, sanToVerifyExact , ignoreCase); } - return verifyDnsNameWildcard(altNameFromCert, sanToVerifyExact , ignoreCase); + return ignoreCase + ? sanToVerifyExact.equalsIgnoreCase(altNameFromCert) + : sanToVerifyExact.equals(altNameFromCert); } private static boolean verifyDnsNameInSanList( From f1265e7370cd240182a2f137f9bbcd3868d15cad Mon Sep 17 00:00:00 2001 From: vimanikag Date: Tue, 9 Sep 2025 15:19:04 +0000 Subject: [PATCH 04/12] 12326 :: DNS SAN matching does not handle wildcards in split patterns that Envoy does --- .../security/trust/XdsX509TrustManager.java | 2 +- .../trust/XdsX509TrustManagerTest.java | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index 075ec63f076..e4106a8a925 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -324,7 +324,7 @@ public X509Certificate[] getAcceptedIssuers() { return delegate.getAcceptedIssuers(); } - private static boolean verifyDnsNameWildcard( + public static boolean verifyDnsNameWildcard( String altNameFromCert, String sanToVerify, boolean ignoreCase) { if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerify)) { return false; diff --git a/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java b/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java index a6562fda970..0f7d2239a2b 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java @@ -25,6 +25,9 @@ import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_SPIFFE_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.WILDCARD_DNS_PEM_FILE; +import static io.grpc.xds.internal.security.trust.XdsX509TrustManager.verifyDnsNameWildcard; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.doReturn; @@ -826,6 +829,39 @@ public void testVerifyDnsName_failsForExtraLabel_ignoreCase() } } + @Test + public void verifyDnsNameWildcard_codeCoverage() { + assertTrue(verifyDnsNameWildcard("a.lyft.com", "*.lyft.com" , true)); + assertTrue(verifyDnsNameWildcard("a.LYFT.com", "*.lyft.COM" , true)); + assertTrue(verifyDnsNameWildcard("lyft.com", "*yft.com" , true)); + assertTrue(verifyDnsNameWildcard("lyft.com", "*lyft.com" , true)); + assertTrue(verifyDnsNameWildcard("lyft.com", "lyf*.com" , true)); + assertTrue(verifyDnsNameWildcard("lyft.com", "lyft*.com" , true)); + assertTrue(verifyDnsNameWildcard("lyft.com", "l*ft.com" , true)); + assertTrue(verifyDnsNameWildcard("t.lyft.com", "t*.lyft.com" , true)); + assertTrue(verifyDnsNameWildcard("test.lyft.com", "t*.lyft.com" , true)); + assertTrue(verifyDnsNameWildcard("l-lots-of-stuff-ft.com", "l*ft.com" , true)); + + assertFalse(verifyDnsNameWildcard("t.lyft.com", "t*t.lyft.com", true)); + assertFalse(verifyDnsNameWildcard("lyft.com", "l*ft.co", true)); + assertFalse(verifyDnsNameWildcard("lyft.com", "ly?t.com", true)); + assertFalse(verifyDnsNameWildcard("lyft.com", "lf*t.com", true)); + assertFalse(verifyDnsNameWildcard(".lyft.com", "*lyft.com", true)); + assertFalse(verifyDnsNameWildcard("lyft.com", "**lyft.com", true)); + assertFalse(verifyDnsNameWildcard("lyft.com", "lyft**.com", true)); + assertFalse(verifyDnsNameWildcard("lyft.com", "ly**ft.com", true)); + assertFalse(verifyDnsNameWildcard("lyft.com", "lyft.c*m", true)); + assertFalse(verifyDnsNameWildcard("lyft.com", "*yft.c*m", true)); + assertFalse(verifyDnsNameWildcard("test.lyft.com.extra", "*.lyft.com", true)); + assertFalse(verifyDnsNameWildcard("a.b.lyft.com", "*.lyft.com", true)); + assertFalse(verifyDnsNameWildcard("foo.test.com", "*.lyft.com", true)); + assertFalse(verifyDnsNameWildcard("lyft.com", "*.lyft.com", true)); + assertFalse(verifyDnsNameWildcard("alyft.com", "*.lyft.com", true)); + assertFalse(verifyDnsNameWildcard("", "*lyft.com", true)); + assertFalse(verifyDnsNameWildcard("lyft.com", "", true)); + assertFalse(verifyDnsNameWildcard("xn--lyft.com", "*.xn--lyft.com", true)); + } + private TestSslEngine buildTrustManagerAndGetSslEngine() throws CertificateException, IOException, CertStoreException { SSLParameters sslParams = buildTrustManagerAndGetSslParameters(); From 35e38ce565933e9adeb563e4b57d498ed4d2c242 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Wed, 10 Sep 2025 02:27:15 +0000 Subject: [PATCH 05/12] 12326 :: DNS SAN matching does not handle wildcards in split patterns that Envoy does --- .../trust/XdsX509TrustManagerTest.java | 140 ++++++++++++++++-- 1 file changed, 130 insertions(+), 10 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java b/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java index 0f7d2239a2b..0ea133bcb28 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java @@ -697,7 +697,7 @@ public void unsupportedAltNameType() throws CertificateException, IOException { } @Test - public void testVerifyDnsName_succeedsForValidWildcardSanNames() + public void testVerifyDnsNameExact_succeedsForValidWildcardSanNames() throws CertificateException, IOException { StringMatcher stringMatcher = StringMatcher.newBuilder() @@ -712,15 +712,16 @@ public void testVerifyDnsName_succeedsForValidWildcardSanNames() trustManager = new XdsX509TrustManager(certContext, mockDelegate); X509Certificate[] certs = CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); + System.out.println(Arrays.toString(certs)); trustManager.verifySubjectAltNameInChain(certs); } @Test - public void testVerifyDnsName_succeedsForValidWildcardSanNames_ignoreCase() + public void testVerifyDnsNameExact_succeedsForValidWildcardSanNames_ignoreCase() throws CertificateException, IOException { StringMatcher stringMatcher = StringMatcher.newBuilder() - .setExact("*.lyft.com") + .setExact("*.LYFT.COM") .setIgnoreCase(true) .build(); @SuppressWarnings("deprecation") @@ -735,7 +736,7 @@ public void testVerifyDnsName_succeedsForValidWildcardSanNames_ignoreCase() } @Test - public void testVerifyDnsName_failsForInvalidWildcard_SanNames() + public void testVerifyDnsNameExact_failsForInvalidWildcard_SanNames() throws CertificateException, IOException { StringMatcher stringMatcher = StringMatcher.newBuilder() @@ -759,11 +760,11 @@ public void testVerifyDnsName_failsForInvalidWildcard_SanNames() } @Test - public void testVerifyDnsName_failsForInvalidWildcardSanNames_ignoreCase() + public void testVerifyDnsNameExact_failsForInvalidWildcardSanNames_ignoreCase() throws CertificateException, IOException { StringMatcher stringMatcher = StringMatcher.newBuilder() - .setExact("lyft.com") + .setExact("LYFT.COM") .setIgnoreCase(true) .build(); @SuppressWarnings("deprecation") @@ -783,7 +784,7 @@ public void testVerifyDnsName_failsForInvalidWildcardSanNames_ignoreCase() } @Test - public void testVerifyDnsName_failsForExtraLabel() throws CertificateException, IOException { + public void testVerifyDnsNameExact_failsForExtraLabel() throws CertificateException, IOException { StringMatcher stringMatcher = StringMatcher.newBuilder() .setExact("test.lyft.com.extra") @@ -806,11 +807,11 @@ public void testVerifyDnsName_failsForExtraLabel() throws CertificateException, } @Test - public void testVerifyDnsName_failsForExtraLabel_ignoreCase() + public void testVerifyDnsNameExact_failsForExtraLabel_ignoreCase() throws CertificateException, IOException { StringMatcher stringMatcher = StringMatcher.newBuilder() - .setExact("test.lyft.com.extra") + .setExact("TEST.LYFT.COM.EXTRA") .setIgnoreCase(true) .build(); @SuppressWarnings("deprecation") @@ -829,6 +830,126 @@ public void testVerifyDnsName_failsForExtraLabel_ignoreCase() } } + @Test + public void testVerifyDnsNameSuffix_succeedsForValidWildcardSanNames_ignoreCase() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setSuffix("*LYFT.COM") + .setIgnoreCase(true) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); + System.out.println(Arrays.toString(certs)); + trustManager.verifySubjectAltNameInChain(certs); + } + + @Test + public void testVerifyDnsNameSuffix_succeedsForValidWildcardSanNames() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setSuffix("*lyft.com") + .setIgnoreCase(false) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); + System.out.println(Arrays.toString(certs)); + trustManager.verifySubjectAltNameInChain(certs); + } + + @Test + public void testVerifyDnsNamePrefix_succeedsForValidWildcardSanNames_ignoreCase() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setPrefix("LYFT*.COM") + .setIgnoreCase(true) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); + System.out.println(Arrays.toString(certs)); + trustManager.verifySubjectAltNameInChain(certs); + } + + @Test + public void testVerifyDnsNamePrefix_succeedsForValidWildcardSanNames() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setPrefix("lyft*.com") + .setIgnoreCase(false) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); + System.out.println(Arrays.toString(certs)); + trustManager.verifySubjectAltNameInChain(certs); + } + + @Test + public void testVerifyDnsNameContains_succeedsForValidWildcardSanNames_ignoreCase() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setContains("zooI.Test.Google") + .setIgnoreCase(true) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); + System.out.println(Arrays.toString(certs)); + trustManager.verifySubjectAltNameInChain(certs); + } + + @Test + public void testVerifyDnsNameContains_succeedsForValidWildcardSanNames() + throws CertificateException, IOException { + StringMatcher stringMatcher = + StringMatcher.newBuilder() + .setContains("zooi.test.google") + .setIgnoreCase(false) + .build(); + @SuppressWarnings("deprecation") + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() + .addMatchSubjectAltNames(stringMatcher) + .build(); + trustManager = new XdsX509TrustManager(certContext, mockDelegate); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); + System.out.println(Arrays.toString(certs)); + trustManager.verifySubjectAltNameInChain(certs); + } + @Test public void verifyDnsNameWildcard_codeCoverage() { assertTrue(verifyDnsNameWildcard("a.lyft.com", "*.lyft.com" , true)); @@ -841,7 +962,6 @@ public void verifyDnsNameWildcard_codeCoverage() { assertTrue(verifyDnsNameWildcard("t.lyft.com", "t*.lyft.com" , true)); assertTrue(verifyDnsNameWildcard("test.lyft.com", "t*.lyft.com" , true)); assertTrue(verifyDnsNameWildcard("l-lots-of-stuff-ft.com", "l*ft.com" , true)); - assertFalse(verifyDnsNameWildcard("t.lyft.com", "t*t.lyft.com", true)); assertFalse(verifyDnsNameWildcard("lyft.com", "l*ft.co", true)); assertFalse(verifyDnsNameWildcard("lyft.com", "ly?t.com", true)); From 1100be458dd0f0893eab160163d29083963cef96 Mon Sep 17 00:00:00 2001 From: Sangamesh1997 Date: Wed, 10 Sep 2025 05:23:49 +0000 Subject: [PATCH 06/12] cosmatic changes --- .../xds/internal/security/trust/XdsX509TrustManager.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index e4106a8a925..f27bbe44d7d 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -120,9 +120,9 @@ private static boolean verifyDnsNamePrefix( return verifyDnsNameWildcard(altNameFromCert, sanToVerifyPrefix , ignoreCase); } return ignoreCase - ? altNameFromCert.toLowerCase(Locale.ROOT).startsWith( + ? altNameFromCert.toLowerCase(Locale.ROOT).startsWith( sanToVerifyPrefix.toLowerCase(Locale.ROOT)) - : altNameFromCert.startsWith(sanToVerifyPrefix); + : altNameFromCert.startsWith(sanToVerifyPrefix); } private static boolean verifyDnsNameSuffix( @@ -168,8 +168,8 @@ private static boolean verifyDnsNameExact( return verifyDnsNameWildcard(altNameFromCert, sanToVerifyExact , ignoreCase); } return ignoreCase - ? sanToVerifyExact.equalsIgnoreCase(altNameFromCert) - : sanToVerifyExact.equals(altNameFromCert); + ? sanToVerifyExact.equalsIgnoreCase(altNameFromCert) + : sanToVerifyExact.equals(altNameFromCert); } private static boolean verifyDnsNameInSanList( From 9e9875d8462dce022acea0fb2e23b30c3d9cb683 Mon Sep 17 00:00:00 2001 From: Sangamesh1997 Date: Wed, 10 Sep 2025 05:38:45 +0000 Subject: [PATCH 07/12] xds : XdsX509TrustManager: cosmetic changes --- .../grpc/xds/internal/security/trust/XdsX509TrustManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index f27bbe44d7d..bf80efb77d7 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -137,7 +137,7 @@ private static boolean verifyDnsNameSuffix( } return ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT).endsWith( - sanToVerifySuffix.toLowerCase(Locale.ROOT)) + sanToVerifySuffix.toLowerCase(Locale.ROOT)) : altNameFromCert.endsWith(sanToVerifySuffix); } @@ -153,7 +153,7 @@ private static boolean verifyDnsNameContains( } return ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT).contains( - sanToVerifySubstring.toLowerCase(Locale.ROOT)) + sanToVerifySubstring.toLowerCase(Locale.ROOT)) : altNameFromCert.contains(sanToVerifySubstring); } From 403faa8d748934e2c4147a6674931c35dc1b2de3 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Tue, 16 Sep 2025 12:02:48 +0000 Subject: [PATCH 08/12] 11246 :: addressed the review comments --- .../certs/bad_wildcard_dns_certificate.pem | 24 -- .../certs/wildcard_dns_certificate.pem | 25 --- .../security/trust/XdsX509TrustManager.java | 50 ++--- .../security/CommonTlsContextTestsUtil.java | 2 - .../trust/XdsX509TrustManagerTest.java | 209 +----------------- 5 files changed, 25 insertions(+), 285 deletions(-) delete mode 100644 testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem delete mode 100644 testing/src/main/resources/certs/wildcard_dns_certificate.pem diff --git a/testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem b/testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem deleted file mode 100644 index c82ae9b32c6..00000000000 --- a/testing/src/main/resources/certs/bad_wildcard_dns_certificate.pem +++ /dev/null @@ -1,24 +0,0 @@ ------BEGIN CERTIFICATE----- -MIID7zCCAtegAwIBAgIUCs5j4C2KXgCRVFa48kc5TYRS1JswDQYJKoZIhvcNAQEL -BQAwGTEXMBUGA1UEAwwOTXkgSW50ZXJuYWwgQ0EwHhcNMjUwOTA4MTI0NTQyWhcN -MjYwOTA4MTI0NTQyWjBlMQswCQYDVQQGEwJVUzERMA8GA1UECAwISWxsaW5vaXMx -EDAOBgNVBAcMB0NoaWNhZ28xFTATBgNVBAoMDEV4YW1wbGUsIENvLjEaMBgGA1UE -AwwRKi50ZXN0Lmdvb2dsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK -AoIBAQCqKnJzYfTFd/Rh+iYwuSlTDejDtSKE8OsByTdCSf5xjcesnCLASvEoCXmE -UdgRKU+lmW2vn9OCgoXAeGfbYjyEjb1AaZhL++qwLHbXAGcEqRdOquMMR1RORa1+ -pjU/IZOOPJgwQxh1FzQE+oP3v1ZbelNF0crru9d4G2atV+iR9vRRuxCdy1Md+Yer -BJL05WWd5ujSa+82KKq2If4EZD4oLT8WjXKF6NIFZuCBHXtLGM9u0lsjR+L/6Ntz -cp0rpTsMeA8BIQTl3pC2+UCRwasDEz8p2jJ3AUCFxfj13rsTfWt80eg0p/oxsINN -PLUtLZ9hbgLyQwZdKWhMpHTq9qzTAgMBAAGjgeIwgd8wCwYDVR0PBAQDAgXgMBMG -A1UdJQQMMAoGCCsGAQUFBwMBMHsGA1UdEQR0MHKCCioqbHlmdC5jb22CCmx5ZnQq -Ki5jb22CCmx5KipmdC5jb22CCGx5ZnQuYyptgggqeWZ0LmMqbYIKKi5seWZ0LmNv -bYIHbCpmdC5jb4IIbHk/dC5jb22CCGxmKnQuY29tggkqbHlmdC5jb22HBMCoAQMw -HQYDVR0OBBYEFGaC9jswbSvwVM0mH4Fw8d4g43CEMB8GA1UdIwQYMBaAFB5bLRTe -Vki0qsiYFA8ugQdM9Aa4MA0GCSqGSIb3DQEBCwUAA4IBAQA4KAJD17VZqzS59mKw -k5mZAmQoY5LbTIusbUuHKvVMJig6bDFwbbeTwcSE492sZQQN/ZP0OlAQGBK/pxl9 -ynrTlh95SqhLgWgVfh//EmVKbMq+tJKlixz7fTgpjMxka4iCzzQtyYUIy3XhrqKY -B8TBt4M2O52clG/xp/2zMvs4zkjXxuHVSHpMWQV4wGqb+/Rk5oPUCqklOfqQHQcf -3EqqVVArk0AzG0tHiXiQUNggioMZfL/pqsLqOsnSVKSCg5avy4sVXDoB5YHtBpx2 -VL77nfG49WbSg5yGqrPAzIeAu6+ffhTt0XhegxvaV/F/ZnvSMSI59ntGYfsoEqtc -O8w2 ------END CERTIFICATE----- diff --git a/testing/src/main/resources/certs/wildcard_dns_certificate.pem b/testing/src/main/resources/certs/wildcard_dns_certificate.pem deleted file mode 100644 index 3c9e62f2b71..00000000000 --- a/testing/src/main/resources/certs/wildcard_dns_certificate.pem +++ /dev/null @@ -1,25 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIEIjCCAwqgAwIBAgIUCs5j4C2KXgCRVFa48kc5TYRS1JkwDQYJKoZIhvcNAQEL -BQAwGTEXMBUGA1UEAwwOTXkgSW50ZXJuYWwgQ0EwHhcNMjUwOTA4MTIzNTI4WhcN -MjYwOTA4MTIzNTI4WjBlMQswCQYDVQQGEwJVUzERMA8GA1UECAwISWxsaW5vaXMx -EDAOBgNVBAcMB0NoaWNhZ28xFTATBgNVBAoMDEV4YW1wbGUsIENvLjEaMBgGA1UE -AwwRKi50ZXN0Lmdvb2dsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK -AoIBAQCqKnJzYfTFd/Rh+iYwuSlTDejDtSKE8OsByTdCSf5xjcesnCLASvEoCXmE -UdgRKU+lmW2vn9OCgoXAeGfbYjyEjb1AaZhL++qwLHbXAGcEqRdOquMMR1RORa1+ -pjU/IZOOPJgwQxh1FzQE+oP3v1ZbelNF0crru9d4G2atV+iR9vRRuxCdy1Md+Yer -BJL05WWd5ujSa+82KKq2If4EZD4oLT8WjXKF6NIFZuCBHXtLGM9u0lsjR+L/6Ntz -cp0rpTsMeA8BIQTl3pC2+UCRwasDEz8p2jJ3AUCFxfj13rsTfWt80eg0p/oxsINN -PLUtLZ9hbgLyQwZdKWhMpHTq9qzTAgMBAAGjggEUMIIBEDALBgNVHQ8EBAMCBeAw -EwYDVR0lBAwwCgYIKwYBBQUHAwEwgasGA1UdEQSBozCBoIIQKi50ZXN0Lmdvb2ds -ZS5mcoIYd2F0ZXJ6b29pLnRlc3QuZ29vZ2xlLmJlghIqLnRlc3QueW91dHViZS5j -b22CCmEubHlmdC5jb22CCmEuTFlGVC5jb22CCWx5ZnQqLmNvbYIJKmx5ZnQuY29t -gghseWYqLmNvbYIJbHlmdCouY29tgghsKmZ0LmNvbYILdCoubHlmdC5jb22HBMCo -AQMwHQYDVR0OBBYEFGaC9jswbSvwVM0mH4Fw8d4g43CEMB8GA1UdIwQYMBaAFB5b -LRTeVki0qsiYFA8ugQdM9Aa4MA0GCSqGSIb3DQEBCwUAA4IBAQC5bu34wiKkck4z -aejXjh2PtW6YyzJS2eIi2MbRtF27WA7okM6ZYpz/Xf7dYygSitfsVgUyciZkkkf9 -I6Qi7M7cImBVpagB9w1HA6Fm30Flphgs+HhFdOB/VwDL1sU7YI4R88tPugnANeVq -cxxUbfkZvUxkbwnkgnA+ZoH6Orwjaz1I8I1mTJtZ6IotU42F2iwBBLv6r3xeiOq/ -gwmnPwO8T002OT5m8GyXd6O7cWMRH/Ys0K/hNpLmYWQxa86F3oWJi8RGF/h3ORnz -w7AAWS0PahXx0tmsaZNTZGOwyTnRL7thiXJajdCwpWWsClfgwhhZaZgUrqKbx3/r -2wZpTadR ------END CERTIFICATE----- diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index bf80efb77d7..63c05729897 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -114,11 +114,6 @@ private static boolean verifyDnsNamePrefix( if (Strings.isNullOrEmpty(sanToVerifyPrefix)) { return false; } - if ((ignoreCase - ? sanToVerifyPrefix.toLowerCase(Locale.ROOT) - : sanToVerifyPrefix).contains("*")) { - return verifyDnsNameWildcard(altNameFromCert, sanToVerifyPrefix , ignoreCase); - } return ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT).startsWith( sanToVerifyPrefix.toLowerCase(Locale.ROOT)) @@ -130,11 +125,6 @@ private static boolean verifyDnsNameSuffix( if (Strings.isNullOrEmpty(sanToVerifySuffix)) { return false; } - if ((ignoreCase - ? sanToVerifySuffix.toLowerCase(Locale.ROOT) - : sanToVerifySuffix).contains("*")) { - return verifyDnsNameWildcard(altNameFromCert, sanToVerifySuffix , ignoreCase); - } return ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT).endsWith( sanToVerifySuffix.toLowerCase(Locale.ROOT)) @@ -146,11 +136,6 @@ private static boolean verifyDnsNameContains( if (Strings.isNullOrEmpty(sanToVerifySubstring)) { return false; } - if ((ignoreCase - ? sanToVerifySubstring.toLowerCase(Locale.ROOT) - : sanToVerifySubstring).contains("*")) { - return verifyDnsNameWildcard(altNameFromCert, sanToVerifySubstring , ignoreCase); - } return ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT).contains( sanToVerifySubstring.toLowerCase(Locale.ROOT)) @@ -326,35 +311,34 @@ public X509Certificate[] getAcceptedIssuers() { public static boolean verifyDnsNameWildcard( String altNameFromCert, String sanToVerify, boolean ignoreCase) { - if (Strings.isNullOrEmpty(altNameFromCert) || Strings.isNullOrEmpty(sanToVerify)) { - return false; - } - String[] certLabels = (ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT) : altNameFromCert) + String[] pattern = (ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT) : altNameFromCert) .split("\\.", -1); - String[] sanLabels = (ignoreCase ? sanToVerify.toLowerCase(Locale.ROOT) : sanToVerify) + String[] dnsLabel = (ignoreCase ? sanToVerify.toLowerCase(Locale.ROOT) : sanToVerify) .split("\\.", -1); - if (certLabels.length != sanLabels.length) { + if (pattern.length != dnsLabel.length) { return false; } - if ((int) sanLabels[0].chars().filter(ch -> ch == '*').count() != 1 - || sanLabels[0].startsWith("xn--")) { + if ((int) dnsLabel[0].chars().filter(ch -> ch == '*').count() != 1 + || dnsLabel[0].startsWith("xn--")) { return false; } - for (int i = 1; i < sanLabels.length; i++) { - if (!sanLabels[i].equals(certLabels[i])) { + for (int i = 1; i < dnsLabel.length; i++) { + if (!dnsLabel[i].equals(pattern[i])) { return false; } } - return labelWildcardMatch(certLabels[0], sanLabels[0]); + return labelWildcardMatch(pattern[0], dnsLabel[0]); } - private static boolean labelWildcardMatch(String certLabel, String sanLabel) { - int starIndex = sanLabel.indexOf('*'); - String prefix = sanLabel.substring(0, starIndex); - String suffix = sanLabel.substring(starIndex + 1); - if (certLabel.length() < prefix.length() + suffix.length()) { - return false; + private static boolean labelWildcardMatch(String pattern, String dnsLabel) { + if ("*".equals(pattern)) { + return true; } - return certLabel.startsWith(prefix) && certLabel.endsWith(suffix); + int starIndex = dnsLabel.indexOf('*'); + String prefix = dnsLabel.substring(0, starIndex); + String suffix = dnsLabel.substring(starIndex + 1); + return pattern.length() >= (dnsLabel.length() - 1) + && pattern.startsWith(prefix) + && pattern.endsWith(suffix); } } diff --git a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java index c133ab04002..48814dece1d 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java @@ -60,8 +60,6 @@ public class CommonTlsContextTestsUtil { public static final String BAD_SERVER_KEY_FILE = "badserver.key"; public static final String BAD_CLIENT_PEM_FILE = "badclient.pem"; public static final String BAD_CLIENT_KEY_FILE = "badclient.key"; - public static final String WILDCARD_DNS_PEM_FILE = "wildcard_dns_certificate.pem"; - public static final String BAD_WILDCARD_DNS_PEM_FILE = "bad_wildcard_dns_certificate.pem"; /** takes additional values and creates CombinedCertificateValidationContext as needed. */ private static CommonTlsContext buildCommonTlsContextWithAdditionalValues( diff --git a/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java b/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java index 0ea133bcb28..b9696690618 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java @@ -18,13 +18,11 @@ import static com.google.common.truth.Truth.assertThat; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.BAD_SERVER_PEM_FILE; -import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.BAD_WILDCARD_DNS_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CA_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_SPIFFE_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_SPIFFE_PEM_FILE; -import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.WILDCARD_DNS_PEM_FILE; import static io.grpc.xds.internal.security.trust.XdsX509TrustManager.verifyDnsNameWildcard; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -701,7 +699,7 @@ public void testVerifyDnsNameExact_succeedsForValidWildcardSanNames() throws CertificateException, IOException { StringMatcher stringMatcher = StringMatcher.newBuilder() - .setExact("*.lyft.com") + .setExact("*.test.youtube.com") .setIgnoreCase(false) .build(); @SuppressWarnings("deprecation") @@ -711,8 +709,7 @@ public void testVerifyDnsNameExact_succeedsForValidWildcardSanNames() .build(); trustManager = new XdsX509TrustManager(certContext, mockDelegate); X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); - System.out.println(Arrays.toString(certs)); + CertificateUtils.toX509Certificates(TlsTesting.loadCert(SERVER_1_PEM_FILE)); trustManager.verifySubjectAltNameInChain(certs); } @@ -721,7 +718,7 @@ public void testVerifyDnsNameExact_succeedsForValidWildcardSanNames_ignoreCase() throws CertificateException, IOException { StringMatcher stringMatcher = StringMatcher.newBuilder() - .setExact("*.LYFT.COM") + .setExact("*.TEST.YOUTUBE.com") .setIgnoreCase(true) .build(); @SuppressWarnings("deprecation") @@ -731,12 +728,12 @@ public void testVerifyDnsNameExact_succeedsForValidWildcardSanNames_ignoreCase() .build(); trustManager = new XdsX509TrustManager(certContext, mockDelegate); X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); + CertificateUtils.toX509Certificates(TlsTesting.loadCert(SERVER_1_PEM_FILE)); trustManager.verifySubjectAltNameInChain(certs); } @Test - public void testVerifyDnsNameExact_failsForInvalidWildcard_SanNames() + public void testVerifyDnsNameExact_failsForNotExactMatch_SanNames() throws CertificateException, IOException { StringMatcher stringMatcher = StringMatcher.newBuilder() @@ -750,78 +747,7 @@ public void testVerifyDnsNameExact_failsForInvalidWildcard_SanNames() .build(); trustManager = new XdsX509TrustManager(certContext, mockDelegate); X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(BAD_WILDCARD_DNS_PEM_FILE)); - try { - trustManager.verifySubjectAltNameInChain(certs); - fail("no exception thrown"); - } catch (CertificateException expected) { - assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); - } - } - - @Test - public void testVerifyDnsNameExact_failsForInvalidWildcardSanNames_ignoreCase() - throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setExact("LYFT.COM") - .setIgnoreCase(true) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(BAD_WILDCARD_DNS_PEM_FILE)); - try { - trustManager.verifySubjectAltNameInChain(certs); - fail("no exception thrown"); - } catch (CertificateException expected) { - assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); - } - } - - @Test - public void testVerifyDnsNameExact_failsForExtraLabel() throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setExact("test.lyft.com.extra") - .setIgnoreCase(false) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(BAD_WILDCARD_DNS_PEM_FILE)); - try { - trustManager.verifySubjectAltNameInChain(certs); - fail("no exception thrown"); - } catch (CertificateException expected) { - assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); - } - } - - @Test - public void testVerifyDnsNameExact_failsForExtraLabel_ignoreCase() - throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setExact("TEST.LYFT.COM.EXTRA") - .setIgnoreCase(true) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(BAD_WILDCARD_DNS_PEM_FILE)); + CertificateUtils.toX509Certificates(TlsTesting.loadCert(SERVER_1_PEM_FILE)); try { trustManager.verifySubjectAltNameInChain(certs); fail("no exception thrown"); @@ -831,127 +757,8 @@ public void testVerifyDnsNameExact_failsForExtraLabel_ignoreCase() } @Test - public void testVerifyDnsNameSuffix_succeedsForValidWildcardSanNames_ignoreCase() - throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setSuffix("*LYFT.COM") - .setIgnoreCase(true) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); - System.out.println(Arrays.toString(certs)); - trustManager.verifySubjectAltNameInChain(certs); - } - - @Test - public void testVerifyDnsNameSuffix_succeedsForValidWildcardSanNames() - throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setSuffix("*lyft.com") - .setIgnoreCase(false) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); - System.out.println(Arrays.toString(certs)); - trustManager.verifySubjectAltNameInChain(certs); - } - - @Test - public void testVerifyDnsNamePrefix_succeedsForValidWildcardSanNames_ignoreCase() - throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setPrefix("LYFT*.COM") - .setIgnoreCase(true) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); - System.out.println(Arrays.toString(certs)); - trustManager.verifySubjectAltNameInChain(certs); - } - - @Test - public void testVerifyDnsNamePrefix_succeedsForValidWildcardSanNames() - throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setPrefix("lyft*.com") - .setIgnoreCase(false) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); - System.out.println(Arrays.toString(certs)); - trustManager.verifySubjectAltNameInChain(certs); - } - - @Test - public void testVerifyDnsNameContains_succeedsForValidWildcardSanNames_ignoreCase() - throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setContains("zooI.Test.Google") - .setIgnoreCase(true) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); - System.out.println(Arrays.toString(certs)); - trustManager.verifySubjectAltNameInChain(certs); - } - - @Test - public void testVerifyDnsNameContains_succeedsForValidWildcardSanNames() - throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setContains("zooi.test.google") - .setIgnoreCase(false) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(WILDCARD_DNS_PEM_FILE)); - System.out.println(Arrays.toString(certs)); - trustManager.verifySubjectAltNameInChain(certs); - } - - @Test - public void verifyDnsNameWildcard_codeCoverage() { + public void testVerifyDnsNameWildcard() { + assertTrue(verifyDnsNameWildcard(".lyft.com", "*.lyft.com" , true)); assertTrue(verifyDnsNameWildcard("a.lyft.com", "*.lyft.com" , true)); assertTrue(verifyDnsNameWildcard("a.LYFT.com", "*.lyft.COM" , true)); assertTrue(verifyDnsNameWildcard("lyft.com", "*yft.com" , true)); From cee6325edbde3ff12eb2971b22973b9f2e8e2ad8 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Wed, 17 Sep 2025 10:00:25 +0000 Subject: [PATCH 09/12] 11246 :: addressed the review comments and updated the logic as per the envoy code. --- .../security/trust/XdsX509TrustManager.java | 57 ++++++++++--------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index 63c05729897..a32362efb64 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -147,10 +147,8 @@ private static boolean verifyDnsNameExact( if (Strings.isNullOrEmpty(sanToVerifyExact)) { return false; } - if ((ignoreCase - ? sanToVerifyExact.toLowerCase(Locale.ROOT) - : sanToVerifyExact).contains("*")) { - return verifyDnsNameWildcard(altNameFromCert, sanToVerifyExact , ignoreCase); + if (sanToVerifyExact.contains("*")) { + return verifyDnsNameWildcard(altNameFromCert, sanToVerifyExact, ignoreCase); } return ignoreCase ? sanToVerifyExact.equalsIgnoreCase(altNameFromCert) @@ -311,34 +309,41 @@ public X509Certificate[] getAcceptedIssuers() { public static boolean verifyDnsNameWildcard( String altNameFromCert, String sanToVerify, boolean ignoreCase) { - String[] pattern = (ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT) : altNameFromCert) - .split("\\.", -1); - String[] dnsLabel = (ignoreCase ? sanToVerify.toLowerCase(Locale.ROOT) : sanToVerify) - .split("\\.", -1); - if (pattern.length != dnsLabel.length) { + String[] splitPattern = splitAtFirstDelimiter(ignoreCase + ? sanToVerify.toLowerCase(Locale.ROOT) : sanToVerify); + String[] splitDnsName = splitAtFirstDelimiter(ignoreCase + ? altNameFromCert.toLowerCase(Locale.ROOT) : altNameFromCert); + if (splitPattern.length < 2 || splitDnsName.length < 2) { return false; } - if ((int) dnsLabel[0].chars().filter(ch -> ch == '*').count() != 1 - || dnsLabel[0].startsWith("xn--")) { - return false; + if (splitPattern[0].contains("*") + && !splitPattern[1].contains("*") + && !splitPattern[0].startsWith("xn--")) { + return splitDnsName[1].equals(splitPattern[1]) + && labelWildcardMatch(splitDnsName[0], splitPattern[0]); } - for (int i = 1; i < dnsLabel.length; i++) { - if (!dnsLabel[i].equals(pattern[i])) { - return false; - } - } - return labelWildcardMatch(pattern[0], dnsLabel[0]); + return false; } - private static boolean labelWildcardMatch(String pattern, String dnsLabel) { - if ("*".equals(pattern)) { + private static boolean labelWildcardMatch(String dnsLabel, String pattern) { + final char glob = '*'; + if (pattern.length() == 1 && pattern.charAt(0) == glob) { return true; } - int starIndex = dnsLabel.indexOf('*'); - String prefix = dnsLabel.substring(0, starIndex); - String suffix = dnsLabel.substring(starIndex + 1); - return pattern.length() >= (dnsLabel.length() - 1) - && pattern.startsWith(prefix) - && pattern.endsWith(suffix); + if (pattern.chars().filter(ch -> ch == glob).count() == 1) { + String[] splitPattern = pattern.split("\\*", -1); + return (pattern.length() <= dnsLabel.length() + 1) + && dnsLabel.startsWith(splitPattern[0]) + && dnsLabel.endsWith(splitPattern[1]); + } + return false; + } + + private static String[] splitAtFirstDelimiter(String s) { + int index = s.indexOf("."); + if (index == -1) { + return new String[]{s}; + } + return new String[]{s.substring(0, index), s.substring(index + 1)}; } } From 2e75fe018c9483cdeecda237c3fb7ba479ca6643 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Thu, 18 Sep 2025 08:17:13 +0000 Subject: [PATCH 10/12] 11246 :: addressed the review comments. --- .../internal/security/trust/XdsX509TrustManager.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index a32362efb64..1483a302609 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -330,17 +330,17 @@ private static boolean labelWildcardMatch(String dnsLabel, String pattern) { if (pattern.length() == 1 && pattern.charAt(0) == glob) { return true; } - if (pattern.chars().filter(ch -> ch == glob).count() == 1) { - String[] splitPattern = pattern.split("\\*", -1); - return (pattern.length() <= dnsLabel.length() + 1) - && dnsLabel.startsWith(splitPattern[0]) - && dnsLabel.endsWith(splitPattern[1]); + int globIndex = pattern.indexOf(glob); + if (pattern.indexOf(glob, globIndex + 1) == -1) { + return dnsLabel.length() >= pattern.length() - 1 + && dnsLabel.startsWith(pattern.substring(0, globIndex)) + && dnsLabel.endsWith(pattern.substring(globIndex + 1)); } return false; } private static String[] splitAtFirstDelimiter(String s) { - int index = s.indexOf("."); + int index = s.indexOf('.'); if (index == -1) { return new String[]{s}; } From e77a957aad97a40362a1f5813d2ae24aad85dd4b Mon Sep 17 00:00:00 2001 From: vimanikag Date: Fri, 19 Sep 2025 07:30:36 +0000 Subject: [PATCH 11/12] 11246 :: addressed the review comments #discussion_r2355624175 --- .../xds/internal/security/trust/XdsX509TrustManager.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index 1483a302609..d2e035522d3 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -313,7 +313,8 @@ public static boolean verifyDnsNameWildcard( ? sanToVerify.toLowerCase(Locale.ROOT) : sanToVerify); String[] splitDnsName = splitAtFirstDelimiter(ignoreCase ? altNameFromCert.toLowerCase(Locale.ROOT) : altNameFromCert); - if (splitPattern.length < 2 || splitDnsName.length < 2) { + if (splitPattern == null || splitDnsName == null + || splitPattern.length < 2 || splitDnsName.length < 2) { return false; } if (splitPattern[0].contains("*") @@ -339,10 +340,11 @@ private static boolean labelWildcardMatch(String dnsLabel, String pattern) { return false; } + @Nullable private static String[] splitAtFirstDelimiter(String s) { int index = s.indexOf('.'); if (index == -1) { - return new String[]{s}; + return null; } return new String[]{s.substring(0, index), s.substring(index + 1)}; } From f1b326290046e55efea3f3a60e41e6611e7ba539 Mon Sep 17 00:00:00 2001 From: vimanikag Date: Fri, 19 Sep 2025 15:19:18 +0000 Subject: [PATCH 12/12] 11246 :: addressed the junit's review comments --- .../security/trust/XdsX509TrustManager.java | 7 +- .../trust/XdsX509TrustManagerTest.java | 98 ++++--------------- 2 files changed, 23 insertions(+), 82 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index d2e035522d3..a40d5f786ae 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -307,8 +307,8 @@ public X509Certificate[] getAcceptedIssuers() { return delegate.getAcceptedIssuers(); } - public static boolean verifyDnsNameWildcard( - String altNameFromCert, String sanToVerify, boolean ignoreCase) { + private static boolean verifyDnsNameWildcard( + String altNameFromCert, String sanToVerify, boolean ignoreCase) { String[] splitPattern = splitAtFirstDelimiter(ignoreCase ? sanToVerify.toLowerCase(Locale.ROOT) : sanToVerify); String[] splitDnsName = splitAtFirstDelimiter(ignoreCase @@ -328,7 +328,8 @@ public static boolean verifyDnsNameWildcard( private static boolean labelWildcardMatch(String dnsLabel, String pattern) { final char glob = '*'; - if (pattern.length() == 1 && pattern.charAt(0) == glob) { + // Check the special case of a single * pattern, as it's common. + if (pattern.equals("*")) { return true; } int globIndex = pattern.indexOf(glob); diff --git a/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java b/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java index b9696690618..132425a76a8 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java @@ -21,10 +21,9 @@ import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CA_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_SPIFFE_PEM_FILE; +import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_0_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_PEM_FILE; import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_SPIFFE_PEM_FILE; -import static io.grpc.xds.internal.security.trust.XdsX509TrustManager.verifyDnsNameWildcard; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.CALLS_REAL_METHODS; @@ -694,51 +693,13 @@ public void unsupportedAltNameType() throws CertificateException, IOException { } } - @Test - public void testVerifyDnsNameExact_succeedsForValidWildcardSanNames() - throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setExact("*.test.youtube.com") - .setIgnoreCase(false) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(SERVER_1_PEM_FILE)); - trustManager.verifySubjectAltNameInChain(certs); - } - - @Test - public void testVerifyDnsNameExact_succeedsForValidWildcardSanNames_ignoreCase() - throws CertificateException, IOException { - StringMatcher stringMatcher = - StringMatcher.newBuilder() - .setExact("*.TEST.YOUTUBE.com") - .setIgnoreCase(true) - .build(); - @SuppressWarnings("deprecation") - CertificateValidationContext certContext = - CertificateValidationContext.newBuilder() - .addMatchSubjectAltNames(stringMatcher) - .build(); - trustManager = new XdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(SERVER_1_PEM_FILE)); - trustManager.verifySubjectAltNameInChain(certs); - } - - @Test - public void testVerifyDnsNameExact_failsForNotExactMatch_SanNames() - throws CertificateException, IOException { + private void runDnsWildcardTest( + String sanPattern, String certFile, boolean ignoreCase, boolean expected) + throws CertificateException, IOException { StringMatcher stringMatcher = StringMatcher.newBuilder() - .setExact("lyft.com") - .setIgnoreCase(false) + .setExact(sanPattern) + .setIgnoreCase(ignoreCase) .build(); @SuppressWarnings("deprecation") CertificateValidationContext certContext = @@ -747,46 +708,25 @@ public void testVerifyDnsNameExact_failsForNotExactMatch_SanNames() .build(); trustManager = new XdsX509TrustManager(certContext, mockDelegate); X509Certificate[] certs = - CertificateUtils.toX509Certificates(TlsTesting.loadCert(SERVER_1_PEM_FILE)); + CertificateUtils.toX509Certificates(TlsTesting.loadCert(certFile)); try { trustManager.verifySubjectAltNameInChain(certs); - fail("no exception thrown"); - } catch (CertificateException expected) { - assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); + assertTrue(expected); + } catch (CertificateException certException) { + assertThat(certException).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); } } @Test - public void testVerifyDnsNameWildcard() { - assertTrue(verifyDnsNameWildcard(".lyft.com", "*.lyft.com" , true)); - assertTrue(verifyDnsNameWildcard("a.lyft.com", "*.lyft.com" , true)); - assertTrue(verifyDnsNameWildcard("a.LYFT.com", "*.lyft.COM" , true)); - assertTrue(verifyDnsNameWildcard("lyft.com", "*yft.com" , true)); - assertTrue(verifyDnsNameWildcard("lyft.com", "*lyft.com" , true)); - assertTrue(verifyDnsNameWildcard("lyft.com", "lyf*.com" , true)); - assertTrue(verifyDnsNameWildcard("lyft.com", "lyft*.com" , true)); - assertTrue(verifyDnsNameWildcard("lyft.com", "l*ft.com" , true)); - assertTrue(verifyDnsNameWildcard("t.lyft.com", "t*.lyft.com" , true)); - assertTrue(verifyDnsNameWildcard("test.lyft.com", "t*.lyft.com" , true)); - assertTrue(verifyDnsNameWildcard("l-lots-of-stuff-ft.com", "l*ft.com" , true)); - assertFalse(verifyDnsNameWildcard("t.lyft.com", "t*t.lyft.com", true)); - assertFalse(verifyDnsNameWildcard("lyft.com", "l*ft.co", true)); - assertFalse(verifyDnsNameWildcard("lyft.com", "ly?t.com", true)); - assertFalse(verifyDnsNameWildcard("lyft.com", "lf*t.com", true)); - assertFalse(verifyDnsNameWildcard(".lyft.com", "*lyft.com", true)); - assertFalse(verifyDnsNameWildcard("lyft.com", "**lyft.com", true)); - assertFalse(verifyDnsNameWildcard("lyft.com", "lyft**.com", true)); - assertFalse(verifyDnsNameWildcard("lyft.com", "ly**ft.com", true)); - assertFalse(verifyDnsNameWildcard("lyft.com", "lyft.c*m", true)); - assertFalse(verifyDnsNameWildcard("lyft.com", "*yft.c*m", true)); - assertFalse(verifyDnsNameWildcard("test.lyft.com.extra", "*.lyft.com", true)); - assertFalse(verifyDnsNameWildcard("a.b.lyft.com", "*.lyft.com", true)); - assertFalse(verifyDnsNameWildcard("foo.test.com", "*.lyft.com", true)); - assertFalse(verifyDnsNameWildcard("lyft.com", "*.lyft.com", true)); - assertFalse(verifyDnsNameWildcard("alyft.com", "*.lyft.com", true)); - assertFalse(verifyDnsNameWildcard("", "*lyft.com", true)); - assertFalse(verifyDnsNameWildcard("lyft.com", "", true)); - assertFalse(verifyDnsNameWildcard("xn--lyft.com", "*.xn--lyft.com", true)); + public void testDnsWildcardPatterns() throws Exception { + runDnsWildcardTest("*.test.google.fr", SERVER_1_PEM_FILE, false, true); + runDnsWildcardTest("*.test.youtube.com", SERVER_1_PEM_FILE, false, true); + runDnsWildcardTest("waterzooi.test.google.be", SERVER_1_PEM_FILE, false, true); + runDnsWildcardTest("192.168.1.3", SERVER_1_PEM_FILE, false, true); + runDnsWildcardTest("*.TEST.YOUTUBE.com", SERVER_1_PEM_FILE, true, true); + runDnsWildcardTest("*.test.google.com.au", SERVER_0_PEM_FILE, false, true); + runDnsWildcardTest("*.TEST.YOUTUBE.com", SERVER_1_PEM_FILE, false, false); + runDnsWildcardTest("*waterzooi", SERVER_1_PEM_FILE, false, false); } private TestSslEngine buildTrustManagerAndGetSslEngine()