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

HTTPS Configuration Considered Extremely Difficult #10

Closed
torrancew opened this issue Nov 24, 2014 · 9 comments
Closed

HTTPS Configuration Considered Extremely Difficult #10

torrancew opened this issue Nov 24, 2014 · 9 comments

Comments

@torrancew
Copy link
Contributor

This was discovered while looking at adding certificate auth support to the Logstash Elasticsearch output, which uses Manticore via the Elasticsearch Ruby client (see this adapter).

A quick overview of the library pointed me upstream to the Apache HTTP Components library for fine-tuned SSL configuration, like per-connection truststore/keystore support. My initial attempt was to use the Manticore client constructor's block support, and set the SSLContext or SSLConnectionSocketFactory on the HttpClientBuilder object in the block. Unfortunately, this did not work. Some digging revealed this note in upstream docs for HttpClientBuilder#setSSLSocketFactory:
Please note this value can be overridden by the setConnectionManager(org.apache.http.conn.HttpClientConnectionManager) method. Unfortunately, this method is called by the Manticore client constructor, indirectly.

Ultimately, (as hinted at in #7 and #8) it seems the only place in the Manticore client where SSL can be configured thoroughly is in Manticore::Client#pool_builder, which is a bit counterintuitive, I think.

I'd like to propose that pool_builder be refactored to always explicitly configure Plain and SSL socket factories, deferring to an ssl_ctx_builder (or similar) method for the latter.

Ultimately, it probably makes sense for Manticore to abstract the common SSL settings, and configure the underlying library on the user's behalf, given the increasing relevance of HTTPS, and the confusing workflow currently required, however I wanted to reach out before adding 4+ configuration options (minimally, truststore+password, keystore+password - ideally support would also include ciphers and other details), to see upstream has a preference on configuration parameter naming. If there's interest, I'm happy to contribute my scratch code towards a patch.

@cheald
Copy link
Owner

cheald commented Nov 24, 2014

I would love a patch. Manticore is basically just scratching my itch right now, so I'm sanding off rough edges as I find them. This sounds like one that needs some sanding! :)

@torrancew
Copy link
Contributor Author

@cheald do you have any preference on key names for SSL-related settings? I considered moving all SSL-related options into a nested hash under options[:ssl], but that is somewhat invasive WRT the current verification-related flag.

@cheald
Copy link
Owner

cheald commented Nov 24, 2014

I think it would be appropriate to move them into an :ssl hash, yeah. I don't mind moving :ignore_ssl_validation to [:ssl][:ignore_validation], though I'd want to support the former with a deprecation notice for a couple of releases.

Manticore is still young and not widely used yet, so it's fine to make breaking changes as long as they're done as elegantly as possible.

@torrancew
Copy link
Contributor Author

@cheald agreed, of course! I'll try to get something over in a few days. In the mean time, what are your thoughts on #8? That issue should be resolved along with this one, but if you'd like to pull #8 in, I'll happily avoid re-implementing it.

@cheald
Copy link
Owner

cheald commented Nov 24, 2014

I'll go ahead and pull it in so that @synhaptein gets credit for it. :)

@torrancew
Copy link
Contributor Author

Thanks!

torrancew pushed a commit to torrancew/manticore that referenced this issue Nov 25, 2014
torrancew pushed a commit to torrancew/manticore that referenced this issue Nov 25, 2014
@torrancew torrancew mentioned this issue Nov 25, 2014
@cheald
Copy link
Owner

cheald commented Nov 29, 2014

I've added support for additional options via :ssl in 8e8cfea

Notably, these include :trust_store and :trust_password, which you can use to specify a custom trust store for validation of SSL certificates.

@cheald
Copy link
Owner

cheald commented Dec 10, 2014

I've just added support for client cert authentication, as well. To use it, simply pass :ssl => {:keystore => "/path/to/keystore", :keystore_password => "password"} to the client constructor. I've additionally added :keystore_type and :truststore_type, so that you can specify JKS or PKCS12 key/trust stores.

I think this should close the issue; if not, let me know, and I'll reopen it.

@cheald cheald closed this as completed Dec 10, 2014
@torrancew
Copy link
Contributor Author

Terrific! Apologies I didn't get around to rebasing in time. Will test ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants