TrustManager to server name from SslEngine instead of from ProtocolNegotiator.

Author: kannanjgithub

xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java

@@ -65,12 +65,12 @@ final class CertProviderClientSslContextProvider extends CertProviderSslContextP
       sslContextBuilder = sslContextBuilder.trustManager(
         new XdsTrustManagerFactory(
             savedSpiffeTrustMap,
-            certificateValidationContext, sniForSanMatching));
+            certificateValidationContext, ((UpstreamTlsContext) tlsContext).getAutoSniSanValidation()));
     } else if (savedTrustedRoots != null) {
       sslContextBuilder = sslContextBuilder.trustManager(
           new XdsTrustManagerFactory(
               savedTrustedRoots.toArray(new X509Certificate[0]),
-              certificateValidationContext, sniForSanMatching));
+              certificateValidationContext, ((UpstreamTlsContext) tlsContext).getAutoSniSanValidation()));
     } else {
       // Should be impossible because of the check in CertProviderClientSslContextProviderFactory
       throw new IllegalStateException("There must be trusted roots or a SPIFFE trust map");
@@ -79,12 +79,12 @@ final class CertProviderClientSslContextProvider extends CertProviderSslContextP
     if (savedSpiffeTrustMap != null) {
       trustManagerFactory = new XdsTrustManagerFactory(
           savedSpiffeTrustMap,
-          certificateValidationContext, sniForSanMatching);
+          certificateValidationContext, ((UpstreamTlsContext) tlsContext).getAutoSniSanValidation());
       sslContextBuilder = sslContextBuilder.trustManager(trustManagerFactory);
     } else {
       trustManagerFactory = new XdsTrustManagerFactory(
           savedTrustedRoots.toArray(new X509Certificate[0]),
-          certificateValidationContext, sniForSanMatching);
+          certificateValidationContext, ((UpstreamTlsContext) tlsContext).getAutoSniSanValidation());
       sslContextBuilder = sslContextBuilder.trustManager(trustManagerFactory);
     }
     if (isMtls()) {

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

@@ -59,26 +59,26 @@ public XdsTrustManagerFactory(CertificateValidationContext certificateValidation
         getTrustedCaFromCertContext(certificateValidationContext),
         certificateValidationContext,
         false,
-        null);
+        false);
   }
 
   public XdsTrustManagerFactory(
       X509Certificate[] certs, CertificateValidationContext staticCertificateValidationContext,
-      String sniForSanMatching) throws CertStoreException {
-    this(certs, staticCertificateValidationContext, true, sniForSanMatching);
+      boolean autoSniSanValidation) throws CertStoreException {
+    this(certs, staticCertificateValidationContext, true, autoSniSanValidation);
   }
 
   public XdsTrustManagerFactory(Map<String, List<X509Certificate>> spiffeTrustMap,
-      CertificateValidationContext staticCertificateValidationContext, String sniForSanMatching)
+      CertificateValidationContext staticCertificateValidationContext, boolean autoSniSanValidation)
       throws CertStoreException {
-    this(spiffeTrustMap, staticCertificateValidationContext, true, sniForSanMatching);
+    this(spiffeTrustMap, staticCertificateValidationContext, true, autoSniSanValidation);
   }
 
   private XdsTrustManagerFactory(
       X509Certificate[] certs,
       CertificateValidationContext certificateValidationContext,
       boolean validationContextIsStatic,
-      String sniForSanMatching)
+      boolean autoSniSanValidation)
       throws CertStoreException {
     if (validationContextIsStatic) {
       checkArgument(
@@ -87,21 +87,21 @@ private XdsTrustManagerFactory(
           "only static certificateValidationContext expected");
     }
     xdsX509TrustManager = createX509TrustManager(
-        certs, certificateValidationContext, sniForSanMatching);
+        certs, certificateValidationContext, autoSniSanValidation);
   }
 
   private XdsTrustManagerFactory(
       Map<String, List<X509Certificate>> spiffeTrustMap,
       CertificateValidationContext certificateValidationContext,
       boolean validationContextIsStatic,
-      String sniForSanMatching)
+      boolean autoSniSanValidation)
       throws CertStoreException {
     if (validationContextIsStatic) {
       checkArgument(
           certificateValidationContext == null || !certificateValidationContext.hasTrustedCa(),
           "only static certificateValidationContext expected");
       xdsX509TrustManager = createX509TrustManager(
-          spiffeTrustMap, certificateValidationContext, sniForSanMatching);
+          spiffeTrustMap, certificateValidationContext, autoSniSanValidation);
     }
   }
 
@@ -128,23 +128,24 @@ private static X509Certificate[] getTrustedCaFromCertContext(
 
   @VisibleForTesting
   static XdsX509TrustManager createX509TrustManager(
-      X509Certificate[] certs, CertificateValidationContext certContext, String sniForSanMatching)
+      X509Certificate[] certs, CertificateValidationContext certContext,
+      boolean autoSniSanValidation)
       throws CertStoreException {
-    return new XdsX509TrustManager(certContext, createTrustManager(certs), sniForSanMatching);
+    return new XdsX509TrustManager(certContext, createTrustManager(certs), autoSniSanValidation);
   }
 
   @VisibleForTesting
   static XdsX509TrustManager createX509TrustManager(
       Map<String, List<X509Certificate>> spiffeTrustMapFile,
-      CertificateValidationContext certContext, String sniForSanMatching)
+      CertificateValidationContext certContext, boolean autoSniSanValidation)
       throws CertStoreException {
     checkNotNull(spiffeTrustMapFile, "spiffeTrustMapFile");
     Map<String, X509ExtendedTrustManager> delegates = new HashMap<>();
     for (Map.Entry<String, List<X509Certificate>> entry:spiffeTrustMapFile.entrySet()) {
       delegates.put(entry.getKey(), createTrustManager(
           entry.getValue().toArray(new X509Certificate[0])));
     }
-    return new XdsX509TrustManager(certContext, delegates, sniForSanMatching);
+    return new XdsX509TrustManager(certContext, delegates, autoSniSanValidation);
   }
 
   private static X509ExtendedTrustManager createTrustManager(X509Certificate[] certs)

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

@@ -21,7 +21,6 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.re2j.Pattern;
 import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
@@ -32,6 +31,7 @@
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateParsingException;
 import java.security.cert.X509Certificate;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
@@ -40,6 +40,8 @@
 import java.util.Map;
 import java.util.Set;
 import javax.annotation.Nullable;
+import javax.net.ssl.SNIHostName;
+import javax.net.ssl.SNIServerName;
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLSocket;
@@ -61,30 +63,24 @@ final class XdsX509TrustManager extends X509ExtendedTrustManager implements X509
   private final X509ExtendedTrustManager delegate;
   private final Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates;
   private final CertificateValidationContext certContext;
-  private final String sniForSanMatching;
+  private final boolean autoSniSanValidation;
 
   XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
-                      X509ExtendedTrustManager delegate) {
-    this(certContext, delegate, null);
-  }
-
-  XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
-                      X509ExtendedTrustManager delegate, @Nullable String sniForSanMatching) {
+      X509ExtendedTrustManager delegate, boolean autoSniSanValidation) {
     checkNotNull(delegate, "delegate");
     this.certContext = certContext;
     this.delegate = delegate;
     this.spiffeTrustMapDelegates = null;
-    this.sniForSanMatching = sniForSanMatching;
+    this.autoSniSanValidation = autoSniSanValidation;
   }
 
   XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
-      Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates,
-      @Nullable String sniForSanMatching) {
+      Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates, boolean autoSniSanValidation) {
     checkNotNull(spiffeTrustMapDelegates, "spiffeTrustMapDelegates");
     this.spiffeTrustMapDelegates = ImmutableMap.copyOf(spiffeTrustMapDelegates);
     this.certContext = certContext;
     this.delegate = null;
-    this.sniForSanMatching = sniForSanMatching;
+    this.autoSniSanValidation = autoSniSanValidation;
   }
 
   private static boolean verifyDnsNameInPattern(
@@ -114,7 +110,7 @@ private static boolean verifyDnsNameInPattern(
   }
 
   private static boolean verifyDnsNameSafeRegex(
-          String altNameFromCert, RegexMatcher sanToVerifySafeRegex) {
+      String altNameFromCert, RegexMatcher sanToVerifySafeRegex) {
     Pattern safeRegExMatch = Pattern.compile(sanToVerifySafeRegex.getRegex());
     return safeRegExMatch.matches(altNameFromCert);
   }
@@ -126,37 +122,40 @@ private static boolean verifyDnsNamePrefix(
     }
     return ignoreCase
         ? altNameFromCert.toLowerCase(Locale.ROOT).startsWith(
-            sanToVerifyPrefix.toLowerCase(Locale.ROOT))
+        sanToVerifyPrefix.toLowerCase(Locale.ROOT))
         : altNameFromCert.startsWith(sanToVerifyPrefix);
   }
 
   private static boolean verifyDnsNameSuffix(
-          String altNameFromCert, String sanToVerifySuffix, boolean ignoreCase) {
+      String altNameFromCert, String sanToVerifySuffix, boolean ignoreCase) {
     if (Strings.isNullOrEmpty(sanToVerifySuffix)) {
       return false;
     }
     return ignoreCase
-            ? altNameFromCert.toLowerCase(Locale.ROOT).endsWith(
-                sanToVerifySuffix.toLowerCase(Locale.ROOT))
-            : altNameFromCert.endsWith(sanToVerifySuffix);
+        ? altNameFromCert.toLowerCase(Locale.ROOT).endsWith(
+        sanToVerifySuffix.toLowerCase(Locale.ROOT))
+        : altNameFromCert.endsWith(sanToVerifySuffix);
   }
 
   private static boolean verifyDnsNameContains(
-          String altNameFromCert, String sanToVerifySubstring, boolean ignoreCase) {
+      String altNameFromCert, String sanToVerifySubstring, boolean ignoreCase) {
     if (Strings.isNullOrEmpty(sanToVerifySubstring)) {
       return false;
     }
     return ignoreCase
-            ? altNameFromCert.toLowerCase(Locale.ROOT).contains(
-                sanToVerifySubstring.toLowerCase(Locale.ROOT))
-            : altNameFromCert.contains(sanToVerifySubstring);
+        ? 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(sanToVerifyExact)) {
       return false;
     }
+    if (sanToVerifyExact.contains("*")) {
+      return verifyDnsNameWildcard(altNameFromCert, sanToVerifyExact, ignoreCase);
+    }
     return ignoreCase
         ? sanToVerifyExact.equalsIgnoreCase(altNameFromCert)
         : sanToVerifyExact.equals(altNameFromCert);
@@ -213,15 +212,11 @@ private static void verifySubjectAltNameInLeaf(
    * This is called from various check*Trusted methods.
    */
   @VisibleForTesting
-  void verifySubjectAltNameInChain(X509Certificate[] peerCertChain) throws CertificateException {
+  void verifySubjectAltNameInChain(X509Certificate[] peerCertChain,
+      List<StringMatcher> verifyList) throws CertificateException {
     if (certContext == null) {
       return;
     }
-    @SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
-    List<StringMatcher> verifyList =
-        CertificateUtils.isXdsSniEnabled && !Strings.isNullOrEmpty(sniForSanMatching)
-            ? ImmutableList.of(StringMatcher.newBuilder().setExact(sniForSanMatching).build())
-            : certContext.getMatchSubjectAltNamesList();
     if (verifyList.isEmpty()) {
       return;
     }
@@ -236,14 +231,14 @@ void verifySubjectAltNameInChain(X509Certificate[] peerCertChain) throws Certifi
   public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket)
       throws CertificateException {
     chooseDelegate(chain).checkClientTrusted(chain, authType, socket);
-    verifySubjectAltNameInChain(chain);
+    verifySubjectAltNameInChain(chain, new ArrayList<>());
   }
 
   @Override
   public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine)
       throws CertificateException {
     chooseDelegate(chain).checkClientTrusted(chain, authType, sslEngine);
-    verifySubjectAltNameInChain(chain);
+    verifySubjectAltNameInChain(chain, new ArrayList<>());
   }
 
   @Override
@@ -254,39 +249,66 @@ public void checkClientTrusted(X509Certificate[] chain, String authType)
   }
 
   @Override
+  @SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
   public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket)
       throws CertificateException {
+    List<StringMatcher> sniMatchers = null;
     if (socket instanceof SSLSocket) {
       SSLSocket sslSocket = (SSLSocket) socket;
       SSLParameters sslParams = sslSocket.getSSLParameters();
       if (sslParams != null) {
         sslParams.setEndpointIdentificationAlgorithm("");
         sslSocket.setSSLParameters(sslParams);
       }
+      sniMatchers = getAutoSniSanMatchers(sslParams);
+    }
+    if (sniMatchers.isEmpty()) {
+      sniMatchers = certContext.getMatchSubjectAltNamesList();
     }
     chooseDelegate(chain).checkServerTrusted(chain, authType, socket);
-    verifySubjectAltNameInChain(chain);
+    verifySubjectAltNameInChain(chain, sniMatchers);
   }
 
   @Override
   public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine)
       throws CertificateException {
+    List<StringMatcher> sniMatchers = null;
     SSLParameters sslParams = sslEngine.getSSLParameters();
     if (sslParams != null) {
       sslParams.setEndpointIdentificationAlgorithm("");
       sslEngine.setSSLParameters(sslParams);
+      sniMatchers = getAutoSniSanMatchers(sslParams);
+    }
+    if (sniMatchers.isEmpty()) {
+      sniMatchers = certContext.getMatchSubjectAltNamesList();
     }
     chooseDelegate(chain).checkServerTrusted(chain, authType, sslEngine);
-    verifySubjectAltNameInChain(chain);
+    verifySubjectAltNameInChain(chain, sniMatchers);
   }
 
   @Override
   public void checkServerTrusted(X509Certificate[] chain, String authType)
       throws CertificateException {
     chooseDelegate(chain).checkServerTrusted(chain, authType);
-    verifySubjectAltNameInChain(chain);
+    verifySubjectAltNameInChain(chain, new ArrayList<>());
   }
 
+  private List<StringMatcher> getAutoSniSanMatchers(SSLParameters sslParams) {
+    List<StringMatcher> sniNamesToMatch = new ArrayList<>();
+    if (CertificateUtils.isXdsSniEnabled) {
+      List<SNIServerName> serverNames = sslParams.getServerNames();
+      if (serverNames != null) {
+        for (SNIServerName serverName : serverNames) {
+          if (serverName instanceof SNIHostName) {
+            SNIHostName sniHostName = (SNIHostName) serverName;
+            String hostName = sniHostName.getAsciiName();
+            sniNamesToMatch.add(StringMatcher.newBuilder().setExact(hostName).build());
+          }
+        }
+      }
+    }
+    return sniNamesToMatch;
+  }
   private X509ExtendedTrustManager chooseDelegate(X509Certificate[] chain)
       throws CertificateException {
     if (spiffeTrustMapDelegates != null) {
@@ -316,4 +338,49 @@ public X509Certificate[] getAcceptedIssuers() {
     }
     return delegate.getAcceptedIssuers();
   }
+
+  private static boolean verifyDnsNameWildcard(
+      String altNameFromCert, String sanToVerify, boolean ignoreCase) {
+    String[] splitPattern = splitAtFirstDelimiter(ignoreCase
+        ? sanToVerify.toLowerCase(Locale.ROOT) : sanToVerify);
+    String[] splitDnsName = splitAtFirstDelimiter(ignoreCase
+        ? altNameFromCert.toLowerCase(Locale.ROOT) : altNameFromCert);
+    if (splitPattern == null || splitDnsName == null) {
+      return false;
+    }
+    if (splitDnsName[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]);
+    }
+    return false;
+  }
+
+  private static boolean labelWildcardMatch(String dnsLabel, String pattern) {
+    final char glob = '*';
+    // Check the special case of a single * pattern, as it's common.
+    if (pattern.equals("*")) {
+      return !dnsLabel.isEmpty();
+    }
+    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;
+  }
+
+  @Nullable
+  private static String[] splitAtFirstDelimiter(String s) {
+    int index = s.indexOf('.');
+    if (index == -1) {
+      return null;
+    }
+    return new String[]{s.substring(0, index), s.substring(index + 1)};
+  }
 }