elasticsearch-keystore `add --stdin` does not behave like `add-file`

[thna123459]

This is related to: #35433

Elasticsearch version (bin/elasticsearch --version):

6.6.1

Description of the problem including expected versus actual behavior:

Keystore data inserted with elasticsearch-keystore add --stdin is unusable for GCS credentials while data inserted using elasticsearch-keystore add-file is perfectly fine.

It is not clear if this really is the intended behaviour:
https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java#L542

If yes, then at least the Puppet code will need adjustments to bind types to credentials because it only supports add --stdin at this point:
https://github.com/elastic/puppet-elasticsearch/blob/master/lib/puppet/provider/elasticsearch_keystore/elasticsearch_keystore.rb

This needs to be described there at least:
https://www.elastic.co/guide/en/elasticsearch/plugins/current/repository-gcs-usage.html

Steps to reproduce:

Loading data with using:

cat /secrets/file | /usr/share/elasticsearch/bin/elasticsearch-keystore add --force --stdin  gcs.client.XYZ.credentials_file

... produces an error upon POST _nodes/reload_secure_settings:

   {
      "name" : "mynode.net",
      "reload_exception" : {
        "type" : "illegal_argument_exception",
        "reason" : "Secret setting gcs.client.XYZ.credentials_file is not a file"
      }
    }

While inserting the same data using add-file does not output any error

/usr/share/elasticsearch/bin/elasticsearch-keystore add-file --force gcs.client.XYZ.credentials_file /secrets/file

Note: The keystores do not have the same file sizes on disk (2559 bytes with --stdin, 2557 with add-file).

[elasticmachine]

Pinging @elastic/es-core-infra

[rjernst]

The add-file type was specifically added for GCS credential support. The plain add is for string (SecureStringSetting in the code) credentials, but for GCS the entire file is encrypted (SecureFileSetting in the code).

You are right the docs are not clear about that; it should be simple to fix. Additionally, long term I would like to have the type required by the setting be enforced when adding to the keystore, but there is a lot of high hanging fruit to get there (we don't even know what setting names are valid when the keystore tool runs).

[thna123459]

Mixing two distinct types of data in a single keystore breaks credential management through Puppet, and I do not see how that can be fixed at this time. Should I open a bug report on puppet-elasticsearch? https://github.com/elastic/puppet-elasticsearch/blob/master/lib/puppet/provider/elasticsearch_keystore/elasticsearch_keystore.rb

[rjernst]

Mixing two distinct types of data in a single keystore breaks credential management

I'm not sure what you mean. The keystore has 2 distinct types of keys right now, there is no mixing. Currently GCS is the only user of the second "file" type. The existence of this second type was partly due encoding issues when I originally added secure GCS settings. At that time, the elasticsearch keystore wrapped a java keystore.

I've opened #40605 to improve the docs. Please do open an issue for puppet-elasticsearch.

[rjernst]

The existence of this second type was partly due encoding issues

Another reason is the interface internally needs to be different for the two types. The "file" type is exposed in code as reading an InputStream, while all the other string settings are exposed as SecureString objects.

[thna123459]

File credentials require an actual clear text file on the file system, therefore:

1. It looks really strange to require a secret to be in a clear text file to add it to an encrypted/obfuscated keystore (?)
2. Puppet is now unable to manage Elasticsearch keystore files because it cannot handle this new corner case. It is possible to work around that by not using Puppet, but that is inconvenient.

I suggest that add-file should read credentials from stdin too (if it already does, please indicate how?).