xds: ignore unknown SAN name type instead of throwing exception (#8183)

grpc · May 19, 2021 · 869b395 · 869b395
1 parent 465c932
commit 869b395
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 3 deletions.
diff --git a/xds/src/main/java/io/grpc/xds/internal/sds/trust/SdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/sds/trust/SdsX509TrustManager.java
@@ -151,14 +151,13 @@ private static boolean verifyOneSanInList(List<?> entry, List<StringMatcher> ver
     if (altNameType == null) {
       throw new CertificateParsingException("Invalid SAN entry: null altNameType");
     }
-    String altNameFromCert = (String) entry.get(1);
     switch (altNameType) {
       case ALT_DNS_NAME:
       case ALT_URI_NAME:
       case ALT_IPA_NAME:
-        return verifyDnsNameInSanList(altNameFromCert, verifySanList);
+        return verifyDnsNameInSanList((String) entry.get(1), verifySanList);
       default:
-        throw new CertificateParsingException("Unsupported altNameType: " + altNameType);
+        return false;
     }
   }
 

diff --git a/xds/src/test/java/io/grpc/xds/internal/sds/trust/SdsX509TrustManagerTest.java b/xds/src/test/java/io/grpc/xds/internal/sds/trust/SdsX509TrustManagerTest.java
@@ -29,6 +29,7 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import com.google.common.collect.ImmutableList;
 import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
 import io.envoyproxy.envoy.type.matcher.v3.RegexMatcher;
 import io.envoyproxy.envoy.type.matcher.v3.StringMatcher;
@@ -37,6 +38,8 @@
 import java.security.cert.CertStoreException;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
+import java.util.Collections;
+import java.util.List;
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLSession;
@@ -551,6 +554,29 @@ public void checkServerTrustedSslSocket_untrustedServer_expectException()
     verify(sslSocket, times(1)).getHandshakeSession();
   }
 
+  @Test
+  public void unsupportedAltNameType() throws CertificateException, IOException {
+    StringMatcher stringMatcher =
+        StringMatcher.newBuilder()
+            .setExact("waterzooi.test.google.be")
+            .setIgnoreCase(false)
+            .build();
+    CertificateValidationContext certContext =
+        CertificateValidationContext.newBuilder().addMatchSubjectAltNames(stringMatcher).build();
+    trustManager = new SdsX509TrustManager(certContext, mockDelegate);
+    X509Certificate mockCert = mock(X509Certificate.class);
+
+    when(mockCert.getSubjectAlternativeNames())
+        .thenReturn(Collections.<List<?>>singleton(ImmutableList.of(Integer.valueOf(1), "foo")));
+    X509Certificate[] certs = new X509Certificate[] {mockCert};
+    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();