Modifying SSL settings to be consistent with the stack by kobelb · Pull Request #9823 · elastic/kibana

kobelb · 2017-01-11T12:01:20Z

Per #7777 we want to move towards consistent SSL settings across the stack.

This introduces the following new settings:

server.ssl.enabled
server.ssl.keyPassphrase
server.ssl.certificateAuthorities
server.ssl.clientAuthentication
server.ssl.supportedProtocols
server.ssl.cipherSuites
elasticsearch.ssl.keyPassphrase

and deprecates the following:

server.ssl.cert -> server.ssl.certificate
elasticsearch.ssl.ca -> elasticsearch.ssl.certificateAuthorities
elasticsearch.ssl.cert -> elasticsearch.ssl.certificate
elasticsearch.ssl.verify -> elasticsearch.ssl.verificationMode
console.proxyConfig

jbudz

I tested all of the new configurations and they're functioning as expected. Deprecation warnings look good, including the more complicated ones like console.proxyConfig.

Lots of tests too, great work on this. A few comments below but otherwise this lgtm.

jbudz · 2017-01-11T16:28:52Z

docs/setup/settings.asciidoc

thoughts on adding these to kibana.yml?

Good question! It's my understanding that we only try to put the common settings in the kibana.yml, I feel like keyPassphrase is a contender to add in there, but I dunno how I feel about clientAuthentication and certiricateAuthorities because I don't see a majority of our users using these. What are your thoughts?

makes sense, on second thought it's probably fine. consistent with the configs for our other products too.

jbudz · 2017-01-12T18:47:31Z

src/core_plugins/console/index.js

can we remove this from the settings docs page and console page?

Great call, I'll make it so.

jbudz · 2017-01-12T18:58:58Z

docs/setup/settings.asciidoc

When using this I'm occasionally seeing

log [18:54:38.465] [error][client][connection] Error: TLS handshake timeout at TLSSocket._handleTimeout (_tls_wrap.js:539:22) at TLSSocket.g (events.js:291:16) at emitNone (events.js:86:13) at TLSSocket.emit (events.js:185:7) at TLSSocket.Socket._onTimeout (net.js:339:8) at ontimeout (timers.js:365:14) at tryOnTimeout (timers.js:237:5) at Timer.listOnTimeout (timers.js:207:5)

I'm not able to reproduce it consistently or often. Functionally it doesn't seem to have an impact, is this something we want in the error log?

Lemme see if I can track down where this is coming from, and see if there's a way to at least downgrade it from error like we're doing elsewhere.

I've been able to consistently get this log message to occur by setting server.ssl.clientAuthentication: required and then opening up Kibana and letting the browser sit on the prompt for me to select a certificate for a long-time. However, I'm sure that other situations could also cause this to occur as well.

This error is being emitted by the tls socket to the https Server to the hapi connection which we then intercept, and downgrade certain errors.

We've been downgrading errors based on their errno, but those only exist on System Errors, and these are just generic Errors.

We could potentially filter these out by matching the err.message of TLS handshake timeout, but that feels rather brittle. Additionally, there are various other generic client errors the underlying ssl library is throwing with uglier messages like the following:

4330128320:error:14094418:SSL routines:ssl3_read_bytes:tlsv1 alert unknown ca:../deps/openssl/openssl/ssl/s3_pkt.c:1487:SSL alert number 48 4330128320:error:140940E5:SSL routines:ssl3_read_bytes:ssl handshake failure:../deps/openssl/openssl/ssl/s3_pkt.c:1210:

I'm beginning to wonder whether we should be downgrading all client errors to debug/info. What are your alls thoughts @spalger @tylersmalley @jbudz?

kobelb · 2017-01-12T20:11:22Z

@jbudz caught that this PR does not currently allow the user to configure the list of cipher-suites.

@epixa - in the chart in #7777 there's a note next to cipher-suites saying 'see list below' and 'language specific', do you happen to recall the original discussion surrounding this issue and whether it impacts how we implement this?

@spalger do you happen to recall why we're customizing the current default list of cipher-suites and whether there is any reason why we wouldn't use the NodeJS default and then allow the user to overwrite this?

tylersmalley · 2017-01-12T23:03:18Z

config/kibana.yml

If we are seeking consistent configurations, why do we camel case our names when ES and others are all lowercase with underscore delimited words?

Should use use the array for this to show multiple CA's can be provided?

elasticsearch.ssl.certificateAuthorities: [ "/path/to/your/CA.pem" ]

we do have #7444. i was on the fence for this one, it is consistent with out current config

Regarding camelCasing, I wanted to keep this consistent with the rest of our settings and then address converting all of them to snake_case in a separate PR. Should we consider trying to do both in the same minor version to reduce the impact on the user? I would prefer to at least do it in a separate PR, even if it's for the same minor version. What are your thoughts?

I agree, let's keep it out of this PR and see if it's something we want to also get into 5.2 to reduce the number of times folks are re-doing their config.

tylersmalley · 2017-01-12T23:21:42Z

docs/setup/settings.asciidoc

I am personally really excited about this as I had been previously needing to remove the passphrase from my generated certs.

tylersmalley · 2017-01-13T00:17:03Z

src/core_plugins/elasticsearch/index.js

bindKey can actually be removed as it's not being used.

tylersmalley · 2017-01-13T00:23:29Z

src/deprecation/deprecations/rename.js

isNull is not used

tylersmalley · 2017-01-13T00:23:38Z

src/deprecation/deprecations/unused.js

isNull is not used

tylersmalley · 2017-01-13T00:35:17Z

When testing the deprecation for pid_file, it appears were now handling the deprecation calls later in the startup processing than before.

In this test, I have defined pid_file which has been renamed to pid.file. The file does not exist so I am getting ENOENT.

When testing for the same thing on master, I immediately get the deprecation warning without a file error.

tylersmalley

A couple feedback items. Only one of concern is when the deprecations are ran.

kobelb · 2017-01-13T15:33:15Z

@tylersmalley, after discussing with Spencer, we decided to move the deprecation logging to use the standard logger, which unfortunately delayed when these deprecations will be output because the logging is currently attached to the kibana server.

It's a trade-off that at the time seemed reasonable, but I'm definitely open to alternatives.

tylersmalley · 2017-01-13T19:46:06Z

@kobelb, I have come around to this as it's actually transforming the config and everything uses the servers logger so this is no different. If @spalger agrees, I am fine moving forward.

spalger · 2017-01-16T22:35:12Z

Yeah, there are only a couple settings that will trigger failures before the deprecation warning, and you found one of them, but I think it's better to use the standard logging format than fix the ordering for those edge cases. That said, if we can do the deprecation logging any earlier I think we should (it runs after http right now which seems unnecessary, but idk)

kobelb · 2017-01-20T12:35:33Z

@spalger good call on moving the deprecations to earlier in the kbn_server. The deprecations currently have to run after http and logging because the log function is exposed on the server, but I was able to put it before the warnings and status.

tylersmalley · 2017-01-20T22:10:57Z

LGTM! By moving the deprecations were now consistent with the behavior in master.

kobelb · 2017-01-24T13:36:25Z

@jbudz you mind giving the changes that I made a look over?

Backports PR #9823 **Commit 1:** Modifying SSL settings to be consistent with the stack * Original sha: 6ea4077 * Authored by = <brandon.kobel@elastic.co> on 2016-10-07T10:19:45Z **Commit 2:** Removing deprecated console settings from the docs * Original sha: 9dc79fd * Authored by kobelb <brandon.kobel@elastic.co> on 2017-01-13T16:08:18Z **Commit 3:** Removing unused code * Original sha: 22713b7 * Authored by kobelb <brandon.kobel@elastic.co> on 2017-01-13T16:09:43Z **Commit 4:** Using array for certificateAuthorities in the kibana.yml * Original sha: a93c9fe * Authored by kobelb <brandon.kobel@elastic.co> on 2017-01-13T16:10:01Z **Commit 5:** Moving the kbn_server deprecation warnings to be earlier * Original sha: 9dba22a * Authored by kobelb <brandon.kobel@elastic.co> on 2017-01-20T12:30:51Z **Commit 6:** Allowing the cipher suites to be configured * Original sha: e224268 * Authored by kobelb <brandon.kobel@elastic.co> on 2017-01-20T19:00:40Z

kobelb · 2017-02-01T20:48:45Z

If you need assistance generating certs with passphrases, this will help: https://gist.github.com/mtigas/952344

Certgen doesn't let you specify passphrases, but it's nice for the other test-cases

rashmivkulkarni · 2017-02-01T20:50:59Z

To Test:

Make sure all the deprecation warnings appear on the Kibana console as you make changes in kibana.yml for every setting .
X-Pack: regression testing on Monitoring( in general) and have a separate Monitoring Cluster.
Test all the new settings mentioned in the ticket work as expected.
using the certgen tool should be good for generating certs. ( be sure to use the pass-phrase : follow: https://gist.github.com/mtigas/952344 for certificates)

Tested on 5.3.0 -BC1 builds From dev ticket #725

kobelb · 2017-02-01T21:12:46Z

To test the server.ssl.cipherSuites and server.ssl.supportedProtocols the following openssl commands will come in handy; however, you'll want to ensure that you're using a newer version of SSL than the one that comes packaged with OS X 10.12 because it's outdated, and technically a version that Apple patched.

# connect using SSLv3
./openssl s_client -connect localhost:5601 -ssl3

# connect using TLSv1.0
./openssl s_client -connect localhost:5601  -tls1

# connect using TLSv1.1
./openssl s_client -connect localhost:5601  -tls1_1

# connect using TLSv1.2
./openssl s_client -connect localhost:5601  -tls1_2

# `openssl ciphers` will return all the ciphers the openssl library supports
./openssl s_client -connect localhost:5601 -cipher [INSERT YOUR CIPHER]

ghost · 2017-03-29T22:42:40Z

docs/setup/settings.asciidoc

+ and `none`. `required` forces a client to present a certificate, while `none` does not.
+`server.ssl.supportedProtocols`:: *Default: TLSv1, TLSv1.1, TLSv1.2*  Supported protocols with versions. Valid protocols: `TLSv1`, `TLSv1.1`, `TLSv1.2`
+`server.ssl.cipherSuites`:: *Default: ECDHE-RSA-AES128-GCM-SHA256, ECDHE-ECDSA-AES128-GCM-SHA256, ECDHE-RSA-AES256-GCM-SHA384, ECDHE-ECDSA-AES256-GCM-SHA384, DHE-RSA-AES128-GCM-SHA256, ECDHE-RSA-AES128-SHA256, DHE-RSA-AES128-SHA256, ECDHE-RSA-AES256-SHA384, DHE-RSA-AES256-SHA384, ECDHE-RSA-AES256-SHA256, DHE-RSA-AES256-SHA256, HIGH,!aNULL, !eNULL, !EXPORT, !DES, !RC4, !MD5, !PSK, !SRP, !CAMELLIA*. Details on the format, and the valid options, are available via the [OpenSSL cipher list format documentation](https://www.openssl.org/docs/man1.0.2/apps/ciphers.html#CIPHER-LIST-FORMAT)
 `elasticsearch.ssl.cert:` and `elasticsearch.ssl.key:`:: Optional settings that provide the paths to the PEM-format SSL


Is this a missed rename to elasticsearch.ssl.certificate?

@Jarpy it is, I'll get a fix up there for this, thanks!

kobelb mentioned this pull request Jan 11, 2017

Consistent server.ssl settings #8855

Closed

kobelb added review v6.0.0 v5.3.0 labels Jan 11, 2017

kobelb assigned jbudz and tylersmalley Jan 11, 2017

kobelb force-pushed the ssl-config branch from 7930d6d to 8be9a5f Compare January 11, 2017 12:18

epixa added the Team:Operations Kibana-Operations Team label Jan 11, 2017

tylersmalley self-requested a review January 11, 2017 19:34

jbudz reviewed Jan 12, 2017

View reviewed changes

tylersmalley reviewed Jan 12, 2017

View reviewed changes

tylersmalley reviewed Jan 13, 2017

View reviewed changes

src/core_plugins/elasticsearch/index.js Outdated

Copy link
Copy Markdown

Member

tylersmalley Jan 13, 2017

Choose a reason for hiding this comment

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

bindKey can actually be removed as it's not being used.

tylersmalley reviewed Jan 13, 2017

View reviewed changes

src/deprecation/deprecations/rename.js Outdated

Copy link
Copy Markdown

Member

tylersmalley Jan 13, 2017

Choose a reason for hiding this comment

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

isNull is not used

tylersmalley reviewed Jan 13, 2017

View reviewed changes

src/deprecation/deprecations/unused.js Outdated

Copy link
Copy Markdown

Member

tylersmalley Jan 13, 2017

Choose a reason for hiding this comment

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

isNull is not used

tylersmalley requested changes Jan 13, 2017

View reviewed changes

kobelb added 6 commits January 20, 2017 15:27

Modifying SSL settings to be consistent with the stack

6ea4077

Removing deprecated console settings from the docs

9dc79fd

Removing unused code

22713b7

Using array for certificateAuthorities in the kibana.yml

a93c9fe

Moving the kbn_server deprecation warnings to be earlier

9dba22a

Allowing the cipher suites to be configured

e224268

kobelb force-pushed the ssl-config branch from de17266 to e224268 Compare January 20, 2017 20:29

tylersmalley approved these changes Jan 20, 2017

View reviewed changes

jbudz approved these changes Jan 25, 2017

View reviewed changes

kobelb merged commit 5a42638 into elastic:master Jan 25, 2017

kobelb deleted the ssl-config branch January 25, 2017 15:59

elastic-jasper mentioned this pull request Jan 25, 2017

[5.x] Modifying SSL settings to be consistent with the stack #10062

Merged

cwurm mentioned this pull request Feb 16, 2017

'Dev Tools' feature does not work with SSL transport (not auth) configured on ES #8932

Closed

This was referenced Mar 29, 2017

Some settings changed in 5.3.0 elastic/kibana-docker#19

Closed

[Doc] Need update Docker Environment Variables #10922

Closed

jbudz mentioned this pull request Mar 29, 2017

Downgrade logging for hapi client errors #10929

Closed

ghost reviewed Mar 29, 2017

View reviewed changes

kobelb mentioned this pull request Apr 5, 2017

Fixing docs reference from elasticsearch.ssl.cert to certificate #11037

Merged

ppf2 mentioned this pull request May 17, 2017

Better error handling when obsolete console.proxyConfig settings are used #11836

Closed

jbudz mentioned this pull request Jul 18, 2017

Allow user to configure multiple CA in kibana.yml #5212

Closed

tylersmalley mentioned this pull request Dec 14, 2017

[Docs] Append more explanation to Configuring Kibana #15615

Merged

lcawl mentioned this pull request Feb 4, 2019

[DOCS] Deprecate SSL settings #29990

Merged

3 tasks

This was referenced Jun 19, 2019

Remove coercion of Console GET requests to POST #39170

Closed

Remove support for proxyConfig and proxyFilter settings in Console in 8.0 #39494

Closed

Conversation

kobelb commented Jan 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbudz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kobelb Jan 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kobelb commented Jan 12, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented Jan 13, 2017

Uh oh!

tylersmalley left a comment

Choose a reason for hiding this comment

Uh oh!

kobelb commented Jan 13, 2017

Uh oh!

tylersmalley commented Jan 13, 2017

Uh oh!

spalger commented Jan 16, 2017

Uh oh!

kobelb commented Jan 20, 2017

Uh oh!

tylersmalley commented Jan 20, 2017

Uh oh!

kobelb commented Jan 24, 2017

Uh oh!

kobelb commented Feb 1, 2017

Uh oh!

rashmivkulkarni commented Feb 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kobelb commented Feb 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

kobelb commented Jan 11, 2017 •

edited

Loading

kobelb Jan 20, 2017 •

edited

Loading

rashmivkulkarni commented Feb 1, 2017 •

edited

Loading

kobelb commented Feb 1, 2017 •

edited

Loading