Skip to content

Commit f135943

Browse files
ejona86ejona86
committed
xds: Plumb system root certs similarly to CertProviders
1 parent 9cddcb4 commit f135943

File tree

3 files changed

+159
-37
lines changed

3 files changed

+159
-37
lines changed

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,20 +55,19 @@ protected final SslContextBuilder getSslContextBuilder(
5555
CertificateValidationContext certificateValidationContextdationContext)
5656
throws CertStoreException {
5757
SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient();
58-
// Null rootCertInstance implies hasSystemRootCerts because of the check in
59-
// CertProviderClientSslContextProviderFactory.
60-
if (rootCertInstance != null) {
61-
if (savedSpiffeTrustMap != null) {
62-
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(
6365
new XdsTrustManagerFactory(
64-
savedSpiffeTrustMap,
66+
savedTrustedRoots.toArray(new X509Certificate[0]),
6567
certificateValidationContextdationContext));
66-
} else {
67-
sslContextBuilder = sslContextBuilder.trustManager(
68-
new XdsTrustManagerFactory(
69-
savedTrustedRoots.toArray(new X509Certificate[0]),
70-
certificateValidationContextdationContext));
71-
}
68+
} else {
69+
// Should be impossible because of the check in CertProviderClientSslContextProviderFactory
70+
throw new IllegalStateException("There must be trusted roots or a SPIFFE trust map");
7271
}
7372
if (isMtls()) {
7473
sslContextBuilder.keyManager(savedKey, savedCertChain);

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

Lines changed: 77 additions & 25 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,15 +38,12 @@
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;
39-
@Nullable private final CertificateProviderInstance certInstance;
40-
@Nullable protected final CertificateProviderInstance rootCertInstance;
41+
@Nullable private final NoExceptionCloseable certHandle;
42+
@Nullable private final NoExceptionCloseable rootCertHandle;
4143
@Nullable protected PrivateKey savedKey;
4244
@Nullable protected List<X509Certificate> savedCertChain;
4345
@Nullable protected List<X509Certificate> savedTrustedRoots;
4446
@Nullable protected Map<String, List<X509Certificate>> savedSpiffeTrustMap;
45-
private final boolean isUsingSystemRootCerts;
4647

4748
protected CertProviderSslContextProvider(
4849
Node node,
@@ -53,40 +54,50 @@ protected CertProviderSslContextProvider(
5354
BaseTlsContext tlsContext,
5455
CertificateProviderStore certificateProviderStore) {
5556
super(tlsContext, staticCertValidationContext);
56-
this.certInstance = certInstance;
57-
this.rootCertInstance = rootCertInstance;
58-
String certInstanceName = null;
59-
if (certInstance != null && certInstance.isInitialized()) {
60-
certInstanceName = certInstance.getInstanceName();
57+
boolean createCertInstance = certInstance != null && certInstance.isInitialized();
58+
boolean createRootCertInstance = rootCertInstance != null && rootCertInstance.isInitialized();
59+
boolean sharedCertInstance = createCertInstance && createRootCertInstance
60+
&& rootCertInstance.getInstanceName().equals(certInstance.getInstanceName());
61+
if (createCertInstance) {
6162
CertificateProviderInfo certProviderInstanceConfig =
62-
getCertProviderConfig(certProviders, certInstanceName);
63+
getCertProviderConfig(certProviders, certInstance.getInstanceName());
64+
CertificateProvider.Watcher watcher = this;
65+
if (!sharedCertInstance) {
66+
watcher = new IgnoreUpdatesWatcher(watcher, /* ignoreRootCertUpdates= */ true);
67+
}
68+
// TODO: Previously we'd hang if certProviderInstanceConfig were null or
69+
// certInstance.isInitialized() == false. Now we'll proceed. Those should be errors, or are
70+
// they impossible and should be assertions?
6371
certHandle = certProviderInstanceConfig == null ? null
6472
: certificateProviderStore.createOrGetProvider(
6573
certInstance.getCertificateName(),
6674
certProviderInstanceConfig.pluginName(),
6775
certProviderInstanceConfig.config(),
68-
this,
69-
true);
76+
watcher,
77+
true)::close;
7078
} else {
7179
certHandle = null;
7280
}
73-
if (rootCertInstance != null
74-
&& rootCertInstance.isInitialized()
75-
&& !rootCertInstance.getInstanceName().equals(certInstanceName)) {
81+
if (createRootCertInstance && sharedCertInstance) {
82+
rootCertHandle = () -> { };
83+
} else if (createRootCertInstance && !sharedCertInstance) {
7684
CertificateProviderInfo certProviderInstanceConfig =
7785
getCertProviderConfig(certProviders, rootCertInstance.getInstanceName());
7886
rootCertHandle = certProviderInstanceConfig == null ? null
7987
: certificateProviderStore.createOrGetProvider(
8088
rootCertInstance.getCertificateName(),
8189
certProviderInstanceConfig.pluginName(),
8290
certProviderInstanceConfig.config(),
83-
this,
84-
true);
91+
new IgnoreUpdatesWatcher(this, /* ignoreRootCertUpdates= */ false),
92+
false)::close;
93+
} else if (rootCertInstance == null
94+
&& CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext())) {
95+
SystemRootCertificateProvider systemRootProvider = new SystemRootCertificateProvider(this);
96+
systemRootProvider.start();
97+
rootCertHandle = systemRootProvider::close;
8598
} else {
8699
rootCertHandle = null;
87100
}
88-
this.isUsingSystemRootCerts = rootCertInstance == null
89-
&& CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext());
90101
}
91102

92103
private static CertificateProviderInfo getCertProviderConfig(
@@ -150,8 +161,7 @@ public final void updateSpiffeTrustMap(Map<String, List<X509Certificate>> spiffe
150161

151162
private void updateSslContextWhenReady() {
152163
if (isMtls()) {
153-
if (savedKey != null
154-
&& (savedTrustedRoots != null || isUsingSystemRootCerts || savedSpiffeTrustMap != null)) {
164+
if (savedKey != null && (savedTrustedRoots != null || savedSpiffeTrustMap != null)) {
155165
updateSslContext();
156166
clearKeysAndCerts();
157167
}
@@ -176,15 +186,15 @@ private void clearKeysAndCerts() {
176186
}
177187

178188
protected final boolean isMtls() {
179-
return certInstance != null && (rootCertInstance != null || isUsingSystemRootCerts);
189+
return certHandle != null && rootCertHandle != null;
180190
}
181191

182192
protected final boolean isClientSideTls() {
183-
return rootCertInstance != null && certInstance == null;
193+
return rootCertHandle != null && certHandle == null;
184194
}
185195

186196
protected final boolean isServerSideTls() {
187-
return certInstance != null && rootCertInstance == null;
197+
return certHandle != null && rootCertHandle == null;
188198
}
189199

190200
@Override
@@ -201,4 +211,46 @@ public final void close() {
201211
rootCertHandle.close();
202212
}
203213
}
214+
215+
interface NoExceptionCloseable extends Closeable {
216+
@Override
217+
void close();
218+
}
219+
220+
static final class IgnoreUpdatesWatcher implements CertificateProvider.Watcher {
221+
private final CertificateProvider.Watcher delegate;
222+
private final boolean ignoreRootCertUpdates;
223+
224+
public IgnoreUpdatesWatcher(
225+
CertificateProvider.Watcher delegate, boolean ignoreRootCertUpdates) {
226+
this.delegate = requireNonNull(delegate, "delegate");
227+
this.ignoreRootCertUpdates = ignoreRootCertUpdates;
228+
}
229+
230+
@Override
231+
public void updateCertificate(PrivateKey key, List<X509Certificate> certChain) {
232+
if (ignoreRootCertUpdates) {
233+
delegate.updateCertificate(key, certChain);
234+
}
235+
}
236+
237+
@Override
238+
public void updateTrustedRoots(List<X509Certificate> trustedRoots) {
239+
if (!ignoreRootCertUpdates) {
240+
delegate.updateTrustedRoots(trustedRoots);
241+
}
242+
}
243+
244+
@Override
245+
public void updateSpiffeTrustMap(Map<String, List<X509Certificate>> spiffeTrustMap) {
246+
if (!ignoreRootCertUpdates) {
247+
delegate.updateSpiffeTrustMap(spiffeTrustMap);
248+
}
249+
}
250+
251+
@Override
252+
public void onError(Status errorStatus) {
253+
delegate.onError(errorStatus);
254+
}
255+
}
204256
}

‎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)