From 5b80941723d6be1fd22589026635a43b33e6cf20 Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@curoverse.com>
Date: Thu, 8 Oct 2015 17:50:22 -0400
Subject: [PATCH] 7167: Fix up comments

---
 sdk/go/keepclient/perms.go | 70 +++++++++++---------------------------
 1 file changed, 19 insertions(+), 51 deletions(-)

diff --git a/sdk/go/keepclient/perms.go b/sdk/go/keepclient/perms.go
index 0797b070b8..7dc06e20af 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@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)
-- 
2.30.2