Skip to content

Commit d6acdfc

Browse files
kannanjgithubkannanjgithub
committed
Merge branch 'ejona86_xds_system_cert' into systemrootcerts-ignore-trusted-root-updates
# Conflicts: # xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java
2 parents 08391fb + 0d5eb0a commit d6acdfc

File tree

3 files changed

+160
-78
lines changed

3 files changed

+160
-78
lines changed

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

Lines changed: 13 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,12 @@
2222
import io.grpc.netty.GrpcSslContexts;
2323
import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext;
2424
import io.grpc.xds.client.Bootstrapper.CertificateProviderInfo;
25-
import io.grpc.xds.internal.security.CommonTlsContextUtil;
2625
import io.grpc.xds.internal.security.trust.XdsTrustManagerFactory;
2726
import io.netty.handler.ssl.SslContextBuilder;
28-
import java.io.IOException;
29-
import java.security.KeyStore;
30-
import java.security.KeyStoreException;
31-
import java.security.NoSuchAlgorithmException;
3227
import java.security.cert.CertStoreException;
33-
import java.security.cert.CertificateException;
3428
import java.security.cert.X509Certificate;
35-
import java.util.Arrays;
36-
import java.util.Collection;
37-
import java.util.List;
3829
import java.util.Map;
39-
import java.util.stream.Collectors;
4030
import javax.annotation.Nullable;
41-
import javax.net.ssl.TrustManager;
42-
import javax.net.ssl.TrustManagerFactory;
43-
import javax.net.ssl.X509TrustManager;
4431

4532
/** A client SslContext provider using CertificateProviderInstance to fetch secrets. */
4633
final class CertProviderClientSslContextProvider extends CertProviderSslContextProvider {
@@ -61,65 +48,30 @@ final class CertProviderClientSslContextProvider extends CertProviderSslContextP
6148
staticCertValidationContext,
6249
upstreamTlsContext,
6350
certificateProviderStore);
64-
if (rootCertInstance == null
65-
&& CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext())
66-
&& !isMtls()) {
67-
try {
68-
// Instantiate sslContext so that addCallback will immediately update the callback with
69-
// the SslContext.
70-
sslContext = getSslContextBuilder(staticCertificateValidationContext).build();
71-
} catch (CertStoreException | CertificateException | IOException e) {
72-
throw new RuntimeException(e);
73-
}
74-
}
7551
}
7652

7753
@Override
7854
protected final SslContextBuilder getSslContextBuilder(
79-
CertificateValidationContext certificateValidationContext)
80-
throws CertificateException, IOException, CertStoreException {
55+
CertificateValidationContext certificateValidationContextdationContext)
56+
throws CertStoreException {
8157
SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient();
82-
if (rootCertInstance != null) {
83-
if (savedSpiffeTrustMap != null) {
84-
sslContextBuilder = sslContextBuilder.trustManager(
58+
if (savedSpiffeTrustMap != null) {
59+
sslContextBuilder = sslContextBuilder.trustManager(
60+
new XdsTrustManagerFactory(
61+
savedSpiffeTrustMap,
62+
certificateValidationContextdationContext));
63+
} else if (savedTrustedRoots != null) {
64+
sslContextBuilder = sslContextBuilder.trustManager(
8565
new XdsTrustManagerFactory(
86-
savedSpiffeTrustMap,
87-
certificateValidationContext));
88-
} else {
89-
sslContextBuilder = sslContextBuilder.trustManager(
90-
new XdsTrustManagerFactory(
91-
savedTrustedRoots.toArray(new X509Certificate[0]),
92-
certificateValidationContext));
93-
}
66+
savedTrustedRoots.toArray(new X509Certificate[0]),
67+
certificateValidationContextdationContext));
9468
} else {
95-
try {
96-
sslContextBuilder = sslContextBuilder.trustManager(
97-
new XdsTrustManagerFactory(
98-
getX509CertificatesFromSystemTrustStore(),
99-
certificateValidationContext));
100-
} catch (KeyStoreException | NoSuchAlgorithmException e) {
101-
throw new CertStoreException(e);
102-
}
69+
// Should be impossible because of the check in CertProviderClientSslContextProviderFactory
70+
throw new IllegalStateException("There must be trusted roots or a SPIFFE trust map");
10371
}
10472
if (isMtls()) {
10573
sslContextBuilder.keyManager(savedKey, savedCertChain);
10674
}
10775
return sslContextBuilder;
10876
}
109-
110-
private X509Certificate[] getX509CertificatesFromSystemTrustStore()
111-
throws KeyStoreException, NoSuchAlgorithmException {
112-
TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(
113-
TrustManagerFactory.getDefaultAlgorithm());
114-
trustManagerFactory.init((KeyStore) null);
115-
116-
List<TrustManager> trustManagers = Arrays.asList(trustManagerFactory.getTrustManagers());
117-
List<X509Certificate> rootCerts = trustManagers.stream()
118-
.filter(X509TrustManager.class::isInstance)
119-
.map(X509TrustManager.class::cast)
120-
.map(trustManager -> Arrays.asList(trustManager.getAcceptedIssuers()))
121-
.flatMap(Collection::stream)
122-
.collect(Collectors.toList());
123-
return rootCerts.toArray(new X509Certificate[rootCerts.size()]);
124-
}
12577
}

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

Lines changed: 76 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,18 @@
1616

1717
package io.grpc.xds.internal.security.certprovider;
1818

19+
import static java.util.Objects.requireNonNull;
20+
1921
import io.envoyproxy.envoy.config.core.v3.Node;
2022
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
2123
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext;
2224
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext.CertificateProviderInstance;
25+
import io.grpc.Status;
2326
import io.grpc.xds.EnvoyServerProtoData.BaseTlsContext;
2427
import io.grpc.xds.client.Bootstrapper.CertificateProviderInfo;
2528
import io.grpc.xds.internal.security.CommonTlsContextUtil;
2629
import io.grpc.xds.internal.security.DynamicSslContextProvider;
30+
import java.io.Closeable;
2731
import java.security.PrivateKey;
2832
import java.security.cert.X509Certificate;
2933
import java.util.List;
@@ -34,8 +38,8 @@
3438
abstract class CertProviderSslContextProvider extends DynamicSslContextProvider implements
3539
CertificateProvider.Watcher {
3640

37-
@Nullable private final CertificateProviderStore.Handle certHandle;
38-
@Nullable private final CertificateProviderStore.Handle rootCertHandle;
41+
@Nullable private final NoExceptionCloseable certHandle;
42+
@Nullable private final NoExceptionCloseable rootCertHandle;
3943
@Nullable private final CertificateProviderInstance certInstance;
4044
@Nullable protected final CertificateProviderInstance rootCertInstance;
4145
@Nullable protected PrivateKey savedKey;
@@ -55,38 +59,52 @@ protected CertProviderSslContextProvider(
5559
super(tlsContext, staticCertValidationContext);
5660
this.certInstance = certInstance;
5761
this.rootCertInstance = rootCertInstance;
58-
String certInstanceName = null;
59-
if (certInstance != null && certInstance.isInitialized()) {
60-
certInstanceName = certInstance.getInstanceName();
62+
this.isUsingSystemRootCerts = rootCertInstance == null
63+
&& CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext());
64+
boolean createCertInstance = certInstance != null && certInstance.isInitialized();
65+
boolean createRootCertInstance = rootCertInstance != null && rootCertInstance.isInitialized();
66+
boolean sharedCertInstance = createCertInstance && createRootCertInstance
67+
&& rootCertInstance.getInstanceName().equals(certInstance.getInstanceName());
68+
if (createCertInstance) {
6169
CertificateProviderInfo certProviderInstanceConfig =
62-
getCertProviderConfig(certProviders, certInstanceName);
70+
getCertProviderConfig(certProviders, certInstance.getInstanceName());
71+
CertificateProvider.Watcher watcher = this;
72+
if (!sharedCertInstance) {
73+
watcher = new IgnoreUpdatesWatcher(watcher, /* ignoreRootCertUpdates= */ true);
74+
}
75+
// TODO: Previously we'd hang if certProviderInstanceConfig were null or
76+
// certInstance.isInitialized() == false. Now we'll proceed. Those should be errors, or are
77+
// they impossible and should be assertions?
6378
certHandle = certProviderInstanceConfig == null ? null
6479
: certificateProviderStore.createOrGetProvider(
6580
certInstance.getCertificateName(),
6681
certProviderInstanceConfig.pluginName(),
6782
certProviderInstanceConfig.config(),
68-
this,
69-
true);
83+
watcher,
84+
true)::close;
7085
} else {
7186
certHandle = null;
7287
}
73-
if (rootCertInstance != null
74-
&& rootCertInstance.isInitialized()
75-
&& !rootCertInstance.getInstanceName().equals(certInstanceName)) {
88+
if (createRootCertInstance && sharedCertInstance) {
89+
rootCertHandle = () -> { };
90+
} else if (createRootCertInstance && !sharedCertInstance) {
7691
CertificateProviderInfo certProviderInstanceConfig =
7792
getCertProviderConfig(certProviders, rootCertInstance.getInstanceName());
7893
rootCertHandle = certProviderInstanceConfig == null ? null
7994
: certificateProviderStore.createOrGetProvider(
8095
rootCertInstance.getCertificateName(),
8196
certProviderInstanceConfig.pluginName(),
8297
certProviderInstanceConfig.config(),
83-
this,
84-
true);
98+
new IgnoreUpdatesWatcher(this, /* ignoreRootCertUpdates= */ false),
99+
false)::close;
100+
} else if (rootCertInstance == null
101+
&& CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext())) {
102+
SystemRootCertificateProvider systemRootProvider = new SystemRootCertificateProvider(this);
103+
systemRootProvider.start();
104+
rootCertHandle = systemRootProvider::close;
85105
} else {
86106
rootCertHandle = null;
87107
}
88-
this.isUsingSystemRootCerts = rootCertInstance == null
89-
&& CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext());
90108
}
91109

92110
private static CertificateProviderInfo getCertProviderConfig(
@@ -153,8 +171,7 @@ public final void updateSpiffeTrustMap(Map<String, List<X509Certificate>> spiffe
153171

154172
private void updateSslContextWhenReady() {
155173
if (isMtls()) {
156-
if (savedKey != null
157-
&& (savedTrustedRoots != null || isUsingSystemRootCerts || savedSpiffeTrustMap != null)) {
174+
if (savedKey != null && (savedTrustedRoots != null || savedSpiffeTrustMap != null)) {
158175
updateSslContext();
159176
clearKeysAndCerts();
160177
}
@@ -207,4 +224,46 @@ public final void close() {
207224
rootCertHandle.close();
208225
}
209226
}
227+
228+
interface NoExceptionCloseable extends Closeable {
229+
@Override
230+
void close();
231+
}
232+
233+
static final class IgnoreUpdatesWatcher implements CertificateProvider.Watcher {
234+
private final CertificateProvider.Watcher delegate;
235+
private final boolean ignoreRootCertUpdates;
236+
237+
public IgnoreUpdatesWatcher(
238+
CertificateProvider.Watcher delegate, boolean ignoreRootCertUpdates) {
239+
this.delegate = requireNonNull(delegate, "delegate");
240+
this.ignoreRootCertUpdates = ignoreRootCertUpdates;
241+
}
242+
243+
@Override
244+
public void updateCertificate(PrivateKey key, List<X509Certificate> certChain) {
245+
if (ignoreRootCertUpdates) {
246+
delegate.updateCertificate(key, certChain);
247+
}
248+
}
249+
250+
@Override
251+
public void updateTrustedRoots(List<X509Certificate> trustedRoots) {
252+
if (!ignoreRootCertUpdates) {
253+
delegate.updateTrustedRoots(trustedRoots);
254+
}
255+
}
256+
257+
@Override
258+
public void updateSpiffeTrustMap(Map<String, List<X509Certificate>> spiffeTrustMap) {
259+
if (!ignoreRootCertUpdates) {
260+
delegate.updateSpiffeTrustMap(spiffeTrustMap);
261+
}
262+
}
263+
264+
@Override
265+
public void onError(Status errorStatus) {
266+
delegate.onError(errorStatus);
267+
}
268+
}
210269
}

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Copyright 2020 The gRPC Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.grpc.xds.internal.security.certprovider;
18+
19+
import io.grpc.Status;
20+
import java.security.KeyStore;
21+
import java.security.KeyStoreException;
22+
import java.security.NoSuchAlgorithmException;
23+
import java.security.cert.X509Certificate;
24+
import java.util.Arrays;
25+
import java.util.Collection;
26+
import java.util.List;
27+
import java.util.stream.Collectors;
28+
import javax.net.ssl.TrustManager;
29+
import javax.net.ssl.TrustManagerFactory;
30+
import javax.net.ssl.X509TrustManager;
31+
32+
/**
33+
* An non-registered provider for CertProviderSslContextProvider to use the same code path for
34+
* system root certs as provider-obtained certs.
35+
*/
36+
final class SystemRootCertificateProvider extends CertificateProvider {
37+
public SystemRootCertificateProvider(CertificateProvider.Watcher watcher) {
38+
super(new DistributorWatcher(), false);
39+
getWatcher().addWatcher(watcher);
40+
}
41+
42+
@Override
43+
public void start() {
44+
try {
45+
TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(
46+
TrustManagerFactory.getDefaultAlgorithm());
47+
trustManagerFactory.init((KeyStore) null);
48+
49+
List<TrustManager> trustManagers = Arrays.asList(trustManagerFactory.getTrustManagers());
50+
List<X509Certificate> rootCerts = trustManagers.stream()
51+
.filter(X509TrustManager.class::isInstance)
52+
.map(X509TrustManager.class::cast)
53+
.map(trustManager -> Arrays.asList(trustManager.getAcceptedIssuers()))
54+
.flatMap(Collection::stream)
55+
.collect(Collectors.toList());
56+
getWatcher().updateTrustedRoots(rootCerts);
57+
} catch (KeyStoreException | NoSuchAlgorithmException ex) {
58+
getWatcher().onError(Status.UNAVAILABLE
59+
.withDescription("Could not load system root certs")
60+
.withCause(ex));
61+
}
62+
}
63+
64+
@Override
65+
public void close() {
66+
// Unnecessary because there's no more callbacks, but do it for good measure
67+
for (Watcher watcher : getWatcher().getDownstreamWatchers()) {
68+
getWatcher().removeWatcher(watcher);
69+
}
70+
}
71+
}

0 commit comments

Comments
 (0)