Add support for certificates in PKCS#12 (P12) key stores #53810

jportner · 2019-12-26T20:38:37Z

Summary

This PR allows Kibana to read certificates and keys from P12 key stores (and trust stores). It adds the following configuration options:

elasticsearch.ssl.keystore.path
elasticsearch.ssl.keystore.password
elasticsearch.ssl.truststore.path
elasticsearch.ssl.truststore.password
server.ssl.keystore.path
server.ssl.keystore.password
server.ssl.truststore.path
server.ssl.truststore.password

Details

I implemented this feature by using node-forge to parse each P12 key store and convert the keys/certs to PEM format. This means that all clients that currently rely on these files do not need any changes made -- at the end of the day, they will still use PEM-formatted keys/certs.

I regenerated the existing certificates in Kibana dev utils, adding some extra ones that weren't included before. Each P12 key store was generated using elasticsearch-certutil, and I extracted PEM certificates / keys from those key stores. The PEM and P12 formats can be used interchangeably (e.g., PEM CA and P12 end-entity key store). I chose not to include the CA private key; if anyone experimenting with Kibana chooses to add that CA to their system's trusted root certificates, it would be trivial to MITM TLS connections for that system. If these certificates need to be regenerated in the future, that can be done using the new instructions detailed in packages/kbn-dev-utils/certs/README.md.

In creating this PR, I made a few design decisions:

I created a new "crypto" utility (src/core/utils/crypto) to handle PKCS12 read operations. I'm not opposed to moving this somewhere else, but this seemed to be the most logical place to put this code.
I changed existing configuration code to read keys/certs, instead of relying on consumers to read those files. This allows us to do additional validation, receive more helpful errors for related failures, reduce filesystem I/O a bit, and avoid an unlikely scenario where keys/certs could be switched out during runtime.
I cleaned up and refactored several tests, removed unnecessary fixtures, and changed some tests to rely on certs/keys provided by kbn-dev-utils.
The Kibana HTTP servers and Elasticsearch clients will be able to use certificate authorities from any or all of the following: P12 key store (that includes an instance key/cert along with CA chain), P12 trust store (that only includes the CA chain), and PEM file/s. This provides a lot of flexibility to users so they won't be forced to try to convert any existing keys/certs that they might want to use.

NOTE: It will be much easier to review this PR commit by commit.

Resolves: #17039

"Release note: Kibana now supports the usage of PKCS#12 (P12) key stores and trust stores for certificates and keys."

elasticmachine · 2019-12-26T21:07:43Z

Pinging @elastic/kibana-security (Team:Security)

jportner

Author's notes for reviewers

jportner · 2019-12-26T21:15:32Z

packages/kbn-dev-utils/src/certs.ts

+export const ES_P12_PATH = resolve(__dirname, '../certs/elasticsearch.p12');
+export const ES_P12_PASSWORD = 'storepass';
+export const ES_EMPTYPASSWORD_P12_PATH = resolve(
+  __dirname,
+  '../certs/elasticsearch_emptypassword.p12'
+);
+export const ES_NOPASSWORD_P12_PATH = resolve(__dirname, '../certs/elasticsearch_nopassword.p12');
+export const KBN_KEY_PATH = resolve(__dirname, '../certs/kibana.key');
+export const KBN_CERT_PATH = resolve(__dirname, '../certs/kibana.crt');
+export const KBN_P12_PATH = resolve(__dirname, '../certs/kibana.p12');
+export const KBN_P12_PASSWORD = 'storepass';


I added a P12 key store that contains the Elasticsearch key / cert / CA cert. I also added other certificates and key stores so we have one place where keys / certs reside for testing purposes. Other unit tests and functional tests now reference these.

jportner · 2019-12-26T21:49:23Z

packages/kbn-es/src/cluster.js

-      esArgs.push(`xpack.security.http.ssl.key=${ES_KEY_PATH}`);
-      esArgs.push(`xpack.security.http.ssl.certificate=${ES_CERT_PATH}`);
-      esArgs.push(`xpack.security.http.ssl.certificate_authorities=${CA_CERT_PATH}`);
+      esArgs.push(`xpack.security.http.ssl.keystore.path=${ES_P12_PATH}`);
+      esArgs.push(`xpack.security.http.ssl.keystore.type=PKCS12`);
+      esArgs.push(`xpack.security.http.ssl.keystore.password=${ES_P12_PASSWORD}`);


This change isn't strictly necessary, but it seemed appropriate to prefer the P12 key store since that is the default output of elasticsearch-certutil.

jportner · 2019-12-26T22:02:57Z

src/core/server/elasticsearch/elasticsearch_client_config.test.ts

+      certificateAuthorities: ['content-of-ca-path-1', 'content-of-ca-path-2'],
+      certificate: 'content-of-certificate-path',
+      key: 'content-of-key-path',


ElasticsearchClientConfig no longer reads the contents of these files -- ElasticsearchConfig handles that. So we don't need to mock the readFileSync implementation here and we can assume that the contents of these certs/keys are being passed in to the ElasticsearchClientConfig params.

jportner · 2019-12-26T22:08:59Z

src/core/server/http/http_server.test.ts

-
-jest.mock('fs', () => ({
-  readFileSync: jest.fn(),
-}));
-
+import { readFileSync } from 'fs';


The old behavior of this unit test resulted in the creation of an HttpServer with SSL enabled, but an undefined key and certificate. Technically this worked without errors, but it seemed that this wasn't the original intent of this test. So, I changed this test to use an actual key/cert instead.

jportner · 2019-12-26T22:18:23Z

x-pack/test/pki_api_integration/fixtures/es_ca.key

-9WQ73QKBgQDAYZzplO4TPoPK9AnxoW/HpSwGEO7Fb8fLyPg94CvHn4QBCFJUKuTn
-hBp/TJgF6CjQWQMr2FKVFF33Ow7+Qa96YGvmYlEjR/71D4Rlprj5JJpuO154DI3I
-YIMNTjvwEQEI+YamMarKsz0Kq+I1EYSAf6bQ4H2PgxDxwTXaLkl0RA==
-----END RSA PRIVATE KEY-----


This key was a copy of the old CA key used in kbn-dev-utils. I regenerated the P12 key stores in this fixtures directory, using the new CA contained in kbn-dev-utils, but I decided not to include the new CA key in the repository. This key is outdated and not needed.

jportner · 2019-12-27T17:45:19Z

packages/kbn-dev-utils/certs/ca.crt

+Bag Attributes
+    friendlyName: elasticsearch
+    localKeyID: 54 69 6D 65 20 31 35 37 37 34 36 36 31 39 38 30 33 37 
+Key Attributes: <No Attributes>
+Bag Attributes
+    friendlyName: ca
+    2.16.840.1.113894.746875.1.1: <Unsupported tag 6>
+subject=/CN=Elastic Certificate Tool Autogenerated CA
+issuer=/CN=Elastic Certificate Tool Autogenerated CA
 -----BEGIN CERTIFICATE-----


These bag attributes are included because the certificate was generated as part of a P12 key store, then exported from that key store using OpenSSL. Any code that reads a PEM-formatted certificate or key will ignore this.

jportner · 2019-12-30T16:54:11Z

x-pack/test/pki_api_integration/config.ts

-        `--server.ssl.key=${ES_KEY_PATH}`,
-        `--server.ssl.certificate=${ES_CERT_PATH}`,
+        `--server.ssl.key=${KBN_KEY_PATH}`,
+        `--server.ssl.certificate=${KBN_CERT_PATH}`,


@kbn/dev-utils now provides a separate certificate/key for Kibana, switched to those to avoid confusion.

kibanamachine · 2019-12-30T19:58:50Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: b3ec790f5603c2d5de5d33f905b9349a7e3ca3a5

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Added Kibana certificate/keypair, and also added P12 formats for both Kibana and Elasticsearch. Removed obsolete dev certs/keys, and changed files and tests where appropriate.

Not positive that this code should remain here, open to suggestions.

The Kibana Platform supports configuration deprecations now, so we can remove the custom deprecation logging that we had previously.

Logger was previously optional to avoid changing tests. Now it is mandatory and the tests have been changed to mock the logger.

This test started failing until I accepted these changes.

These never should have been grouped together. Split them out to improve maintainability and reduce clutter.

This includes: * HTTPS server * HTTPS redirect server * BasePath proxy server

docs/settings/security-settings.asciidoc

packages/kbn-dev-utils/certs/README.md

src/cli/serve/serve.js

src/core/server/elasticsearch/elasticsearch_config.ts

src/core/utils/crypto/pkcs12.test.ts

src/core/server/http/ssl_config.test.ts

docs/setup/settings.asciidoc

Also changed readPkcs12Keystore to determine which end-entity key and cert to return based on if they have a matching public key. The old behavior would simply return the first key and cert as the EE key and cert.

pgayvallet

LGTM for platform changes

src/core/server/elasticsearch/elasticsearch_config.ts

jportner · 2020-01-06T21:39:23Z

@kobelb ready for re-review!

kobelb

This is looking and working great! As discussed on Slack, the only configuration that we'll want to manually verify works properly is a CA that uses an intermediate certificate

src/core/utils/crypto/pkcs12.ts

src/core/utils/crypto/pkcs12.test.ts

Also simplified README regarding cert generation

kobelb

LGTM on green CI

kibanamachine · 2020-01-09T17:40:08Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 835a3fd

History

💔 Build #19028 failed e306b72
💚 Build #18218 succeeded 7857a1b
💚 Build #18144 succeeded b05c004
💔 Build #18127 failed 6eefb34
💔 Build #18092 failed 0ff4b1d

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Kibana now supports the usage of PKCS#12 (P12) key stores and trust stores for certificates and keys.

…4383) Kibana now supports the usage of PKCS#12 (P12) key stores and trust stores for certificates and keys.

The Elasticsearch config should error out if a PKCS12 keystore does not contain a key *or* a certificate. This was intended to be the functionality in PR elastic#53810, but it was overlooked. Changing it now since this PR is changing code in the same file.

jportner added v7.6.0 v8.0.0 release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Dec 26, 2019

jportner force-pushed the issue-17039-support-p12-certificates branch 2 times, most recently from 160111e to 4dc7117 Compare December 26, 2019 22:01

jportner commented Dec 26, 2019

View reviewed changes

elastic deleted a comment from kibanamachine Dec 26, 2019

jportner force-pushed the issue-17039-support-p12-certificates branch from 4dc7117 to 0ff505d Compare December 27, 2019 17:42

jportner commented Dec 27, 2019

View reviewed changes

jportner force-pushed the issue-17039-support-p12-certificates branch 3 times, most recently from e8ec4f9 to 0593259 Compare December 30, 2019 16:45

jportner commented Dec 30, 2019

View reviewed changes

jportner force-pushed the issue-17039-support-p12-certificates branch from 0593259 to b3ec790 Compare December 30, 2019 18:41

jportner added 12 commits December 30, 2019 15:47

Generate new certificates and keys for kbn-dev-utils

7cf86da

Added Kibana certificate/keypair, and also added P12 formats for both Kibana and Elasticsearch. Removed obsolete dev certs/keys, and changed files and tests where appropriate.

Change certificate/keypair that are used for kbn-es

3224ee1

Add crypto utils and node-forge module

5a3837a

Not positive that this code should remain here, open to suggestions.

Migrated elasticsearch username deprecation

e4a77dd

The Kibana Platform supports configuration deprecations now, so we can remove the custom deprecation logging that we had previously.

Require logger for ElasticsearchConfig

b7ea8a6

Logger was previously optional to avoid changing tests. Now it is mandatory and the tests have been changed to mock the logger.

Change Elasticsearch clients to read certs/keys upon config parse

76d1441

scripts/check_core_api_changes.js --accept

7212b66

This test started failing until I accepted these changes.

Change Elasticsearch clients to support P12 key stores and trust stores

78177d8

Split out ssl_config unit tests from http_config unit tests

73660b6

These never should have been grouped together. Split them out to improve maintainability and reduce clutter.

Change HTTP servers to read certs/keys upon config parse

afdc378

This includes: * HTTPS server * HTTPS redirect server * BasePath proxy server

Change HTTP servers to support P12 key stores and trust stores

eb557e0

Update documentation

c4d5163

jportner force-pushed the issue-17039-support-p12-certificates branch from 7afb594 to c4d5163 Compare December 30, 2019 20:50

Changing server integration tests to use explicit CAs

035b718

kobelb reviewed Jan 3, 2020

View reviewed changes

Address feedback from code review

0ff4b1d

Also changed readPkcs12Keystore to determine which end-entity key and cert to return based on if they have a matching public key. The old behavior would simply return the first key and cert as the EE key and cert.

pgayvallet approved these changes Jan 6, 2020

View reviewed changes

src/core/server/elasticsearch/elasticsearch_config.ts Outdated Show resolved Hide resolved

jportner added 4 commits January 6, 2020 11:39

Merge branch 'master' into issue-17039-support-p12-certificates

6eefb34

Rebuild @kbn/pm to fix CI errors

b05c004

Extract utility method in elasticsearch_config.ts

015bf17

Add extra PKCS12 tests and fix outdated jsdoc

7857a1b

kobelb reviewed Jan 8, 2020

View reviewed changes

src/core/utils/crypto/pkcs12.ts Outdated Show resolved Hide resolved

src/core/utils/crypto/pkcs12.ts Outdated Show resolved Hide resolved

src/core/utils/crypto/pkcs12.ts Outdated Show resolved Hide resolved

src/core/utils/crypto/pkcs12.test.ts Show resolved Hide resolved

jportner added 4 commits January 8, 2020 17:59

Merge branch 'master' into issue-17039-support-p12-certificates

91178f6

Address final PR review feedback

3330bdc

Also simplified README regarding cert generation

Integration test for Kibana with intermediate CA

e306b72

Fix CI (renovate)

835a3fd

kobelb approved these changes Jan 9, 2020

View reviewed changes

jportner merged commit 719ff25 into elastic:master Jan 9, 2020

jportner mentioned this pull request Jan 9, 2020

[7.x] Add support for certificates in PKCS#12 (P12) key stores (#53810) #54383

Merged

jportner added a commit to jportner/kibana that referenced this pull request Jan 9, 2020

Add support for certificates in PKCS#12 (P12) key stores (elastic#53810)

15f573a

Kibana now supports the usage of PKCS#12 (P12) key stores and trust stores for certificates and keys.

jportner mentioned this pull request Jan 9, 2020

Deprecate using elasticsearch.ssl.certificate without elasticsearch.ssl.key and vice versa #54392

Merged

jportner added a commit that referenced this pull request Jan 9, 2020

Add support for certificates in PKCS#12 (P12) key stores (#53810) (#5…

74ebdf9

…4383) Kibana now supports the usage of PKCS#12 (P12) key stores and trust stores for certificates and keys.

jportner added the backported label Jan 9, 2020

jportner mentioned this pull request Jan 9, 2020

Enhance integration tests for SSL/TLS scenarios #54396

Open

This was referenced Jan 16, 2020

Stack Monitoring can't use SSL certificates/keys #55073

Closed

[DOCS] Adds instructions to extract files from a .p12 keystore #54981

Merged

Clarify TLS encryption setup #55112

Closed

Update Monitoring plugin's Elasticsearch configuration #55119

Merged

jinmu03 added the release_note:roadmap label Feb 9, 2020

jportner mentioned this pull request Jun 1, 2021

Respond to TLS certificate/key changes without requiring a restart #101072

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for certificates in PKCS#12 (P12) key stores #53810

Add support for certificates in PKCS#12 (P12) key stores #53810

jportner commented Dec 26, 2019 •

edited

Loading

elasticmachine commented Dec 26, 2019

jportner left a comment

jportner Dec 26, 2019

jportner Dec 26, 2019

jportner Dec 26, 2019 •

edited

Loading

jportner Dec 26, 2019 •

edited

Loading

jportner Dec 26, 2019 •

edited

Loading

jportner Dec 27, 2019

jportner Dec 30, 2019

kibanamachine commented Dec 30, 2019

pgayvallet left a comment

jportner commented Jan 6, 2020

kobelb left a comment

kobelb left a comment

kibanamachine commented Jan 9, 2020

Add support for certificates in PKCS#12 (P12) key stores #53810

Add support for certificates in PKCS#12 (P12) key stores #53810

Conversation

jportner commented Dec 26, 2019 • edited Loading

Summary

Details

elasticmachine commented Dec 26, 2019

jportner left a comment

Choose a reason for hiding this comment

jportner Dec 26, 2019

Choose a reason for hiding this comment

jportner Dec 26, 2019

Choose a reason for hiding this comment

jportner Dec 26, 2019 • edited Loading

Choose a reason for hiding this comment

jportner Dec 26, 2019 • edited Loading

Choose a reason for hiding this comment

jportner Dec 26, 2019 • edited Loading

Choose a reason for hiding this comment

jportner Dec 27, 2019

Choose a reason for hiding this comment

jportner Dec 30, 2019

Choose a reason for hiding this comment

kibanamachine commented Dec 30, 2019

💚 Build Succeeded

pgayvallet left a comment

Choose a reason for hiding this comment

jportner commented Jan 6, 2020

kobelb left a comment

Choose a reason for hiding this comment

kobelb left a comment

Choose a reason for hiding this comment

kibanamachine commented Jan 9, 2020

💚 Build Succeeded

History

jportner commented Dec 26, 2019 •

edited

Loading

jportner Dec 26, 2019 •

edited

Loading

jportner Dec 26, 2019 •

edited

Loading

jportner Dec 26, 2019 •

edited

Loading