elastic / logstash

Logstash - transport and process your logs, events, or other data
https://www.elastic.co/products/logstash
Other
14.17k stars 3.49k forks source link

Implement secrets/credentials container #8353

Open jordansissel opened 6 years ago

jordansissel commented 6 years ago

Parent ticket: https://github.com/elastic/logstash/issues/6892

This issue is focused on designing the following:

The above could be split into separate issues if desired.

Rough specification:

Safety goals:

jordansissel commented 6 years ago

Some notes from a planning discussion with @andrewvc

Secret Store
    Key Areas
    1 Secret Store Library: API + Implementation of the secret store for setting and retrieving secrets
        - Implementation detail: The actual file store itself (structure, encryption tech, etc)
    2 The command line interface
        - Can only set, list, and remove secrets, cannot read secrets.
        - *CANNOT* read secret values, only list entries, not decrypt.
        - Must be protected by a passphrase (plus encryption)
    3 Logstash configuration
        - Making it possible for users to say "get this credential from the secret store"
        - Logstash needs a way for an operator to provide the decryption key:
            - overload `$FOO` lookup to check Environment and/or Secret Store depending on user config?
            - Rejected idea: overload `:validate => :password` to make the user value be a lookup key
        - Plugins need a way to, with permission, fetch secrets

output {
    elasticsearch {
        hosts => [ ... ]
        user => "whatever"
        # `$secret` prefix would make it look up `blah` in the credential store.
        password => $secret.blah
    }

}
jakelandis commented 6 years ago
1 Secret Store Library: API + Implementation of the secret store for setting and retrieving secrets

PR #8566

The API is general purpose, allowing arbitrary byte[] to be persisted. The key (or alias) for the secret store is a versioned URN. The URN format is urn:logstash:secret:v1:<key> where key is the unique bit. In the example above it would be urn:logstash:secret:v1:blah.

Using a versioned URN as the key into the secret store allows for storing different types of secrets, for example, if we ever stored tokens it could be urn:logstash:token:v1:blah. The version is there to help future proof it, such that if the underlying format (encoding) changes we can flex our logic based on the version. The version also allows for adding additional semantics to the secret independent of the underlying store. For example, if we wanted to add optional expiry to secrets we could create urn:logstash:secret:v2:<key>:<expiry>.

While it is a bit of a pain to use byte[] everywhere, it isn't much worse then char[] and Java String's violate this goal "Plaintext values should be only held in memory as long as they are needed.". Further, byte[] allows arbitrary non-text secrets to be stored without any changes, and is the most flexible for alternative implementations.

Implementation detail: The actual file store itself (structure, encryption tech, etc)

The implementation provided in the PR is a JavaKeyStore of type pkcs12 using PBE (password based encryption) secret keys. This results in each KeyStore entry encrypted (3DES I think), and the whole keystore signed upon save. This ensures that the data is encrypted and has integrity. A password required to read/write to the KeyStore, though it can be an empty String. These are implementation details of the JavaKeyStore (not my code).

Interestingly the PBE Key that is required to be used with the JavaKeyStore only supports ASCII based encoding, and further the KeyStore type may impose more/less restrictions on the encoding. To avoid this, I am encoding the byte[] as Base64 (which is by definition ASCII) and storing the base64 bytes, and decoding on retrieval. This blows up the size a bit by re-encoding character data, but size on disk really isn't a concern here since it still relatively small and buys us full unicode and/or any arbitrary encoding can be stored.

jakelandis commented 6 years ago

@jordansissel / @andrewvc / @yaauie

re: password => $secret.blah

What do you think about using $!{blah} to represent a secret ? (as i am sure you know) This differs from the environment substitution by only the ! . In concept this and the environment entries are doing the same thing (substitution), and in ruby-likeness adding a ! changes the behavior slightly.

Also, I would guess that environment substitution is the general workaround for the lack of a secret store, so the migration from ${BLAH} to $!{BLAH} seems intuitive. ( or migrate to $!{blah} since case doesn't matter here). LS won't start up with a helpful message if you configure a secret that can't be replaced

Further, wrapping in brackets {...} greatly simplifies any edge cases w/r/t to parsing out the values.

andrewvc commented 6 years ago

I'd rather have a one byte solution instead of $!. Maybe ^{secret.blah}?

Definitely +1 on the brackets.

On Wed, Nov 15, 2017 at 7:05 PM, Jake Landis notifications@github.com wrote:

@jordansissel https://github.com/jordansissel / @andrewvc https://github.com/andrewvc / @yaauie https://github.com/yaauie

re: password => $secret.blah

What do you think about using $!{blah} to represent a secret ? (as i am sure you know) This differs from the environment substitution by only the ! . In concept this and the environment entries are doing the same thing (substitution), and in ruby-likeness adding a ! changes the behavior slightly.

Also, I would guess that environment substitution is the general workaround for the lack of a secret store, so the migration from ${BLAH} to $!{BLAH} seems intuitive. ( or migrate to $!{blah} since case doesn't matter here). LS won't start up with a helpful message if you configure a secret that can't be replaced

Further, wrapping in brackets {...} greatly simplifies any edge cases w/r/t to parsing out the values.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/elastic/logstash/issues/8353#issuecomment-344781743, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIBY5cFwzQ3rtXuiZ1tVOiNkzvu-i5Hks5s24pegaJpZM4PfmxZ .

jordansissel commented 6 years ago

I was thinking we could use the same syntax. It could check both env and secret store for the name. This is what puppet does and it seems to work well

On Wed, Nov 15, 2017 at 6:37 PM Andrew Cholakian notifications@github.com wrote:

I'd rather have a one byte solution instead of $!. Maybe ^{secret.blah}?

Definitely +1 on the brackets.

On Wed, Nov 15, 2017 at 7:05 PM, Jake Landis notifications@github.com wrote:

@jordansissel https://github.com/jordansissel / @andrewvc https://github.com/andrewvc / @yaauie https://github.com/yaauie

re: password => $secret.blah

What do you think about using $!{blah} to represent a secret ? (as i am sure you know) This differs from the environment substitution by only the ! . In concept this and the environment entries are doing the same thing (substitution), and in ruby-likeness adding a ! changes the behavior slightly.

Also, I would guess that environment substitution is the general workaround for the lack of a secret store, so the migration from ${BLAH} to $!{BLAH} seems intuitive. ( or migrate to $!{blah} since case doesn't matter here). LS won't start up with a helpful message if you configure a secret that can't be replaced

Further, wrapping in brackets {...} greatly simplifies any edge cases w/r/t to parsing out the values.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/elastic/logstash/issues/8353#issuecomment-344781743 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AAIBY5cFwzQ3rtXuiZ1tVOiNkzvu-i5Hks5s24pegaJpZM4PfmxZ

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/elastic/logstash/issues/8353#issuecomment-344797295, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIC6l4WUmaGIG0fC9LIKmTfkP24Ys9dks5s25_6gaJpZM4PfmxZ .

andrewvc commented 6 years ago

Oh, I like that idea as well. WDYT @jakelandis ?

jakelandis commented 6 years ago

@jordansissel / @andrewvc

Re-using ${...} will work for secret store substitution. I was abit concerned about confusion, but a bit documentation should solve that. Also probably less confusing then introducing something new for the same concept.

...so the evaluation precedence will be:

  1. secret store
  2. environment variable
  3. default (e.g. ${var:default}
andrewvc commented 6 years ago

It sounds like we have agreement then.

jakelandis commented 6 years ago

The CLI for storing/purging credentials from the credential store.

The CLI will strongly mirror Elasticsearch's. (It won't be a perfect copy, but pretty close)

Since the keystore has optional logstash.yml settings, the CLI will be implemented using the Ruby based command line (Clamp). This is to allow easy access logstash.yml and have access to all the pre-existing defaults (such as data dir), but at a slight cost of startup time (it won't need to load the whole thing, just enough to get the settings from the correct place) .

This means that a user can interact with the secret store via bin/logstash. Specifically, the commands will be --keystore.command <command> and --keystore.secret.id <id>.

For example: bin/logstash --keystore.command add --keystore.secret.id db.pass

However, a shell script will be created that will convert those (kinda awkward) command's into something that matches Elasticsearch's CLI.

Just wanted to solicit any feedback on this approach but I get far down this road.

jordansissel commented 6 years ago

bin/logstash --keystore.command add --keystore.secret.id db.pass

This is a weird implementation detail. This would mean there are two ways for users to invoke the keystore cli, and I'd prefer only one.

We have the plugin manager as a separate command line interface, and I think we can do the same for the secret manager.

I am +1 on copying the CLI that Elasticsearch has. The implementation details are up for discussion. My intuition is that the bin/logstash --keystore.command ... stuff feels really weird to have in bin/logstash command (even if we have a script wrap this).

jakelandis commented 6 years ago

This is a weird implementation detail.

Agreed.

We have the plugin manager as a separate command line interface, and I think we can do the same for the secret manager.

The main difference is the requirement to access to logstash.yml. However, in retrospect, the ability to read the settings is already de-coupled from the 'bin/logstash' CLI, so should be able to re-use reading without the CLI. Let me take another swing at this.

thanks!

jordansissel commented 6 years ago

@jakelandis I enjoy these efforts to try and reuse components :)

tylersmalley commented 6 years ago

Why use substitution for the configuration file instead of what ES does, which merges settings from the keystore after loading the configuration yaml? It would be nice if all our applications behaved similarly.

jakelandis commented 6 years ago

@tylersmalley - We have 2 sets of configuration's to protect. One configuration sets up our pipelines, and the other for the Logstash service. The former is a set of configuration files written in Logstash's config language, and defines the pipelines (for example, use beats input with es output).

The latter is a single YAML file and defines attributes specific to Logstash itself (for example which queue type to use).

For our pipeline config the keys used aren't guaranteed to be unique across the set of configurations. For example two outputs could have defined password as the key. To further complicate things, the same pipeline may have the same type of output multiple times. For example a pipeline may send to 2 different http outputs, both configured with different password values.

For our pipeline config, I do not believe it is possible to merge without substitution placeholders.

For our single YAML configuration it is possible, but we also want symmetry within our own configuration first.

It would be nice if all our applications behaved similarly.

Agree'd , I will look into support a merge in addition to a placeholder for our YAML configuration.