[ARVADOS] updated: 2.1.0-112-g0cfb2b064

[Git user]

Summary of changes:
 services/keep-web/s3.go      | 33 ++++++++++++++++++---------------
 services/keep-web/s3_test.go |  5 +++++
 2 files changed, 23 insertions(+), 15 deletions(-)

       via  0cfb2b0646ad8129c82883717af7a51d28e6876a (commit)
      from  199ca290ab259ba21f798bb059bb808fe3b609ba (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 0cfb2b0646ad8129c82883717af7a51d28e6876a
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Mon Nov 23 14:52:57 2020 -0500

    17106: Allow use of URL-encoded token as S3 access/secret key.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index 603198684..379afe37e 100644
--- 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 786e68afe..5936f03c9 100644
--- 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},

-----------------------------------------------------------------------


hooks/post-receive
--