[ARVADOS] updated: 7940276742ab045a83c389690e10d748e16b7f64

git at public.curoverse.com git at public.curoverse.com
Thu Oct 8 17:41:28 EDT 2015


Summary of changes:
 sdk/go/keepclient/perms.go       |  96 +++++++++++-------------------------
 services/keepstore/perms.go      |  51 +++----------------
 services/keepstore/perms_test.go | 104 ++++++++-------------------------------
 3 files changed, 55 insertions(+), 196 deletions(-)

  discards  b938fd6d63a7dade2124d0873b20329d8a27f224 (commit)
       via  7940276742ab045a83c389690e10d748e16b7f64 (commit)
       via  2bd15f5413cd6e76b30c0df81434a337170d4db5 (commit)
       via  20c5b73d598463a7dfb4ce711993480d56e23838 (commit)
       via  3ee62426a5ebfc056f58cd0655ba1e9f7ed0a722 (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (b938fd6d63a7dade2124d0873b20329d8a27f224)
            \
             N -- N -- N (7940276742ab045a83c389690e10d748e16b7f64)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

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 7940276742ab045a83c389690e10d748e16b7f64
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Oct 8 17:50:22 2015 -0400

    7167: Fix up comments

diff --git a/sdk/go/keepclient/perms.go b/sdk/go/keepclient/perms.go
index 0797b07..7dc06e2 100644
--- a/sdk/go/keepclient/perms.go
+++ b/sdk/go/keepclient/perms.go
@@ -1,37 +1,6 @@
-/*
-Permissions management on Arvados locator hashes.
-
-The permissions structure for Arvados is as follows (from
-https://arvados.org/issues/2328)
-
-A Keep locator string has the following format:
-
-    [hash]+[size]+A[signature]@[timestamp]
-
-The "signature" string here is a cryptographic hash, expressed as a
-string of hexadecimal digits, and timestamp is a 32-bit Unix timestamp
-expressed as a hexadecimal number.  e.g.:
-
-    acbd18db4cc2f85cedef654fccc4a4d8+3+A257f3f5f5f0a4e4626a18fc74bd42ec34dcb228a at 7fffffff
-
-The signature represents a guarantee that this locator was generated
-by either Keep or the API server for use with the supplied API token.
-If a request to Keep includes a locator with a valid signature and is
-accompanied by the proper API token, the user has permission to GET
-that object.
-
-The signature may be generated either by Keep (after the user writes a
-block) or by the API server (if the user has can_read permission on
-the specified object). Keep and API server share a secret that is used
-to generate signatures.
-
-To verify a permission hint, Keep generates a new hint for the
-requested object (using the locator string, the timestamp, the
-permission secret and the user's API token, which must appear in the
-request headers) and compares it against the hint included in the
-request. If the permissions do not match, or if the API token is not
-present, Keep returns a 401 error.
-*/
+// Generate and verify permission signatures for Keep locators.
+//
+// See https://dev.arvados.org/projects/arvados/wiki/Keep_locator_format
 
 package keepclient
 
@@ -53,12 +22,9 @@ var (
 	ErrSignatureMissing   = errors.New("Missing signature")
 )
 
-// makePermSignature returns a string representing the signed permission
-// hint for the blob identified by blobHash, apiToken, expiration timestamp, and permission secret.
-//
-// The permissionSecret is the secret key used to generate SHA1 digests
-// for permission hints. apiserver and Keep must use the same key.
-func makePermSignature(blobHash string, apiToken string, expiry string, permissionSecret []byte) string {
+// makePermSignature generates a SHA-1 HMAC digest for the given blob,
+// token, expiry, and site secret.
+func makePermSignature(blobHash, apiToken, expiry string, permissionSecret []byte) string {
 	hmac := hmac.New(sha1.New, permissionSecret)
 	hmac.Write([]byte(blobHash))
 	hmac.Write([]byte("@"))
@@ -69,17 +35,18 @@ func makePermSignature(blobHash string, apiToken string, expiry string, permissi
 	return fmt.Sprintf("%x", digest)
 }
 
-// SignLocator takes a blobLocator, an apiToken, an expiry time, and a permission secret
-// and returns a signed locator string.
-func SignLocator(blobLocator string, apiToken string, expiry time.Time, permissionSecret []byte) string {
-	// If no permission secret or API token is available,
-	// return an unsigned locator.
-	if permissionSecret == nil || apiToken == "" {
+// SignLocator returns blobLocator with a permission signature
+// added. If either permissionSecret or apiToken is empty, blobLocator
+// is returned untouched.
+//
+// This function is intended to be used by system components and admin
+// utilities: userland programs do not know the permissionSecret.
+func SignLocator(blobLocator, apiToken string, expiry time.Time, permissionSecret []byte) string {
+	if len(permissionSecret) == 0 || apiToken == "" {
 		return blobLocator
 	}
-	// Extract the hash from the blob locator, omitting any size hint that may be present.
+	// Strip off all hints: only the hash is used to sign.
 	blobHash := strings.Split(blobLocator, "+")[0]
-	// Return the signed locator string.
 	timestampHex := fmt.Sprintf("%08x", expiry.Unix())
 	return blobLocator +
 		"+A" + makePermSignature(blobHash, apiToken, timestampHex, permissionSecret) +
@@ -93,10 +60,12 @@ var signedLocatorRe = regexp.MustCompile(`^([[:xdigit:]]{32}).*\+A([[:xdigit:]]{
 // either ExpiredError (if the timestamp has expired, which is
 // something the client could have figured out independently) or
 // PermissionError.
-func VerifySignature(signedLocator string, apiToken string, permissionSecret []byte) error {
+//
+// This function is intended to be used by system components and admin
+// utilities: userland programs do not know the permissionSecret.
+func VerifySignature(signedLocator, apiToken string, permissionSecret []byte) error {
 	matches := signedLocatorRe.FindStringSubmatch(signedLocator)
 	if matches == nil {
-		// Could not find a permission signature at all
 		return ErrSignatureMissing
 	}
 	blobHash := matches[1]
@@ -113,7 +82,6 @@ func VerifySignature(signedLocator string, apiToken string, permissionSecret []b
 	return nil
 }
 
-// parseHexTimestamp parses timestamp
 func parseHexTimestamp(timestampHex string) (ts time.Time, err error) {
 	if tsInt, e := strconv.ParseInt(timestampHex, 16, 0); e == nil {
 		ts = time.Unix(tsInt, 0)

commit 2bd15f5413cd6e76b30c0df81434a337170d4db5
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Oct 8 17:33:55 2015 -0400

    7167: Replace duplicate tests with PermissionSecret tests

diff --git a/services/keepstore/perms_test.go b/services/keepstore/perms_test.go
index 9b4e30a..59cb8bd 100644
--- a/services/keepstore/perms_test.go
+++ b/services/keepstore/perms_test.go
@@ -23,104 +23,40 @@ const (
 	knownSignedLocator = knownLocator + knownSigHint
 )
 
-func TestSignLocator(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
+func TestSignLocatorUsesPermissionSecret(t *testing.T) {
+	defer func(b []byte) {
+		PermissionSecret = b
+	}(PermissionSecret)
 
 	tsInt, err := strconv.ParseInt(knownTimestamp, 16, 0)
 	if err != nil {
-		t.Fail()
-	}
-	if knownSignedLocator != SignLocator(knownLocator, knownToken, time.Unix(tsInt, 0)) {
-		t.Fail()
+		t.Fatal(err)
 	}
-}
+	t0 := time.Unix(tsInt, 0)
 
-func TestVerifySignature(t *testing.T) {
 	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	if VerifySignature(knownSignedLocator, knownToken) != nil {
-		t.Fail()
-	}
-}
-
-func TestVerifySignatureExtraHints(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	if VerifySignature(knownLocator+"+K at xyzzy"+knownSigHint, knownToken) != nil {
-		t.Fatal("Verify cannot handle hint before permission signature")
+	if x := SignLocator(knownLocator, knownToken, t0); x != knownSignedLocator {
+		t.Fatalf("Got %+q, expected %+q", x, knownSignedLocator)
 	}
 
-	if VerifySignature(knownLocator+knownSigHint+"+Zfoo", knownToken) != nil {
-		t.Fatal("Verify cannot handle hint after permission signature")
-	}
-
-	if VerifySignature(knownLocator+"+K at xyzzy"+knownSigHint+"+Zfoo", knownToken) != nil {
-		t.Fatal("Verify cannot handle hints around permission signature")
+	PermissionSecret = []byte("arbitrarykey")
+	if x := SignLocator(knownLocator, knownToken, t0); x == knownSignedLocator {
+		t.Fatalf("Got same signature %+q, even though PermissionSecret changed", x)
 	}
 }
 
-// The size hint on the locator string should not affect signature validation.
-func TestVerifySignatureWrongSize(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	if VerifySignature(knownHash+"+999999"+knownSigHint, knownToken) != nil {
-		t.Fatal("Verify cannot handle incorrect size hint")
-	}
+func TestVerifyLocatorUsesPermissionSecret(t *testing.T) {
+	defer func(b []byte) {
+		PermissionSecret = b
+	}(PermissionSecret)
 
-	if VerifySignature(knownHash+knownSigHint, knownToken) != nil {
-		t.Fatal("Verify cannot handle missing size hint")
-	}
-}
-
-func TestVerifySignatureBadSig(t *testing.T) {
 	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	badLocator := knownLocator + "+Aaaaaaaaaaaaaaaa@" + knownTimestamp
-	if VerifySignature(badLocator, knownToken) != PermissionError {
-		t.Fail()
+	if err := VerifySignature(knownSignedLocator, knownToken); err != nil {
+		t.Fatal(err)
 	}
-}
-
-func TestVerifySignatureBadTimestamp(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	badLocator := knownLocator + "+A" + knownSignature + "@OOOOOOOl"
-	if VerifySignature(badLocator, knownToken) != PermissionError {
-		t.Fail()
-	}
-}
-
-func TestVerifySignatureBadSecret(t *testing.T) {
-	PermissionSecret = []byte("00000000000000000000")
-	defer func() { PermissionSecret = nil }()
-
-	if VerifySignature(knownSignedLocator, knownToken) != PermissionError {
-		t.Fail()
-	}
-}
-
-func TestVerifySignatureBadToken(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	if VerifySignature(knownSignedLocator, "00000000") != PermissionError {
-		t.Fail()
-	}
-}
-
-func TestVerifySignatureExpired(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
 
-	yesterday := time.Now().AddDate(0, 0, -1)
-	expiredLocator := SignLocator(knownHash, knownToken, yesterday)
-	if VerifySignature(expiredLocator, knownToken) != ExpiredError {
-		t.Fail()
+	PermissionSecret = []byte("arbitrarykey")
+	if err := VerifySignature(knownSignedLocator, knownToken); err == nil {
+		t.Fatal("Verified signature even with wrong PermissionSecret")
 	}
 }

commit 20c5b73d598463a7dfb4ce711993480d56e23838
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Oct 8 17:17:46 2015 -0400

    7167: Tidy up errors. Remove extra comment copy.

diff --git a/sdk/go/keepclient/perms.go b/sdk/go/keepclient/perms.go
index 5b792a8..0797b07 100644
--- a/sdk/go/keepclient/perms.go
+++ b/sdk/go/keepclient/perms.go
@@ -38,6 +38,7 @@ package keepclient
 import (
 	"crypto/hmac"
 	"crypto/sha1"
+	"errors"
 	"fmt"
 	"regexp"
 	"strconv"
@@ -45,22 +46,13 @@ import (
 	"time"
 )
 
-// KeepError types.
-//
-type KeepError struct {
-	HTTPCode int
-	ErrMsg   string
-}
-
 var (
-	PermissionError = &KeepError{403, "Forbidden"}
-	ExpiredError    = &KeepError{401, "Expired permission signature"}
+	ErrSignatureExpired   = errors.New("Signature expired")
+	ErrSignatureInvalid   = errors.New("Invalid signature")
+	ErrSignatureMalformed = errors.New("Malformed signature")
+	ErrSignatureMissing   = errors.New("Missing signature")
 )
 
-func (e *KeepError) Error() string {
-	return e.ErrMsg
-}
-
 // makePermSignature returns a string representing the signed permission
 // hint for the blob identified by blobHash, apiToken, expiration timestamp, and permission secret.
 //
@@ -105,18 +97,18 @@ func VerifySignature(signedLocator string, apiToken string, permissionSecret []b
 	matches := signedLocatorRe.FindStringSubmatch(signedLocator)
 	if matches == nil {
 		// Could not find a permission signature at all
-		return PermissionError
+		return ErrSignatureMissing
 	}
 	blobHash := matches[1]
 	sigHex := matches[2]
 	expHex := matches[3]
 	if expTime, err := parseHexTimestamp(expHex); err != nil {
-		return PermissionError
+		return ErrSignatureMalformed
 	} else if expTime.Before(time.Now()) {
-		return ExpiredError
+		return ErrSignatureExpired
 	}
 	if sigHex != makePermSignature(blobHash, apiToken, expHex, permissionSecret) {
-		return PermissionError
+		return ErrSignatureInvalid
 	}
 	return nil
 }
diff --git a/services/keepstore/perms.go b/services/keepstore/perms.go
index 494e6b7..6168a32 100644
--- a/services/keepstore/perms.go
+++ b/services/keepstore/perms.go
@@ -1,38 +1,3 @@
-/*
-Permissions management on Arvados locator hashes.
-
-The permissions structure for Arvados is as follows (from
-https://arvados.org/issues/2328)
-
-A Keep locator string has the following format:
-
-    [hash]+[size]+A[signature]@[timestamp]
-
-The "signature" string here is a cryptographic hash, expressed as a
-string of hexadecimal digits, and timestamp is a 32-bit Unix timestamp
-expressed as a hexadecimal number.  e.g.:
-
-    acbd18db4cc2f85cedef654fccc4a4d8+3+A257f3f5f5f0a4e4626a18fc74bd42ec34dcb228a at 7fffffff
-
-The signature represents a guarantee that this locator was generated
-by either Keep or the API server for use with the supplied API token.
-If a request to Keep includes a locator with a valid signature and is
-accompanied by the proper API token, the user has permission to GET
-that object.
-
-The signature may be generated either by Keep (after the user writes a
-block) or by the API server (if the user has can_read permission on
-the specified object). Keep and API server share a secret that is used
-to generate signatures.
-
-To verify a permission hint, Keep generates a new hint for the
-requested object (using the locator string, the timestamp, the
-permission secret and the user's API token, which must appear in the
-request headers) and compares it against the hint included in the
-request. If the permissions do not match, or if the API token is not
-present, Keep returns a 401 error.
-*/
-
 package main
 
 import (
@@ -47,7 +12,7 @@ var PermissionSecret []byte
 
 // SignLocator takes a blobLocator, an apiToken and an expiry time, and
 // returns a signed locator string.
-func SignLocator(blobLocator string, apiToken string, expiry time.Time) string {
+func SignLocator(blobLocator, apiToken string, expiry time.Time) string {
 	return keepclient.SignLocator(blobLocator, apiToken, expiry, PermissionSecret)
 }
 
@@ -56,14 +21,12 @@ func SignLocator(blobLocator string, apiToken string, expiry time.Time) string {
 // either ExpiredError (if the timestamp has expired, which is
 // something the client could have figured out independently) or
 // PermissionError.
-func VerifySignature(signedLocator string, apiToken string) error {
+func VerifySignature(signedLocator, apiToken string) error {
 	err := keepclient.VerifySignature(signedLocator, apiToken, PermissionSecret)
-	if err != nil {
-		if err == keepclient.PermissionError {
-			return PermissionError
-		} else if err == keepclient.ExpiredError {
-			return ExpiredError
-		}
+	if err == keepclient.ErrSignatureExpired {
+		return ExpiredError
+	} else if err != nil {
+		return PermissionError
 	}
-	return err
+	return nil
 }

commit 3ee62426a5ebfc056f58cd0655ba1e9f7ed0a722
Author: radhika <radhika at curoverse.com>
Date:   Wed Oct 7 16:47:56 2015 -0400

    7167: move perms code from keepstore into keepclient go SDK.

diff --git a/services/keepstore/perms.go b/sdk/go/keepclient/perms.go
similarity index 68%
copy from services/keepstore/perms.go
copy to sdk/go/keepclient/perms.go
index 5579238..5b792a8 100644
--- a/services/keepstore/perms.go
+++ b/sdk/go/keepclient/perms.go
@@ -33,7 +33,7 @@ request. If the permissions do not match, or if the API token is not
 present, Keep returns a 401 error.
 */
 
-package main
+package keepclient
 
 import (
 	"crypto/hmac"
@@ -45,15 +45,29 @@ import (
 	"time"
 )
 
-// The PermissionSecret is the secret key used to generate SHA1
-// digests for permission hints. apiserver and Keep must use the same
-// key.
-var PermissionSecret []byte
+// KeepError types.
+//
+type KeepError struct {
+	HTTPCode int
+	ErrMsg   string
+}
+
+var (
+	PermissionError = &KeepError{403, "Forbidden"}
+	ExpiredError    = &KeepError{401, "Expired permission signature"}
+)
+
+func (e *KeepError) Error() string {
+	return e.ErrMsg
+}
 
-// MakePermSignature returns a string representing the signed permission
-// hint for the blob identified by blobHash, apiToken and expiration timestamp.
-func MakePermSignature(blobHash string, apiToken string, expiry string) string {
-	hmac := hmac.New(sha1.New, PermissionSecret)
+// makePermSignature returns a string representing the signed permission
+// hint for the blob identified by blobHash, apiToken, expiration timestamp, and permission secret.
+//
+// The permissionSecret is the secret key used to generate SHA1 digests
+// for permission hints. apiserver and Keep must use the same key.
+func makePermSignature(blobHash string, apiToken string, expiry string, permissionSecret []byte) string {
+	hmac := hmac.New(sha1.New, permissionSecret)
 	hmac.Write([]byte(blobHash))
 	hmac.Write([]byte("@"))
 	hmac.Write([]byte(apiToken))
@@ -63,12 +77,12 @@ func MakePermSignature(blobHash string, apiToken string, expiry string) string {
 	return fmt.Sprintf("%x", digest)
 }
 
-// SignLocator takes a blobLocator, an apiToken and an expiry time, and
-// returns a signed locator string.
-func SignLocator(blobLocator string, apiToken string, expiry time.Time) string {
+// SignLocator takes a blobLocator, an apiToken, an expiry time, and a permission secret
+// and returns a signed locator string.
+func SignLocator(blobLocator string, apiToken string, expiry time.Time, permissionSecret []byte) string {
 	// If no permission secret or API token is available,
 	// return an unsigned locator.
-	if PermissionSecret == nil || apiToken == "" {
+	if permissionSecret == nil || apiToken == "" {
 		return blobLocator
 	}
 	// Extract the hash from the blob locator, omitting any size hint that may be present.
@@ -76,7 +90,7 @@ func SignLocator(blobLocator string, apiToken string, expiry time.Time) string {
 	// Return the signed locator string.
 	timestampHex := fmt.Sprintf("%08x", expiry.Unix())
 	return blobLocator +
-		"+A" + MakePermSignature(blobHash, apiToken, timestampHex) +
+		"+A" + makePermSignature(blobHash, apiToken, timestampHex, permissionSecret) +
 		"@" + timestampHex
 }
 
@@ -87,7 +101,7 @@ var signedLocatorRe = regexp.MustCompile(`^([[:xdigit:]]{32}).*\+A([[:xdigit:]]{
 // either ExpiredError (if the timestamp has expired, which is
 // something the client could have figured out independently) or
 // PermissionError.
-func VerifySignature(signedLocator string, apiToken string) error {
+func VerifySignature(signedLocator string, apiToken string, permissionSecret []byte) error {
 	matches := signedLocatorRe.FindStringSubmatch(signedLocator)
 	if matches == nil {
 		// Could not find a permission signature at all
@@ -96,19 +110,19 @@ func VerifySignature(signedLocator string, apiToken string) error {
 	blobHash := matches[1]
 	sigHex := matches[2]
 	expHex := matches[3]
-	if expTime, err := ParseHexTimestamp(expHex); err != nil {
+	if expTime, err := parseHexTimestamp(expHex); err != nil {
 		return PermissionError
 	} else if expTime.Before(time.Now()) {
 		return ExpiredError
 	}
-	if sigHex != MakePermSignature(blobHash, apiToken, expHex) {
+	if sigHex != makePermSignature(blobHash, apiToken, expHex, permissionSecret) {
 		return PermissionError
 	}
 	return nil
 }
 
-// ParseHexTimestamp parses timestamp
-func ParseHexTimestamp(timestampHex string) (ts time.Time, err error) {
+// parseHexTimestamp parses timestamp
+func parseHexTimestamp(timestampHex string) (ts time.Time, err error) {
 	if tsInt, e := strconv.ParseInt(timestampHex, 16, 0); e == nil {
 		ts = time.Unix(tsInt, 0)
 	} else {
diff --git a/services/keepstore/perms_test.go b/sdk/go/keepclient/perms_test.go
similarity index 62%
copy from services/keepstore/perms_test.go
copy to sdk/go/keepclient/perms_test.go
index 59516af..61d10c1 100644
--- a/services/keepstore/perms_test.go
+++ b/sdk/go/keepclient/perms_test.go
@@ -1,4 +1,4 @@
-package main
+package keepclient
 
 import (
 	"testing"
@@ -23,103 +23,76 @@ const (
 )
 
 func TestSignLocator(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	if ts, err := ParseHexTimestamp(knownTimestamp); err != nil {
+	if ts, err := parseHexTimestamp(knownTimestamp); err != nil {
 		t.Errorf("bad knownTimestamp %s", knownTimestamp)
 	} else {
-		if knownSignedLocator != SignLocator(knownLocator, knownToken, ts) {
+		if knownSignedLocator != SignLocator(knownLocator, knownToken, ts, []byte(knownKey)) {
 			t.Fail()
 		}
 	}
 }
 
 func TestVerifySignature(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	if VerifySignature(knownSignedLocator, knownToken) != nil {
+	if VerifySignature(knownSignedLocator, knownToken, []byte(knownKey)) != nil {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureExtraHints(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	if VerifySignature(knownLocator+"+K at xyzzy"+knownSigHint, knownToken) != nil {
+	if VerifySignature(knownLocator+"+K at xyzzy"+knownSigHint, knownToken, []byte(knownKey)) != nil {
 		t.Fatal("Verify cannot handle hint before permission signature")
 	}
 
-	if VerifySignature(knownLocator+knownSigHint+"+Zfoo", knownToken) != nil {
+	if VerifySignature(knownLocator+knownSigHint+"+Zfoo", knownToken, []byte(knownKey)) != nil {
 		t.Fatal("Verify cannot handle hint after permission signature")
 	}
 
-	if VerifySignature(knownLocator+"+K at xyzzy"+knownSigHint+"+Zfoo", knownToken) != nil {
+	if VerifySignature(knownLocator+"+K at xyzzy"+knownSigHint+"+Zfoo", knownToken, []byte(knownKey)) != nil {
 		t.Fatal("Verify cannot handle hints around permission signature")
 	}
 }
 
 // The size hint on the locator string should not affect signature validation.
 func TestVerifySignatureWrongSize(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	if VerifySignature(knownHash+"+999999"+knownSigHint, knownToken) != nil {
+	if VerifySignature(knownHash+"+999999"+knownSigHint, knownToken, []byte(knownKey)) != nil {
 		t.Fatal("Verify cannot handle incorrect size hint")
 	}
 
-	if VerifySignature(knownHash+knownSigHint, knownToken) != nil {
+	if VerifySignature(knownHash+knownSigHint, knownToken, []byte(knownKey)) != nil {
 		t.Fatal("Verify cannot handle missing size hint")
 	}
 }
 
 func TestVerifySignatureBadSig(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
 	badLocator := knownLocator + "+Aaaaaaaaaaaaaaaa@" + knownTimestamp
-	if VerifySignature(badLocator, knownToken) != PermissionError {
+	if VerifySignature(badLocator, knownToken, []byte(knownKey)) != PermissionError {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureBadTimestamp(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
 	badLocator := knownLocator + "+A" + knownSignature + "@OOOOOOOl"
-	if VerifySignature(badLocator, knownToken) != PermissionError {
+	if VerifySignature(badLocator, knownToken, []byte(knownKey)) != PermissionError {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureBadSecret(t *testing.T) {
-	PermissionSecret = []byte("00000000000000000000")
-	defer func() { PermissionSecret = nil }()
-
-	if VerifySignature(knownSignedLocator, knownToken) != PermissionError {
+	if VerifySignature(knownSignedLocator, knownToken, []byte("00000000000000000000")) != PermissionError {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureBadToken(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
-	if VerifySignature(knownSignedLocator, "00000000") != PermissionError {
+	if VerifySignature(knownSignedLocator, "00000000", []byte(knownKey)) != PermissionError {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureExpired(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
-
 	yesterday := time.Now().AddDate(0, 0, -1)
-	expiredLocator := SignLocator(knownHash, knownToken, yesterday)
-	if VerifySignature(expiredLocator, knownToken) != ExpiredError {
+	expiredLocator := SignLocator(knownHash, knownToken, yesterday, []byte(knownKey))
+	if VerifySignature(expiredLocator, knownToken, []byte(knownKey)) != ExpiredError {
 		t.Fail()
 	}
 }
diff --git a/services/keepstore/perms.go b/services/keepstore/perms.go
index 5579238..494e6b7 100644
--- a/services/keepstore/perms.go
+++ b/services/keepstore/perms.go
@@ -36,12 +36,7 @@ present, Keep returns a 401 error.
 package main
 
 import (
-	"crypto/hmac"
-	"crypto/sha1"
-	"fmt"
-	"regexp"
-	"strconv"
-	"strings"
+	"git.curoverse.com/arvados.git/sdk/go/keepclient"
 	"time"
 )
 
@@ -50,69 +45,25 @@ import (
 // key.
 var PermissionSecret []byte
 
-// MakePermSignature returns a string representing the signed permission
-// hint for the blob identified by blobHash, apiToken and expiration timestamp.
-func MakePermSignature(blobHash string, apiToken string, expiry string) string {
-	hmac := hmac.New(sha1.New, PermissionSecret)
-	hmac.Write([]byte(blobHash))
-	hmac.Write([]byte("@"))
-	hmac.Write([]byte(apiToken))
-	hmac.Write([]byte("@"))
-	hmac.Write([]byte(expiry))
-	digest := hmac.Sum(nil)
-	return fmt.Sprintf("%x", digest)
-}
-
 // SignLocator takes a blobLocator, an apiToken and an expiry time, and
 // returns a signed locator string.
 func SignLocator(blobLocator string, apiToken string, expiry time.Time) string {
-	// If no permission secret or API token is available,
-	// return an unsigned locator.
-	if PermissionSecret == nil || apiToken == "" {
-		return blobLocator
-	}
-	// Extract the hash from the blob locator, omitting any size hint that may be present.
-	blobHash := strings.Split(blobLocator, "+")[0]
-	// Return the signed locator string.
-	timestampHex := fmt.Sprintf("%08x", expiry.Unix())
-	return blobLocator +
-		"+A" + MakePermSignature(blobHash, apiToken, timestampHex) +
-		"@" + timestampHex
+	return keepclient.SignLocator(blobLocator, apiToken, expiry, PermissionSecret)
 }
 
-var signedLocatorRe = regexp.MustCompile(`^([[:xdigit:]]{32}).*\+A([[:xdigit:]]{40})@([[:xdigit:]]{8})`)
-
 // VerifySignature returns nil if the signature on the signedLocator
 // can be verified using the given apiToken. Otherwise it returns
 // either ExpiredError (if the timestamp has expired, which is
 // something the client could have figured out independently) or
 // PermissionError.
 func VerifySignature(signedLocator string, apiToken string) error {
-	matches := signedLocatorRe.FindStringSubmatch(signedLocator)
-	if matches == nil {
-		// Could not find a permission signature at all
-		return PermissionError
-	}
-	blobHash := matches[1]
-	sigHex := matches[2]
-	expHex := matches[3]
-	if expTime, err := ParseHexTimestamp(expHex); err != nil {
-		return PermissionError
-	} else if expTime.Before(time.Now()) {
-		return ExpiredError
-	}
-	if sigHex != MakePermSignature(blobHash, apiToken, expHex) {
-		return PermissionError
-	}
-	return nil
-}
-
-// ParseHexTimestamp parses timestamp
-func ParseHexTimestamp(timestampHex string) (ts time.Time, err error) {
-	if tsInt, e := strconv.ParseInt(timestampHex, 16, 0); e == nil {
-		ts = time.Unix(tsInt, 0)
-	} else {
-		err = e
+	err := keepclient.VerifySignature(signedLocator, apiToken, PermissionSecret)
+	if err != nil {
+		if err == keepclient.PermissionError {
+			return PermissionError
+		} else if err == keepclient.ExpiredError {
+			return ExpiredError
+		}
 	}
-	return ts, err
+	return err
 }
diff --git a/services/keepstore/perms_test.go b/services/keepstore/perms_test.go
index 59516af..9b4e30a 100644
--- a/services/keepstore/perms_test.go
+++ b/services/keepstore/perms_test.go
@@ -1,6 +1,7 @@
 package main
 
 import (
+	"strconv"
 	"testing"
 	"time"
 )
@@ -26,12 +27,12 @@ func TestSignLocator(t *testing.T) {
 	PermissionSecret = []byte(knownKey)
 	defer func() { PermissionSecret = nil }()
 
-	if ts, err := ParseHexTimestamp(knownTimestamp); err != nil {
-		t.Errorf("bad knownTimestamp %s", knownTimestamp)
-	} else {
-		if knownSignedLocator != SignLocator(knownLocator, knownToken, ts) {
-			t.Fail()
-		}
+	tsInt, err := strconv.ParseInt(knownTimestamp, 16, 0)
+	if err != nil {
+		t.Fail()
+	}
+	if knownSignedLocator != SignLocator(knownLocator, knownToken, time.Unix(tsInt, 0)) {
+		t.Fail()
 	}
 }
 

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list