Fixes 500 error when using PKI authentication with an incomplete certificate chain

x-pack/plugins/security/server/authentication/providers/pki.test.ts

@@ -42,6 +42,13 @@ function getMockPeerCertificate(chain: string[] | string) {
       // Imitate self-signed certificate that is issuer for itself.
       certificate.issuerCertificate = index === fingerprintChain.length - 1 ? certificate : {};
 
+      // Imitate other fields for logging assertions
+      certificate.subject = 'mock subject';
+      certificate.issuer = 'mock issuer';
+      certificate.subjectaltname = 'mock subjectaltname';
+      certificate.valid_from = 'mock valid_from';
+      certificate.valid_to = 'mock valid_to';
+
       return certificate.issuerCertificate;
     },
     mockPeerCertificate as Record<string, any>
@@ -59,6 +66,9 @@ function getMockSocket({
 } = {}) {
   const socket = new TLSSocket(new Socket());
   socket.authorized = authorized;
+  if (!authorized) {
+    socket.authorizationError = new Error('mock authorization error');
+  }
   socket.getPeerCertificate = jest.fn().mockReturnValue(peerCertificate);
   return socket;
 }
@@ -88,26 +98,58 @@ describe('PKIAuthenticationProvider', () => {
   function defineCommonLoginAndAuthenticateTests(
     operation: (request: KibanaRequest) => Promise<AuthenticationResult>
   ) {
-    it('does not handle requests without certificate.', async () => {
+    it('does not handle unauthorized requests.', async () => {
       const request = httpServerMock.createKibanaRequest({
-        socket: getMockSocket({ authorized: true }),
+        socket: getMockSocket({
+          authorized: false,
+          peerCertificate: getMockPeerCertificate('2A:7A:C2:DD'),
+        }),
       });
 
       await expect(operation(request)).resolves.toEqual(AuthenticationResult.notHandled());
 
       expect(mockOptions.client.asScoped).not.toHaveBeenCalled();
       expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
+      expect(mockOptions.logger.debug).toHaveBeenCalledWith(
+        'Peer certificate chain: [{"subject":"mock subject","issuer":"mock issuer","issuerCertType":"object","subjectaltname":"mock subjectaltname","validFrom":"mock valid_from","validTo":"mock valid_to"}]'
+      );
+      expect(mockOptions.logger.debug).toHaveBeenCalledWith(
+        'Authentication is not possible since peer certificate was not authorized: Error: mock authorization error.'
+      );
     });
 
-    it('does not handle unauthorized requests.', async () => {
+    it('does not handle requests with a missing certificate chain.', async () => {
       const request = httpServerMock.createKibanaRequest({
-        socket: getMockSocket({ peerCertificate: getMockPeerCertificate('2A:7A:C2:DD') }),
+        socket: getMockSocket({ authorized: true, peerCertificate: null }),
       });
 
       await expect(operation(request)).resolves.toEqual(AuthenticationResult.notHandled());
 
       expect(mockOptions.client.asScoped).not.toHaveBeenCalled();
       expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
+      expect(mockOptions.logger.debug).toHaveBeenCalledWith('Peer certificate chain: []');
+      expect(mockOptions.logger.debug).toHaveBeenCalledWith(
+        'Authentication is not possible due to missing peer certificate chain.'
+      );
+    });
+
+    it('does not handle requests with an incomplete certificate chain.', async () => {
+      const peerCertificate = getMockPeerCertificate('2A:7A:C2:DD');
+      (peerCertificate as any).issuerCertificate = undefined; // This behavior has been observed, even though it's not valid according to the type definition
+      const request = httpServerMock.createKibanaRequest({
+        socket: getMockSocket({ authorized: true, peerCertificate }),
+      });
+
+      await expect(operation(request)).resolves.toEqual(AuthenticationResult.notHandled());
+
+      expect(mockOptions.client.asScoped).not.toHaveBeenCalled();
+      expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
+      expect(mockOptions.logger.debug).toHaveBeenCalledWith(
+        'Peer certificate chain: [{"subject":"mock subject","issuer":"mock issuer","issuerCertType":"undefined","subjectaltname":"mock subjectaltname","validFrom":"mock valid_from","validTo":"mock valid_to"}]'
+      );
+      expect(mockOptions.logger.debug).toHaveBeenCalledWith(
+        'Authentication is not possible due to incomplete peer certificate chain.'
+      );
     });
 
     it('gets an access token in exchange to peer certificate chain and stores it in the state.', async () => {

x-pack/plugins/security/server/authentication/providers/pki.ts

@@ -40,6 +40,39 @@ function canStartNewSession(request: KibanaRequest) {
   return canRedirectRequest(request) && request.route.options.authRequired === true;
 }
 
+/**
+ * Returns a stringified version of a certificate, including metadata
+ * @param peerCertificate DetailedPeerCertificate instance.
+ */
+function stringifyCertificate(peerCertificate: DetailedPeerCertificate) {
+  const {
+    subject,
+    issuer,
+    issuerCertificate,
+    subjectaltname,
+    valid_from: validFrom,
+    valid_to: validTo,
+  } = peerCertificate;
+
+  // The issuerCertificate field can be three different values:
+  //  * Object: In this case, the issuer certificate is an object
+  //  * null: In this case, the issuer certificate is a null value; this should not happen according to the type definition but historically there was code in place to account for this
+  //  * undefined: The issuer certificate chain is broken; this should not happen according to the type definition but we have observed this edge case behavior with certain client/server configurations
+  // This distinction can be useful for troubleshooting mutual TLS connection problems, so we include it in the stringified certificate that is printed to the debug logs.
+  // There are situations where a partial client certificate chain is accepted by Node, but we cannot verify the chain in Kibana because an intermediate issuerCertificate is undefined.
+  // If this happens, Kibana will reject the authentication attempt, and the client and/or server need to ensure that the entire CA chain is installed.
+  let issuerCertType: string;
+  if (issuerCertificate === undefined) {
+    issuerCertType = 'undefined';
+  } else if (issuerCertificate === null) {
+    issuerCertType = 'null';
+  } else {
+    issuerCertType = 'object';
+  }
+
+  return JSON.stringify({ subject, issuer, issuerCertType, subjectaltname, validFrom, validTo });
+}
+
 /**
  * Provider that supports PKI request authentication.
  */
@@ -204,21 +237,27 @@ export class PKIAuthenticationProvider extends BaseAuthenticationProvider {
   private async authenticateViaPeerCertificate(request: KibanaRequest) {
     this.logger.debug('Trying to authenticate request via peer certificate chain.');
 
+    // We should collect entire certificate chain as an ordered array of certificates encoded as base64 strings.
+    const peerCertificate = request.socket.getPeerCertificate(true);
+    const { certificateChain, isChainIncomplete } = this.getCertificateChain(peerCertificate);
+
     if (!request.socket.authorized) {
       this.logger.debug(
         `Authentication is not possible since peer certificate was not authorized: ${request.socket.authorizationError}.`
       );
       return AuthenticationResult.notHandled();
     }
 
-    const peerCertificate = request.socket.getPeerCertificate(true);
     if (peerCertificate === null) {
       this.logger.debug('Authentication is not possible due to missing peer certificate chain.');
       return AuthenticationResult.notHandled();
     }
 
-    // We should collect entire certificate chain as an ordered array of certificates encoded as base64 strings.
-    const certificateChain = this.getCertificateChain(peerCertificate);
+    if (isChainIncomplete) {
+      this.logger.debug('Authentication is not possible due to incomplete peer certificate chain.');
+      return AuthenticationResult.notHandled();
+    }
+
     let result: { access_token: string; authentication: AuthenticationInfo };
     try {
       result = await this.options.client.callAsInternalUser('shield.delegatePKI', {
@@ -255,23 +294,31 @@ export class PKIAuthenticationProvider extends BaseAuthenticationProvider {
    */
   private getCertificateChain(peerCertificate: DetailedPeerCertificate | null) {
     const certificateChain = [];
+    const certificateStrings = [];
+    let isChainIncomplete = false;
     let certificate: DetailedPeerCertificate | null = peerCertificate;
-    while (certificate !== null && Object.keys(certificate).length > 0) {
+
+    while (certificate && Object.keys(certificate).length > 0) {
       certificateChain.push(certificate.raw.toString('base64'));
+      certificateStrings.push(stringifyCertificate(certificate));
 
       // For self-signed certificates, `issuerCertificate` may be a circular reference.
       if (certificate === certificate.issuerCertificate) {
         this.logger.debug('Self-signed certificate is detected in certificate chain');
-        certificate = null;
+        break;
+      } else if (certificate.issuerCertificate === undefined) {
+        // The chain is only considered to be incomplete if one or more issuerCertificate values is undefined;
+        // this is not an expected return value from Node, but it can happen in some edge cases
+        isChainIncomplete = true;
+        break;
       } else {
+        // Repeat the loop
         certificate = certificate.issuerCertificate;
       }
     }
 
-    this.logger.debug(
-      `Peer certificate chain consists of ${certificateChain.length} certificates.`
-    );
+    this.logger.debug(`Peer certificate chain: [${certificateStrings.join(', ')}]`);
 
-    return certificateChain;
+    return { certificateChain, isChainIncomplete };
   }
 }