Fixes incomplete client cert chain when using PKI authentication with the login selector (#88229)

jportner · web-flow · commit a62d6939bd4f · 2021-01-14T09:15:41.000-05:00
diff --git a/docs/development/core/server/kibana-plugin-core-server.ikibanasocket.getprotocol.md b/docs/development/core/server/kibana-plugin-core-server.ikibanasocket.getprotocol.md
@@ -0,0 +1,17 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [IKibanaSocket](./kibana-plugin-core-server.ikibanasocket.md) &gt; [getProtocol](./kibana-plugin-core-server.ikibanasocket.getprotocol.md)
+
+## IKibanaSocket.getProtocol() method
+
+Returns a string containing the negotiated SSL/TLS protocol version of the current connection. The value 'unknown' will be returned for connected sockets that have not completed the handshaking process. The value null will be returned for server sockets or disconnected client sockets. See https://www.openssl.org/docs/man1.0.2/ssl/SSL\_get\_version.html for more information.
+
+<b>Signature:</b>
+
+```typescript
+getProtocol(): string | null;
+```
+<b>Returns:</b>
+
+`string | null`
+
diff --git a/docs/development/core/server/kibana-plugin-core-server.ikibanasocket.md b/docs/development/core/server/kibana-plugin-core-server.ikibanasocket.md
@@ -26,4 +26,6 @@ export interface IKibanaSocket
 |  [getPeerCertificate(detailed)](./kibana-plugin-core-server.ikibanasocket.getpeercertificate.md) |  |
 |  [getPeerCertificate(detailed)](./kibana-plugin-core-server.ikibanasocket.getpeercertificate_1.md) |  |
 |  [getPeerCertificate(detailed)](./kibana-plugin-core-server.ikibanasocket.getpeercertificate_2.md) | Returns an object representing the peer's certificate. The returned object has some properties corresponding to the field of the certificate. If detailed argument is true the full chain with issuer property will be returned, if false only the top certificate without issuer property. If the peer does not provide a certificate, it returns null. |
+|  [getProtocol()](./kibana-plugin-core-server.ikibanasocket.getprotocol.md) | Returns a string containing the negotiated SSL/TLS protocol version of the current connection. The value 'unknown' will be returned for connected sockets that have not completed the handshaking process. The value null will be returned for server sockets or disconnected client sockets. See https://www.openssl.org/docs/man1.0.2/ssl/SSL\_get\_version.html for more information. |
+|  [renegotiate(options)](./kibana-plugin-core-server.ikibanasocket.renegotiate.md) | Renegotiates a connection to obtain the peer's certificate. This cannot be used when the protocol version is TLSv1.3. |
 
diff --git a/docs/development/core/server/kibana-plugin-core-server.ikibanasocket.renegotiate.md b/docs/development/core/server/kibana-plugin-core-server.ikibanasocket.renegotiate.md
@@ -0,0 +1,29 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [IKibanaSocket](./kibana-plugin-core-server.ikibanasocket.md) &gt; [renegotiate](./kibana-plugin-core-server.ikibanasocket.renegotiate.md)
+
+## IKibanaSocket.renegotiate() method
+
+Renegotiates a connection to obtain the peer's certificate. This cannot be used when the protocol version is TLSv1.3.
+
+<b>Signature:</b>
+
+```typescript
+renegotiate(options: {
+        rejectUnauthorized?: boolean;
+        requestCert?: boolean;
+    }): Promise<void>;
+```
+
+## Parameters
+
+|  Parameter | Type | Description |
+|  --- | --- | --- |
+|  options | <code>{</code><br/><code>        rejectUnauthorized?: boolean;</code><br/><code>        requestCert?: boolean;</code><br/><code>    }</code> | The options may contain the following fields: rejectUnauthorized, requestCert (See tls.createServer() for details). |
+
+<b>Returns:</b>
+
+`Promise<void>`
+
+A Promise that will be resolved if renegotiation succeeded, or will be rejected if renegotiation failed.
+
diff --git a/src/core/server/http/router/socket.test.ts b/src/core/server/http/router/socket.test.ts
@@ -22,10 +22,10 @@ import { KibanaSocket } from './socket';
 
 describe('KibanaSocket', () => {
   describe('getPeerCertificate', () => {
-    it('returns null for net.Socket instance', () => {
+    it('returns `null` for net.Socket instance', () => {
       const socket = new KibanaSocket(new Socket());
 
-      expect(socket.getPeerCertificate()).toBe(null);
+      expect(socket.getPeerCertificate()).toBeNull();
     });
 
     it('delegates a call to tls.Socket instance', () => {
@@ -40,20 +40,82 @@ describe('KibanaSocket', () => {
       expect(result).toBe(cert);
     });
 
-    it('returns null if tls.Socket getPeerCertificate returns null', () => {
+    it('returns `null` if tls.Socket getPeerCertificate returns null', () => {
       const tlsSocket = new TLSSocket(new Socket());
       jest.spyOn(tlsSocket, 'getPeerCertificate').mockImplementation(() => null as any);
       const socket = new KibanaSocket(tlsSocket);
 
-      expect(socket.getPeerCertificate()).toBe(null);
+      expect(socket.getPeerCertificate()).toBeNull();
     });
 
-    it('returns null if tls.Socket getPeerCertificate returns empty object', () => {
+    it('returns `null` if tls.Socket getPeerCertificate returns empty object', () => {
       const tlsSocket = new TLSSocket(new Socket());
       jest.spyOn(tlsSocket, 'getPeerCertificate').mockImplementation(() => ({} as any));
       const socket = new KibanaSocket(tlsSocket);
 
-      expect(socket.getPeerCertificate()).toBe(null);
+      expect(socket.getPeerCertificate()).toBeNull();
+    });
+  });
+
+  describe('getProtocol', () => {
+    it('returns `null` for net.Socket instance', () => {
+      const socket = new KibanaSocket(new Socket());
+
+      expect(socket.getProtocol()).toBeNull();
+    });
+
+    it('delegates a call to tls.Socket instance', () => {
+      const tlsSocket = new TLSSocket(new Socket());
+      const protocol = 'TLSv1.2';
+      const spy = jest.spyOn(tlsSocket, 'getProtocol').mockImplementation(() => protocol);
+      const socket = new KibanaSocket(tlsSocket);
+      const result = socket.getProtocol();
+
+      expect(spy).toBeCalledTimes(1);
+      expect(result).toBe(protocol);
+    });
+
+    it('returns `null` if tls.Socket getProtocol returns null', () => {
+      const tlsSocket = new TLSSocket(new Socket());
+      jest.spyOn(tlsSocket, 'getProtocol').mockImplementation(() => null as any);
+      const socket = new KibanaSocket(tlsSocket);
+
+      expect(socket.getProtocol()).toBeNull();
+    });
+  });
+
+  describe('renegotiate', () => {
+    it('throws error for net.Socket instance', async () => {
+      const socket = new KibanaSocket(new Socket());
+
+      expect(() => socket.renegotiate({})).rejects.toThrowErrorMatchingInlineSnapshot(
+        `"Cannot renegotiate a connection when TLS is not enabled."`
+      );
+    });
+
+    it('delegates a call to tls.Socket instance', async () => {
+      const tlsSocket = new TLSSocket(new Socket());
+      const result = Symbol();
+      const spy = jest.spyOn(tlsSocket, 'renegotiate').mockImplementation((_, callback) => {
+        callback(result as any);
+        return undefined;
+      });
+      const socket = new KibanaSocket(tlsSocket);
+
+      expect(socket.renegotiate({})).resolves.toBe(result);
+      expect(spy).toBeCalledTimes(1);
+    });
+
+    it('throws error if tls.Socket renegotiate returns error', async () => {
+      const tlsSocket = new TLSSocket(new Socket());
+      const error = new Error('Oh no!');
+      jest.spyOn(tlsSocket, 'renegotiate').mockImplementation((_, callback) => {
+        callback(error);
+        return undefined;
+      });
+      const socket = new KibanaSocket(tlsSocket);
+
+      expect(() => socket.renegotiate({})).rejects.toThrow(error);
     });
   });
 
@@ -68,12 +130,11 @@ describe('KibanaSocket', () => {
       const tlsSocket = new TLSSocket(new Socket());
 
       tlsSocket.authorized = true;
-      let socket = new KibanaSocket(tlsSocket);
+      const socket = new KibanaSocket(tlsSocket);
       expect(tlsSocket.authorized).toBe(true);
       expect(socket.authorized).toBe(true);
 
       tlsSocket.authorized = false;
-      socket = new KibanaSocket(tlsSocket);
       expect(tlsSocket.authorized).toBe(false);
       expect(socket.authorized).toBe(false);
     });
@@ -90,13 +151,12 @@ describe('KibanaSocket', () => {
       const tlsSocket = new TLSSocket(new Socket());
       tlsSocket.authorizationError = undefined as any;
 
-      let socket = new KibanaSocket(tlsSocket);
+      const socket = new KibanaSocket(tlsSocket);
       expect(tlsSocket.authorizationError).toBeUndefined();
       expect(socket.authorizationError).toBeUndefined();
 
       const authorizationError = new Error('some error');
       tlsSocket.authorizationError = authorizationError;
-      socket = new KibanaSocket(tlsSocket);
 
       expect(tlsSocket.authorizationError).toBe(authorizationError);
       expect(socket.authorizationError).toBe(authorizationError);
diff --git a/src/core/server/http/router/socket.ts b/src/core/server/http/router/socket.ts
@@ -19,6 +19,7 @@
 
 import { Socket } from 'net';
 import { DetailedPeerCertificate, PeerCertificate, TLSSocket } from 'tls';
+import { promisify } from 'util';
 
 /**
  * A tiny abstraction for TCP socket.
@@ -38,6 +39,20 @@ export interface IKibanaSocket {
    */
   getPeerCertificate(detailed?: boolean): PeerCertificate | DetailedPeerCertificate | null;
 
+  /**
+   * Returns a string containing the negotiated SSL/TLS protocol version of the current connection. The value 'unknown' will be returned for
+   * connected sockets that have not completed the handshaking process. The value null will be returned for server sockets or disconnected
+   * client sockets. See https://www.openssl.org/docs/man1.0.2/ssl/SSL_get_version.html for more information.
+   */
+  getProtocol(): string | null;
+
+  /**
+   * Renegotiates a connection to obtain the peer's certificate. This cannot be used when the protocol version is TLSv1.3.
+   * @param options - The options may contain the following fields: rejectUnauthorized, requestCert (See tls.createServer() for details).
+   * @returns A Promise that will be resolved if renegotiation succeeded, or will be rejected if renegotiation failed.
+   */
+  renegotiate(options: { rejectUnauthorized?: boolean; requestCert?: boolean }): Promise<void>;
+
   /**
    * Indicates whether or not the peer certificate was signed by one of the specified CAs. When TLS
    * isn't used the value is `undefined`.
@@ -52,15 +67,14 @@ export interface IKibanaSocket {
 }
 
 export class KibanaSocket implements IKibanaSocket {
-  readonly authorized?: boolean;
-  readonly authorizationError?: Error;
-
-  constructor(private readonly socket: Socket) {
-    if (this.socket instanceof TLSSocket) {
-      this.authorized = this.socket.authorized;
-      this.authorizationError = this.socket.authorizationError;
-    }
+  public get authorized() {
+    return this.socket instanceof TLSSocket ? this.socket.authorized : undefined;
   }
+  public get authorizationError() {
+    return this.socket instanceof TLSSocket ? this.socket.authorizationError : undefined;
+  }
+
+  constructor(private readonly socket: Socket) {}
 
   getPeerCertificate(detailed: true): DetailedPeerCertificate | null;
   getPeerCertificate(detailed: false): PeerCertificate | null;
@@ -76,4 +90,18 @@ export class KibanaSocket implements IKibanaSocket {
     }
     return null;
   }
+
+  public getProtocol() {
+    if (this.socket instanceof TLSSocket) {
+      return this.socket.getProtocol();
+    }
+    return null;
+  }
+
+  public async renegotiate(options: { rejectUnauthorized?: boolean; requestCert?: boolean }) {
+    if (this.socket instanceof TLSSocket) {
+      return promisify(this.socket.renegotiate.bind(this.socket))(options);
+    }
+    return Promise.reject(new Error('Cannot renegotiate a connection when TLS is not enabled.'));
+  }
 }
diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md
@@ -1101,6 +1101,11 @@ export interface IKibanaSocket {
     // (undocumented)
     getPeerCertificate(detailed: false): PeerCertificate | null;
     getPeerCertificate(detailed?: boolean): PeerCertificate | DetailedPeerCertificate | null;
+    getProtocol(): string | null;
+    renegotiate(options: {
+        rejectUnauthorized?: boolean;
+        requestCert?: boolean;
+    }): Promise<void>;
 }
 
 // @public @deprecated
diff --git a/x-pack/plugins/security/server/authentication/providers/pki.test.ts b/x-pack/plugins/security/server/authentication/providers/pki.test.ts
diff --git a/x-pack/plugins/security/server/authentication/providers/pki.ts b/x-pack/plugins/security/server/authentication/providers/pki.ts
