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

Raft (retry_join/CLI) leader-... options (file paths vs PEM data) #8753

Closed
exo-cedric opened this issue Apr 16, 2020 · 6 comments
Closed

Raft (retry_join/CLI) leader-... options (file paths vs PEM data) #8753

exo-cedric opened this issue Apr 16, 2020 · 6 comments

Comments

@exo-cedric
Copy link

Describe the bug

As per raft's retry_join documentation - https://www.vaultproject.io/docs/configuration/storage/raft#retry_join-stanza - leader_... options are supposedly path to CA/certificate/private-key files

Those must actually be the PEM data in order for the joining to work.

This also stands true for the vault operator raft join CLI command - https://www.vaultproject.io/docs/commands/operator/raft#join - although it's less clear those options are not path to files (although other TLS-related options are; e.g. -ca-cert)

Expected behavior

Ideally, leader-... options should be path to files (which makes it less cumbersome to specify multiple retry_join stanzas in the configuration file).

But if providing PEM data is the intended behavior, then the documentation should be corrected and maybe the CLI options explicited.

Environment:

  • Vault Server Version (retrieve with vault status): 1.4.0
  • Vault CLI Version (retrieve with vault version): 1.4.0
  • Server Operating System/Architecture: n/a
@vvidovic
Copy link

Maybe "file options" should be provided for three certificate/key properties, for example:
leader_ca_cert_file

This would enable me to use a simple configuration in the OpenShift cluster, something like:

 "leader_ca_cert_file": "/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt"

Instead of:

"leader_ca_cert": "-----BEGIN CERTIFICATE-----\nMIID...\n-----END CERTIFICATE-----"

@vvidovic
Copy link

In the similar configuration - seal/transit (https://www.vaultproject.io/docs/configuration/seal/transit) "tls_ca_cert" parameter takes a path to the cert file.

@Lucretius
Copy link

I ran into this as well. This function appears to be the culprit. The retry_join stanza is read but the file paths are never actually read by something like ioutil.ReadFile to get the actual byte contents. Instead, the file content strings are just casted to byte arrays and passed in to the TLS Configuration function. The fix is to read the file contents, handling errors, before passing their contents in.

@zelch
Copy link

zelch commented May 1, 2020

Looking at things, SetupTLSConfig and ClientTLSConfig really appear to be intended for the exact same thing, except one takes (in theory) byte arrays and one takes a conf map which holds things like tls_cert_file.

The only user in the vault repo of ClientTLSConfig is raft, which uses it incorrectly.

A slight refactor of both functions might be in order so we don't duplicate quite so much code, and so we actually handle the raft TLS settings correctly.

@calvn
Copy link
Contributor

calvn commented May 12, 2020

Thanks for bringing this up! This should be addressed by #8894.

@calvn calvn closed this as completed May 12, 2020
@exo-cedric
Copy link
Author

Thanks for #8894 !
For consistency's sake, shouldn't the CLI part also be addressed (in command/operator_raft_join.go) ?

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

No branches or pull requests

5 participants