Updates to the Amazon S3 Encryption Client - This change includes fix…

…es for issues that were reported by Sophie Schmieg from the Google ISE team, and for issues that were discovered by AWS Cryptography.

CHANGELOG_PENDING.md

@@ -1,4 +1,5 @@
 ### SDK Features
+* `service/s3/s3crypto`: Updates to the Amazon S3 Encryption Client - This change includes fixes for issues that were reported by Sophie Schmieg from the Google ISE team, and for issues that were discovered by AWS Cryptography.
 
 ### SDK Enhancements
 

aws/csm/reporter_test.go

@@ -183,7 +183,7 @@ func TestReportingMetrics(t *testing.T) {
 				{
 					"Type":                "ApiCallAttempt",
 					"SdkException":        request.ErrCodeRequestError,
-					"SdkExceptionMessage": request.ErrCodeRequestError+": sdk error",
+					"SdkExceptionMessage": request.ErrCodeRequestError + ": sdk error",
 					"HttpStatusCode":      float64(500),
 				},
 				{

aws/ec2metadata/service.go

@@ -41,7 +41,7 @@ const (
 	enableTokenProviderHandlerName = "enableTokenProviderHandler"
 
 	// TTL constants
-	defaultTTL = 21600 * time.Second
+	defaultTTL          = 21600 * time.Second
 	ttlExpirationWindow = 30 * time.Second
 )
 

service/s3/doc_custom.go

@@ -104,19 +104,6 @@
 // content from S3. The Encryption and Decryption clients can be used concurrently
 // once the client is created.
 //
-//    sess := session.Must(session.NewSession())
-//
-//    // Create the decryption client.
-//    svc := s3crypto.NewDecryptionClient(sess)
-//
-//    // The object will be downloaded from S3 and decrypted locally. By metadata
-//    // about the object's encryption will instruct the decryption client how
-//    // decrypt the content of the object. By default KMS is used for keys.
-//    result, err := svc.GetObject(&s3.GetObjectInput {
-//        Bucket: aws.String(myBucket),
-//        Key: aws.String(myKey),
-//    })
-//
 // See the s3crypto package documentation for more information.
 // https://docs.aws.amazon.com/sdk-for-go/api/service/s3/s3crypto/
 //

service/s3/s3crypto/aes_cbc_content_cipher.go

@@ -2,7 +2,6 @@ package s3crypto
 
 import (
 	"io"
-	"strings"
 )
 
 const (
@@ -15,18 +14,38 @@ type cbcContentCipherBuilder struct {
 	padder    Padder
 }
 
-func (cbcContentCipherBuilder) isUsingDeprecatedFeatures() error {
-	return errDeprecatedCipherBuilder
-}
-
-// AESCBCContentCipherBuilder returns a new encryption only mode structure with a specific cipher
-// for the master key
+// AESCBCContentCipherBuilder returns a new encryption only AES/CBC mode structure using the provided padder. The provided cipher data generator
+// will be used to provide keys for content encryption.
 //
-// deprecated: This content cipher builder has been deprecated. Users should migrate to AESGCMContentCipherBuilder
+// deprecated: This feature is in maintenance mode, no new updates will be released. Please see https://docs.aws.amazon.com/general/latest/gr/aws_sdk_cryptography.html for more information.
 func AESCBCContentCipherBuilder(generator CipherDataGenerator, padder Padder) ContentCipherBuilder {
 	return cbcContentCipherBuilder{generator: generator, padder: padder}
 }
 
+// RegisterAESCBCContentCipher registers the AES/CBC cipher and padder with the provided CryptoRegistry.
+//
+// Example:
+//	cr := s3crypto.NewCryptoRegistry()
+//	if err := s3crypto.RegisterAESCBCContentCipher(cr, s3crypto.AESCBCPadder); err != nil {
+//		panic(err) // handle error
+//	}
+//
+// deprecated: This feature is in maintenance mode, no new updates will be released. Please see https://docs.aws.amazon.com/general/latest/gr/aws_sdk_cryptography.html for more information.
+func RegisterAESCBCContentCipher(registry *CryptoRegistry, padder Padder) error {
+	if registry == nil {
+		return errNilCryptoRegistry
+	}
+	name := AESCBC + "/" + padder.Name()
+	err := registry.AddCEK(name, newAESCBCContentCipher)
+	if err != nil {
+		return err
+	}
+	if err := registry.AddPadder(name, padder); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (builder cbcContentCipherBuilder) ContentCipher() (ContentCipher, error) {
 	cd, err := builder.generator.GenerateCipherData(cbcKeySize, cbcNonceSize)
 	if err != nil {
@@ -37,11 +56,22 @@ func (builder cbcContentCipherBuilder) ContentCipher() (ContentCipher, error) {
 	return newAESCBCContentCipher(cd)
 }
 
+func (builder cbcContentCipherBuilder) isAWSFixture() bool {
+	return true
+}
+
+func (cbcContentCipherBuilder) isEncryptionVersionCompatible(version clientVersion) error {
+	if version != v1ClientVersion {
+		return errDeprecatedIncompatibleCipherBuilder
+	}
+	return nil
+}
+
 // newAESCBCContentCipher will create a new aes cbc content cipher. If the cipher data's
-// will set the CEK algorithm if it hasn't been set.
+// will set the cek algorithm if it hasn't been set.
 func newAESCBCContentCipher(cd CipherData) (ContentCipher, error) {
 	if len(cd.CEKAlgorithm) == 0 {
-		cd.CEKAlgorithm = strings.Join([]string{AESCBC, cd.Padder.Name()}, "/")
+		cd.CEKAlgorithm = AESCBC + "/" + cd.Padder.Name()
 	}
 	cipher, err := newAESCBC(cd, cd.Padder)
 	if err != nil {
@@ -77,3 +107,11 @@ func (cc *aesCBCContentCipher) DecryptContents(src io.ReadCloser) (io.ReadCloser
 func (cc aesCBCContentCipher) GetCipherData() CipherData {
 	return cc.CipherData
 }
+
+var (
+	_ ContentCipherBuilder        = (*cbcContentCipherBuilder)(nil)
+	_ compatibleEncryptionFixture = (*cbcContentCipherBuilder)(nil)
+	_ awsFixture                  = (*cbcContentCipherBuilder)(nil)
+
+	_ ContentCipher = (*aesCBCContentCipher)(nil)
+)

service/s3/s3crypto/aes_cbc_content_cipher_test.go

@@ -1,14 +1,13 @@
-package s3crypto_test
+package s3crypto
 
 import (
+	"strings"
 	"testing"
-
-	"github.com/aws/aws-sdk-go/service/s3/s3crypto"
 )
 
 func TestAESCBCBuilder(t *testing.T) {
 	generator := mockGenerator{}
-	builder := s3crypto.AESCBCContentCipherBuilder(generator, s3crypto.NoPadder)
+	builder := AESCBCContentCipherBuilder(generator, NoPadder)
 	if builder == nil {
 		t.Fatal(builder)
 	}
@@ -18,3 +17,64 @@ func TestAESCBCBuilder(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestAesCBCContentCipher_isFixtureEncryptionCompatible(t *testing.T) {
+	generator := mockGenerator{}
+	builder := AESCBCContentCipherBuilder(generator, NoPadder)
+	if builder == nil {
+		t.Fatal("expected builder to not be nil")
+	}
+
+	compatibility, ok := builder.(compatibleEncryptionFixture)
+	if !ok {
+		t.Fatal("expected builder to implement compatibleEncryptionFixture interface")
+	}
+
+	if err := compatibility.isEncryptionVersionCompatible(v1ClientVersion); err != nil {
+		t.Errorf("expected builder to be compatible with v1 client")
+	}
+
+	if err := compatibility.isEncryptionVersionCompatible(v2ClientVersion); err == nil {
+		t.Errorf("expected builder to not be compatible with v2 client")
+	}
+}
+
+func TestRegisterAESCBCContentCipher(t *testing.T) {
+	cr := NewCryptoRegistry()
+	padder := AESCBCPadder
+	err := RegisterAESCBCContentCipher(cr, padder)
+	if err != nil {
+		t.Fatalf("expected no error, got %v", err)
+	}
+
+	if v, ok := cr.GetCEK("AES/CBC/PKCS5Padding"); !ok {
+		t.Fatal("expected cek algorithm handler to registered")
+	} else if v == nil {
+		t.Fatal("expected non-nil cek handler to be registered")
+	}
+
+	if v, ok := cr.GetPadder("AES/CBC/PKCS5Padding"); !ok {
+		t.Fatal("expected padder to be registered")
+	} else if v != padder {
+		t.Fatal("padder did not match provided value")
+	}
+
+	// try to register padder again
+	err = RegisterAESCBCContentCipher(cr, padder)
+	if err == nil {
+		t.Fatal("expected error, got none")
+	} else if !strings.Contains(err.Error(), "duplicate cek registry entry") {
+		t.Errorf("expected duplicate cek entry, got %v", err)
+	}
+
+	// try to regster padder with cek removed but padder entry still present
+	if _, ok := cr.RemoveCEK("AES/CBC/PKCS5Padding"); !ok {
+		t.Fatalf("expected value to be removed")
+	}
+	err = RegisterAESCBCContentCipher(cr, padder)
+	if err == nil {
+		t.Fatal("expected error, got none")
+	} else if !strings.Contains(err.Error(), "duplicate padder registry entry") {
+		t.Errorf("expected duplicate padder entry, got %v", err)
+	}
+}

service/s3/s3crypto/aes_gcm_content_cipher.go

@@ -1,6 +1,7 @@
 package s3crypto
 
 import (
+	"fmt"
 	"io"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -11,21 +12,62 @@ const (
 	gcmNonceSize = 12
 )
 
-type gcmContentCipherBuilder struct {
-	generator CipherDataGenerator
+// AESGCMContentCipherBuilder returns a new encryption only AES/GCM mode structure with a specific cipher data generator
+// that will provide keys to be used for content encryption.
+//
+// Note: This uses the Go stdlib AEAD implementation for AES/GCM. Due to this objects to be encrypted or decrypted
+// will be fully loaded into memory before encryption or decryption can occur. Caution must be taken to avoid memory
+// allocation failures.
+//
+// deprecated: This feature is in maintenance mode, no new updates will be released. Please see https://docs.aws.amazon.com/general/latest/gr/aws_sdk_cryptography.html for more information.
+func AESGCMContentCipherBuilder(generator CipherDataGenerator) ContentCipherBuilder {
+	return gcmContentCipherBuilder{generator}
+}
+
+// AESGCMContentCipherBuilderV2 returns a new encryption only AES/GCM mode structure with a specific cipher data generator
+// that will provide keys to be used for content encryption. This type is compatible with the V2 encryption client.
+//
+// Note: This uses the Go stdlib AEAD implementation for AES/GCM. Due to this objects to be encrypted or decrypted
+// will be fully loaded into memory before encryption or decryption can occur. Caution must be taken to avoid memory
+// allocation failures.
+func AESGCMContentCipherBuilderV2(generator CipherDataGeneratorWithCEKAlg) ContentCipherBuilder {
+	return gcmContentCipherBuilderV2{generator}
 }
 
-func (builder gcmContentCipherBuilder) isUsingDeprecatedFeatures() error {
-	if feature, ok := builder.generator.(deprecatedFeatures); ok {
-		return feature.isUsingDeprecatedFeatures()
+// RegisterAESGCMContentCipher registers the AES/GCM content cipher algorithm with the provided CryptoRegistry.
+//
+// Example:
+//	cr := s3crypto.NewCryptoRegistry()
+//	if err := s3crypto.RegisterAESGCMContentCipher(cr); err != nil {
+//		panic(err) // handle error
+//	}
+//
+func RegisterAESGCMContentCipher(registry *CryptoRegistry) error {
+	if registry == nil {
+		return errNilCryptoRegistry
+	}
+
+	err := registry.AddCEK(AESGCMNoPadding, newAESGCMContentCipher)
+	if err != nil {
+		return err
+	}
+
+	// NoPadder is generic but required by this algorithm, so if it is already registered and is the expected implementation
+	// don't error.
+	padderName := NoPadder.Name()
+	if v, ok := registry.GetPadder(padderName); !ok {
+		if err := registry.AddPadder(padderName, NoPadder); err != nil {
+			return err
+		}
+	} else if _, ok := v.(noPadder); !ok {
+		return fmt.Errorf("%s is already registred but does not match expected type %T", padderName, NoPadder)
 	}
 	return nil
 }
 
-// AESGCMContentCipherBuilder returns a new encryption only mode structure with a specific cipher
-// for the master key
-func AESGCMContentCipherBuilder(generator CipherDataGenerator) ContentCipherBuilder {
-	return gcmContentCipherBuilder{generator}
+// gcmContentCipherBuilder is a AES/GCM content cipher to be used with the V1 client CipherDataGenerator interface
+type gcmContentCipherBuilder struct {
+	generator CipherDataGenerator
 }
 
 func (builder gcmContentCipherBuilder) ContentCipher() (ContentCipher, error) {
@@ -37,10 +79,6 @@ func (builder gcmContentCipherBuilder) ContentCipherWithContext(ctx aws.Context)
 	var err error
 
 	switch v := builder.generator.(type) {
-	case CipherDataGeneratorWithCEKAlgWithContext:
-		cd, err = v.GenerateCipherDataWithCEKAlgWithContext(ctx, gcmKeySize, gcmNonceSize, AESGCMNoPadding)
-	case CipherDataGeneratorWithCEKAlg:
-		cd, err = v.GenerateCipherDataWithCEKAlg(gcmKeySize, gcmNonceSize, AESGCMNoPadding)
 	case CipherDataGeneratorWithContext:
 		cd, err = v.GenerateCipherDataWithContext(ctx, gcmKeySize, gcmNonceSize)
 	default:
@@ -53,6 +91,52 @@ func (builder gcmContentCipherBuilder) ContentCipherWithContext(ctx aws.Context)
 	return newAESGCMContentCipher(cd)
 }
 
+// isFixtureEncryptionCompatible will ensure that this type may only be used with the V1 client
+func (builder gcmContentCipherBuilder) isEncryptionVersionCompatible(version clientVersion) error {
+	if version != v1ClientVersion {
+		return errDeprecatedIncompatibleCipherBuilder
+	}
+	return nil
+}
+
+func (builder gcmContentCipherBuilder) isAWSFixture() bool {
+	return true
+}
+
+// gcmContentCipherBuilderV2 return a new builder for encryption content using AES/GCM/NoPadding. This type is meant
+// to be used with key wrapping implementations that allow the cek algorithm to be provided when calling the
+// cipher data generator.
+type gcmContentCipherBuilderV2 struct {
+	generator CipherDataGeneratorWithCEKAlg
+}
+
+func (builder gcmContentCipherBuilderV2) ContentCipher() (ContentCipher, error) {
+	return builder.ContentCipherWithContext(aws.BackgroundContext())
+}
+
+func (builder gcmContentCipherBuilderV2) ContentCipherWithContext(ctx aws.Context) (ContentCipher, error) {
+	cd, err := builder.generator.GenerateCipherDataWithCEKAlg(ctx, gcmKeySize, gcmNonceSize, AESGCMNoPadding)
+	if err != nil {
+		return nil, err
+	}
+
+	return newAESGCMContentCipher(cd)
+}
+
+// isFixtureEncryptionCompatible will ensure that this type may only be used with the V2 client
+func (builder gcmContentCipherBuilderV2) isEncryptionVersionCompatible(version clientVersion) error {
+	if version != v2ClientVersion {
+		return errDeprecatedIncompatibleCipherBuilder
+	}
+	return nil
+}
+
+// isAWSFixture will return whether this type was constructed with an AWS provided CipherDataGenerator
+func (builder gcmContentCipherBuilderV2) isAWSFixture() bool {
+	v, ok := builder.generator.(awsFixture)
+	return ok && v.isAWSFixture()
+}
+
 func newAESGCMContentCipher(cd CipherData) (ContentCipher, error) {
 	cd.CEKAlgorithm = AESGCMNoPadding
 	cd.TagLength = "128"
@@ -91,3 +175,25 @@ func (cc *aesGCMContentCipher) DecryptContents(src io.ReadCloser) (io.ReadCloser
 func (cc aesGCMContentCipher) GetCipherData() CipherData {
 	return cc.CipherData
 }
+
+// assert ContentCipherBuilder implementations
+var (
+	_ ContentCipherBuilder = (*gcmContentCipherBuilder)(nil)
+	_ ContentCipherBuilder = (*gcmContentCipherBuilderV2)(nil)
+)
+
+// assert ContentCipherBuilderWithContext implementations
+var (
+	_ ContentCipherBuilderWithContext = (*gcmContentCipherBuilder)(nil)
+	_ ContentCipherBuilderWithContext = (*gcmContentCipherBuilderV2)(nil)
+)
+
+// assert ContentCipher implementations
+var (
+	_ ContentCipher = (*aesGCMContentCipher)(nil)
+)
+
+// assert awsFixture implementations
+var (
+	_ awsFixture = (*gcmContentCipherBuilderV2)(nil)
+)