16312: Use V4 signatures for all AWS regions. Add config override.

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 23d71204385af2e8d7a7a9ae17514b7223bea9c8..dff4fc7e34c7c7c51055db42dc98dbced6ee62df 100644 (file)
--- a/doc/admin/upgrading.html.textile.liquid
+++ b/doc/admin/upgrading.html.textile.liquid
@@ -40,6 +40,10 @@ h2(#master). development master (as of 2020-02-07)
 
 None in current development master.
 
+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 12f4bd9ded026471fe8fb8ed3d8641d1479d8b6e..ebe39e5b223d5dd3283ebae5fb8050dfe1549ace 100644 (file)
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -951,6 +951,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 88cb9139a23e0361444969fd307bf34d399c05d5..42c4374c0a13d53ef1c8610aaae59400df854ec5 100644 (file)
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -957,6 +957,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 69de3f05e231d29321ddbaa0b0f5f6dc1d5659e0..880a91ee6965fdd0d4adc2aef3d397a156caf821 100644 (file)
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -236,12 +236,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..5c642a942dba244c385d582276bac6be3b90c93b 100644 (file)
--- a/services/keepstore/s3_volume_test.go
+++ b/services/keepstore/s3_volume_test.go
@@ -122,13 +122,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 +470,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,