Add tests and fix bugs

grpc · Oct 14, 2024 · 1a1024a · 1a1024a
1 parent c87767e
commit 1a1024a
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 10 deletions.
diff --git a/packages/grpc-js-xds/src/xds-bootstrap.ts b/packages/grpc-js-xds/src/xds-bootstrap.ts
@@ -345,7 +345,7 @@ function validateFileWatcherPluginConfig(obj: any, instanceName: string): FileWa
   return {
     certificateFile: obj.certificate_file,
     privateKeyFile: obj.private_key_file,
-    caCertificateFile: obj.caCertificateFile,
+    caCertificateFile: obj.ca_certificate_file,
     refreshIntervalMs: durationToMs(refreshDuration)
   };
 }

diff --git a/packages/grpc-js-xds/src/xds-resource-type/listener-resource-type.ts b/packages/grpc-js-xds/src/xds-resource-type/listener-resource-type.ts
@@ -143,34 +143,46 @@ function validateFilterChain(context: XdsDecodeContext, filterChain: FilterChain
   if (filterChain.transport_socket) {
     const transportSocket = filterChain.transport_socket;
     if (transportSocket.name !== 'envoy.transport_sockets.tls') {
+      trace('Wrong transportSocket.name');
+      return false;
+    }
+    if (!transportSocket.typed_config) {
+      trace('No typed_config');
       return false;
     }
     if (transportSocket.typed_config?.type_url !== DOWNSTREAM_TLS_CONTEXT_TYPE_URL) {
+      trace(`Wrong typed_config type_url: ${transportSocket.typed_config?.type_url}`);
       return false;
     }
     const downstreamTlsContext = decodeSingleResource(DOWNSTREAM_TLS_CONTEXT_TYPE_URL, transportSocket.typed_config.value);
     if (!downstreamTlsContext.common_tls_context) {
+      trace('No common_tls_context');
       return false;
     }
     const commonTlsContext = downstreamTlsContext.common_tls_context;
     if (!commonTlsContext.tls_certificate_provider_instance) {
+      trace('No tls_certificate_provider_instance');
       return false;
     }
     if (!(commonTlsContext.tls_certificate_provider_instance.instance_name in context.bootstrap.certificateProviders)) {
+      trace('Unmatched tls_certificate_provider_instance instance_name');
       return false;
     }
     let validationContext: CertificateValidationContext__Output | null;
     switch (commonTlsContext.validation_context_type) {
       case 'validation_context_sds_secret_config':
+        trace('Unexpected validation_context_sds_secret_config')
         return false;
       case 'validation_context':
         if (!commonTlsContext.validation_context) {
+          trace('Missing validation_context');
           return false;
         }
         validationContext = commonTlsContext.validation_context;
         break;
       case 'combined_validation_context':
         if (!commonTlsContext.combined_validation_context) {
+          trace('Missing combined_validation_context')
           return false;
         }
         validationContext = commonTlsContext.combined_validation_context.default_validation_context;
@@ -179,21 +191,27 @@ function validateFilterChain(context: XdsDecodeContext, filterChain: FilterChain
         return false;
     }
     if (validationContext?.ca_certificate_provider_instance && !(validationContext.ca_certificate_provider_instance.instance_name in context.bootstrap.certificateProviders)) {
+      trace('Unmatched validationContext instance_name');
       return false;
     }
     if (downstreamTlsContext.require_client_certificate && !validationContext) {
+      trace('require_client_certificate set without validationContext');
       return false;
     }
     if (commonTlsContext.tls_params) {
+      trace('tls_params set');
       return false;
     }
     if (commonTlsContext.custom_handshaker) {
+      trace('custom_handshaker set');
       return false;
     }
     if (downstreamTlsContext.require_sni?.value) {
+      trace('require_sni set');
       return false;
     }
     if (downstreamTlsContext.ocsp_staple_policy !== 'LENIENT_STAPLING') {
+      trace('Unexpected ocsp_staple_policy');
       return false;
     }
   }

diff --git a/packages/grpc-js-xds/test/test-xds-credentials.ts b/packages/grpc-js-xds/test/test-xds-credentials.ts
@@ -75,9 +75,64 @@ describe('Server xDS Credentials', () => {
       common_tls_context: {
         tls_certificate_provider_instance: {
           instance_name: 'test_certificates'
+        },
+        validation_context: {}
+      },
+      ocsp_staple_policy: 'LENIENT_STAPLING'
+    }
+    const baseServerListener: Listener = {
+      default_filter_chain: {
+        filter_chain_match: {
+          source_type: 'SAME_IP_OR_LOOPBACK'
+        },
+        transport_socket: {
+          name: 'envoy.transport_sockets.tls',
+          typed_config: downstreamTlsContext
         }
       }
     }
+    const serverRoute = new FakeServerRoute(backend.getPort(), 'serverRoute', baseServerListener);
+    xdsServer.setRdsResource(serverRoute.getRouteConfiguration());
+    xdsServer.setLdsResource(serverRoute.getListener());
+    xdsServer.addResponseListener((typeUrl, responseState) => {
+      if (responseState.state === 'NACKED') {
+        client?.stopCalls();
+        assert.fail(`Client NACKED ${typeUrl} resource with message ${responseState.errorMessage}`);
+      }
+    });
+    const cluster = new FakeEdsCluster('cluster1', 'endpoint1', [{backends: [backend], locality:{region: 'region1'}}]);
+    const routeGroup = new FakeRouteGroup('listener1', 'route1', [{cluster: cluster}]);
+    await routeGroup.startAllBackends(xdsServer);
+    xdsServer.setEdsResource(cluster.getEndpointConfig());
+    xdsServer.setCdsResource(cluster.getClusterConfig());
+    xdsServer.setRdsResource(routeGroup.getRouteConfiguration());
+    xdsServer.setLdsResource(routeGroup.getListener());
+    client = XdsTestClient.createFromServer('listener1', xdsServer, credentials.createSsl(ca), {
+      'grpc.ssl_target_name_override': 'foo.test.google.fr',
+      'grpc.default_authority': 'foo.test.google.fr',
+    });
+    const error = await client.sendOneCallAsync();
+    assert.strictEqual(error, null);
+  });
+  it('Should use identity and CA certificates when configured', async () => {
+    const [backend] = await createBackends(1, true, new XdsServerCredentials(ServerCredentials.createInsecure()));
+    const downstreamTlsContext: DownstreamTlsContext & AnyExtension = {
+      '@type': DOWNSTREAM_TLS_CONTEXT_TYPE_URL,
+      common_tls_context: {
+        tls_certificate_provider_instance: {
+          instance_name: 'test_certificates'
+        },
+        validation_context: {
+          ca_certificate_provider_instance: {
+            instance_name: 'test_certificates'
+          }
+        }
+      },
+      ocsp_staple_policy: 'LENIENT_STAPLING',
+      require_client_certificate: {
+        value: true
+      }
+    }
     const baseServerListener: Listener = {
       default_filter_chain: {
         filter_chain_match: {
@@ -105,7 +160,10 @@ describe('Server xDS Credentials', () => {
     xdsServer.setCdsResource(cluster.getClusterConfig());
     xdsServer.setRdsResource(routeGroup.getRouteConfiguration());
     xdsServer.setLdsResource(routeGroup.getListener());
-    client = XdsTestClient.createFromServer('listener1', xdsServer, credentials.createSsl(ca));
+    client = XdsTestClient.createFromServer('listener1', xdsServer, credentials.createSsl(ca, key, cert), {
+      'grpc.ssl_target_name_override': 'foo.test.google.fr',
+      'grpc.default_authority': 'foo.test.google.fr',
+    });
     const error = await client.sendOneCallAsync();
     assert.strictEqual(error, null);
   });

diff --git a/packages/grpc-js-xds/test/xds-server.ts b/packages/grpc-js-xds/test/xds-server.ts
@@ -53,6 +53,7 @@ const loadedProtos = loadPackageDefinition(loadSync(
     'envoy/extensions/load_balancing_policies/wrr_locality/v3/wrr_locality.proto',
     'envoy/extensions/load_balancing_policies/ring_hash/v3/ring_hash.proto',
     'envoy/extensions/load_balancing_policies/pick_first/v3/pick_first.proto',
+    'envoy/extensions/transport_sockets/tls/v3/tls.proto',
     'xds/type/v3/typed_struct.proto'
   ],
   {

diff --git a/packages/grpc-js/src/certificate-provider.ts b/packages/grpc-js/src/certificate-provider.ts
@@ -90,7 +90,7 @@ export class FileWatcherCertificateProvider implements CertificateProvider {
       if (!this.refreshTimer) {
         return;
       }
-      trace('File watcher read certificates certificate' + (certificateResult ? '!=' : '==') + 'null, privateKey' + (privateKeyResult ? '!=' : '==') + 'null, CA certificate' + (caCertificateResult ? '!=' : '==') + 'null');
+      trace('File watcher read certificates certificate ' + certificateResult.status + ', privateKey ' + privateKeyResult.status + ', CA certificate ' + caCertificateResult.status);
       this.lastUpdateTime = new Date();
       this.fileResultPromise = null;
       if (certificateResult.status === 'fulfilled' && privateKeyResult.status === 'fulfilled') {

diff --git a/packages/grpc-js/src/server-credentials.ts b/packages/grpc-js/src/server-credentials.ts
@@ -225,24 +225,24 @@ class CertificateProviderServerCredentials extends ServerCredentials {
   private caCertificateUpdateListener: CaCertificateUpdateListener = this.handleCaCertificateUpdate.bind(this);
   private identityCertificateUpdateListener: IdentityCertificateUpdateListener = this.handleIdentityCertitificateUpdate.bind(this);
   constructor(
-    private caCertificateProvider: CertificateProvider,
-    private identityCertificateProvider: CertificateProvider | null,
+    private identityCertificateProvider: CertificateProvider,
+    private caCertificateProvider: CertificateProvider | null,
     private requireClientCertificate: boolean
   ) {
     super();
   }
   _addWatcher(watcher: SecureContextWatcher): void {
     if (this.getWatcherCount() === 0) {
-      this.caCertificateProvider.addCaCertificateListener(this.caCertificateUpdateListener);
-      this.identityCertificateProvider?.addIdentityCertificateListener(this.identityCertificateUpdateListener);
+      this.caCertificateProvider?.addCaCertificateListener(this.caCertificateUpdateListener);
+      this.identityCertificateProvider.addIdentityCertificateListener(this.identityCertificateUpdateListener);
     }
     super._addWatcher(watcher);
   }
   _removeWatcher(watcher: SecureContextWatcher): void {
     super._removeWatcher(watcher);
     if (this.getWatcherCount() === 0) {
-      this.caCertificateProvider.removeCaCertificateListener(this.caCertificateUpdateListener);
-      this.identityCertificateProvider?.removeIdentityCertificateListener(this.identityCertificateUpdateListener);
+      this.caCertificateProvider?.removeCaCertificateListener(this.caCertificateUpdateListener);
+      this.identityCertificateProvider.removeIdentityCertificateListener(this.identityCertificateUpdateListener);
     }
   }
   _isSecure(): boolean {
@@ -279,7 +279,8 @@ class CertificateProviderServerCredentials extends ServerCredentials {
   }
 
   private finalizeUpdate() {
-    this.updateSecureContextOptions(this.calculateSecureContextOptions());
+    const secureContextOptions = this.calculateSecureContextOptions();
+    this.updateSecureContextOptions(secureContextOptions);
   }
 
   private handleCaCertificateUpdate(update: CaCertificateUpdate | null) {