Secure API

[yaauie]

Release notes

• Adds options for securing the Logstash API using TLS and/or HTTP Basic auth.

What does this PR do?

Partial implementation of #13196 adding SSL and Basic auth to the Logstash API

Why is it important/What is the impact to the user?

Empowers users to secure the Logstash API, and to bind to non-loopback interfaces by default when the API is secured.

This in turn will empower future development of the API features.

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files (and/or docker env variables)
• I have added tests that prove my fix is effective or that my feature works

Author's Checklist

• regroups API-related settings under top-level api.*
• adds api.ssl settings
  • api.ssl.enabled
  • api.ssl.keystore.path and api.ssl.keystore.password
  • api.client_authentication
  • api.ssl.supported_protocols
• adds api.auth settings
  • api.auth.type: none and basic
  • api.auth.basic.username and api.auth.basic.password
• api.http.host defaults to localhost when api.ssl.enabled: true and api.auth.type: basic
• api.http.host defaults to localhost when api.ssl.enabled: true and api.ssl.client_authentication: required

[yaauie]

@jsvd I would appreciate a WIP-review to make sure this is headed in the right direction.

• Currently, testing for SSL is entirely manual. I used openssl@3 to generate and sign my keys/certs, but ran into problems exporting those to pkcs12 with openssl@3 and had to fall back to doing so with openssl@1.1. Manual tests show it works with either the .p12 or the jks keystores (certs: shared-ca.tar.gz). These will need to be automated to avoid putting expiring artifacts into source control.
• The spec calls for api.ssl.client_authorization (none, optional, required), but I'm getting weird hangs in puma that are taking a very long time to chase down -- this feature will likely get pared off for later.
• The spec also calls for configuring a list of supported TLS protocols. This may be pared off as well, since matching the config param that Elasticsearch has for similar is proving tricky.

[jsvd]

Good stuff!! I've been playing with this and I like the details around deprecating the old settings and the warnings when incompatible settings are used.

Regarding authorization, I'm wondering if we should prepare the settings to account for multiple users instead of 1 single user. As soon as we add 1 feature that is more security sensitive like adding a pipeline we'll want separate users. For example, 1 for elastic agent doing monitoring and 1 for operators manipulating pipelines (and maybe even 1 other that can only grab metrics, for diagnostics).

As for encryption, I'm worried about how much Puma's MiniSSL's mini nature will harm our ability to provide a good tls experience to users. Besides the issue you've hit with client authentication, here's a couple of other limitations:

1. minissl does not allow having separate stores: one for trust, other for node-specific certs/keys. It's normal to have a trust store bundle you can hand out to all nodes, while keystores will be password protected and for each node.

2. minissl for jruby doesn't support tls1.3. As tls1.1 is deprecated in jdk17 (see jdk.tls.disabledAlgorithms on $JAVA_HOME/conf/security/java.security) we'll want, asap, to also provide tls1.3 support.

3. maybe other fine tuning settings such as allowing configuration of verification_mode (on top of existing client_authentication).

[yaauie]

My primary goal with this PR is to provide an interface that we can maintain and grow for the years to come, along-side an implementation that provides some value now.

With that in mind:

Regarding authorization, I'm wondering if we should prepare the settings to account for multiple users instead of 1 single user. As soon as we add 1 feature that is more security sensitive like adding a pipeline we'll want separate users. For example, 1 for elastic agent doing monitoring and 1 for operators manipulating pipelines (and maybe even 1 other that can only grab metrics, for diagnostics).

I agree that a single user isn't going to be enough for features we want to add in the future, but I believe that the interface provides us room to add this in a backward-compatible way if and when those requirements become clear. For example, if we were to add role-based safeguards to different end-points, we could easily add api.auth.basic.role setting to specify the role that our single user gets, defaulting to something read-only like monitoring. We could add additional auth providers that are smarter about assigning roles (separate config file? external RBAC with Elasticsearch?) without breaking users who are using the minimal api.auth.basic implementation provided here.

As for encryption, I'm worried about how much Puma's MiniSSL's mini nature will harm our ability to provide a good tls experience to users. Besides the issue you've hit with client authentication, here's a couple of other limitations:

1. minissl does not allow having separate stores: one for trust, other for node-specific certs/keys. It's normal to have a trust store bundle you can hand out to all nodes, while keystores will be password protected and for each node.

When we swap out minissl for a different implementation, we can add an option like api.ssl.truststore, and we can default to the value provided to api.ssl.keystore to maintain backward compatibility when a user doesn't provide one.

2. minissl for jruby doesn't support tls1.3. As tls1.1 is deprecated in jdk17 (see jdk.tls.disabledAlgorithms on $JAVA_HOME/conf/security/java.security) we'll want, asap, to also provide tls1.3 support.
3. maybe other fine tuning settings such as allowing configuration of verification_mode (on top of existing client_authentication).

Absolutely. This interface is meant to be an ever-expanding subset of the one provided by Elasticsearch's ssl.* options for its API, as we are able. It is not meant to fully-replicate their implementation in one go, but rather to provide a baseline value for some of our users and a path forward for adding value to the rest.

[jsvd]

I've left a few comments, mostly minor.

git-wise I'd prefer for this PR to be split into two. wdyt?

One could group all internal changes to settings:

• docker/data/logstash/env2yaml/env2yaml.go
• logstash-core/lib/logstash/patches/clamp.rb
• logstash-core/lib/logstash/patches/puma.rb
• logstash-core/lib/logstash/settings.rb
• logstash-core/logstash-core.gemspec
• maybe parts of logstash-core/lib/logstash/webserver.rb

A second PR would have all the code for the new api settings, deprecating old ones:

• logstash-core/lib/logstash/agent.rb
• logstash-core/lib/logstash/runner.rb
• logstash-core/lib/logstash/environment.rb
• yml/docs

[jsvd]

The functionality LGTM, I added only 3 nitpicks in this review.
You've mentioned this before and I'd appreciate a test, even if just for the happy path, confirming the correct emission of certificates from a jks/p12 keystore during TLS handshake.

For the assets, if needed we can have a separate repository with pregenerated root certs, server certs, etc, and a github action scheduled to, every 6 months, call the generate script to avoid expirations.

[yaauie]

@jsvd I just now saw your comment requesting this be split into two PR's. I've just finished adding the requested tests and getting everything back to green, and will attempt to pare it off into two separate PR's along those lines shortly. I've been maintaining "one thing" per commit, rebasing and squashing as I go, so in theory it shouldn't be too difficult.

[yaauie]

@jsvd this should be ready for a final review; I've addressed your concerns and appreciate your help in building out the integration tests.

[jsvd]

I see 4 leftover files from the client cert cleanup, that should be deleted:

qa/integration/fixtures/webserver_certs/generated/client_from_intermediate.jks
qa/integration/fixtures/webserver_certs/generated/client_from_intermediate.p12
qa/integration/fixtures/webserver_certs/generated/client_from_root.jks
qa/integration/fixtures/webserver_certs/generated/client_from_root.p12

Other than that, it all LGTM!! 🎉

[yaauie]

It looks like I checked in a mixed-generation of certificates, such that the root.crt didn't match the one used to sign the server_*.crt's. I've generated a fresh generation and validated that it all works locally, and have updated this branch to trigger a build.

[yaauie]

🤦🏼 I was using git add --patch, which blows past binary files, causing the updated .p12 and jks files to not get included. I've updated those and we should be green after that.

[yaauie]

jenkins test this again please

(ES failed to download in integration test setup)

[yaauie]

jenkins test this again please

[jsvd]

Tested a few scenarios locally: enabling tls, setting up basic auth, using passwords from logstash keystore, using wrong credentials.

All good, LGTM 🚢