tls: plumbing for multiple TLS certificate ingestion.#5095
tls: plumbing for multiple TLS certificate ingestion.#5095htuch merged 3 commits intoenvoyproxy:masterfrom
Conversation
This PR starts to plumb multiple TLS certs from the proto level into the SSL context. We stop short of enabling multiple TLS certificates, but instead have sufficient mechanism and interface changes to propagate them to the SSL context. Future PRs will extend this with the SSL context implementation. Risk Level: Low Testing: bazel test //test/... Part of envoyproxy#1319. Signed-off-by: Harvey Tuch <htuch@google.com>
| return providers; | ||
| } | ||
| // TODO(htuch): Support multiple SDS secret configs; could we mix them with | ||
| // static? |
There was a problem hiding this comment.
We could mix them (in theory), but we shouldn't, IMHO, and there is no valid reason to do that, really...
| tls_certificate_configs_.clear(); | ||
| tls_certificate_configs_.emplace_back(*tls_certficate_providers_[0]->secret()); | ||
| callback(); | ||
| }); |
There was a problem hiding this comment.
This will break with multiple certificates, clearing list of configured certificates on each callback, won't it? Though, I guess, this will only happen with SDS, where we support only a single certificate right now.
Maybe add a TODO to fix it?
| const std::string ecdh_curves_; | ||
|
|
||
| Ssl::TlsCertificateConfigPtr tls_certificate_config_; | ||
| std::vector<Ssl::TlsCertificateConfigImpl> tls_certificate_configs_; |
There was a problem hiding this comment.
Aren't we losing std::unique_ptr wrapping here?
There was a problem hiding this comment.
Yeah, but std::vector is RAII-aware, see https://en.cppreference.com/w/cpp/language/raii.
There was a problem hiding this comment.
Hmm... I think we're using std::vector<TypePtr> in majority of the codebase...
But if you say it's fine, then it's fine :) Thanks!
There was a problem hiding this comment.
Yeah, in general you can only do this if the class has a sensible copy/move constructor to allow vector to grow. In this case, TlsCertificateConfigImpl is just a bunch of const strings, so it works fine.
Signed-off-by: Harvey Tuch <htuch@google.com>
This PR starts to plumb multiple TLS certs from the proto level into the SSL context. We stop short of enabling multiple TLS certificates, but instead have sufficient mechanism and interface changes to propagate them to the SSL context. Future PRs will extend this with the SSL context implementation. Risk Level: Low Testing: bazel test //test/... Part of envoyproxy#1319. Signed-off-by: Harvey Tuch <htuch@google.com> Signed-off-by: Fred Douglas <fredlas@google.com>
This PR starts to plumb multiple TLS certs from the proto level into the SSL context. We stop short
of enabling multiple TLS certificates, but instead have sufficient mechanism and interface changes
to propagate them to the SSL context. Future PRs will extend this with the SSL context
implementation.
Risk Level: Low
Testing: bazel test //test/...
Part of #1319.
Signed-off-by: Harvey Tuch htuch@google.com