Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Jan 14, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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`

Original file line number Diff line number Diff line change
Expand Up @@ -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. |

Original file line number Diff line number Diff line change
@@ -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.

80 changes: 70 additions & 10 deletions src/core/server/http/router/socket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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);
});
});

Expand All @@ -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);
});
Expand All @@ -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);
Expand Down
44 changes: 36 additions & 8 deletions src/core/server/http/router/socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import { Socket } from 'net';
import { DetailedPeerCertificate, PeerCertificate, TLSSocket } from 'tls';
import { promisify } from 'util';

/**
* A tiny abstraction for TCP socket.
Expand All @@ -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`.
Expand All @@ -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(public readonly socket: Socket) {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we intentionally make the socket public (it was private)?

Copy link
Contributor Author

@jportner jportner Jan 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬 Nope, that was intended to be a temporary change while I was testing how renegotiate worked. Thanks for catching, I'll change that back.

Edit: done in 044a062.


getPeerCertificate(detailed: true): DetailedPeerCertificate | null;
getPeerCertificate(detailed: false): PeerCertificate | null;
Expand All @@ -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.'));
}
}
5 changes: 5 additions & 0 deletions src/core/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading