xds: fix identity cert rotation with separate CA provider · grpc/grpc-java@e2b268a · GitHub
Skip to content

Commit e2b268a

Browse files
author
Anshika Pandey
committed
xds: fix identity cert rotation with separate CA provider
1 parent 0b62dce commit e2b268a

3 files changed

Lines changed: 88 additions & 14 deletions

File tree

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

Lines changed: 7 additions & 4 deletions

‎xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderTest.java‎

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
import com.google.common.annotations.VisibleForTesting;
3232
import com.google.common.collect.ImmutableList;
33+
import com.google.common.collect.ImmutableMap;
3334
import com.google.common.util.concurrent.MoreExecutors;
3435
import io.envoyproxy.envoy.config.core.v3.DataSource;
3536
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
@@ -155,7 +156,10 @@ public void testProviderForClient_mtls() throws Exception {
155156
assertThat(provider.getSslContextAndTrustManager()).isNotNull();
156157
assertThat(provider.savedKey).isNull();
157158
assertThat(provider.savedCertChain).isNull();
158-
assertThat(provider.savedTrustedRoots).isNull();
159+
// Trust roots are not cleared: the root provider is independent of the identity provider and
160+
// may not push another update for a long time (or ever), so the last known-good value must
161+
// be retained to allow future identity-only rotations to still trigger a rebuild.
162+
assertThat(provider.savedTrustedRoots).isNotNull();
159163

160164
TestCallback testCallback =
161165
CommonTlsContextTestsUtil.getValueThruCallback(provider);
@@ -180,12 +184,70 @@ public void testProviderForClient_mtls() throws Exception {
180184
ImmutableList.of(getCertFromResourceName(SERVER_1_PEM_FILE)));
181185
assertThat(provider.savedKey).isNull();
182186
assertThat(provider.savedCertChain).isNull();
183-
assertThat(provider.savedTrustedRoots).isNull();
187+
assertThat(provider.savedTrustedRoots).isNotNull();
184188
assertThat(provider.getSslContextAndTrustManager()).isNotNull();
185189
testCallback1 = CommonTlsContextTestsUtil.getValueThruCallback(provider);
186190
assertThat(testCallback1.updatedSslContext).isNotSameInstanceAs(testCallback.updatedSslContext);
187191
}
188192

193+
/**
194+
* Regression test for https://github.com/grpc/grpc-java/issues/13058: the identity cert and
195+
* the CA trust bundle are served by two *separate* certificate provider instances (as happens
196+
* with two file_watcher instances on independent refresh schedules). Once the root/CA
197+
* instance stops sending updates (e.g. its backing file never changes again), a later
198+
* identity-cert-only rotation on the other, independent instance must still trigger a rebuild,
199+
* reusing the last known-good trust roots rather than getting stuck forever.
200+
*/
201+
@Test
202+
public void testProviderForClient_mtls_separateRootInstance_identityRotationRebuildsContext()
203+
throws Exception {
204+
final CertificateProvider.DistributorWatcher[] watcherCaptor =
205+
new CertificateProvider.DistributorWatcher[2];
206+
TestCertificateProvider.createAndRegisterProviderProvider(
207+
certificateProviderRegistry, watcherCaptor, "testca_identity", 0);
208+
TestCertificateProvider.createAndRegisterProviderProvider(
209+
certificateProviderRegistry, watcherCaptor, "testca_root", 1);
210+
211+
Bootstrapper.BootstrapInfo bootstrapInfo =
212+
Bootstrapper.BootstrapInfo.builder()
213+
.servers(ImmutableList.<Bootstrapper.ServerInfo>of())
214+
.node(CommonBootstrapperTestUtils.getTestBootstrapInfo().node())
215+
.certProviders(ImmutableMap.of(
216+
"identity_instance",
217+
Bootstrapper.CertificateProviderInfo.create("testca_identity", ImmutableMap.of()),
218+
"ca_instance",
219+
Bootstrapper.CertificateProviderInfo.create("testca_root", ImmutableMap.of())))
220+
.build();
221+
222+
CertProviderClientSslContextProvider provider =
223+
getSslContextProvider(
224+
"identity_instance",
225+
"ca_instance",
226+
bootstrapInfo,
227+
/* alpnProtocols= */ null,
228+
/* staticCertValidationContext= */ null, false);
229+
230+
// Initial mTLS build: the identity provider and the independent root provider each fire once.
231+
watcherCaptor[0].updateCertificate(
232+
CommonCertProviderTestUtils.getPrivateKey(CLIENT_KEY_FILE),
233+
ImmutableList.of(getCertFromResourceName(CLIENT_PEM_FILE)));
234+
watcherCaptor[1].updateTrustedRoots(ImmutableList.of(getCertFromResourceName(CA_PEM_FILE)));
235+
assertThat(provider.getSslContextAndTrustManager()).isNotNull();
236+
237+
TestCallback initialCallback = CommonTlsContextTestsUtil.getValueThruCallback(provider);
238+
assertThat(initialCallback.updatedSslContext).isNotNull();
239+
240+
// The CA/root provider never fires again (its file never changes), but the identity cert
241+
// rotates on its own, independent schedule.
242+
watcherCaptor[0].updateCertificate(
243+
CommonCertProviderTestUtils.getPrivateKey(SERVER_1_KEY_FILE),
244+
ImmutableList.of(getCertFromResourceName(SERVER_1_PEM_FILE)));
245+
246+
TestCallback afterRotationCallback = CommonTlsContextTestsUtil.getValueThruCallback(provider);
247+
assertThat(afterRotationCallback.updatedSslContext)
248+
.isNotSameInstanceAs(initialCallback.updatedSslContext);
249+
}
250+
189251
@Test
190252
public void testProviderForClient_systemRootCerts_mtls() throws Exception {
191253
final CertificateProvider.DistributorWatcher[] watcherCaptor =
@@ -299,7 +361,10 @@ public void testProviderForClient_mtls_newXds() throws Exception {
299361
assertThat(provider.getSslContextAndTrustManager()).isNotNull();
300362
assertThat(provider.savedKey).isNull();
301363
assertThat(provider.savedCertChain).isNull();
302-
assertThat(provider.savedTrustedRoots).isNull();
364+
// Trust roots are not cleared: the root provider is independent of the identity provider and
365+
// may not push another update for a long time (or ever), so the last known-good value must
366+
// be retained to allow future identity-only rotations to still trigger a rebuild.
367+
assertThat(provider.savedTrustedRoots).isNotNull();
303368

304369
TestCallback testCallback =
305370
CommonTlsContextTestsUtil.getValueThruCallback(provider);
@@ -324,7 +389,7 @@ public void testProviderForClient_mtls_newXds() throws Exception {
324389
ImmutableList.of(getCertFromResourceName(SERVER_1_PEM_FILE)));
325390
assertThat(provider.savedKey).isNull();
326391
assertThat(provider.savedCertChain).isNull();
327-
assertThat(provider.savedTrustedRoots).isNull();
392+
assertThat(provider.savedTrustedRoots).isNotNull();
328393
assertThat(provider.getSslContextAndTrustManager()).isNotNull();
329394
testCallback1 = CommonTlsContextTestsUtil.getValueThruCallback(provider);
330395
assertThat(testCallback1.updatedSslContext).isNotSameInstanceAs(testCallback.updatedSslContext);
@@ -387,7 +452,7 @@ public void testProviderForClient_tls() throws Exception {
387452
assertThat(provider.getSslContextAndTrustManager()).isNotNull();
388453
assertThat(provider.savedKey).isNull();
389454
assertThat(provider.savedCertChain).isNull();
390-
assertThat(provider.savedTrustedRoots).isNull();
455+
assertThat(provider.savedTrustedRoots).isNotNull();
391456

392457
TestCallback testCallback =
393458
CommonTlsContextTestsUtil.getValueThruCallback(provider);
@@ -514,7 +579,7 @@ public void testProviderForClient_deprecatedCertProviderField() throws Exception
514579
assertThat(provider.getSslContextAndTrustManager()).isNotNull();
515580
assertThat(provider.savedKey).isNull();
516581
assertThat(provider.savedCertChain).isNull();
517-
assertThat(provider.savedTrustedRoots).isNull();
582+
assertThat(provider.savedTrustedRoots).isNotNull();
518583

519584
TestCallback testCallback =
520585
CommonTlsContextTestsUtil.getValueThruCallback(provider);

‎xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderServerSslContextProviderTest.java‎

Lines changed: 10 additions & 4 deletions

0 commit comments

Comments
 (0)