Merge branch 'sh-add-s3-encryption' into 'master'

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

See merge request gitlab-org/gitlab-workhorse!537

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