secrets/gcpkms: add support for additional authenticated data (AAD) #3658
Description
First of all I'd like to take a moment to thank all the contributors and maintainers of the Go Cloud library. It is a fantastic resource available to the community and has been an absolutely wonderful, well documented, easy-to-use, and well structured library for our purposes. Specifically, the foresight to provide concrete types on top of driver interfaces for both flexibility and common operations, with the additional ability to break out with the As methodology, simply chef's kiss.
Thank you!!
Is your feature request related to a problem? Please describe.
GCP has support for what's called 'Additional Authenticated Data' within the KMS product:
https://docs.cloud.google.com/kms/docs/additional-authenticated-data
Additional authenticated data (AAD) is any string that you pass to Cloud Key Management Service as part of an encrypt or decrypt request. AAD is used as an integrity check and can help protect your data from a confused deputy attack. The AAD string must be no larger than 64 KiB.
In the SDK, this is provided as a []byte in the EncryptRequest and DecryptRequest structs:
// Optional. Optional data that must match the data originally supplied in
// [EncryptRequest.additional_authenticated_data][google.cloud.kms.v1.EncryptRequest.additional_authenticated_data].
AdditionalAuthenticatedData [][byte](https://pkg.go.dev/builtin#byte)
There is currently support for this type of data in the AWS KMS keeper via what's called Encryption Context (added in PR 3154), which is used in the Encrypt and Decrypt methods:
https://github.com/google/go-cloud/blob/master/secrets/awskms/kms.go#L238
type KeeperOptions struct {
// EncryptionContext parameters.
// See https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html#encrypt_context.
EncryptionContext map[string]string
}
But, there is currently no support for this similar feature within the GCP KMS keeper. It would be mighty fine if something similar was available.
Describe the solution you'd like
Summary
At the simplest level, allow a additional authentication data []byte to be provided within the existing gcpkms.KeeperOptions and then use that value within the Encrypt and Decrypt methods.
Details
To flesh out the idea a bit further...
Keeper Opts
Modify the gcpkms.KeeperOptions to hold this data:
type KeeperOptions struct {
// AdditionalAuthenticatedData is non-secret data used as an optional integrity check which must
// match between encrypt and decrypt operations. This prevents confused deputy attacks by
// binding context to encrypted data without being stored in the ciphertext.
// See here for more info: https://docs.cloud.google.com/kms/docs/additional-authenticated-data
AdditionalAuthenticatedData []byte
}
Update the unexported keeper struct to hold the options provided during construction:
type keeper struct {
keyResourceID string
client *cloudkms.KeyManagementClient
opts KeeperOptions
}
Modify the OpenKeeper so the keeper's field is set and defensively checks for nil opts:
func OpenKeeper(client *cloudkms.KeyManagementClient, keyResourceID string, opts *KeeperOptions) *secrets.Keeper {
if opts == nil {
opts = &KeeperOptions{}
}
return secrets.NewKeeper(&keeper{
keyResourceID: keyResourceID,
client: client,
opts: *opts,
})
}
Method Updates
Update the Encrypt method to use this option:
func (k *keeper) Encrypt(ctx context.Context, plaintext []byte) ([]byte, error) {
req := &kmspb.EncryptRequest{
Name: k.keyResourceID,
Plaintext: plaintext,
AdditionalAuthenticatedData: k.opts.AdditionalAuthenticatedData,
}
...
Do the same for the Decrypt method:
func (k *keeper) Decrypt(ctx context.Context, ciphertext []byte) ([]byte, error) {
req := &kmspb.DecryptRequest{
Name: k.keyResourceID,
Ciphertext: ciphertext,
AdditionalAuthenticatedData: k.opts.AdditionalAuthenticatedData,
}
...
URL Changes
We'll almost certainly want to allow providing this value via a URL, so a modification there is probably desirable. One option is to allow the value to come in as a Base64 URL encoded string, which we can decode and store as part of OpenKeeperURL:
func (o *URLOpener) OpenKeeperURL(ctx context.Context, u *url.URL) (*secrets.Keeper, error) {
q := u.Query()
for param := range q {
if param != "additional_authenticated_data" {
return nil, fmt.Errorf("open keeper %v: invalid query parameter %q", u, param)
}
}
opts := o.Options
if aad := q.Get("additional_authenticated_data"); aad != "" {
dec, err := base64.URLEncoding.DecodeString(aad)
if err != nil {
return nil, fmt.Errorf("base64 url decoding query parameter \"additional_authenticated_data\": %w", err)
}
opts.AdditionalAuthenticatedData = dec
}
return OpenKeeper(o.Client, path.Join(u.Host, u.Path), &opts), nil
}
We'll then want to update docs on the URLOpener so they're accurate:
// URLOpener opens GCP KMS URLs like
// "gcpkms://projects/[PROJECT_ID]/locations/[LOCATION]/keyRings/[KEY_RING]/cryptoKeys/[KEY]".
//
// The URL host+path are used as the key resource ID; see
// https://cloud.google.com/kms/docs/object-hierarchy#key for more details.
//
// The following query parameters are supported:
//
// - additional_authenticated_data: a base64 URL encoded string of AAD provided to encrypt and
// decrypt calls
type URLOpener struct {
Tests
And then tests can be updated to fix MakeDriver's keeper construction:
func (h *harness) MakeDriver(ctx context.Context) (driver.Keeper, driver.Keeper, error) {
return &keeper{KeyResourceID(project, location, keyRing, keyID1), h.client, KeeperOptions{}},
&keeper{KeyResourceID(project, location, keyRing, keyID2), h.client, KeeperOptions{}}, nil
}
And a test to validate that the query param is properly parsed:
func TestOpenKeeper(t *testing.T) {
cleanup := setup.FakeGCPDefaultCredentials(t)
defer cleanup()
tests := []struct {
URL string
WantErr bool
}{
// OK.
{"gcpkms://projects/MYPROJECT/locations/MYLOCATION/keyRings/MYKEYRING/cryptoKeys/MYKEY", false},
// Valid base64 url encoded additional_authenticated_data.
{"gcpkms://projects/MYPROJECT/locations/MYLOCATION/keyRings/MYKEYRING/cryptoKeys/MYKEY?additional_authenticated_data=aGVsbG8gd29ybGQh", false},
// Invalid base64 url encoded additional_authenticated_data.
{"gcpkms://projects/MYPROJECT/locations/MYLOCATION/keyRings/MYKEYRING/cryptoKeys/MYKEY?additional_authenticated_data=test@123", true},
// Invalid query parameter.
{"gcpkms://projects/MYPROJECT/locations/MYLOCATION/keyRings/MYKEYRING/cryptoKeys/MYKEY?param=val", true},
}
...
Describe alternatives you've considered
-
Prefix-based URL param approach (like
aad_...), similar to the AWS implementation (which usescontext_...)- Pro - easier for a human to deduce what data is being provided as AAD
- Pro - more closely mirrors AWS implementations if services are using AWS and GCP, their URLs can be more similar
- Con - forces a map-like structure onto the data that can be any slice of bytes
- Con - requires the library to choose an explicit encoding strategy when translating to what the SDK needs (I mean it can be JSON, but still)
-
(similar to above) Alternative type for
AdditionalAuthenticatedDatain theKeeperOptions, like amap[string]string- Basically precipitates and the same pros/cons from the above
-
Alternative URL encoding methodology (like hex)
- More space efficient to use base64 encoding and hex doesn't seem to provide any real benefit
Additional context
Note, happy to supply a PR if desired! Figured it was worth a chat first in an issue before just submitting a PR, in case there are disagreements regarding supporting this type of functionality or concerns with the overall approach.