[ARVADOS] updated: f814b4b5d0c6168c95d2eda87a45c87df07c72c9

git at public.curoverse.com git at public.curoverse.com
Fri Oct 9 14:23:50 EDT 2015


Summary of changes:
 sdk/go/keepclient/perms.go           | 33 ++++++++++++++++++++-------------
 sdk/go/keepclient/perms_test.go      | 10 +++++-----
 services/keepstore/keepstore_test.go |  2 +-
 services/keepstore/perms_test.go     |  4 ++--
 4 files changed, 28 insertions(+), 21 deletions(-)

  discards  7940276742ab045a83c389690e10d748e16b7f64 (commit)
  discards  2bd15f5413cd6e76b30c0df81434a337170d4db5 (commit)
       via  f814b4b5d0c6168c95d2eda87a45c87df07c72c9 (commit)
       via  5e2bf20f75a532625719f0f779d1e26f9d4466c4 (commit)
       via  5b80941723d6be1fd22589026635a43b33e6cf20 (commit)
       via  5089c3bb94101d32028d0fc06283f43fd70af53c (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 (7940276742ab045a83c389690e10d748e16b7f64)
            \
             N -- N -- N (f814b4b5d0c6168c95d2eda87a45c87df07c72c9)

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 f814b4b5d0c6168c95d2eda87a45c87df07c72c9
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri Oct 9 14:28:29 2015 -0400

    7167: Deobfuscate variable names

diff --git a/sdk/go/keepclient/perms.go b/sdk/go/keepclient/perms.go
index a73db76..12105c6 100644
--- a/sdk/go/keepclient/perms.go
+++ b/sdk/go/keepclient/perms.go
@@ -76,14 +76,14 @@ func VerifySignature(signedLocator, apiToken string, permissionSecret []byte) er
 		return ErrSignatureMissing
 	}
 	blobHash := matches[1]
-	sigHex := matches[2]
-	expHex := matches[3]
-	if expTime, err := parseHexTimestamp(expHex); err != nil {
+	signatureHex := matches[2]
+	expiryHex := matches[3]
+	if expiryTime, err := parseHexTimestamp(expiryHex); err != nil {
 		return ErrSignatureInvalid
-	} else if expTime.Before(time.Now()) {
+	} else if expiryTime.Before(time.Now()) {
 		return ErrSignatureExpired
 	}
-	if sigHex != makePermSignature(blobHash, apiToken, expHex, permissionSecret) {
+	if signatureHex != makePermSignature(blobHash, apiToken, expiryHex, permissionSecret) {
 		return ErrSignatureInvalid
 	}
 	return nil

commit 5e2bf20f75a532625719f0f779d1e26f9d4466c4
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri Oct 9 14:20:15 2015 -0400

    7167: Update tests and comments to new error vars.

diff --git a/sdk/go/keepclient/perms.go b/sdk/go/keepclient/perms.go
index 7dc06e2..a73db76 100644
--- a/sdk/go/keepclient/perms.go
+++ b/sdk/go/keepclient/perms.go
@@ -16,10 +16,15 @@ import (
 )
 
 var (
-	ErrSignatureExpired   = errors.New("Signature expired")
-	ErrSignatureInvalid   = errors.New("Invalid signature")
-	ErrSignatureMalformed = errors.New("Malformed signature")
-	ErrSignatureMissing   = errors.New("Missing signature")
+	// ErrSignatureExpired - a signature was rejected because the
+	// expiry time has passed.
+	ErrSignatureExpired = errors.New("Signature expired")
+	// ErrSignatureInvalid - a signature was rejected because it
+	// was badly formatted or did not match the given secret key.
+	ErrSignatureInvalid = errors.New("Invalid signature")
+	// ErrSignatureMissing - the given locator does not have a
+	// signature hint.
+	ErrSignatureMissing = errors.New("Missing signature")
 )
 
 // makePermSignature generates a SHA-1 HMAC digest for the given blob,
@@ -57,9 +62,11 @@ var signedLocatorRe = regexp.MustCompile(`^([[:xdigit:]]{32}).*\+A([[:xdigit:]]{
 
 // 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.
+// ErrSignatureExpired (if the signature's expiry time has passed,
+// which is something the client could have figured out
+// independently), ErrSignatureMissing (if there is no signature hint
+// at all), or ErrSignatureInvalid (if the signature is present but
+// badly formatted or incorrect).
 //
 // This function is intended to be used by system components and admin
 // utilities: userland programs do not know the permissionSecret.
@@ -72,7 +79,7 @@ func VerifySignature(signedLocator, apiToken string, permissionSecret []byte) er
 	sigHex := matches[2]
 	expHex := matches[3]
 	if expTime, err := parseHexTimestamp(expHex); err != nil {
-		return ErrSignatureMalformed
+		return ErrSignatureInvalid
 	} else if expTime.Before(time.Now()) {
 		return ErrSignatureExpired
 	}
diff --git a/sdk/go/keepclient/perms_test.go b/sdk/go/keepclient/perms_test.go
index 61d10c1..1380795 100644
--- a/sdk/go/keepclient/perms_test.go
+++ b/sdk/go/keepclient/perms_test.go
@@ -65,26 +65,26 @@ func TestVerifySignatureWrongSize(t *testing.T) {
 
 func TestVerifySignatureBadSig(t *testing.T) {
 	badLocator := knownLocator + "+Aaaaaaaaaaaaaaaa@" + knownTimestamp
-	if VerifySignature(badLocator, knownToken, []byte(knownKey)) != PermissionError {
+	if VerifySignature(badLocator, knownToken, []byte(knownKey)) != ErrSignatureMissing {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureBadTimestamp(t *testing.T) {
 	badLocator := knownLocator + "+A" + knownSignature + "@OOOOOOOl"
-	if VerifySignature(badLocator, knownToken, []byte(knownKey)) != PermissionError {
+	if VerifySignature(badLocator, knownToken, []byte(knownKey)) != ErrSignatureMissing {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureBadSecret(t *testing.T) {
-	if VerifySignature(knownSignedLocator, knownToken, []byte("00000000000000000000")) != PermissionError {
+	if VerifySignature(knownSignedLocator, knownToken, []byte("00000000000000000000")) != ErrSignatureInvalid {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureBadToken(t *testing.T) {
-	if VerifySignature(knownSignedLocator, "00000000", []byte(knownKey)) != PermissionError {
+	if VerifySignature(knownSignedLocator, "00000000", []byte(knownKey)) != ErrSignatureInvalid {
 		t.Fail()
 	}
 }
@@ -92,7 +92,7 @@ func TestVerifySignatureBadToken(t *testing.T) {
 func TestVerifySignatureExpired(t *testing.T) {
 	yesterday := time.Now().AddDate(0, 0, -1)
 	expiredLocator := SignLocator(knownHash, knownToken, yesterday, []byte(knownKey))
-	if VerifySignature(expiredLocator, knownToken, []byte(knownKey)) != ExpiredError {
+	if VerifySignature(expiredLocator, knownToken, []byte(knownKey)) != ErrSignatureExpired {
 		t.Fail()
 	}
 }
diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 9b13292..8a004b7 100644
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -185,7 +185,7 @@ func TestPutBlockMD5Fail(t *testing.T) {
 	// Check that PutBlock returns the expected error when the hash does
 	// not match the block.
 	if _, err := PutBlock(BadBlock, TestHash); err != RequestHashError {
-		t.Error("Expected RequestHashError, got %v", err)
+		t.Errorf("Expected RequestHashError, got %v", err)
 	}
 
 	// Confirm that GetBlock fails to return anything.

commit 5b80941723d6be1fd22589026635a43b33e6cf20
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 5089c3bb94101d32028d0fc06283f43fd70af53c
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..f4443fc 100644
--- a/services/keepstore/perms_test.go
+++ b/services/keepstore/perms_test.go
@@ -24,103 +24,39 @@ const (
 )
 
 func TestSignLocator(t *testing.T) {
-	PermissionSecret = []byte(knownKey)
-	defer func() { PermissionSecret = nil }()
+	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 TestVerifyLocator(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")
 	}
 }

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list