Elasticsearch: Teach reindex to honor `xpack.ssl.certificate_authorities`

Created on 10 Feb 2017  路  14Comments  路  Source: elastic/elasticsearch

Original comment by @nik9000:

Right now reindex is fine with ssl but doesn't read xpack.ssl.certificate_authorities. It probably should, but I'm not really sure how to make that work because reindex and x-pack don't know about each other. I'm sure we could get it to work by making some common thing inside core that x-pack implements and reindex uses, but I'm not sure it is worth the effort. Maybe? Thus, discuss label.

:SecuritNetwork >enhancement

Most helpful comment

_Note, this issue is specifically about how to make X-Pack based SSL config available in _reindex, not general reindex/SSL questions, but the two questions can't be separated perfectly_

I propose that we do something based around the following

  • Not use xpack.ssl.* settings because:

    1. these tend to interfere with other X-Pack features (like watcher connections to https URLs), which prompted us to change the docs in 42f9a990d17c220766614c7b8888bfd12143ce6c
    2. we're looking to remove those fallback settings anyway (https://github.com/elastic/elasticsearch/issues/29797)
  • Not add overwrite the system trust config (SSLContext.setDefault) because configuring the default system trust incorrectly will also break other features (like watcher) and experience shows that configure SSL well is tricky, and customers often don't completely understand the implication of changes they're making. Making changes to the default SSL context seems trappy. Even if we only supported _adding_ additional CAs, that still has the risk that the JVM would start trusting certs that it shouldn't.

    • As part of a fix I'm working on for #30344, we're planning to introduce _named_ ssl contexts. These allow code that uses the X-Pack SSL Service to ask for a context by name, without needing to have a copy of the actual settings.
  • We now have an SPI concept via #27881. This would allow the reindex module and x-pack module to share code in ways that weren't possible when this issue was raised.

Putting those together, would mean that

  • X-Pack can add some settings to configure an SSL context for "_reindex"
  • We can implement a basic SSL provider concept, either in server (core) or in the reindex module through SPI
  • X-Pack implements this provider interface, to make SSL contexts available to non X-Pack code
  • Reindex uses the discovered service providers to configure an appropriate SSL context.

There's some open questions there like:

  • do you need to tell reindex that it should look for a named SSL context?
  • is it too lenient to: use x-pack if configured, otherwise use the system default? (We typically lean towards explicit over implicit)
  • is SPI the correct approach, or would a standard plugin make more sense?
  • do we want to support multiple SSL providers, or require that there be only one?

but all those questions are relatively minor and could be resolved as part of the implementation.

All 14 comments

Original comment by @jaymode:

I'm sure we could get it to work by making some common thing inside core that x-pack implements and reindex uses

I wonder if we should have the notion of a default keystore/truststore in core for things that need SSL. For core, this would point to the jdk defaults that are configured using system properties. Plugins would need to be allowed to provide a default and then xpack would set it.

but I'm not sure it is worth the effort

I am also on the fence about the effort and usefulness vs asking the user to configure the default JDK one (or if reindex has other settings).

A simpler and possibly cleaner idea is to have xpack set the default SSLContext based on what has been configured using xpack.ssl.*, under the assumption that this is what reindex will use for reindex to remote.

Original comment by @nik9000:

A simpler and possibly cleaner idea is to have xpack set the default SSLContext based on what has been configured using xpack.ssl.*, under the assumption that this is what reindex will use for reindex to remote.

It is certainly worth testing that assumption, but if it proves true I think it is fine.

Original comment by @joshbressers:

This just showed up on my radar. I assume that with the coming TLS everywhere, we will want this working properly.

@jaymode @tvernum Any thoughts on how to best fix this?

Original comment by @tvernum:

I'll do some of tests on this and see whether setting the default SSL context does what we want.

Original comment by @tvernum:

This doesn't work out of the box.
The REST client uses Apache HTTP client, which by default doesn't use the default SSL Context, and the REST client doesn't configure an SSL context (or enable the default context).

It's possible to make it work, but it would require changes to the TransportReindexAction to configure the SSLContext on the REST client.

Original comment by @jaymode:

I opened a core issue for the rest client aspect previously LINK REDACTED I'll go ahead and take a look at that since no one else has gotten around to it yet.

Original comment by @eskibars:

I see https://github.com/elastic/elasticsearch/issues/23231 closed. Is there anything here still outstanding?

Original comment by @jaymode:

The core issue allowed for us to use the system default context, but I think the item left to do here is to consider making the SSLContext defined by the settings xpack.ssl.* the default system context. I'm on the fence on whether we should actually do this; it provides a nicer experience but then we are changing system defaults. Maybe we only do this if no default context is set and/or provide a setting to disable it.

Original comment by @ppf2:

+1 on making it work with xpack.ssl.*. On 5.0+, we made an effort to actively promote all users to use the xpack.ssl.* settings to directly reference the pem certs instead of using JKS - and our documentation for setting up encryption in x-pack has since been rewritten with all examples referencing pem (with jks information hidden away only in the x-pack settings section, not in the actual installation instructions). I have a customer who dislikes JKS so much that they are very happy with our move away from JKS, but the restclient + reindexing api are the pieces that are forcing them to go back to set up JKS stores. So allowing users to use xpack.ssl.* (as an option) for both the low level restclient + reindexing API will allow users to control this only via xpack.ssl.* and be consistent everywhere.

Original comment by @ppf2:

Related: https://github.com/elastic/elasticsearch/issues/25604

Original comment by @PhaedrusTheGreek:

Related: https://github.com/elastic/elasticsearch/issues/27267

_Note, this issue is specifically about how to make X-Pack based SSL config available in _reindex, not general reindex/SSL questions, but the two questions can't be separated perfectly_

I propose that we do something based around the following

  • Not use xpack.ssl.* settings because:

    1. these tend to interfere with other X-Pack features (like watcher connections to https URLs), which prompted us to change the docs in 42f9a990d17c220766614c7b8888bfd12143ce6c
    2. we're looking to remove those fallback settings anyway (https://github.com/elastic/elasticsearch/issues/29797)
  • Not add overwrite the system trust config (SSLContext.setDefault) because configuring the default system trust incorrectly will also break other features (like watcher) and experience shows that configure SSL well is tricky, and customers often don't completely understand the implication of changes they're making. Making changes to the default SSL context seems trappy. Even if we only supported _adding_ additional CAs, that still has the risk that the JVM would start trusting certs that it shouldn't.

    • As part of a fix I'm working on for #30344, we're planning to introduce _named_ ssl contexts. These allow code that uses the X-Pack SSL Service to ask for a context by name, without needing to have a copy of the actual settings.
  • We now have an SPI concept via #27881. This would allow the reindex module and x-pack module to share code in ways that weren't possible when this issue was raised.

Putting those together, would mean that

  • X-Pack can add some settings to configure an SSL context for "_reindex"
  • We can implement a basic SSL provider concept, either in server (core) or in the reindex module through SPI
  • X-Pack implements this provider interface, to make SSL contexts available to non X-Pack code
  • Reindex uses the discovered service providers to configure an appropriate SSL context.

There's some open questions there like:

  • do you need to tell reindex that it should look for a named SSL context?
  • is it too lenient to: use x-pack if configured, otherwise use the system default? (We typically lean towards explicit over implicit)
  • is SPI the correct approach, or would a standard plugin make more sense?
  • do we want to support multiple SSL providers, or require that there be only one?

but all those questions are relatively minor and could be resolved as part of the implementation.

ping: @elastic/es-security , @joshbressers , @eskibars

@eskibars Do you know if there's an ETA on this? If so, approximately when?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jasontedor picture jasontedor  路  3Comments

dadoonet picture dadoonet  路  3Comments

malpani picture malpani  路  3Comments

dawi picture dawi  路  3Comments

clintongormley picture clintongormley  路  3Comments