xds: Plumb system root certs similarly to CertProviders

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

@@ -55,20 +55,19 @@ protected final SslContextBuilder getSslContextBuilder(
           CertificateValidationContext certificateValidationContextdationContext)
       throws CertStoreException {
     SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient();
-    // Null rootCertInstance implies hasSystemRootCerts because of the check in
-    // CertProviderClientSslContextProviderFactory.
-    if (rootCertInstance != null) {
-      if (savedSpiffeTrustMap != null) {
-        sslContextBuilder = sslContextBuilder.trustManager(
+    if (savedSpiffeTrustMap != null) {
+      sslContextBuilder = sslContextBuilder.trustManager(
+        new XdsTrustManagerFactory(
+            savedSpiffeTrustMap,
+            certificateValidationContextdationContext));
+    } else if (savedTrustedRoots != null) {
+      sslContextBuilder = sslContextBuilder.trustManager(
           new XdsTrustManagerFactory(
-              savedSpiffeTrustMap,
+              savedTrustedRoots.toArray(new X509Certificate[0]),
               certificateValidationContextdationContext));
-      } else {
-        sslContextBuilder = sslContextBuilder.trustManager(
-            new XdsTrustManagerFactory(
-                savedTrustedRoots.toArray(new X509Certificate[0]),
-                certificateValidationContextdationContext));
-      }
+    } else {
+      // Should be impossible because of the check in CertProviderClientSslContextProviderFactory
+      throw new IllegalStateException("There must be trusted roots or a SPIFFE trust map");
     }
     if (isMtls()) {
       sslContextBuilder.keyManager(savedKey, savedCertChain);

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

@@ -16,14 +16,18 @@
 
 package io.grpc.xds.internal.security.certprovider;
 
+import static java.util.Objects.requireNonNull;
+
 import io.envoyproxy.envoy.config.core.v3.Node;
 import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
 import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext;
 import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext.CertificateProviderInstance;
+import io.grpc.Status;
 import io.grpc.xds.EnvoyServerProtoData.BaseTlsContext;
 import io.grpc.xds.client.Bootstrapper.CertificateProviderInfo;
 import io.grpc.xds.internal.security.CommonTlsContextUtil;
 import io.grpc.xds.internal.security.DynamicSslContextProvider;
+import java.io.Closeable;
 import java.security.PrivateKey;
 import java.security.cert.X509Certificate;
 import java.util.List;
@@ -34,15 +38,12 @@
 abstract class CertProviderSslContextProvider extends DynamicSslContextProvider implements
     CertificateProvider.Watcher {
 
-  @Nullable private final CertificateProviderStore.Handle certHandle;
-  @Nullable private final CertificateProviderStore.Handle rootCertHandle;
-  @Nullable private final CertificateProviderInstance certInstance;
-  @Nullable protected final CertificateProviderInstance rootCertInstance;
+  @Nullable private final NoExceptionCloseable certHandle;
+  @Nullable private final NoExceptionCloseable rootCertHandle;
   @Nullable protected PrivateKey savedKey;
   @Nullable protected List<X509Certificate> savedCertChain;
   @Nullable protected List<X509Certificate> savedTrustedRoots;
   @Nullable protected Map<String, List<X509Certificate>> savedSpiffeTrustMap;
-  private final boolean isUsingSystemRootCerts;
 
   protected CertProviderSslContextProvider(
       Node node,
@@ -53,40 +54,50 @@ protected CertProviderSslContextProvider(
       BaseTlsContext tlsContext,
       CertificateProviderStore certificateProviderStore) {
     super(tlsContext, staticCertValidationContext);
-    this.certInstance = certInstance;
-    this.rootCertInstance = rootCertInstance;
-    String certInstanceName = null;
-    if (certInstance != null && certInstance.isInitialized()) {
-      certInstanceName = certInstance.getInstanceName();
+    boolean createCertInstance = certInstance != null && certInstance.isInitialized();
+    boolean createRootCertInstance = rootCertInstance != null && rootCertInstance.isInitialized();
+    boolean sharedCertInstance = createCertInstance && createRootCertInstance
+        && rootCertInstance.getInstanceName().equals(certInstance.getInstanceName());
+    if (createCertInstance) {
       CertificateProviderInfo certProviderInstanceConfig =
-          getCertProviderConfig(certProviders, certInstanceName);
+          getCertProviderConfig(certProviders, certInstance.getInstanceName());
+      CertificateProvider.Watcher watcher = this;
+      if (!sharedCertInstance) {
+        watcher = new IgnoreUpdatesWatcher(watcher, /* ignoreRootCertUpdates= */ true);
+      }
+      // TODO: Previously we'd hang if certProviderInstanceConfig were null or
+      // certInstance.isInitialized() == false. Now we'll proceed. Those should be errors, or are
+      // they impossible and should be assertions?
       certHandle = certProviderInstanceConfig == null ? null
           : certificateProviderStore.createOrGetProvider(
               certInstance.getCertificateName(),
               certProviderInstanceConfig.pluginName(),
               certProviderInstanceConfig.config(),
-              this,
-              true);
+              watcher,
+              true)::close;
     } else {
       certHandle = null;
     }
-    if (rootCertInstance != null
-        && rootCertInstance.isInitialized()
-        && !rootCertInstance.getInstanceName().equals(certInstanceName)) {
+    if (createRootCertInstance && sharedCertInstance) {
+      rootCertHandle = () -> { };
+    } else if (createRootCertInstance && !sharedCertInstance) {
       CertificateProviderInfo certProviderInstanceConfig =
           getCertProviderConfig(certProviders, rootCertInstance.getInstanceName());
       rootCertHandle = certProviderInstanceConfig == null ? null
           : certificateProviderStore.createOrGetProvider(
               rootCertInstance.getCertificateName(),
               certProviderInstanceConfig.pluginName(),
               certProviderInstanceConfig.config(),
-              this,
-              true);
+              new IgnoreUpdatesWatcher(this, /* ignoreRootCertUpdates= */ false),
+              false)::close;
+    } else if (rootCertInstance == null
+        && CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext())) {
+      SystemRootCertificateProvider systemRootProvider = new SystemRootCertificateProvider(this);
+      systemRootProvider.start();
+      rootCertHandle = systemRootProvider::close;
     } else {
       rootCertHandle = null;
     }
-    this.isUsingSystemRootCerts = rootCertInstance == null
-        && CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext());
   }
 
   private static CertificateProviderInfo getCertProviderConfig(
@@ -150,8 +161,7 @@ public final void updateSpiffeTrustMap(Map<String, List<X509Certificate>> spiffe
 
   private void updateSslContextWhenReady() {
     if (isMtls()) {
-      if (savedKey != null
-          && (savedTrustedRoots != null || isUsingSystemRootCerts || savedSpiffeTrustMap != null)) {
+      if (savedKey != null && (savedTrustedRoots != null || savedSpiffeTrustMap != null)) {
         updateSslContext();
         clearKeysAndCerts();
       }
@@ -176,15 +186,15 @@ private void clearKeysAndCerts() {
   }
 
   protected final boolean isMtls() {
-    return certInstance != null && (rootCertInstance != null || isUsingSystemRootCerts);
+    return certHandle != null && rootCertHandle != null;
   }
 
   protected final boolean isClientSideTls() {
-    return rootCertInstance != null && certInstance == null;
+    return rootCertHandle != null && certHandle == null;
   }
 
   protected final boolean isServerSideTls() {
-    return certInstance != null && rootCertInstance == null;
+    return certHandle != null && rootCertHandle == null;
   }
 
   @Override
@@ -201,4 +211,46 @@ public final void close() {
       rootCertHandle.close();
     }
   }
+
+  interface NoExceptionCloseable extends Closeable {
+    @Override
+    void close();
+  }
+
+  static final class IgnoreUpdatesWatcher implements CertificateProvider.Watcher {
+    private final CertificateProvider.Watcher delegate;
+    private final boolean ignoreRootCertUpdates;
+
+    public IgnoreUpdatesWatcher(
+        CertificateProvider.Watcher delegate, boolean ignoreRootCertUpdates) {
+      this.delegate = requireNonNull(delegate, "delegate");
+      this.ignoreRootCertUpdates = ignoreRootCertUpdates;
+    }
+
+    @Override
+    public void updateCertificate(PrivateKey key, List<X509Certificate> certChain) {
+      if (ignoreRootCertUpdates) {
+        delegate.updateCertificate(key, certChain);
+      }
+    }
+
+    @Override
+    public void updateTrustedRoots(List<X509Certificate> trustedRoots) {
+      if (!ignoreRootCertUpdates) {
+        delegate.updateTrustedRoots(trustedRoots);
+      }
+    }
+
+    @Override
+    public void updateSpiffeTrustMap(Map<String, List<X509Certificate>> spiffeTrustMap) {
+      if (!ignoreRootCertUpdates) {
+        delegate.updateSpiffeTrustMap(spiffeTrustMap);
+      }
+    }
+
+    @Override
+    public void onError(Status errorStatus) {
+      delegate.onError(errorStatus);
+    }
+  }
 }

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

@@ -0,0 +1,71 @@
+/*
+ * Copyright 2020 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.xds.internal.security.certprovider;
+
+import io.grpc.Status;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+
+/**
+ * An non-registered provider for CertProviderSslContextProvider to use the same code path for
+ * system root certs as provider-obtained certs.
+ */
+final class SystemRootCertificateProvider extends CertificateProvider {
+  public SystemRootCertificateProvider(CertificateProvider.Watcher watcher) {
+    super(new DistributorWatcher(), false);
+    getWatcher().addWatcher(watcher);
+  }
+
+  @Override
+  public void start() {
+    try {
+      TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(
+          TrustManagerFactory.getDefaultAlgorithm());
+      trustManagerFactory.init((KeyStore) null);
+
+      List<TrustManager> trustManagers = Arrays.asList(trustManagerFactory.getTrustManagers());
+      List<X509Certificate> rootCerts = trustManagers.stream()
+          .filter(X509TrustManager.class::isInstance)
+          .map(X509TrustManager.class::cast)
+          .map(trustManager -> Arrays.asList(trustManager.getAcceptedIssuers()))
+          .flatMap(Collection::stream)
+          .collect(Collectors.toList());
+      getWatcher().updateTrustedRoots(rootCerts);
+    } catch (KeyStoreException | NoSuchAlgorithmException ex) {
+      getWatcher().onError(Status.UNAVAILABLE
+          .withDescription("Could not load system root certs")
+          .withCause(ex));
+    }
+  }
+
+  @Override
+  public void close() {
+    // Unnecessary because there's no more callbacks, but do it for good measure
+    for (Watcher watcher : getWatcher().getDownstreamWatchers()) {
+      getWatcher().removeWatcher(watcher);
+    }
+  }
+}