17106: Allow use of URL-encoded token as S3 access/secret key.

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

diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index 603198684c629b79b044e82632c7f17527075a51..379afe37e94e23dc273cac8907d8c5516060d555 100644 (file)
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -16,6 +16,7 @@ import (
        "net/url"
        "os"
        "path/filepath"
+       "regexp"
        "sort"
        "strconv"
        "strings"
@@ -111,6 +112,21 @@ func s3signature(secretKey, scope, signedHeaders, stringToSign string) (string,
        return hashdigest(hmac.New(sha256.New, key), stringToSign), nil
 }
 
+var v2tokenUnderscore = regexp.MustCompile(`^v2_[a-z0-9]{5}-gj3su-[a-z0-9]{15}_`)
+
+func unescapeKey(key string) string {
+       if v2tokenUnderscore.MatchString(key) {
+               // Entire Arvados token, with "/" replaced by "_" to
+               // avoid colliding with the Authorization header
+               // format.
+               return strings.Replace(key, "_", "/", -1)
+       } else if s, err := url.QueryUnescape(key); err == nil {
+               return s
+       } else {
+               return key
+       }
+}
+
 // 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
@@ -152,14 +168,7 @@ func (h *handler) checks3signature(r *http.Request) (string, error) {
        } else {
                // Access key and secret key are both an entire
                // Arvados token or OIDC access token.
-               mungedKey := key
-               if strings.HasPrefix(key, "v2_") {
-                       // Entire Arvados token, with "/" replaced by
-                       // "_" to avoid colliding with the
-                       // Authorization header format.
-                       mungedKey = strings.Replace(key, "_", "/", -1)
-               }
-               ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+mungedKey)
+               ctx := arvados.ContextWithAuthorization(r.Context(), "Bearer "+unescapeKey(key))
                err = client.RequestAndDecodeContext(ctx, &aca, "GET", "arvados/v1/api_client_authorizations/current", nil, nil)
                secret = key
        }
@@ -190,13 +199,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
                        http.Error(w, "malformed Authorization header", http.StatusUnauthorized)
                        return true
                }
-               token = split[0]
-               if strings.HasPrefix(token, "v2_") {
-                       // User provided a full Arvados token with "/"
-                       // munged to "_" (see V4 signature validation)
-                       // but client software used S3 V2 signature.
-                       token = strings.Replace(token, "_", "/", -1)
-               }
+               token = unescapeKey(split[0])
        } else if strings.HasPrefix(auth, s3SignAlgorithm+" ") {
                t, err := h.checks3signature(r)
                if err != nil {

diff --git a/services/keep-web/s3_test.go b/services/keep-web/s3_test.go
index 786e68afec4ca197980b56270e3f0bc66ab7494d..5936f03c963042c84f2e4e0004a09db47da9f422 100644 (file)
--- a/services/keep-web/s3_test.go
+++ b/services/keep-web/s3_test.go
@@ -10,6 +10,7 @@ import (
        "fmt"
        "io/ioutil"
        "net/http"
+       "net/url"
        "os"
        "os/exec"
        "strings"
@@ -118,11 +119,15 @@ func (s *IntegrationSuite) TestS3Signatures(c *check.C) {
                secretkey string
        }{
                {true, aws.V2Signature, arvadostest.ActiveToken, "none"},
+               {true, aws.V2Signature, url.QueryEscape(arvadostest.ActiveTokenV2), "none"},
+               {true, aws.V2Signature, strings.Replace(arvadostest.ActiveTokenV2, "/", "_", -1), "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},
+               {true, aws.V4Signature, url.QueryEscape(arvadostest.ActiveTokenV2), url.QueryEscape(arvadostest.ActiveTokenV2)},
+               {true, aws.V4Signature, strings.Replace(arvadostest.ActiveTokenV2, "/", "_", -1), strings.Replace(arvadostest.ActiveTokenV2, "/", "_", -1)},
                {false, aws.V4Signature, arvadostest.ActiveToken, ""},
                {false, aws.V4Signature, arvadostest.ActiveToken, "none"},
                {false, aws.V4Signature, "none", arvadostest.ActiveToken},