change to have kibana --ssl cli option use more recent certs #57933

pmuellr · 2020-02-18T22:45:14Z

Summary

Trying to fix the situation when running Kibana via yarn start --ssl, where:

it seems to be impossible to convince Chrome this is a safe server (I think, no CA)
the following RED message is logged in Kibana for every incoming https request

server   error  [15:35:21.596] [error][client][connection] Error: 4394542528:error:14094416:SSL \
  routines:ssl3_read_bytes:sslv3 alert certificate unknown:\
  ../deps/openssl/openssl/ssl/record/rec_layer_s3.c:1544:SSL alert number 46

The cert/key used currently are here:

kibana/src/cli/dev_ssl.js

Lines 20 to 22 in 76d475a

    
           import { resolve } from 'path'; 
        
           export const DEV_SSL_CERT_PATH = resolve(__dirname, '../../test/dev_certs/server.crt'); 
        
           export const DEV_SSL_KEY_PATH = resolve(__dirname, '../../test/dev_certs/server.key');

This PR changes to use these instead: https://github.com/elastic/kibana/tree/master/packages/kbn-dev-utils/certs

The good news is when I run this code via yarn start --ssl, I'm able to get a Chrome session not complaining about invalid certs, and the red error log messages certificate unknown are no longer printed.

Note that the source location where these paths are changed includes the following TODO comment, which I deleted for this PR:

kibana/src/cli/serve/serve.js

Lines 124 to 125 in 76d475a

    
           // TODO: change this cert/key to KBN_CERT_PATH and KBN_KEY_PATH from '@kbn/dev-utils'; will require some work to avoid breaking 
        
           // functional tests. Once that is done, the existing test cert/key at DEV_SSL_CERT_PATH and DEV_SSL_KEY_PATH can be deleted.

As such, I expect CI to fail, but I'm curious how badly it will fail :-)

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

jportner

I tested this locally and it works, LGTM on behalf of the Security team. Thanks!

jportner · 2020-02-19T05:39:04Z

src/cli/serve/serve.js

-      // functional tests. Once that is done, the existing test cert/key at DEV_SSL_CERT_PATH and DEV_SSL_KEY_PATH can be deleted.
-      set('server.ssl.certificate', DEV_SSL_CERT_PATH);
-      set('server.ssl.key', DEV_SSL_KEY_PATH);
+      set('server.ssl.certificate', KBN_CERT_PATH);


Suggested change

set('server.ssl.certificate', KBN_CERT_PATH);

set('server.ssl.certificateAuthorities', CA_CERT_PATH);

set('server.ssl.certificate', KBN_CERT_PATH);

Nit: This isn't required to make things work, but I would set the server.ssl.certificateAuthorities here. This ensures that the Kibana server will send the CA certificate to clients along with the server certificate.

Also you'd need to add another ensureNotDefined above for this setting.

Before and after
Before:
$ keytool -printcert -sslserver localhost:5601 Certificate #0 ==================================== Owner: CN=kibana Issuer: CN=Elastic Certificate Tool Autogenerated CA Serial number: d356920f65ccd88ba8d90c16114a1dc5f26999aa Valid from: Fri Dec 27 12:03:42 EST 2019 until: Sat Dec 14 12:03:42 EST 2069 Certificate fingerprints: SHA1: 10:A4:C7:7E:07:C4:9B:A0:A6:57:FD:70:78:7A:EE:BF:BD:CB:27:2C SHA256: 66:7B:31:92:7D:4F:68:C0:40:08:24:E3:9B:2B:7F:CD:D5:0F:8B:E6:13:0D:E5:FF:62:2B:B9:24:DF:65:46:1C Signature algorithm name: SHA256withRSA Subject Public Key Algorithm: 2048-bit RSA key Version: 3 Extensions: #1: ObjectId: 2.5.29.35 Criticality=false AuthorityKeyIdentifier [ KeyIdentifier [ 0000: 1A ED 26 0E 7B C3 46 70 5D 34 11 BC 0F 1C 0F 75 ..&...Fp]4.....u 0010: 60 2B 07 49 `+.I ] ] #2: ObjectId: 2.5.29.19 Criticality=false BasicConstraints:[ CA:false PathLen: undefined ] #3: ObjectId: 2.5.29.17 Criticality=false SubjectAlternativeName [ DNSName: localhost ] #4: ObjectId: 2.5.29.14 Criticality=false SubjectKeyIdentifier [ KeyIdentifier [ 0000: 0B 07 51 28 DB 73 37 4A CF 0C B0 56 B3 57 E4 F0 ..Q(.s7J...V.W.. 0010: 71 BD CF 98 q... ] ]

After:

$ keytool -printcert -sslserver localhost:5601 Certificate #0 ==================================== Owner: CN=kibana Issuer: CN=Elastic Certificate Tool Autogenerated CA Serial number: d356920f65ccd88ba8d90c16114a1dc5f26999aa Valid from: Fri Dec 27 12:03:42 EST 2019 until: Sat Dec 14 12:03:42 EST 2069 Certificate fingerprints: SHA1: 10:A4:C7:7E:07:C4:9B:A0:A6:57:FD:70:78:7A:EE:BF:BD:CB:27:2C SHA256: 66:7B:31:92:7D:4F:68:C0:40:08:24:E3:9B:2B:7F:CD:D5:0F:8B:E6:13:0D:E5:FF:62:2B:B9:24:DF:65:46:1C Signature algorithm name: SHA256withRSA Subject Public Key Algorithm: 2048-bit RSA key Version: 3 Extensions: #1: ObjectId: 2.5.29.35 Criticality=false AuthorityKeyIdentifier [ KeyIdentifier [ 0000: 1A ED 26 0E 7B C3 46 70 5D 34 11 BC 0F 1C 0F 75 ..&...Fp]4.....u 0010: 60 2B 07 49 `+.I ] ] #2: ObjectId: 2.5.29.19 Criticality=false BasicConstraints:[ CA:false PathLen: undefined ] #3: ObjectId: 2.5.29.17 Criticality=false SubjectAlternativeName [ DNSName: localhost ] #4: ObjectId: 2.5.29.14 Criticality=false SubjectKeyIdentifier [ KeyIdentifier [ 0000: 0B 07 51 28 DB 73 37 4A CF 0C B0 56 B3 57 E4 F0 ..Q(.s7J...V.W.. 0010: 71 BD CF 98 q... ] ] Certificate #1 ==================================== Owner: CN=Elastic Certificate Tool Autogenerated CA Issuer: CN=Elastic Certificate Tool Autogenerated CA Serial number: 5b46eb844b582b7b540589579d46be00c9805fa9 Valid from: Fri Dec 27 12:02:32 EST 2019 until: Sat Dec 14 12:02:32 EST 2069 Certificate fingerprints: SHA1: 86:5D:45:49:A1:18:5A:33:66:CD:01:40:A3:4D:2C:45:C1:60:7C:7B SHA256: AE:E4:15:52:63:D1:8B:E3:69:DB:BD:48:08:F0:E5:9D:55:2D:3E:F1:56:69:12:80:71:74:78:BF:E1:7D:BE:55 Signature algorithm name: SHA256withRSA Subject Public Key Algorithm: 2048-bit RSA key Version: 3 Extensions: #1: ObjectId: 2.5.29.35 Criticality=false AuthorityKeyIdentifier [ KeyIdentifier [ 0000: 1A ED 26 0E 7B C3 46 70 5D 34 11 BC 0F 1C 0F 75 ..&...Fp]4.....u 0010: 60 2B 07 49 `+.I ] ] #2: ObjectId: 2.5.29.19 Criticality=true BasicConstraints:[ CA:true PathLen:2147483647 ] #3: ObjectId: 2.5.29.14 Criticality=false SubjectKeyIdentifier [ KeyIdentifier [ 0000: 1A ED 26 0E 7B C3 46 70 5D 34 11 BC 0F 1C 0F 75 ..&...Fp]4.....u 0010: 60 2B 07 49 `+.I ] ]

validated that my "before" looks like yours, before making the change ...

and validated that my "after" also looks like yours

pmuellr · 2020-02-19T21:58:34Z

Seems like the following files are no longer needed:

test/dev_certs/server.crt
test/dev_certs/server.key
src/cli/dev_ssl.js

Doing some searching on the file names, and exports in dev_ssl.js - DEV_SSL_CERT_PATH and DEV_SSL_KEY_PATH - yields nothing. Since the only two files in test/dev_certs are ones no longer used, the test/dev_certs directory should also no longer be needed.

I'll try deleting all these in a separate commit, see how CI reacts.

pmuellr · 2020-02-19T22:05:51Z

I was pretty conservative in the summary regarding the "check list", I don't anything that is there now is relevant.

I don't think there are breaking API changes, unless the new "disallowing server.ssl.certificateAuthorities if --ssl is used" constraint would be considered breaking API.

The only thing I'm kinda wondering about is whether we have docs-for-devs that might point them at the old creds (eg, "you can add these to your Keychain" kinda thing).

pmuellr · 2020-02-19T22:07:05Z

Are there some appropriate feature / team labels I should use for this PR?

elasticmachine · 2020-02-20T17:07:06Z

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

jportner · 2020-02-20T17:07:17Z

Are there some appropriate feature / team labels I should use for this PR?

Added labels

pmuellr · 2020-02-26T16:16:17Z

@jportner Are there any other approvals needed, or other concerns to be addressed, or is this good to merge to master?

pmuellr · 2020-02-26T16:16:29Z

@elasticmachine merge upstream

pmuellr · 2020-02-28T17:25:02Z

@elasticmachine merge upstream

kibanamachine · 2020-02-28T19:17:39Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 7f3ac60

History

💚 Build #29173 succeeded fbcfd0f
💚 Build #27748 succeeded c10fb02
💚 Build #27513 succeeded dd0e68d

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

…#57933) * change to have --ssl cli option use more recent certs * also configure 'server.ssl.certificateAuthorities' per PR review * delete theoretically now-unused ssl creds

…#58949) * change to have --ssl cli option use more recent certs * also configure 'server.ssl.certificateAuthorities' per PR review * delete theoretically now-unused ssl creds

…dex-server-side * 'master' of github.com:elastic/kibana: (34 commits) [Upgrade Assistant] Remove "boom" from reindex service (elastic#58715) [data] Clean up QueryStringInput unit tests (elastic#58704) [SIEM] Detection Fix typo in Adobe Hijack Persistence rule (elastic#58804) Restores [SIEM][CASE] Init Configure Case Page (elastic#58121) (elastic#58924) Skips additional failing Ingest Manager integration tests Skips failing Ingest Manager integration tests Move dev tools styles to NP (elastic#58855) change to have kibana --ssl cli option use more recent certs (elastic#57933) disable failing suite (elastic#58942) Don't start pollEsNodesVersion unless someone subscribes (elastic#56923) Do not write UUID file during optimize process (elastic#58899) [Endpoint] Task/add nav bar (elastic#58604) [Metric Alerts] Add backend support for multiple expressions per alert (elastic#58672) [Metrics Alerts] Fix alerting on a rate aggregation (elastic#58789) disable flaky suite (elastic#55953) Revert "[SIEM] apollo@3 (elastic#51926)" and "[SIEM][CASE] Init Confi… (elastic#58806) [resubmit] Prep agg types for new platform (elastic#58893) [Lens] Allow number formatting within Lens (elastic#56253) [Autocomplete] Use settings from config rather than UI settings (elastic#58784) Improve action and trigger types (elastic#58657) ... # Conflicts: # x-pack/plugins/upgrade_assistant/server/routes/reindex_indices/reindex_indices.ts

change to have --ssl cli option use more recent certs

dd0e68d

pmuellr added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Feb 18, 2020

jportner approved these changes Feb 19, 2020

View reviewed changes

also configure 'server.ssl.certificateAuthorities' per PR review

fa7b00c

delete theoretically now-unused ssl creds

c10fb02

pmuellr marked this pull request as ready for review February 20, 2020 01:51

jportner added chore Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Feb 20, 2020

Merge branch 'master' into kibana-dev/ssl-certs

fbcfd0f

Merge branch 'master' into kibana-dev/ssl-certs

7f3ac60

pmuellr merged commit 100c570 into elastic:master Feb 28, 2020

pmuellr mentioned this pull request Feb 28, 2020

[7.x] change to have kibana --ssl cli option use more recent certs (#57933) #58949

Merged

pmuellr added the backported label Mar 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change to have kibana --ssl cli option use more recent certs #57933

change to have kibana --ssl cli option use more recent certs #57933

pmuellr commented Feb 18, 2020 •

edited

Loading

jportner left a comment

jportner Feb 19, 2020

pmuellr Feb 19, 2020

pmuellr Feb 19, 2020

pmuellr commented Feb 19, 2020

pmuellr commented Feb 19, 2020

pmuellr commented Feb 19, 2020

elasticmachine commented Feb 20, 2020

jportner commented Feb 20, 2020

pmuellr commented Feb 26, 2020

pmuellr commented Feb 26, 2020

pmuellr commented Feb 28, 2020

kibanamachine commented Feb 28, 2020

	import { resolve } from 'path';
	export const DEV_SSL_CERT_PATH = resolve(__dirname, '../../test/dev_certs/server.crt');
	export const DEV_SSL_KEY_PATH = resolve(__dirname, '../../test/dev_certs/server.key');

	// TODO: change this cert/key to KBN_CERT_PATH and KBN_KEY_PATH from '@kbn/dev-utils'; will require some work to avoid breaking
	// functional tests. Once that is done, the existing test cert/key at DEV_SSL_CERT_PATH and DEV_SSL_KEY_PATH can be deleted.

	set('server.ssl.certificate', KBN_CERT_PATH);
	set('server.ssl.certificateAuthorities', CA_CERT_PATH);
	set('server.ssl.certificate', KBN_CERT_PATH);

change to have kibana --ssl cli option use more recent certs #57933

change to have kibana --ssl cli option use more recent certs #57933

Conversation

pmuellr commented Feb 18, 2020 • edited Loading

Summary

Checklist

For maintainers

jportner left a comment

Choose a reason for hiding this comment

jportner Feb 19, 2020

Choose a reason for hiding this comment

pmuellr Feb 19, 2020

Choose a reason for hiding this comment

pmuellr Feb 19, 2020

Choose a reason for hiding this comment

pmuellr commented Feb 19, 2020

pmuellr commented Feb 19, 2020

pmuellr commented Feb 19, 2020

elasticmachine commented Feb 20, 2020

jportner commented Feb 20, 2020

pmuellr commented Feb 26, 2020

pmuellr commented Feb 26, 2020

pmuellr commented Feb 28, 2020

kibanamachine commented Feb 28, 2020

💚 Build Succeeded

History

pmuellr commented Feb 18, 2020 •

edited

Loading