Closed PheonixS closed 2 years ago
Looks like (udpa.annotations.sensitive) = true
is either missing somewhere or ignored.
cc @htuch @junr03
I think we have the annotation, https://github.com/envoyproxy/envoy/blob/2bf847854610db8bc5a44ef3046fcc8f3a23518e/api/envoy/extensions/transport_sockets/tls/v3/common.proto#L249, so seems like a bug in config dump redaction. The OP seems to be asking about something different though, they're not seeing an update reflected in the config dump. I suspect that is because the update does not happen on the client (I don't see it in the logs).
@htuch you right there are 2 points:
Config dump reveal session ticket keys.
Values are not updated in the config dump.
I see this lines in the log I provider: [source/extensions/transport_sockets/tls/ssl_socket.cc:432] Secret is updated..
I assume that means that secret was updated on the Envoy side.
Yeah, some secret is updated. I can't line up the timestamps of the logs and your config dump though. It might also help to ahve some trace output around secret manager and SDS (you might want to redact if there is anything sensitive manually).
Hi @htuch , here is synchronised logs of envoy. debug.log
Logs of SDS server:
2021/12/10 12:29:41.598231 [INFO] Program started
2021/12/10 12:29:41.598439 [INFO] sds start, interval: 30s
2021/12/10 12:29:41.598467 [INFO] sds iteration
2021/12/10 12:29:41.598483 [INFO] creating snapshot Version 1639135781598
2021/12/10 12:29:41.600377 [INFO] management server listening on port 18000
2021/12/10 12:29:50.557976 [DEBUG] OnStreamOpen 1 type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret
2021/12/10 12:29:50.558624 [DEBUG] NewNodeDiscovery: %!(EXTRA string=1)
2021/12/10 12:29:50.558650 [DEBUG] open watch 1 for type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret[apimgateway_session_ticket_keys] from nodeID "1", version ""
2021/12/10 12:29:50.558667 [DEBUG] respond open watch 1[apimgateway_session_ticket_keys] with new version "1639135781598"
2021/12/10 12:29:50.558687 [DEBUG] respond type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret[apimgateway_session_ticket_keys] version "" with version "1639135781598"
2021/12/10 12:29:50.563514 [DEBUG] open watch 2 for type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret[apimgateway_session_ticket_keys] from nodeID "1", version "1639135781598"
2021/12/10 12:30:11.600103 [INFO] sds iteration
2021/12/10 12:30:11.600157 [INFO] creating snapshot Version 1639135811600
2021/12/10 12:30:11.600216 [DEBUG] respond open watch 2[apimgateway_session_ticket_keys] with new version "1639135811600"
2021/12/10 12:30:11.600230 [DEBUG] respond type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret[apimgateway_session_ticket_keys] version "1639135781598" with version "1639135811600"
2021/12/10 12:30:11.601898 [DEBUG] open watch 3 for type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret[apimgateway_session_ticket_keys] from nodeID "1", version "1639135811600"
2021/12/10 12:30:41.599165 [INFO] sds iteration
2021/12/10 12:30:41.599207 [INFO] creating snapshot Version 1639135841599
2021/12/10 12:30:41.599241 [DEBUG] respond open watch 3[apimgateway_session_ticket_keys] with new version "1639135841599"
2021/12/10 12:30:41.599275 [DEBUG] respond type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret[apimgateway_session_ticket_keys] version "1639135811600" with version "1639135841599"
2021/12/10 12:30:41.600442 [DEBUG] open watch 4 for type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret[apimgateway_session_ticket_keys] from nodeID "1", version "1639135841599"
^C
2021/12/10 12:30:53.368932 [INFO] SIGTERM received, exiting
2021/12/10 12:30:53.369061 [INFO] ctx is done, exiting
I'm not seeing anything obvious, @JimmyCYJ @lizan any thoughts on this one?
When I look at the code, I think the relevant sites are the dump code for tickets, at https://github.com/envoyproxy/envoy/blob/fef9121735e2dc680b58a192d04b2f9251f1a0a5/source/common/secret/secret_manager_impl.cc#L283 and the SDS update at https://github.com/envoyproxy/envoy/blob/fef9121735e2dc680b58a192d04b2f9251f1a0a5/source/common/secret/sds_api.cc#L95. A couple of observations:
SecretManagerImpl::dumpSecretConfigs
should be refactored. This is very verbose and boilier platey, making it hard to follow logic and check for consistency across dumps.@PheonixS is it just the inline bytes that is not updated? I.e. is the timestamp/version bumped?
@htuch yes, only inline bytes are not updated. Timestamp/version correctly bumped.
So, I figured out what is going on and it's kind of hilarious (specifically funny that I didn't spot this earlier). Redaction is taking place, if you base64 decode W3JlZGFjdGVkXQ==
, you get [redacted]
. So, we're not actually failing to update config dump or redact. It's a bit confusing but makes sense when you think of the underlying proto JSON representation for bytes
. You can also see this captured in a test at https://github.com/envoyproxy/envoy/blob/9050cfdc683856a7b0c7d43483e6f4152e91206d/test/common/secret/secret_manager_impl_test.cc#L693.
I'm going to close this out, as I don't think there is anything broken or actionable, but feel free to reopen if you have some suggestion on how to make this less confusing to the next person who encounters.
Description: session_ticket_keys retrieved via SDS should be redacted from the output.
Looks like only initial state in revealed in the config dump. Next iterations of pushing new configuration is not updating value of inline_bytes.
Repro steps:
Run SDS server which will send the following schema to Envoy:
Section of dynamic secrets:
Admin and Stats Output: Clusters:
Listeners:
Server info:
Config:
Logs: