Repository navigation
[encryption]: Add skipEncodings to avoid double-encrypting payloads - #201
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
| encryption: | ||
| enabled: true | ||
| cacheSize: 200 | ||
| # Encodings already encrypted before they reach the proxy, forwarded unsealed. |
There was a problem hiding this comment.
Nit:
Maybe note in the commend that this is a header?
It looks like a header, but might as well be very explicit.
There was a problem hiding this comment.
They're actually fields (Payload.Metadata["encoding"]) set by PayloadCodecs.
| // With no keys there is no encryption codec, so the list has nothing to skip. | ||
| if len(cfg.Encryption.SkipEncodings) > 0 && o.vault == nil { | ||
| o.logger.Warn("encryption.skipEncodings is set but no encryption keys are configured, so it has no effect") | ||
| } | ||
|
|
There was a problem hiding this comment.
im fine with this but there is an argument for just failing invalid/non-effect configs
There was a problem hiding this comment.
Agreed. So far, I've been failing cases where behaviour would be invalid (e.g., enabling encryption without a default key policy) and warning about things that are ineffectual or don't break an invariant (e.g., apiTranslations defined with no Cloud upstream).
I absolutely don't feel super strongly about this, so if you'd prefer an error case here, say the word.
The proxy seals every outbound payload when encryption is enabled, even
one a Worker's own codec already encrypted, or one an earlier proxy in
a chain already sealed. A chained hop was worse...on the response it
tried to open the first hop's payloads, failed with unknown key, and
failed the whole call.
`encryption.skipEncodings` lists encodings to treat as already
encrypted. Payloads under a listed encoding are forwarded unchanged.
Matching is exact on the _encoding_ metadata, so sample codecs that also
use binary/encrypted, for example, are skipped too. Listing
`binary/encrypted` additionally makes `Decode` return payloads sealed
under a KEK this proxy doesn't hold instead of failing. That entry is
opt-in so a standalone proxy that lost a KEK still fails loudly. With it
set, you can observe it via `vault_ops_total{result="unknown_key"}`.
Listing an encoding trusts every Worker that sets it, since the proxy
cannot tell ciphertext from plaintext labeled that way. Blank entries
are rejected so a payload with no encoding can never match, and the
dataplane warns when a list is set with no keys configured.
Skips are counted in a new metric (`payloads_skipped_total`) by operation, encoding,
and namespace. The encoding label only ever holds a configured entry.
`vault_ops_total` reports an unknown KEK as _unknown_key_ rather than
error.
9545a12 to
9d79f38
Compare
3 participants
The proxy seals every outbound payload when encryption is enabled, even if a Worker's own codec already encrypted it or an earlier proxy in the chain already sealed it. A chained hop was worse...on the response, it tried to open the first hop's payloads, failed with an unknown key, and failed the whole call.
encryption.skipEncodingslists encodings to treat as already encrypted. Payloads under a listed encoding are forwarded unchanged. Matching is exact on the encoding metadata, so sample codecs that also use binary/encrypted, for example, are skipped too. Listingbinary/encryptedadditionally makesDecodereturn payloads sealed under a KEK this proxy doesn't hold instead of failing. That entry is opt-in, so a standalone proxy that lost a KEK still fails loudly. With it set, you can observe it viavault_ops_total{result="unknown_key"}.Listing an encoding trusts every Worker that sets it, since the proxy cannot tell ciphertext from plaintext labelled that way. Blank entries are rejected so a payload with no encoding can never match, and the dataplane warns when a list is set with no keys configured.
Skips are counted in a new metric (
payloads_skipped_total) by operation, encoding, and namespace. The encoding label only ever holds a configured entry.vault_ops_totalreports an unknown KEK as unknown_key rather than error.