Skip to content

Commit 4334877

Browse files
kannanjgithubAgraVator
kannanjgithub
authored and
AgraVator
committed
xds: SslContext updates handling when using system root certs (grpc#12340)
Fixes the issues in SslContext not updated when using system root certs because it was using `CertProviderClientSslContextProvider` that relies on watcher updates from the certificate file watcher. This change creates a separate handler for system root certs and updates the SslContext on the `SslContextProvider` callback as soon as the provider is created.
1 parent 37e6244 commit 4334877

File tree

8 files changed

+399
-80
lines changed

8 files changed

+399
-80
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: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import io.grpc.xds.client.Bootstrapper.CertificateProviderInfo;
2525
import io.grpc.xds.internal.security.CommonTlsContextUtil;
2626
import io.grpc.xds.internal.security.DynamicSslContextProvider;
27+
import java.io.Closeable;
2728
import java.security.PrivateKey;
2829
import java.security.cert.X509Certificate;
2930
import java.util.List;
@@ -34,8 +35,8 @@
3435
abstract class CertProviderSslContextProvider extends DynamicSslContextProvider implements
3536
CertificateProvider.Watcher {
3637

37-
@Nullable private final CertificateProviderStore.Handle certHandle;
38-
@Nullable private final CertificateProviderStore.Handle rootCertHandle;
38+
@Nullable private final NoExceptionCloseable certHandle;
39+
@Nullable private final NoExceptionCloseable rootCertHandle;
3940
@Nullable private final CertificateProviderInstance certInstance;
4041
@Nullable protected final CertificateProviderInstance rootCertInstance;
4142
@Nullable protected PrivateKey savedKey;
@@ -55,38 +56,50 @@ protected CertProviderSslContextProvider(
5556
super(tlsContext, staticCertValidationContext);
5657
this.certInstance = certInstance;
5758
this.rootCertInstance = rootCertInstance;
58-
String certInstanceName = null;
59-
if (certInstance != null && certInstance.isInitialized()) {
60-
certInstanceName = certInstance.getInstanceName();
59+
this.isUsingSystemRootCerts = rootCertInstance == null
60+
&& CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext());
61+
boolean createCertInstance = certInstance != null && certInstance.isInitialized();
62+
boolean createRootCertInstance = rootCertInstance != null && rootCertInstance.isInitialized();
63+
boolean sharedCertInstance = createCertInstance && createRootCertInstance
64+
&& rootCertInstance.getInstanceName().equals(certInstance.getInstanceName());
65+
if (createCertInstance) {
6166
CertificateProviderInfo certProviderInstanceConfig =
62-
getCertProviderConfig(certProviders, certInstanceName);
67+
getCertProviderConfig(certProviders, certInstance.getInstanceName());
68+
CertificateProvider.Watcher watcher = this;
69+
if (!sharedCertInstance && !isUsingSystemRootCerts) {
70+
watcher = new IgnoreUpdatesWatcher(watcher, /* ignoreRootCertUpdates= */ true);
71+
}
72+
// TODO: Previously we'd hang if certProviderInstanceConfig were null or
73+
// certInstance.isInitialized() == false. Now we'll proceed. Those should be errors, or are
74+
// they impossible and should be assertions?
6375
certHandle = certProviderInstanceConfig == null ? null
6476
: certificateProviderStore.createOrGetProvider(
6577
certInstance.getCertificateName(),
6678
certProviderInstanceConfig.pluginName(),
6779
certProviderInstanceConfig.config(),
68-
this,
69-
true);
80+
watcher,
81+
true)::close;
7082
} else {
7183
certHandle = null;
7284
}
73-
if (rootCertInstance != null
74-
&& rootCertInstance.isInitialized()
75-
&& !rootCertInstance.getInstanceName().equals(certInstanceName)) {
85+
if (createRootCertInstance && !sharedCertInstance) {
7686
CertificateProviderInfo certProviderInstanceConfig =
7787
getCertProviderConfig(certProviders, rootCertInstance.getInstanceName());
7888
rootCertHandle = certProviderInstanceConfig == null ? null
7989
: certificateProviderStore.createOrGetProvider(
8090
rootCertInstance.getCertificateName(),
8191
certProviderInstanceConfig.pluginName(),
8292
certProviderInstanceConfig.config(),
83-
this,
84-
true);
93+
new IgnoreUpdatesWatcher(this, /* ignoreRootCertUpdates= */ false),
94+
false)::close;
95+
} else if (rootCertInstance == null
96+
&& CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext())) {
97+
SystemRootCertificateProvider systemRootProvider = new SystemRootCertificateProvider(this);
98+
systemRootProvider.start();
99+
rootCertHandle = systemRootProvider::close;
85100
} else {
86101
rootCertHandle = null;
87102
}
88-
this.isUsingSystemRootCerts = rootCertInstance == null
89-
&& CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext());
90103
}
91104

92105
private static CertificateProviderInfo getCertProviderConfig(
@@ -150,17 +163,16 @@ public final void updateSpiffeTrustMap(Map<String, List<X509Certificate>> spiffe
150163

151164
private void updateSslContextWhenReady() {
152165
if (isMtls()) {
153-
if (savedKey != null
154-
&& (savedTrustedRoots != null || isUsingSystemRootCerts || savedSpiffeTrustMap != null)) {
166+
if (savedKey != null && (savedTrustedRoots != null || savedSpiffeTrustMap != null)) {
155167
updateSslContext();
156168
clearKeysAndCerts();
157169
}
158-
} else if (isClientSideTls()) {
170+
} else if (isRegularTlsAndClientSide()) {
159171
if (savedTrustedRoots != null || savedSpiffeTrustMap != null) {
160172
updateSslContext();
161173
clearKeysAndCerts();
162174
}
163-
} else if (isServerSideTls()) {
175+
} else if (isRegularTlsAndServerSide()) {
164176
if (savedKey != null) {
165177
updateSslContext();
166178
clearKeysAndCerts();
@@ -170,20 +182,22 @@ private void updateSslContextWhenReady() {
170182

171183
private void clearKeysAndCerts() {
172184
savedKey = null;
173-
savedTrustedRoots = null;
174-
savedSpiffeTrustMap = null;
185+
if (!isUsingSystemRootCerts) {
186+
savedTrustedRoots = null;
187+
savedSpiffeTrustMap = null;
188+
}
175189
savedCertChain = null;
176190
}
177191

178192
protected final boolean isMtls() {
179193
return certInstance != null && (rootCertInstance != null || isUsingSystemRootCerts);
180194
}
181195

182-
protected final boolean isClientSideTls() {
183-
return rootCertInstance != null && certInstance == null;
196+
protected final boolean isRegularTlsAndClientSide() {
197+
return (rootCertInstance != null || isUsingSystemRootCerts) && certInstance == null;
184198
}
185199

186-
protected final boolean isServerSideTls() {
200+
protected final boolean isRegularTlsAndServerSide() {
187201
return certInstance != null && rootCertInstance == null;
188202
}
189203

@@ -201,4 +215,9 @@ public final void close() {
201215
rootCertHandle.close();
202216
}
203217
}
218+
219+
interface NoExceptionCloseable extends Closeable {
220+
@Override
221+
void close();
222+
}
204223
}

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright 2025 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 static java.util.Objects.requireNonNull;
20+
21+
import com.google.common.annotations.VisibleForTesting;
22+
import io.grpc.Status;
23+
import java.security.PrivateKey;
24+
import java.security.cert.X509Certificate;
25+
import java.util.List;
26+
import java.util.Map;
27+
28+
public final class IgnoreUpdatesWatcher implements CertificateProvider.Watcher {
29+
private final CertificateProvider.Watcher delegate;
30+
private final boolean ignoreRootCertUpdates;
31+
32+
public IgnoreUpdatesWatcher(
33+
CertificateProvider.Watcher delegate, boolean ignoreRootCertUpdates) {
34+
this.delegate = requireNonNull(delegate, "delegate");
35+
this.ignoreRootCertUpdates = ignoreRootCertUpdates;
36+
}
37+
38+
@Override
39+
public void updateCertificate(PrivateKey key, List<X509Certificate> certChain) {
40+
if (ignoreRootCertUpdates) {
41+
delegate.updateCertificate(key, certChain);
42+
}
43+
}
44+
45+
@Override
46+
public void updateTrustedRoots(List<X509Certificate> trustedRoots) {
47+
if (!ignoreRootCertUpdates) {
48+
delegate.updateTrustedRoots(trustedRoots);
49+
}
50+
}
51+
52+
@Override
53+
public void updateSpiffeTrustMap(Map<String, List<X509Certificate>> spiffeTrustMap) {
54+
if (!ignoreRootCertUpdates) {
55+
delegate.updateSpiffeTrustMap(spiffeTrustMap);
56+
}
57+
}
58+
59+
@Override
60+
public void onError(Status errorStatus) {
61+
delegate.onError(errorStatus);
62+
}
63+
64+
@VisibleForTesting
65+
public CertificateProvider.Watcher getDelegate() {
66+
return delegate;
67+
}
68+
}

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