Merge branch '16312-s3-signature-v4'

closes #16312

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>

diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid
index 070e58983a50fc01c8943d6d29aa46b3d0453361..edd92fa0ea1a117a91d60f3453dc6c3bd146ca3d 100644 (file)
--- a/doc/admin/upgrading.html.textile.liquid
+++ b/doc/admin/upgrading.html.textile.liquid
@@ -44,6 +44,10 @@ The SSO (single sign-on) component is deprecated and will not be supported in fu
 
 After migrating your configuration, uninstall the @arvados-sso-provider@ package.
 
+h3. S3 signatures
+
+Keepstore now uses "V4 signatures":https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-authenticating-requests.html by default for S3 requests. If you are using Amazon S3, no action is needed; all regions support V4 signatures. If you are using a different S3-compatible service that does not support V4 signatures, add @V2Signature: true@ to your volume driver parameters to preserve the old behavior. See "configuring S3 object storage":{{site.baseurl}}/install/configure-s3-object-storage.html.
+
 h2(#v2_0_0). v2.0.0 (2020-02-07)
 
 "Upgrading from 1.4":#v1_4_1

diff --git a/doc/install/configure-s3-object-storage.html.textile.liquid b/doc/install/configure-s3-object-storage.html.textile.liquid
index e953f660fbc0defa81bc13ca34ab2138f4f7dc08..b960ac1fda0c2ab1fbaae77e4ae3c875b8dec0bc 100644 (file)
--- a/doc/install/configure-s3-object-storage.html.textile.liquid
+++ b/doc/install/configure-s3-object-storage.html.textile.liquid
@@ -59,6 +59,11 @@ Volumes are configured in the @Volumes@ section of the cluster configuration fil
           # declaration.
           LocationConstraint: false
 
+          # Use V2 signatures instead of the default V4. Amazon S3
+          # supports V4 signatures in all regions, but this option
+          # might be needed for other S3-compatible services.
+          V2Signature: false
+
           # Requested page size for "list bucket contents" requests.
           IndexPageSize: 1000
 

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 0efe49c1cb9331621ebb6d141ef55697c52464fe..204f7538bad5fc268d191f9fa58a38aa5f7389a0 100644 (file)
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -1020,6 +1020,7 @@ Clusters:
           Region: us-east-1a
           Bucket: aaaaa
           LocationConstraint: false
+          V2Signature: false
           IndexPageSize: 1000
           ConnectTimeout: 1m
           ReadTimeout: 10m

diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index c9d29f814ddddc6d3d443006b823199d2f3e9e93..ec5bc187d7625d504918d8feb8e23abf16e7018c 100644 (file)
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -1026,6 +1026,7 @@ Clusters:
           Region: us-east-1a
           Bucket: aaaaa
           LocationConstraint: false
+          V2Signature: false
           IndexPageSize: 1000
           ConnectTimeout: 1m
           ReadTimeout: 10m

diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 9f9f00e6445ec676b7ca19877cef1e7b304912e2..1efc87ea72ac6f67496e0b4df931905092f2c6fa 100644 (file)
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -259,12 +259,14 @@ type Volume struct {
 }
 
 type S3VolumeDriverParameters struct {
+       IAMRole            string
        AccessKey          string
        SecretKey          string
        Endpoint           string
        Region             string
        Bucket             string
        LocationConstraint bool
+       V2Signature        bool
        IndexPageSize      int
        ConnectTimeout     Duration
        ReadTimeout        Duration

diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 80aa5ec3bb8fe13fe449f8069afc5e0d306d9b11..96f2e7db3965704570f3906c78ab6e624072e013 100644 (file)
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -129,20 +129,9 @@ func s3regions() (okList []string) {
 
 // S3Volume implements Volume using an S3 bucket.
 type S3Volume struct {
-       AccessKey          string
-       SecretKey          string
-       AuthToken          string    // populated automatically when IAMRole is used
-       AuthExpiration     time.Time // populated automatically when IAMRole is used
-       IAMRole            string
-       Endpoint           string
-       Region             string
-       Bucket             string
-       LocationConstraint bool
-       IndexPageSize      int
-       ConnectTimeout     arvados.Duration
-       ReadTimeout        arvados.Duration
-       RaceWindow         arvados.Duration
-       UnsafeDelete       bool
+       arvados.S3VolumeDriverParameters
+       AuthToken      string    // populated automatically when IAMRole is used
+       AuthExpiration time.Time // populated automatically when IAMRole is used
 
        cluster   *arvados.Cluster
        volume    arvados.Volume
@@ -188,8 +177,7 @@ func (v *S3Volume) bootstrapIAMCredentials() error {
 func (v *S3Volume) newS3Client() *s3.S3 {
        auth := aws.NewAuth(v.AccessKey, v.SecretKey, v.AuthToken, v.AuthExpiration)
        client := s3.New(*auth, v.region)
-       if v.region.EC2Endpoint.Signer == aws.V4Signature {
-               // Currently affects only eu-central-1
+       if !v.V2Signature {
                client.Signature = aws.V4Signature
        }
        client.ConnectTimeout = time.Duration(v.ConnectTimeout)

diff --git a/services/keepstore/s3_volume_test.go b/services/keepstore/s3_volume_test.go
index 2c5cdf5b99fa3255d03626933d280ac2e7e21a8a..2736f00b743c791502f78886e716b521a0585eb1 100644 (file)
--- a/services/keepstore/s3_volume_test.go
+++ b/services/keepstore/s3_volume_test.go
@@ -101,6 +101,53 @@ func (s *StubbedS3Suite) TestIndex(c *check.C) {
        }
 }
 
+func (s *StubbedS3Suite) TestSignatureVersion(c *check.C) {
+       var header http.Header
+       stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+               header = r.Header
+       }))
+       defer stub.Close()
+
+       // Default V4 signature
+       vol := S3Volume{
+               S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
+                       AccessKey: "xxx",
+                       SecretKey: "xxx",
+                       Endpoint:  stub.URL,
+                       Region:    "test-region-1",
+                       Bucket:    "test-bucket-name",
+               },
+               cluster: s.cluster,
+               logger:  ctxlog.TestLogger(c),
+               metrics: newVolumeMetricsVecs(prometheus.NewRegistry()),
+       }
+       err := vol.check()
+       c.Check(err, check.IsNil)
+       err = vol.Put(context.Background(), "acbd18db4cc2f85cedef654fccc4a4d8", []byte("foo"))
+       c.Check(err, check.IsNil)
+       c.Check(header.Get("Authorization"), check.Matches, `AWS4-HMAC-SHA256 .*`)
+
+       // Force V2 signature
+       vol = S3Volume{
+               S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
+                       AccessKey:   "xxx",
+                       SecretKey:   "xxx",
+                       Endpoint:    stub.URL,
+                       Region:      "test-region-1",
+                       Bucket:      "test-bucket-name",
+                       V2Signature: true,
+               },
+               cluster: s.cluster,
+               logger:  ctxlog.TestLogger(c),
+               metrics: newVolumeMetricsVecs(prometheus.NewRegistry()),
+       }
+       err = vol.check()
+       c.Check(err, check.IsNil)
+       err = vol.Put(context.Background(), "acbd18db4cc2f85cedef654fccc4a4d8", []byte("foo"))
+       c.Check(err, check.IsNil)
+       c.Check(header.Get("Authorization"), check.Matches, `AWS xxx:.*`)
+}
+
 func (s *StubbedS3Suite) TestIAMRoleCredentials(c *check.C) {
        s.metadata = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
                upd := time.Now().UTC().Add(-time.Hour).Format(time.RFC3339)
@@ -122,13 +169,15 @@ func (s *StubbedS3Suite) TestIAMRoleCredentials(c *check.C) {
                w.WriteHeader(http.StatusNotFound)
        }))
        deadv := &S3Volume{
-               IAMRole:  s.metadata.URL + "/fake-metadata/test-role",
-               Endpoint: "http://localhost:12345",
-               Region:   "test-region-1",
-               Bucket:   "test-bucket-name",
-               cluster:  s.cluster,
-               logger:   ctxlog.TestLogger(c),
-               metrics:  newVolumeMetricsVecs(prometheus.NewRegistry()),
+               S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
+                       IAMRole:  s.metadata.URL + "/fake-metadata/test-role",
+                       Endpoint: "http://localhost:12345",
+                       Region:   "test-region-1",
+                       Bucket:   "test-bucket-name",
+               },
+               cluster: s.cluster,
+               logger:  ctxlog.TestLogger(c),
+               metrics: newVolumeMetricsVecs(prometheus.NewRegistry()),
        }
        err := deadv.check()
        c.Check(err, check.ErrorMatches, `.*/fake-metadata/test-role.*`)
@@ -468,19 +517,21 @@ func (s *StubbedS3Suite) newTestableVolume(c *check.C, cluster *arvados.Cluster,
 
        v := &TestableS3Volume{
                S3Volume: &S3Volume{
-                       AccessKey:          accessKey,
-                       SecretKey:          secretKey,
-                       IAMRole:            iamRole,
-                       Bucket:             TestBucketName,
-                       Endpoint:           endpoint,
-                       Region:             "test-region-1",
-                       LocationConstraint: true,
-                       UnsafeDelete:       true,
-                       IndexPageSize:      1000,
-                       cluster:            cluster,
-                       volume:             volume,
-                       logger:             ctxlog.TestLogger(c),
-                       metrics:            metrics,
+                       S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
+                               IAMRole:            iamRole,
+                               AccessKey:          accessKey,
+                               SecretKey:          secretKey,
+                               Bucket:             TestBucketName,
+                               Endpoint:           endpoint,
+                               Region:             "test-region-1",
+                               LocationConstraint: true,
+                               UnsafeDelete:       true,
+                               IndexPageSize:      1000,
+                       },
+                       cluster: cluster,
+                       volume:  volume,
+                       logger:  ctxlog.TestLogger(c),
+                       metrics: metrics,
                },
                c:           c,
                server:      srv,