Handle secure context errors, fix server constructor argument handling

packages/grpc-js-xds/src/xds-credentials.ts

@@ -64,7 +64,7 @@ export class XdsChannelCredentials extends ChannelCredentials {
 
 export class XdsServerCredentials extends ServerCredentials {
   constructor(private fallbackCredentials: ServerCredentials) {
-    super();
+    super({});
   }
 
   getFallbackCredentials() {

packages/grpc-js-xds/src/xds-resource-type/listener-resource-type.ts

@@ -152,6 +152,7 @@ function validateTransportSocket(context: XdsDecodeContext, transportSocket: Tra
     return errors;
   }
   const downstreamTlsContext = decodeSingleResource(DOWNSTREAM_TLS_CONTEXT_TYPE_URL, transportSocket.typed_config.value);
+  trace('Decoded DownstreamTlsContext: ' + JSON.stringify(downstreamTlsContext, undefined, 2));
   if (downstreamTlsContext.require_sni?.value) {
     errors.push(`DownstreamTlsContext.require_sni set`);
   }

packages/grpc-js-xds/test/test-xds-credentials.ts

@@ -21,7 +21,7 @@ import { FakeEdsCluster, FakeRouteGroup, FakeServerRoute } from './framework';
 import { ControlPlaneServer } from './xds-server';
 import { XdsTestClient } from './client';
 import { XdsChannelCredentials, XdsServerCredentials } from '../src';
-import { credentials, ServerCredentials, experimental } from '@grpc/grpc-js';
+import { credentials, ServerCredentials, experimental, status } from '@grpc/grpc-js';
 import { readFileSync } from 'fs';
 import * as path from 'path';
 import { Listener } from '../src/generated/envoy/config/listener/v3/Listener';
@@ -441,6 +441,65 @@ describe('Client xDS credentials', () => {
       const error = await client.sendOneCallAsync();
       assert.strictEqual(error, null);
     });
-
+    it('Should fail when the server expects a certificate and does not get one', 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: {
+            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 upstreamTlsContext: UpstreamTlsContext = {
+        common_tls_context: {
+          validation_context: {
+            ca_certificate_provider_instance: {
+              instance_name: 'test_certificates'
+            }
+          }
+        }
+      };
+      const cluster = new FakeEdsCluster('cluster1', 'endpoint1', [{backends: [backend], locality:{region: 'region1'}}], undefined, upstreamTlsContext);
+      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, new XdsChannelCredentials(credentials.createInsecure()));
+      const error = await client.sendOneCallAsync();
+      assert(error);
+      assert.strictEqual(error.code, status.UNAVAILABLE);
+    });
   });
 });

packages/grpc-js/src/channel-credentials.ts

@@ -31,6 +31,8 @@ import { Socket } from 'net';
 import { ChannelOptions } from './channel-options';
 import { GrpcUri, parseUri, splitHostPort } from './uri-parser';
 import { getDefaultAuthority } from './resolver';
+import { log } from './logging';
+import { LogVerbosity } from './constants';
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 function verifyIsBufferOrNull(obj: any, friendlyName: string): void {
@@ -475,12 +477,17 @@ class CertificateProviderChannelCredentialsImpl extends ChannelCredentials {
     if (this.identityCertificateProvider !== null && !this.latestIdentityUpdate) {
       return null;
     }
-    return createSecureContext({
-      ca: this.latestCaUpdate.caCertificate,
-      key: this.latestIdentityUpdate?.privateKey,
-      cert: this.latestIdentityUpdate?.certificate,
-      ciphers: CIPHER_SUITES
-    });
+    try {
+      return createSecureContext({
+        ca: this.latestCaUpdate.caCertificate,
+        key: this.latestIdentityUpdate?.privateKey,
+        cert: this.latestIdentityUpdate?.certificate,
+        ciphers: CIPHER_SUITES
+      });
+    } catch (e) {
+      log(LogVerbosity.ERROR, 'Failed to createSecureContext with error ' + (e as Error).message);
+      return null;
+    }
   }
 }
 

packages/grpc-js/src/server-credentials.ts

@@ -32,7 +32,11 @@ export interface SecureContextWatcher {
 
 export abstract class ServerCredentials {
   private watchers: Set<SecureContextWatcher> = new Set();
-  private latestContextOptions: SecureServerOptions | null = null;
+  private latestContextOptions: SecureContextOptions | null = null;
+  constructor(private serverConstructorOptions: SecureServerOptions | null, contextOptions?: SecureContextOptions) {
+    this.latestContextOptions = contextOptions ?? null;
+  }
+
   _addWatcher(watcher: SecureContextWatcher) {
     this.watchers.add(watcher);
   }
@@ -42,16 +46,21 @@ export abstract class ServerCredentials {
   protected getWatcherCount() {
     return this.watchers.size;
   }
-  protected updateSecureContextOptions(options: SecureServerOptions | null) {
+  protected updateSecureContextOptions(options: SecureContextOptions | null) {
     this.latestContextOptions = options;
     for (const watcher of this.watchers) {
       watcher(this.latestContextOptions);
     }
   }
-  abstract _isSecure(): boolean;
-  _getSettings(): SecureServerOptions | null {
+  _isSecure(): boolean {
+    return this.serverConstructorOptions !== null;
+  }
+  _getSecureContextOptions(): SecureContextOptions | null {
     return this.latestContextOptions;
   }
+  _getConstructorOptions(): SecureServerOptions | null {
+    return this.serverConstructorOptions;
+  }
   _getInterceptors(): ServerInterceptor[] {
     return [];
   }
@@ -101,18 +110,19 @@ export abstract class ServerCredentials {
     }
 
     return new SecureServerCredentials({
+      requestCert: checkClientCertificate,
+      ciphers: CIPHER_SUITES,
+    }, {
       ca: rootCerts ?? getDefaultRootsData() ?? undefined,
       cert,
       key,
-      requestCert: checkClientCertificate,
-      ciphers: CIPHER_SUITES,
     });
   }
 }
 
 class InsecureServerCredentials extends ServerCredentials {
-  _isSecure(): boolean {
-    return false;
+  constructor() {
+    super(null);
   }
 
   _getSettings(): null {
@@ -127,17 +137,9 @@ class InsecureServerCredentials extends ServerCredentials {
 class SecureServerCredentials extends ServerCredentials {
   private options: SecureServerOptions;
 
-  constructor(options: SecureServerOptions) {
-    super();
-    this.options = options;
-  }
-
-  _isSecure(): boolean {
-    return true;
-  }
-
-  _getSettings(): SecureServerOptions {
-    return this.options;
+  constructor(constructorOptions: SecureServerOptions, contextOptions: SecureContextOptions) {
+    super(constructorOptions, contextOptions);
+    this.options = {...constructorOptions, ...contextOptions};
   }
 
   /**
@@ -229,7 +231,11 @@ class CertificateProviderServerCredentials extends ServerCredentials {
     private caCertificateProvider: CertificateProvider | null,
     private requireClientCertificate: boolean
   ) {
-    super();
+    super({
+      requestCert: caCertificateProvider !== null,
+      rejectUnauthorized: requireClientCertificate,
+      ciphers: CIPHER_SUITES
+    });
   }
   _addWatcher(watcher: SecureContextWatcher): void {
     if (this.getWatcherCount() === 0) {
@@ -245,9 +251,6 @@ class CertificateProviderServerCredentials extends ServerCredentials {
       this.identityCertificateProvider.removeIdentityCertificateListener(this.identityCertificateUpdateListener);
     }
   }
-  _isSecure(): boolean {
-    return true;
-  }
   _equals(other: ServerCredentials): boolean {
     if (this === other) {
       return true;
@@ -262,7 +265,7 @@ class CertificateProviderServerCredentials extends ServerCredentials {
     )
   }
 
-  private calculateSecureContextOptions(): SecureServerOptions | null {
+  private calculateSecureContextOptions(): SecureContextOptions | null {
     if (this.latestIdentityUpdate === null) {
       return null;
     }
@@ -271,10 +274,8 @@ class CertificateProviderServerCredentials extends ServerCredentials {
     }
     return {
       ca: this.latestCaUpdate?.caCertificate,
-      cert: this.latestIdentityUpdate.certificate,
-      key: this.latestIdentityUpdate.privateKey,
-      requestCert: this.latestIdentityUpdate !== null,
-      rejectUnauthorized: this.requireClientCertificate
+      cert: [this.latestIdentityUpdate.certificate],
+      key: [this.latestIdentityUpdate.privateKey],
     };
   }
 
@@ -307,7 +308,7 @@ export function createCertificateProviderServerCredentials(
 
 class InterceptorServerCredentials extends ServerCredentials {
   constructor(private readonly childCredentials: ServerCredentials, private readonly interceptors: ServerInterceptor[]) {
-    super();
+    super({});
   }
   _isSecure(): boolean {
     return this.childCredentials._isSecure();
@@ -338,8 +339,11 @@ class InterceptorServerCredentials extends ServerCredentials {
   override _removeWatcher(watcher: SecureContextWatcher): void {
     this.childCredentials._removeWatcher(watcher);
   }
-  override _getSettings(): SecureServerOptions | null {
-    return this.childCredentials._getSettings();
+  override _getConstructorOptions(): SecureServerOptions | null {
+    return this.childCredentials._getConstructorOptions();
+  }
+  override _getSecureContextOptions(): SecureContextOptions | null {
+    return this.childCredentials._getSecureContextOptions();
   }
 }
 

packages/grpc-js/src/server.ts

@@ -574,13 +574,15 @@ export class Server {
   private createHttp2Server(credentials: ServerCredentials) {
     let http2Server: http2.Http2Server | http2.Http2SecureServer;
     if (credentials._isSecure()) {
-      const credentialsSettings = credentials._getSettings();
+      const constructorOptions = credentials._getConstructorOptions();
+      const contextOptions = credentials._getSecureContextOptions();
       const secureServerOptions: http2.SecureServerOptions = {
         ...this.commonServerOptions,
-        ...credentialsSettings,
+        ...constructorOptions,
+        ...contextOptions,
         enableTrace: this.options['grpc-node.tls_enable_trace'] === 1
       };
-      let areCredentialsValid = credentialsSettings !== null;
+      let areCredentialsValid = contextOptions !== null;
       this.trace('Initial credentials valid: ' + areCredentialsValid);
       http2Server = http2.createSecureServer(secureServerOptions);
       http2Server.prependListener('connection', (socket: Socket) => {
@@ -600,7 +602,13 @@ export class Server {
       });
       const credsWatcher: SecureContextWatcher = options => {
         if (options) {
-          (http2Server as http2.Http2SecureServer).setSecureContext(options);
+          const secureServer = http2Server as http2.Http2SecureServer;
+          try {
+            secureServer.setSecureContext(options);
+          } catch (e) {
+            logging.log(LogVerbosity.ERROR, 'Failed to set secure context with error ' + (e as Error).message);
+            options = null;
+          }
         }
         areCredentialsValid = options !== null;
         this.trace('Post-update credentials valid: ' + areCredentialsValid);

packages/grpc-js/src/transport.ts

@@ -659,6 +659,10 @@ export class Http2SubchannelConnector implements SubchannelConnector {
       return Promise.reject();
     }
 
+    if (secureConnectResult.socket.closed) {
+      return Promise.reject('Connection closed before starting HTTP/2 handshake');
+    }
+
     return new Promise<Http2Transport>((resolve, reject) => {
       let remoteName: string | null = null;
       let realTarget: GrpcUri = this.channelTarget;
@@ -671,6 +675,26 @@ export class Http2SubchannelConnector implements SubchannelConnector {
       }
       const scheme = secureConnectResult.secure ? 'https' : 'http';
       const targetPath = getDefaultAuthority(realTarget);
+      const closeHandler = () => {
+        this.session?.destroy();
+        this.session = null;
+        // Leave time for error event to happen before rejecting
+        setImmediate(() => {
+          if (!reportedError) {
+            reportedError = true;
+            reject(`${errorMessage.trim()} (${new Date().toISOString()})`);
+          }
+        });
+      };
+      const errorHandler = (error: Error) => {
+        this.session?.destroy();
+        errorMessage = (error as Error).message;
+        this.trace('connection failed with error ' + errorMessage);
+        if (!reportedError) {
+          reportedError = true;
+          reject(`${errorMessage} (${new Date().toISOString()})`);
+        }
+      };
       const session = http2.connect(`${scheme}://${targetPath}`, {
         createConnection: (authority, option) => {
           return secureConnectResult.socket;
@@ -687,27 +711,15 @@ export class Http2SubchannelConnector implements SubchannelConnector {
       session.unref();
       session.once('remoteSettings', () => {
         session.removeAllListeners();
+        secureConnectResult.socket.removeListener('close', closeHandler);
+        secureConnectResult.socket.removeListener('error', errorHandler);
         resolve(new Http2Transport(session, address, options, remoteName));
         this.session = null;
       });
-      session.once('close', () => {
-        this.session = null;
-        // Leave time for error event to happen before rejecting
-        setImmediate(() => {
-          if (!reportedError) {
-            reportedError = true;
-            reject(`${errorMessage} (${new Date().toISOString()})`);
-          }
-        });
-      });
-      session.once('error', error => {
-        errorMessage = (error as Error).message;
-        this.trace('connection failed with error ' + errorMessage);
-        if (!reportedError) {
-          reportedError = true;
-          reject(`${errorMessage} (${new Date().toISOString()})`);
-        }
-      });
+      session.once('close', closeHandler);
+      session.once('error', errorHandler);
+      secureConnectResult.socket.once('close', closeHandler);
+      secureConnectResult.socket.once('error', errorHandler);
     });
   }
 
@@ -747,11 +759,13 @@ export class Http2SubchannelConnector implements SubchannelConnector {
     let secureConnectResult: SecureConnectResult | null  = null;
     const addressString = subchannelAddressToString(address);
     try {
+      this.trace(addressString + ' Waiting for secureConnector to be ready');
       await secureConnector.waitForReady();
+      this.trace(addressString + ' secureConnector is ready');
       tcpConnection = await this.tcpConnect(address, options);
-      this.trace(addressString + ' ' + 'Established TCP connection');
+      this.trace(addressString + ' Established TCP connection');
       secureConnectResult = await secureConnector.connect(tcpConnection);
-      this.trace(addressString + ' ' + 'Established secure connection');
+      this.trace(addressString + ' Established secure connection');
       return this.createSession(secureConnectResult, address, options);
     } catch (e) {
       tcpConnection?.destroy();