api: redact more fields.

[PiotrSikora]

Signed-off-by: Piotr Sikora piotrsikora@google.com

[repokitteh-read-only]

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #9692 was opened by PiotrSikora.

see: more, trace.

[mergeconflict]

Are you thinking to add tests similar to SecretManagerImplTest::ConfigDumpHandler?

[PiotrSikora]

Are you thinking to add tests similar to SecretManagerImplTest::ConfigDumpHandler?

Not really, no... Extensive testing for protos annotated with udpa.annotations.sensitive in /config_dump is already done in #9315, and that's the only place where those fields are emitted, AFAIK (other than logs, but you're working on that separately).

[incfly]

pieces that i'm famliar with are already redacted, so lgtm

just curious whats the PrivateKeyProvider.typed_config is about? That seems used to implement opaque private operation backed by boringssl hook, correct? is that security sensitive?
#6326

[PiotrSikora]

just curious whats the PrivateKeyProvider.typed_config is about? That seems used to implement opaque private operation backed by boringssl hook, correct? is that security sensitive?
#6326

It's the configuration for the asynchronous private key provider, which, in principle, may contain the private key itself, or an information on how to perform the signing.

I consider this information sensitive.

[mergeconflict]

LGTM, thanks for doing this!

[htuch]

LGTM, thanks. I think we don't have to be perfect here, only grow the number of sites we annotate and encourage folks to use these going forward. Config dump and logs are still sensitive and contain PII, so we are not making any assertions around them being scrubbed of all sensitive data.

[PiotrSikora]

This won't make for a great git history, but presumably this can be still merged after the freeze?

[htuch]

Yeah, sensitive annotations are not breaking. By freezing, we just mean that we won't do breaking changes as per policy.

[mattklein123]

LGTM with one question.

[mattklein123]

Doesn't this effectively not do anything since redaction doesn't work inside the struct?

[mergeconflict]

No, this works fine: it recursively redacts everything inside the struct.
The case that wouldn't work is if the struct weren't annotated as sensitive, but contained some field that, if it were part of a strongly-typed message, would have been annotated sensitive. That is, when we're looking inside the struct, we have no info about which fields should be redacted.

[mattklein123]

OK got it. Since this is a struct that gets converted into a strongly typed message everything works. 👍