-
Notifications
You must be signed in to change notification settings - Fork 1
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
Send SNI information when connecting via TLS #7
base: vast/0.18.x
Are you sure you want to change the base?
Conversation
2b55680
to
7703f97
Compare
Without this setting, OpenSSL would only validate that the certificate has a valid signature from a trusted CA, but not that it actually matches the host to whom we were trying to connect.
CAF currently expects to receive OpenSSL certificates and keys as filenames. This matches the expectations of libopenssl, but is cumbersome to use in a Cloud context. In particular, ECS can only pass secrets as environment variables. So we extend the command-line option to also accept a string like `env:SOME_VARIABLE` and read the contents from the environment variable `SOME_VARIABLE`.
0a7d5d4
to
edf4fb7
Compare
ofs << *contents; | ||
ofs.close(); | ||
return filename; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized that this approach will only work for docker-compose
, there's no good way to inject environment variables when the user uses his own vast
and a vast.yaml
.
However adding the value without the indirection has the disadvantage that the secrets will appear everywhere the VAST config is printed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make it possible to pass the file path in the env variable for the second case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A file path is how it already works, before our patches.
I think the cleanest solution is to not pass this via config at all but to provide a /get-client-certificates
endpoint where we can download everything before starting CAF. (and to only use this mechanism for the manager nodes) But for the short term I just added support for putting the PEM directly into the option.
* Support for reading private keys and certificates directly as opposed to specifying the name of a separate environment variable, since the latter is not possible when using a `vast.yaml` config file. Note that this is just a crux, eventually we want to have a cloud endpoint where the node can download the required TLS files on the fly given some auth token. * Support for OpenSSL 1.1.1 series. The OSSL_DECODER API is not available on that version, which is used on Debian Bullseye.
1cca273
to
1804776
Compare
Unlike its lower-level variants, `PEM_read_bio_PrivateKey` is not officially deprecated yet so we can avoid maintaining two versions of the private key loading by relying on that.
Co-Authored-By: Tobias Mayer <[email protected]>
No description provided.