[kms]: Add KeyMaterial to avoid hand-rolled framing - #172
pseudomuto wants to merge 1 commit into
Conversation
Decrypt receives nothing but the ciphertext Encrypt returned, so anything an extension server needs to find its wrapping key again has to ride inside it. Every server writing one of these hits that problem and answers it with its own solution. For example, the example used a binary frame, with a version byte, a length-prefixed namespace, and the sealed DEK. Add api.ext.v1.KeyMaterial as an optional framing to use instead. The wrapped DEK is the only required field; version and namespace cover what a server needs (or may need) to select the key again. The `opaque` field belongs to the server itself for custom data and is round-tripped untouched, so one that needs more than those two fields doesn't need to roll their own solution necessarily. Everything but the DEK is metadata in the clear, which the message says plainly, since this is framing and not encryption. Switch the kms example over, which drops the frame offsets, the uint16 namespace length check, and the version byte. The nonce goes in opaque, where it doubles as a worked example of that field. The example derives its key from the version the material carries rather than checking it, so bumping currentVersion seals new payloads under a new key while everything already sealed keeps opening. This simulates the way a provider backed by a real key service (e.g. Vault) would address a key version.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
| if len(ciphertext) < headerSize { | ||
| return nil, errors.New("server: ciphertext is too short to hold a header") | ||
| material := &extv1.KeyMaterial{ | ||
| EncryptedDek: gcm.Seal(nil, nonce, dek, []byte(namespace)), |
There was a problem hiding this comment.
This is a demo, but IRL, passing nonce here instead of nil would include it in the ciphertext and make it available during decryption. I omitted it to show how opaque works.
| // it. Rather than hand-roll a binary frame, a server can marshal this message as | ||
| // its ciphertext and unmarshal it on the way back. | ||
| // | ||
| // The fields other than encrypted_dek are metadata in the clear: this is |
There was a problem hiding this comment.
Can we clarify here that if cleartext metadata is used for key selection or decrypt, changing it must cause decrypt to fail? KeyMaterial only defines how these fields are stored; it does not protect them on its own. NewKeyWrapper handles this by including the fields in AEAD additional data. An extension that uses KeyMaterial directly would need to get the same protection from its own wrapping operation or key-management backend.
There was a problem hiding this comment.
The other fields (version, namespace, opaque) aren't used for decryption (directly) and are not sensitive or secret values. Instead, they identify which key produced the encrypted dek. Changing any of them would cause decryption to fail because it couldn't find the key.
Adding additional data (AD) ensures that if the values were tampered with, decryption would fail even if the correct key was somehow found (e.g., the implementation provides a fallback key). This is because we use the values from the proto to construct the AD, which, if different, would preemptively fail to decrypt.
3 participants
Decrypt receives only the ciphertext Encrypt returns, so anything an extension server needs to find its wrapping key again has to ride inside it. Every server writing one of these hits that problem and answers it with its own solution. For example, the example used a binary frame, with a version byte, a length-prefixed namespace, and the sealed DEK.
Add api.ext.v1.KeyMaterial as an optional framing to use instead. The wrapped DEK is the only required field; version and namespace cover what a server needs (or may need) to select the key again. The
opaquefield belongs to the server for custom data and is round-tripped untouched, so anything that needs more than those two fields doesn't necessarily need to roll its own solution. Everything but the DEK is metadata in the clear, as the message says plainly, since this is framing, not encryption.Switch the KMS example over, which drops the frame offsets, the uint16 namespace length check, and the version byte. The nonce goes in opaque (it's not a secret) simply as a working example of that field. The example derives its key from the version the material carries rather than just validating it. Bumping
currentVersionseals new payloads under a new key while everything already sealed keeps opening. This simulates how a provider backed by a real key service (e.g., Vault) would handle key versions.