Add support for AWS S3 Server Side Encryption (SSE-KMS)

Prior to this change, uploads to AWS S3 were only encrypted on the
server if a default encryption were specified on the bucket.

With this change, admins can now configure the encryption and the AWS
Key Management Service (KMS) key ID in GitLab Rails, and the
configuration will be used in uploads. Bucket policies to enforce
encryption can now be used since Workhorse sends the required headers
(`x-amz-server-side-encryption` and
`x-amz-server-side-encryption-aws-kms-key-id`). The bucket policy cannot
be enforced with default encryption, since that is applied after the
check.

This requires the changes in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38240 to work.

Part of https://gitlab.com/gitlab-org/gitlab/-/issues/22200

changelogs/unreleased/sh-add-s3-encryption.yml

+---
+title: Add support for AWS S3 Server Side Encryption (SSE-KMS)
+merge_request: 537
+author:
+type: added

internal/config/config.go

@@ -39,11 +39,13 @@ type S3Credentials struct {
 }
 
 type S3Config struct {
-	Region        string `toml:"-"`
-	Bucket        string `toml:"-"`
-	PathStyle     bool   `toml:"-"`
-	Endpoint      string `toml:"-"`
-	UseIamProfile bool   `toml:"-"`
+	Region               string `toml:"-"`
+	Bucket               string `toml:"-"`
+	PathStyle            bool   `toml:"-"`
+	Endpoint             string `toml:"-"`
+	UseIamProfile        bool   `toml:"-"`
+	ServerSideEncryption string `toml:"-"` // Server-side encryption mode (e.g. AES256, aws:kms)
+	SSEKMSKeyID          string `toml:"-"` // Server-side encryption key-management service key ID (e.g. arn:aws:xxx)
 }
 
 type RedisConfig struct {

internal/filestore/file_handler_test.go

@@ -276,7 +276,7 @@ func TestSaveFile(t *testing.T) {
 }
 
 func TestSaveFileWithWorkhorseClient(t *testing.T) {
-	s3Creds, s3Config, sess, ts := test.SetupS3(t)
+	s3Creds, s3Config, sess, ts := test.SetupS3(t, "")
 	defer ts.Close()
 
 	ctx, cancel := context.WithCancel(context.Background())

internal/objectstore/s3_object.go

@@ -22,6 +22,16 @@ type S3Object struct {
 	uploader
 }
 
+func setEncryptionOptions(input *s3manager.UploadInput, s3Config config.S3Config) {
+	if s3Config.ServerSideEncryption != "" {
+		input.ServerSideEncryption = aws.String(s3Config.ServerSideEncryption)
+
+		if s3Config.ServerSideEncryption == s3.ServerSideEncryptionAwsKms && s3Config.SSEKMSKeyID != "" {
+			input.SSEKMSKeyId = aws.String(s3Config.SSEKMSKeyID)
+		}
+	}
+}
+
 func NewS3Object(ctx context.Context, objectName string, s3Credentials config.S3Credentials, s3Config config.S3Config, deadline time.Time) (*S3Object, error) {
 	pr, pw := io.Pipe()
 	objectStorageUploadsOpen.Inc()
@@ -54,11 +64,15 @@ func NewS3Object(ctx context.Context, objectName string, s3Credentials config.S3
 		o.objectName = objectName
 		uploader := s3manager.NewUploader(sess)
 
-		_, err = uploader.UploadWithContext(uploadCtx, &s3manager.UploadInput{
+		input := &s3manager.UploadInput{
 			Bucket: aws.String(s3Config.Bucket),
 			Key:    aws.String(objectName),
 			Body:   pr,
-		})
+		}
+
+		setEncryptionOptions(input, s3Config)
+
+		_, err = uploader.UploadWithContext(uploadCtx, input)
 		if err != nil {
 			o.uploadError = err
 			objectStorageUploadRequestsRequestFailed.Inc()

internal/objectstore/s3_object_test.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws/session"
+	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/config"
@@ -21,52 +22,65 @@ import (
 )
 
 func TestS3ObjectUpload(t *testing.T) {
-	creds, config, sess, ts := test.SetupS3(t)
-	defer ts.Close()
-
-	deadline := time.Now().Add(testTimeout)
-	tmpDir, err := ioutil.TempDir("", "workhorse-test-")
-	require.NoError(t, err)
-	defer os.Remove(tmpDir)
+	testCases := []struct {
+		encryption string
+	}{
+		{encryption: ""},
+		{encryption: s3.ServerSideEncryptionAes256},
+		{encryption: s3.ServerSideEncryptionAwsKms},
+	}
 
-	objectName := filepath.Join(tmpDir, "s3-test-data")
-	ctx, cancel := context.WithCancel(context.Background())
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("encryption=%v", tc.encryption), func(t *testing.T) {
+			creds, config, sess, ts := test.SetupS3(t, tc.encryption)
+			defer ts.Close()
 
-	object, err := objectstore.NewS3Object(ctx, objectName, creds, config, deadline)
-	require.NoError(t, err)
+			deadline := time.Now().Add(testTimeout)
+			tmpDir, err := ioutil.TempDir("", "workhorse-test-")
+			require.NoError(t, err)
+			defer os.Remove(tmpDir)
 
-	// copy data
-	n, err := io.Copy(object, strings.NewReader(test.ObjectContent))
-	require.NoError(t, err)
-	require.Equal(t, test.ObjectSize, n, "Uploaded file mismatch")
+			objectName := filepath.Join(tmpDir, "s3-test-data")
+			ctx, cancel := context.WithCancel(context.Background())
 
-	// close HTTP stream
-	err = object.Close()
-	require.NoError(t, err)
+			object, err := objectstore.NewS3Object(ctx, objectName, creds, config, deadline)
+			require.NoError(t, err)
 
-	test.S3ObjectExists(t, sess, config, objectName, test.ObjectContent)
+			// copy data
+			n, err := io.Copy(object, strings.NewReader(test.ObjectContent))
+			require.NoError(t, err)
+			require.Equal(t, test.ObjectSize, n, "Uploaded file mismatch")
 
-	cancel()
-	deleted := false
-
-	retry(3, time.Second, func() error {
-		if test.S3ObjectDoesNotExist(t, sess, config, objectName) {
-			deleted = true
-			return nil
-		} else {
-			return fmt.Errorf("file is still present, retrying")
-		}
-	})
+			// close HTTP stream
+			err = object.Close()
+			require.NoError(t, err)
 
-	require.True(t, deleted)
+			test.S3ObjectExists(t, sess, config, objectName, test.ObjectContent)
+			test.CheckS3Metadata(t, sess, config, objectName)
+
+			cancel()
+			deleted := false
+
+			retry(3, time.Second, func() error {
+				if test.S3ObjectDoesNotExist(t, sess, config, objectName) {
+					deleted = true
+					return nil
+				} else {
+					return fmt.Errorf("file is still present, retrying")
+				}
+			})
+
+			require.True(t, deleted)
+		})
+	}
 }
 
 func TestConcurrentS3ObjectUpload(t *testing.T) {
-	creds, uploadsConfig, uploadsSession, uploadServer := test.SetupS3WithBucket(t, "uploads")
+	creds, uploadsConfig, uploadsSession, uploadServer := test.SetupS3WithBucket(t, "uploads", "")
 	defer uploadServer.Close()
 
 	// This will return a separate S3 endpoint
-	_, artifactsConfig, artifactsSession, artifactsServer := test.SetupS3WithBucket(t, "artifacts")
+	_, artifactsConfig, artifactsSession, artifactsServer := test.SetupS3WithBucket(t, "artifacts", "")
 	defer artifactsServer.Close()
 
 	deadline := time.Now().Add(testTimeout)
@@ -116,7 +130,7 @@ func TestConcurrentS3ObjectUpload(t *testing.T) {
 }
 
 func TestS3ObjectUploadCancel(t *testing.T) {
-	creds, config, _, ts := test.SetupS3(t)
+	creds, config, _, ts := test.SetupS3(t, "")
 	defer ts.Close()
 
 	ctx, cancel := context.WithCancel(context.Background())

internal/objectstore/test/s3_stub.go

@@ -21,11 +21,11 @@ import (
 	"github.com/johannesboyne/gofakes3/backend/s3mem"
 )
 
-func SetupS3(t *testing.T) (config.S3Credentials, config.S3Config, *session.Session, *httptest.Server) {
-	return SetupS3WithBucket(t, "test-bucket")
+func SetupS3(t *testing.T, encryption string) (config.S3Credentials, config.S3Config, *session.Session, *httptest.Server) {
+	return SetupS3WithBucket(t, "test-bucket", encryption)
 }
 
-func SetupS3WithBucket(t *testing.T, bucket string) (config.S3Credentials, config.S3Config, *session.Session, *httptest.Server) {
+func SetupS3WithBucket(t *testing.T, bucket string, encryption string) (config.S3Credentials, config.S3Config, *session.Session, *httptest.Server) {
 	backend := s3mem.New()
 	faker := gofakes3.New(backend)
 	ts := httptest.NewServer(faker.Server())
@@ -42,6 +42,14 @@ func SetupS3WithBucket(t *testing.T, bucket string) (config.S3Credentials, confi
 		PathStyle: true,
 	}
 
+	if encryption != "" {
+		config.ServerSideEncryption = encryption
+
+		if encryption == s3.ServerSideEncryptionAwsKms {
+			config.SSEKMSKeyID = "arn:aws:1234"
+		}
+	}
+
 	sess, err := session.NewSession(&aws.Config{
 		Credentials:      credentials.NewStaticCredentials(creds.AwsAccessKeyID, creds.AwsSecretAccessKey, ""),
 		Endpoint:         aws.String(ts.URL),
@@ -75,6 +83,29 @@ func S3ObjectExists(t *testing.T, sess *session.Session, config config.S3Config,
 	})
 }
 
+func CheckS3Metadata(t *testing.T, sess *session.Session, config config.S3Config, objectName string) {
+	// In a real S3 provider, s3crypto.NewDecryptionClient should probably be used
+	svc := s3.New(sess)
+	result, err := svc.GetObject(&s3.GetObjectInput{
+		Bucket: aws.String(config.Bucket),
+		Key:    aws.String(objectName),
+	})
+	require.NoError(t, err)
+
+	if config.ServerSideEncryption != "" {
+		require.Equal(t, aws.String(config.ServerSideEncryption), result.ServerSideEncryption)
+
+		if config.ServerSideEncryption == s3.ServerSideEncryptionAwsKms {
+			require.Equal(t, aws.String(config.SSEKMSKeyID), result.SSEKMSKeyId)
+		} else {
+			require.Nil(t, result.SSEKMSKeyId)
+		}
+	} else {
+		require.Nil(t, result.ServerSideEncryption)
+		require.Nil(t, result.SSEKMSKeyId)
+	}
+}
+
 // S3ObjectDoesNotExist returns true if the object has been deleted,
 // false otherwise. The return signature is different from
 // S3ObjectExists because deletion may need to be retried since deferred