Merge branch '16809-s3-v4-signature'
authorTom Clegg <tom@tomclegg.ca>
Tue, 6 Oct 2020 17:56:07 +0000 (13:56 -0400)
committerTom Clegg <tom@tomclegg.ca>
Tue, 6 Oct 2020 17:56:07 +0000 (13:56 -0400)
closes #16809

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

lib/install/deps.go
services/api/app/controllers/arvados/v1/api_client_authorizations_controller.rb
services/keep-web/s3.go
services/keep-web/s3_test.go
services/keep-web/s3aws_test.go [new file with mode: 0644]
services/keep-web/server_test.go

index 4378023158a085d0500b2d88164d0babc21671e7..15ff0607a9a927e9659ac581d786e393b51f6c44 100644 (file)
@@ -142,6 +142,8 @@ func (installCommand) RunCommand(prog string, args []string, stdin io.Reader, st
                        "postgresql",
                        "postgresql-contrib",
                        "python3-dev",
+                       "python3-venv",
+                       "python3-virtualenv",
                        "r-base",
                        "r-cran-testthat",
                        "r-cran-devtools",
@@ -150,8 +152,6 @@ func (installCommand) RunCommand(prog string, args []string, stdin io.Reader, st
                        "r-cran-roxygen2",
                        "r-cran-xml",
                        "sudo",
-                       "python3-virtualenv",
-                       "python3-venv",
                        "wget",
                        "xvfb",
                        "zlib1g-dev",
index cd466cf1fb2565b79e1f796d9981c0dd20750636..59e359232e834fbeb1f12a9c6daec6c52168debd 100644 (file)
@@ -81,7 +81,9 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
         val.is_a?(String) && (attr == 'uuid' || attr == 'api_token')
       }
     end
-    @objects = model_class.where('user_id=?', current_user.id)
+    if current_api_client_authorization.andand.api_token != Rails.configuration.SystemRootToken
+      @objects = model_class.where('user_id=?', current_user.id)
+    end
     if wanted_scopes.compact.any?
       # We can't filter on scopes effectively using AR/postgres.
       # Instead we get the entire result set, do our own filtering on
@@ -122,8 +124,8 @@ class Arvados::V1::ApiClientAuthorizationsController < ApplicationController
 
   def find_object_by_uuid
     uuid_param = params[:uuid] || params[:id]
-    if (uuid_param != current_api_client_authorization.andand.uuid and
-        not Thread.current[:api_client].andand.is_trusted)
+    if (uuid_param != current_api_client_authorization.andand.uuid &&
+        !Thread.current[:api_client].andand.is_trusted)
       return forbidden
     end
     @limit = 1
index 7cd1b1a896e3fa56eaa60db2f5d4851ac9430b39..49fb2456f5851662bec9573af6e06978d930d741 100644 (file)
 package main
 
 import (
+       "crypto/hmac"
+       "crypto/sha256"
        "encoding/xml"
        "errors"
        "fmt"
+       "hash"
        "io"
        "net/http"
+       "net/url"
        "os"
        "path/filepath"
        "sort"
        "strconv"
        "strings"
+       "time"
 
        "git.arvados.org/arvados.git/sdk/go/arvados"
        "git.arvados.org/arvados.git/sdk/go/ctxlog"
        "github.com/AdRoll/goamz/s3"
 )
 
-const s3MaxKeys = 1000
+const (
+       s3MaxKeys       = 1000
+       s3SignAlgorithm = "AWS4-HMAC-SHA256"
+       s3MaxClockSkew  = 5 * time.Minute
+)
+
+func hmacstring(msg string, key []byte) []byte {
+       h := hmac.New(sha256.New, key)
+       io.WriteString(h, msg)
+       return h.Sum(nil)
+}
+
+func hashdigest(h hash.Hash, payload string) string {
+       io.WriteString(h, payload)
+       return fmt.Sprintf("%x", h.Sum(nil))
+}
+
+// Signing key for given secret key and request attrs.
+func s3signatureKey(key, datestamp, regionName, serviceName string) []byte {
+       return hmacstring("aws4_request",
+               hmacstring(serviceName,
+                       hmacstring(regionName,
+                               hmacstring(datestamp, []byte("AWS4"+key)))))
+}
+
+// Canonical query string for S3 V4 signature: sorted keys, spaces
+// escaped as %20 instead of +, keyvalues joined with &.
+func s3querystring(u *url.URL) string {
+       keys := make([]string, 0, len(u.Query()))
+       values := make(map[string]string, len(u.Query()))
+       for k, vs := range u.Query() {
+               k = strings.Replace(url.QueryEscape(k), "+", "%20", -1)
+               keys = append(keys, k)
+               for _, v := range vs {
+                       v = strings.Replace(url.QueryEscape(v), "+", "%20", -1)
+                       if values[k] != "" {
+                               values[k] += "&"
+                       }
+                       values[k] += k + "=" + v
+               }
+       }
+       sort.Strings(keys)
+       for i, k := range keys {
+               keys[i] = values[k]
+       }
+       return strings.Join(keys, "&")
+}
+
+func s3stringToSign(alg, scope, signedHeaders string, r *http.Request) (string, error) {
+       timefmt, timestr := "20060102T150405Z", r.Header.Get("X-Amz-Date")
+       if timestr == "" {
+               timefmt, timestr = time.RFC1123, r.Header.Get("Date")
+       }
+       t, err := time.Parse(timefmt, timestr)
+       if err != nil {
+               return "", fmt.Errorf("invalid timestamp %q: %s", timestr, err)
+       }
+       if skew := time.Now().Sub(t); skew < -s3MaxClockSkew || skew > s3MaxClockSkew {
+               return "", errors.New("exceeded max clock skew")
+       }
+
+       var canonicalHeaders string
+       for _, h := range strings.Split(signedHeaders, ";") {
+               if h == "host" {
+                       canonicalHeaders += h + ":" + r.Host + "\n"
+               } else {
+                       canonicalHeaders += h + ":" + r.Header.Get(h) + "\n"
+               }
+       }
+
+       canonicalRequest := fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n%s", r.Method, r.URL.EscapedPath(), s3querystring(r.URL), canonicalHeaders, signedHeaders, r.Header.Get("X-Amz-Content-Sha256"))
+       ctxlog.FromContext(r.Context()).Debugf("s3stringToSign: canonicalRequest %s", canonicalRequest)
+       return fmt.Sprintf("%s\n%s\n%s\n%s", alg, r.Header.Get("X-Amz-Date"), scope, hashdigest(sha256.New(), canonicalRequest)), nil
+}
+
+func s3signature(secretKey, scope, signedHeaders, stringToSign string) (string, error) {
+       // scope is {datestamp}/{region}/{service}/aws4_request
+       drs := strings.Split(scope, "/")
+       if len(drs) != 4 {
+               return "", fmt.Errorf("invalid scope %q", scope)
+       }
+       key := s3signatureKey(secretKey, drs[0], drs[1], drs[2])
+       return hashdigest(hmac.New(sha256.New, key), stringToSign), nil
+}
+
+// checks3signature verifies the given S3 V4 signature and returns the
+// Arvados token that corresponds to the given accessKey. An error is
+// returned if accessKey is not a valid token UUID or the signature
+// does not match.
+func (h *handler) checks3signature(r *http.Request) (string, error) {
+       var key, scope, signedHeaders, signature string
+       authstring := strings.TrimPrefix(r.Header.Get("Authorization"), s3SignAlgorithm+" ")
+       for _, cmpt := range strings.Split(authstring, ",") {
+               cmpt = strings.TrimSpace(cmpt)
+               split := strings.SplitN(cmpt, "=", 2)
+               switch {
+               case len(split) != 2:
+                       // (?) ignore
+               case split[0] == "Credential":
+                       keyandscope := strings.SplitN(split[1], "/", 2)
+                       if len(keyandscope) == 2 {
+                               key, scope = keyandscope[0], keyandscope[1]
+                       }
+               case split[0] == "SignedHeaders":
+                       signedHeaders = split[1]
+               case split[0] == "Signature":
+                       signature = split[1]
+               }
+       }
+
+       client := (&arvados.Client{
+               APIHost:  h.Config.cluster.Services.Controller.ExternalURL.Host,
+               Insecure: h.Config.cluster.TLS.Insecure,
+       }).WithRequestID(r.Header.Get("X-Request-Id"))
+       var aca arvados.APIClientAuthorization
+       var secret string
+       var err error
+       if len(key) == 27 && key[5:12] == "-gj3su-" {
+               // Access key is the UUID of an Arvados token, secret
+               // key is the secret part.
+               ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+h.Config.cluster.SystemRootToken)
+               err = client.RequestAndDecodeContext(ctx, &aca, "GET", "arvados/v1/api_client_authorizations/"+key, nil, nil)
+               secret = aca.APIToken
+       } else {
+               // Access key and secret key are both an entire
+               // Arvados token or OIDC access token.
+               ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+key)
+               err = client.RequestAndDecodeContext(ctx, &aca, "GET", "arvados/v1/api_client_authorizations/current", nil, nil)
+               secret = key
+       }
+       if err != nil {
+               ctxlog.FromContext(r.Context()).WithError(err).WithField("UUID", key).Info("token lookup failed")
+               return "", errors.New("invalid access key")
+       }
+       stringToSign, err := s3stringToSign(s3SignAlgorithm, scope, signedHeaders, r)
+       if err != nil {
+               return "", err
+       }
+       expect, err := s3signature(secret, scope, signedHeaders, stringToSign)
+       if err != nil {
+               return "", err
+       } else if expect != signature {
+               return "", fmt.Errorf("signature does not match (scope %q signedHeaders %q stringToSign %q)", scope, signedHeaders, stringToSign)
+       }
+       return secret, nil
+}
 
 // serveS3 handles r and returns true if r is a request from an S3
 // client, otherwise it returns false.
@@ -30,27 +180,17 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
        if auth := r.Header.Get("Authorization"); strings.HasPrefix(auth, "AWS ") {
                split := strings.SplitN(auth[4:], ":", 2)
                if len(split) < 2 {
-                       w.WriteHeader(http.StatusUnauthorized)
+                       http.Error(w, "malformed Authorization header", http.StatusUnauthorized)
                        return true
                }
                token = split[0]
-       } else if strings.HasPrefix(auth, "AWS4-HMAC-SHA256 ") {
-               for _, cmpt := range strings.Split(auth[17:], ",") {
-                       cmpt = strings.TrimSpace(cmpt)
-                       split := strings.SplitN(cmpt, "=", 2)
-                       if len(split) == 2 && split[0] == "Credential" {
-                               keyandscope := strings.Split(split[1], "/")
-                               if len(keyandscope[0]) > 0 {
-                                       token = keyandscope[0]
-                                       break
-                               }
-                       }
-               }
-               if token == "" {
-                       w.WriteHeader(http.StatusBadRequest)
-                       fmt.Println(w, "invalid V4 signature")
+       } else if strings.HasPrefix(auth, s3SignAlgorithm+" ") {
+               t, err := h.checks3signature(r)
+               if err != nil {
+                       http.Error(w, "signature verification failed: "+err.Error(), http.StatusForbidden)
                        return true
                }
+               token = t
        } else {
                return false
        }
index 33e978c3b7acec6e196e06db884bd6e519eb8964..786e68afec4ca197980b56270e3f0bc66ab7494d 100644 (file)
@@ -11,6 +11,7 @@ import (
        "io/ioutil"
        "net/http"
        "os"
+       "os/exec"
        "strings"
        "sync"
        "time"
@@ -70,12 +71,13 @@ func (s *IntegrationSuite) s3setup(c *check.C) s3stage {
        err = arv.RequestAndDecode(&coll, "GET", "arvados/v1/collections/"+coll.UUID, nil, nil)
        c.Assert(err, check.IsNil)
 
-       auth := aws.NewAuth(arvadostest.ActiveTokenV2, arvadostest.ActiveTokenV2, "", time.Now().Add(time.Hour))
+       auth := aws.NewAuth(arvadostest.ActiveTokenUUID, arvadostest.ActiveToken, "", time.Now().Add(time.Hour))
        region := aws.Region{
                Name:       s.testServer.Addr,
                S3Endpoint: "http://" + s.testServer.Addr,
        }
        client := s3.New(*auth, region)
+       client.Signature = aws.V4Signature
        return s3stage{
                arv:  arv,
                ac:   ac,
@@ -104,6 +106,40 @@ func (stage s3stage) teardown(c *check.C) {
        }
 }
 
+func (s *IntegrationSuite) TestS3Signatures(c *check.C) {
+       stage := s.s3setup(c)
+       defer stage.teardown(c)
+
+       bucket := stage.collbucket
+       for _, trial := range []struct {
+               success   bool
+               signature int
+               accesskey string
+               secretkey string
+       }{
+               {true, aws.V2Signature, arvadostest.ActiveToken, "none"},
+               {false, aws.V2Signature, "none", "none"},
+               {false, aws.V2Signature, "none", arvadostest.ActiveToken},
+
+               {true, aws.V4Signature, arvadostest.ActiveTokenUUID, arvadostest.ActiveToken},
+               {true, aws.V4Signature, arvadostest.ActiveToken, arvadostest.ActiveToken},
+               {false, aws.V4Signature, arvadostest.ActiveToken, ""},
+               {false, aws.V4Signature, arvadostest.ActiveToken, "none"},
+               {false, aws.V4Signature, "none", arvadostest.ActiveToken},
+               {false, aws.V4Signature, "none", "none"},
+       } {
+               c.Logf("%#v", trial)
+               bucket.S3.Auth = *(aws.NewAuth(trial.accesskey, trial.secretkey, "", time.Now().Add(time.Hour)))
+               bucket.S3.Signature = trial.signature
+               _, err := bucket.GetReader("emptyfile")
+               if trial.success {
+                       c.Check(err, check.IsNil)
+               } else {
+                       c.Check(err, check.NotNil)
+               }
+       }
+}
+
 func (s *IntegrationSuite) TestS3HeadBucket(c *check.C) {
        stage := s.s3setup(c)
        defer stage.teardown(c)
@@ -310,6 +346,15 @@ func (s *IntegrationSuite) TestS3ProjectPutObjectFailure(c *check.C) {
 }
 func (s *IntegrationSuite) testS3PutObjectFailure(c *check.C, bucket *s3.Bucket, prefix string) {
        s.testServer.Config.cluster.Collections.S3FolderObjects = false
+
+       // Can't use V4 signature for these tests, because
+       // double-slash is incorrectly cleaned by the aws.V4Signature,
+       // resulting in a "bad signature" error. (Cleaning the path is
+       // appropriate for other services, but not in S3 where object
+       // names "foo//bar" and "foo/bar" are semantically different.)
+       bucket.S3.Auth = *(aws.NewAuth(arvadostest.ActiveToken, "none", "", time.Now().Add(time.Hour)))
+       bucket.S3.Signature = aws.V2Signature
+
        var wg sync.WaitGroup
        for _, trial := range []struct {
                path string
@@ -636,3 +681,22 @@ func (s *IntegrationSuite) testS3CollectionListRollup(c *check.C) {
                c.Logf("=== trial %+v keys %q prefixes %q nextMarker %q", trial, gotKeys, gotPrefixes, resp.NextMarker)
        }
 }
+
+// TestS3cmd checks compatibility with the s3cmd command line tool, if
+// it's installed. As of Debian buster, s3cmd is only in backports, so
+// `arvados-server install` don't install it, and this test skips if
+// it's not installed.
+func (s *IntegrationSuite) TestS3cmd(c *check.C) {
+       if _, err := exec.LookPath("s3cmd"); err != nil {
+               c.Skip("s3cmd not found")
+               return
+       }
+
+       stage := s.s3setup(c)
+       defer stage.teardown(c)
+
+       cmd := exec.Command("s3cmd", "--no-ssl", "--host="+s.testServer.Addr, "--host-bucket="+s.testServer.Addr, "--access_key="+arvadostest.ActiveTokenUUID, "--secret_key="+arvadostest.ActiveToken, "ls", "s3://"+arvadostest.FooCollection)
+       buf, err := cmd.CombinedOutput()
+       c.Check(err, check.IsNil)
+       c.Check(string(buf), check.Matches, `.* 3 +s3://`+arvadostest.FooCollection+`/foo\n`)
+}
diff --git a/services/keep-web/s3aws_test.go b/services/keep-web/s3aws_test.go
new file mode 100644 (file)
index 0000000..d3db94e
--- /dev/null
@@ -0,0 +1,78 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package main
+
+import (
+       "bytes"
+       "context"
+       "io/ioutil"
+
+       "git.arvados.org/arvados.git/sdk/go/arvadostest"
+       "github.com/aws/aws-sdk-go-v2/aws"
+       "github.com/aws/aws-sdk-go-v2/aws/defaults"
+       "github.com/aws/aws-sdk-go-v2/aws/ec2metadata"
+       "github.com/aws/aws-sdk-go-v2/aws/ec2rolecreds"
+       "github.com/aws/aws-sdk-go-v2/aws/endpoints"
+       "github.com/aws/aws-sdk-go-v2/service/s3"
+       check "gopkg.in/check.v1"
+)
+
+func (s *IntegrationSuite) TestS3AWSSDK(c *check.C) {
+       stage := s.s3setup(c)
+       defer stage.teardown(c)
+
+       cfg := defaults.Config()
+       cfg.Credentials = aws.NewChainProvider([]aws.CredentialsProvider{
+               aws.NewStaticCredentialsProvider(arvadostest.ActiveTokenUUID, arvadostest.ActiveToken, ""),
+               ec2rolecreds.New(ec2metadata.New(cfg)),
+       })
+       cfg.EndpointResolver = aws.EndpointResolverFunc(func(service, region string) (aws.Endpoint, error) {
+               if service == "s3" {
+                       return aws.Endpoint{
+                               URL:           "http://" + s.testServer.Addr,
+                               SigningRegion: "custom-signing-region",
+                       }, nil
+               } else {
+                       return endpoints.NewDefaultResolver().ResolveEndpoint(service, region)
+               }
+       })
+       client := s3.New(cfg)
+       client.ForcePathStyle = true
+       listreq := client.ListObjectsV2Request(&s3.ListObjectsV2Input{
+               Bucket:            aws.String(arvadostest.FooCollection),
+               MaxKeys:           aws.Int64(100),
+               Prefix:            aws.String(""),
+               ContinuationToken: nil,
+       })
+       resp, err := listreq.Send(context.Background())
+       c.Assert(err, check.IsNil)
+       c.Check(resp.Contents, check.HasLen, 1)
+       for _, key := range resp.Contents {
+               c.Check(*key.Key, check.Equals, "foo")
+       }
+
+       p := make([]byte, 100000000)
+       for i := range p {
+               p[i] = byte('a')
+       }
+       putreq := client.PutObjectRequest(&s3.PutObjectInput{
+               Body:        bytes.NewReader(p),
+               Bucket:      aws.String(stage.collbucket.Name),
+               ContentType: aws.String("application/octet-stream"),
+               Key:         aws.String("aaaa"),
+       })
+       _, err = putreq.Send(context.Background())
+       c.Assert(err, check.IsNil)
+
+       getreq := client.GetObjectRequest(&s3.GetObjectInput{
+               Bucket: aws.String(stage.collbucket.Name),
+               Key:    aws.String("aaaa"),
+       })
+       getresp, err := getreq.Send(context.Background())
+       c.Assert(err, check.IsNil)
+       getdata, err := ioutil.ReadAll(getresp.Body)
+       c.Assert(err, check.IsNil)
+       c.Check(bytes.Equal(getdata, p), check.Equals, true)
+}
index c37852a128bbaa9571ebf1527f3f9f6b6cee41ae..acdc11b305335fd25afe3fed4c27122c3488c84f 100644 (file)
@@ -440,6 +440,7 @@ func (s *IntegrationSuite) SetUpTest(c *check.C) {
        cfg.cluster.Services.WebDAV.InternalURLs[arvados.URL{Host: listen}] = arvados.ServiceInstance{}
        cfg.cluster.Services.WebDAVDownload.InternalURLs[arvados.URL{Host: listen}] = arvados.ServiceInstance{}
        cfg.cluster.ManagementToken = arvadostest.ManagementToken
+       cfg.cluster.SystemRootToken = arvadostest.SystemRootToken
        cfg.cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken
        s.testServer = &server{Config: cfg}
        err = s.testServer.Start(ctxlog.TestLogger(c))