Skip to content

Commit 5b876cc

Browse files
authored
xds: Handle wildcards in split patterns in DNS SAN exact matching (#12345)
Makes DNS SAN exact matching in xds handle wildcards in split patterns similar to Envoy. Fixes #12326
1 parent 63fdaac commit 5b876cc

File tree

4 files changed

+144
-2
lines changed

4 files changed

+144
-2
lines changed
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIDsjCCApqgAwIBAgIUCs5j4C2KXgCRVFa48kc5TYRS1JwwDQYJKoZIhvcNAQEL
3+
BQAwGTEXMBUGA1UEAwwOTXkgSW50ZXJuYWwgQ0EwIBcNMjUwOTIzMDc1NDUzWhgP
4+
MjEyNTA4MzAwNzU0NTNaMGUxCzAJBgNVBAYTAlVTMREwDwYDVQQIDAhJbGxpbm9p
5+
czEQMA4GA1UEBwwHQ2hpY2FnbzEVMBMGA1UECgwMRXhhbXBsZSwgQ28uMRowGAYD
6+
VQQDDBEqLnRlc3QuZ29vZ2xlLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCC
7+
AQoCggEBAKoqcnNh9MV39GH6JjC5KVMN6MO1IoTw6wHJN0JJ/nGNx6ycIsBK8SgJ
8+
eYRR2BEpT6WZba+f04KChcB4Z9tiPISNvUBpmEv76rAsdtcAZwSpF06q4wxHVE5F
9+
rX6mNT8hk448mDBDGHUXNAT6g/e/Vlt6U0XRyuu713gbZq1X6JH29FG7EJ3LUx35
10+
h6sEkvTlZZ3m6NJr7zYoqrYh/gRkPigtPxaNcoXo0gVm4IEde0sYz27SWyNH4v/o
11+
23NynSulOwx4DwEhBOXekLb5QJHBqwMTPynaMncBQIXF+PXeuxN9a3zR6DSn+jGw
12+
g008tS0tn2FuAvJDBl0paEykdOr2rNMCAwEAAaOBozCBoDALBgNVHQ8EBAMCBeAw
13+
EwYDVR0lBAwwCgYIKwYBBQUHAwEwPAYDVR0RBDUwM4IKbHkqKmZ0LmNvbYIIKnlm
14+
dC5jKm2CCS5seWZ0LmNvbYIOeG4tLSoubHlmdC5jb22CADAdBgNVHQ4EFgQUZoL2
15+
OzBtK/BUzSYfgXDx3iDjcIQwHwYDVR0jBBgwFoAUHlstFN5WSLSqyJgUDy6BB0z0
16+
BrgwDQYJKoZIhvcNAQELBQADggEBAMYwVOT7XZJMQ6n32pQqtZhJ/Z0wlVfCAbm0
17+
7xospeBt6KtOz2zIsvPpq0aqPjowMAeL1EZaBvmfm/XgWUU5e/3hLUIHOHyKfswB
18+
czDbY0RE8nfVDoF4Ck1ljPjvrFr4tSAxTzVA4JU5o3UXkblBg0LG6tTuLlZ3x5aF
19+
KtkZnszxjE+vOg6J9MDbFP/xtA1oVHyCvk+cUgnBxAoPShI+87DINGVTmztBSetK
20+
nJN9dOh7Q88NhTLHOe67Ora9Y0ZP+uFKHaqFv8qj8B/Q6ptb0CAksdL5EunkIHrq
21+
glKdVdYgIP2JpRwtvVHK5FzWBlGXCi3DxTyYi6FWqsSJ+heCS2w=
22+
-----END CERTIFICATE-----

xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,9 @@ private static boolean verifyDnsNameExact(
147147
if (Strings.isNullOrEmpty(sanToVerifyExact)) {
148148
return false;
149149
}
150+
if (sanToVerifyExact.contains("*")) {
151+
return verifyDnsNameWildcard(altNameFromCert, sanToVerifyExact, ignoreCase);
152+
}
150153
return ignoreCase
151154
? sanToVerifyExact.equalsIgnoreCase(altNameFromCert)
152155
: sanToVerifyExact.equals(altNameFromCert);
@@ -303,4 +306,49 @@ public X509Certificate[] getAcceptedIssuers() {
303306
}
304307
return delegate.getAcceptedIssuers();
305308
}
309+
310+
private static boolean verifyDnsNameWildcard(
311+
String altNameFromCert, String sanToVerify, boolean ignoreCase) {
312+
String[] splitPattern = splitAtFirstDelimiter(ignoreCase
313+
? sanToVerify.toLowerCase(Locale.ROOT) : sanToVerify);
314+
String[] splitDnsName = splitAtFirstDelimiter(ignoreCase
315+
? altNameFromCert.toLowerCase(Locale.ROOT) : altNameFromCert);
316+
if (splitPattern == null || splitDnsName == null) {
317+
return false;
318+
}
319+
if (splitDnsName[0].startsWith("xn--")) {
320+
return false;
321+
}
322+
if (splitPattern[0].contains("*")
323+
&& !splitPattern[1].contains("*")
324+
&& !splitPattern[0].startsWith("xn--")) {
325+
return splitDnsName[1].equals(splitPattern[1])
326+
&& labelWildcardMatch(splitDnsName[0], splitPattern[0]);
327+
}
328+
return false;
329+
}
330+
331+
private static boolean labelWildcardMatch(String dnsLabel, String pattern) {
332+
final char glob = '*';
333+
// Check the special case of a single * pattern, as it's common.
334+
if (pattern.equals("*")) {
335+
return !dnsLabel.isEmpty();
336+
}
337+
int globIndex = pattern.indexOf(glob);
338+
if (pattern.indexOf(glob, globIndex + 1) == -1) {
339+
return dnsLabel.length() >= pattern.length() - 1
340+
&& dnsLabel.startsWith(pattern.substring(0, globIndex))
341+
&& dnsLabel.endsWith(pattern.substring(globIndex + 1));
342+
}
343+
return false;
344+
}
345+
346+
@Nullable
347+
private static String[] splitAtFirstDelimiter(String s) {
348+
int index = s.indexOf('.');
349+
if (index == -1) {
350+
return null;
351+
}
352+
return new String[]{s.substring(0, index), s.substring(index + 1)};
353+
}
306354
}

xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ public class CommonTlsContextTestsUtil {
6060
public static final String BAD_SERVER_KEY_FILE = "badserver.key";
6161
public static final String BAD_CLIENT_PEM_FILE = "badclient.pem";
6262
public static final String BAD_CLIENT_KEY_FILE = "badclient.key";
63+
public static final String BAD_WILDCARD_DNS_PEM_FILE =
64+
"sni-test-certs/bad_wildcard_dns_certificate.pem";
6365

6466
/** takes additional values and creates CombinedCertificateValidationContext as needed. */
6567
private static CommonTlsContext buildCommonTlsContextWithAdditionalValues(

xds/src/test/java/io/grpc/xds/internal/security/trust/XdsX509TrustManagerTest.java

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@
1818

1919
import static com.google.common.truth.Truth.assertThat;
2020
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.BAD_SERVER_PEM_FILE;
21+
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.BAD_WILDCARD_DNS_PEM_FILE;
2122
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CA_PEM_FILE;
2223
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_PEM_FILE;
2324
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.CLIENT_SPIFFE_PEM_FILE;
25+
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_0_PEM_FILE;
2426
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_PEM_FILE;
2527
import static io.grpc.xds.internal.security.CommonTlsContextTestsUtil.SERVER_1_SPIFFE_PEM_FILE;
2628
import static org.junit.Assert.fail;
@@ -42,6 +44,7 @@
4244
import java.security.cert.CertificateException;
4345
import java.security.cert.X509Certificate;
4446
import java.util.Arrays;
47+
import java.util.Collection;
4548
import java.util.Collections;
4649
import java.util.List;
4750
import javax.net.ssl.SSLEngine;
@@ -52,15 +55,16 @@
5255
import org.junit.Rule;
5356
import org.junit.Test;
5457
import org.junit.runner.RunWith;
55-
import org.junit.runners.JUnit4;
58+
import org.junit.runners.Parameterized;
59+
import org.junit.runners.Parameterized.Parameters;
5660
import org.mockito.Mock;
5761
import org.mockito.junit.MockitoJUnit;
5862
import org.mockito.junit.MockitoRule;
5963

6064
/**
6165
* Unit tests for {@link XdsX509TrustManager}.
6266
*/
63-
@RunWith(JUnit4.class)
67+
@RunWith(Parameterized.class)
6468
public class XdsX509TrustManagerTest {
6569

6670
@Rule
@@ -74,6 +78,12 @@ public class XdsX509TrustManagerTest {
7478

7579
private XdsX509TrustManager trustManager;
7680

81+
private final TestParam testParam;
82+
83+
public XdsX509TrustManagerTest(TestParam testParam) {
84+
this.testParam = testParam;
85+
}
86+
7787
@Test
7888
public void nullCertContextTest() throws CertificateException, IOException {
7989
trustManager = new XdsX509TrustManager(null, mockDelegate);
@@ -691,6 +701,52 @@ public void unsupportedAltNameType() throws CertificateException, IOException {
691701
}
692702
}
693703

704+
@Test
705+
public void testDnsWildcardPatterns()
706+
throws CertificateException, IOException {
707+
StringMatcher stringMatcher =
708+
StringMatcher.newBuilder()
709+
.setExact(testParam.sanPattern)
710+
.setIgnoreCase(testParam.ignoreCase)
711+
.build();
712+
@SuppressWarnings("deprecation")
713+
CertificateValidationContext certContext =
714+
CertificateValidationContext.newBuilder()
715+
.addMatchSubjectAltNames(stringMatcher)
716+
.build();
717+
trustManager = new XdsX509TrustManager(certContext, mockDelegate);
718+
X509Certificate[] certs =
719+
CertificateUtils.toX509Certificates(TlsTesting.loadCert(testParam.certFile));
720+
try {
721+
trustManager.verifySubjectAltNameInChain(certs);
722+
assertThat(testParam.expected).isTrue();
723+
} catch (CertificateException certException) {
724+
assertThat(testParam.expected).isFalse();
725+
assertThat(certException).hasMessageThat().isEqualTo("Peer certificate SAN check failed");
726+
}
727+
}
728+
729+
@Parameters(name = "{index}: {0}")
730+
public static Collection<Object[]> getParameters() {
731+
return Arrays.asList(new Object[][] {
732+
{new TestParam("*.test.google.fr", SERVER_1_PEM_FILE, false, true)},
733+
{new TestParam("*.test.youtube.com", SERVER_1_PEM_FILE, false, true)},
734+
{new TestParam("waterzooi.test.google.be", SERVER_1_PEM_FILE, false, true)},
735+
{new TestParam("192.168.1.3", SERVER_1_PEM_FILE, false, true)},
736+
{new TestParam("*.TEST.YOUTUBE.com", SERVER_1_PEM_FILE, true, true)},
737+
{new TestParam("w*i.test.google.be", SERVER_1_PEM_FILE, false, true)},
738+
{new TestParam("w*a.test.google.be", SERVER_1_PEM_FILE, false, false)},
739+
{new TestParam("*.test.google.com.au", SERVER_0_PEM_FILE, false, false)},
740+
{new TestParam("*.TEST.YOUTUBE.com", SERVER_1_PEM_FILE, false, false)},
741+
{new TestParam("*waterzooi", SERVER_1_PEM_FILE, false, false)},
742+
{new TestParam("*.lyft.com", BAD_WILDCARD_DNS_PEM_FILE, false, false)},
743+
{new TestParam("ly**ft.com", BAD_WILDCARD_DNS_PEM_FILE, false, false)},
744+
{new TestParam("*yft.c*m", BAD_WILDCARD_DNS_PEM_FILE, false, false)},
745+
{new TestParam("xn--*.lyft.com", BAD_WILDCARD_DNS_PEM_FILE, false, false)},
746+
{new TestParam("", BAD_WILDCARD_DNS_PEM_FILE, false, false)},
747+
});
748+
}
749+
694750
private TestSslEngine buildTrustManagerAndGetSslEngine()
695751
throws CertificateException, IOException, CertStoreException {
696752
SSLParameters sslParams = buildTrustManagerAndGetSslParameters();
@@ -754,4 +810,18 @@ public void setSSLParameters(SSLParameters sslParameters) {
754810

755811
private SSLParameters sslParameters;
756812
}
813+
814+
private static class TestParam {
815+
final String sanPattern;
816+
final String certFile;
817+
final boolean ignoreCase;
818+
final boolean expected;
819+
820+
TestParam(String sanPattern, String certFile, boolean ignoreCase, boolean expected) {
821+
this.sanPattern = sanPattern;
822+
this.certFile = certFile;
823+
this.ignoreCase = ignoreCase;
824+
this.expected = expected;
825+
}
826+
}
757827
}

0 commit comments

Comments
 (0)