ssl: expose upstream TLS client context config#6178
ssl: expose upstream TLS client context config#6178kyessenov wants to merge 2 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Kuat Yessenov <kuat@google.com>
Signed-off-by: Kuat Yessenov <kuat@google.com>
|
Do you have a WIP PR that is relying on this in the filter? I don’t think this should be in the factory interface but would like to see how you’re using it. |
|
Ideally, I could just access the security info from cluster info, but I don't see a way to do it without going through a factory right now. |
|
I think ideally you would like to see what is actually being used for the upstream connection? so something like #6144 but for upstream in StreamInfo sounds more appropriate, no? |
|
Yes and no. I think we do want an actual connection info for telemetry. But we also want a policy check using the intended connection settings. I realize there is an issue that upstream cluster info is inaccessible during decodeHeaders ATM, but that can be fixed separately. |
| /** | ||
| * @return CertificateValidationContextConfig the certificate validation context config. | ||
| */ | ||
| virtual const Ssl::CertificateValidationContextConfig* certificateValidationContext() const PURE; |
There was a problem hiding this comment.
We need a bit of abstraction for this by not leaking Ssl namespace into here, perhaps a Protobuf::Message& config() or abstract class TransportSocketConfig. Ssl::Connection in TransportSocket is due to historical reason and that is to be removed.
There was a problem hiding this comment.
It looks like ATLS is not using certificate validation context. How do we tell whether ATLS config is meant for mutual TLS?
There was a problem hiding this comment.
did you mean ALTS? If you expose TransportSocket proto does that work?
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
stale
bot
added
the
stale
|
This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
2 participants
Description: Populate certificate validation context for upstream host descriptions. We would like to collect whether upstream has client cert defined for telemetry purposes in a custom filter.
/assign @lizan
Risk Level: Small
Testing: None, looking for a recommendation how to test this properly.
Docs Changes: None
Release Notes: