[ARVADOS] updated: 4d9ece42c8482948e4eca9f587cdb36f62a00e2b

Git user git at public.curoverse.com
Thu Apr 21 09:32:36 EDT 2016


Summary of changes:
 doc/install/install-keepstore.html.textile.liquid |  2 +-
 sdk/go/keepclient/perms.go                        | 14 +++++++-----
 sdk/go/keepclient/perms_test.go                   | 28 +++++++++++------------
 services/api/app/models/blob.rb                   | 10 ++++----
 services/api/config/application.default.yml       | 21 +++++++++--------
 services/keepstore/perms_test.go                  |  4 ++--
 tools/keep-block-check/keep-block-check.go        | 22 +++++++++---------
 tools/keep-block-check/keep-block-check_test.go   | 18 +++++++--------
 tools/keep-rsync/keep-rsync.go                    | 28 +++++++++++------------
 tools/keep-rsync/keep-rsync_test.go               | 16 ++++++-------
 10 files changed, 85 insertions(+), 78 deletions(-)

       via  4d9ece42c8482948e4eca9f587cdb36f62a00e2b (commit)
       via  aec99288fab5ceed8dc746e62efaeee33ff38a82 (commit)
       via  af754a64c48e30aec75bbcb1b741a768faacb94c (commit)
      from  6a202a2765bdbac5062fd34231804527edfb6a14 (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 4d9ece42c8482948e4eca9f587cdb36f62a00e2b
Author: radhika <radhika at curoverse.com>
Date:   Thu Apr 21 09:32:11 2016 -0400

    8936: address review comments

diff --git a/doc/install/install-keepstore.html.textile.liquid b/doc/install/install-keepstore.html.textile.liquid
index 0f36c5d..b211ce6 100644
--- a/doc/install/install-keepstore.html.textile.liquid
+++ b/doc/install/install-keepstore.html.textile.liquid
@@ -41,7 +41,7 @@ Usage of ./keepstore:
   -azure-storage-account-name="": Azure storage account name used for subsequent --azure-storage-container-volume arguments.
   -azure-storage-container-volume=[]: Use the given container as a storage volume. Can be given multiple times.
   -azure-storage-replication=3: Replication level to report to clients when data is stored in an Azure container.
-  -blob-signature-ttl=1209600: Lifetime of blob permission signatures. This will become a part of the signing key, and will cause clients to retry or fail if changed while they are in progress. See services/api/config/application.default.yml.
+  -blob-signature-ttl=1209600: Lifetime of blob permission signatures. Modifying the ttl will invalidate all existing signatures. See services/api/config/application.default.yml.
   -blob-signing-key-file="": File containing the secret key for generating and verifying blob permission signatures.
   -data-manager-token-file="": File with the API token used by the Data Manager. All DELETE requests or GET /index requests must carry this token.
   -enforce-permissions=false: Enforce permission signatures on requests.
diff --git a/sdk/go/keepclient/perms.go b/sdk/go/keepclient/perms.go
index 378fdcd..d650f0d 100644
--- a/sdk/go/keepclient/perms.go
+++ b/sdk/go/keepclient/perms.go
@@ -29,7 +29,7 @@ var (
 
 // makePermSignature generates a SHA-1 HMAC digest for the given blob,
 // token, expiry, and site secret.
-func makePermSignature(blobHash, apiToken, expiry string, blobSigningTTL time.Duration, permissionSecret []byte) string {
+func makePermSignature(blobHash, apiToken, expiry, blobSignatureTTL string, permissionSecret []byte) string {
 	hmac := hmac.New(sha1.New, permissionSecret)
 	hmac.Write([]byte(blobHash))
 	hmac.Write([]byte("@"))
@@ -37,7 +37,7 @@ func makePermSignature(blobHash, apiToken, expiry string, blobSigningTTL time.Du
 	hmac.Write([]byte("@"))
 	hmac.Write([]byte(expiry))
 	hmac.Write([]byte("@"))
-	hmac.Write([]byte(strconv.Itoa(int(blobSigningTTL.Seconds()))))
+	hmac.Write([]byte(blobSignatureTTL))
 	digest := hmac.Sum(nil)
 	return fmt.Sprintf("%x", digest)
 }
@@ -48,15 +48,16 @@ func makePermSignature(blobHash, apiToken, expiry string, blobSigningTTL time.Du
 //
 // 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, blobSigningTTL time.Duration, permissionSecret []byte) string {
+func SignLocator(blobLocator, apiToken string, expiry time.Time, blobSignatureTTL time.Duration, permissionSecret []byte) string {
 	if len(permissionSecret) == 0 || apiToken == "" {
 		return blobLocator
 	}
 	// Strip off all hints: only the hash is used to sign.
 	blobHash := strings.Split(blobLocator, "+")[0]
 	timestampHex := fmt.Sprintf("%08x", expiry.Unix())
+	blobSignatureTTLHex := strconv.FormatInt(int64(blobSignatureTTL.Seconds()), 16)
 	return blobLocator +
-		"+A" + makePermSignature(blobHash, apiToken, timestampHex, blobSigningTTL, permissionSecret) +
+		"+A" + makePermSignature(blobHash, apiToken, timestampHex, blobSignatureTTLHex, permissionSecret) +
 		"@" + timestampHex
 }
 
@@ -72,7 +73,7 @@ var signedLocatorRe = regexp.MustCompile(`^([[:xdigit:]]{32}).*\+A([[:xdigit:]]{
 //
 // 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, blobSigningTTL time.Duration, permissionSecret []byte) error {
+func VerifySignature(signedLocator, apiToken string, blobSignatureTTL time.Duration, permissionSecret []byte) error {
 	matches := signedLocatorRe.FindStringSubmatch(signedLocator)
 	if matches == nil {
 		return ErrSignatureMissing
@@ -85,7 +86,8 @@ func VerifySignature(signedLocator, apiToken string, blobSigningTTL time.Duratio
 	} else if expiryTime.Before(time.Now()) {
 		return ErrSignatureExpired
 	}
-	if signatureHex != makePermSignature(blobHash, apiToken, expiryHex, blobSigningTTL, permissionSecret) {
+	blobSignatureTTLHex := strconv.FormatInt(int64(blobSignatureTTL.Seconds()), 16)
+	if signatureHex != makePermSignature(blobHash, apiToken, expiryHex, blobSignatureTTLHex, permissionSecret) {
 		return ErrSignatureInvalid
 	}
 	return nil
diff --git a/sdk/go/keepclient/perms_test.go b/sdk/go/keepclient/perms_test.go
index f0b8f96..a566e0e 100644
--- a/sdk/go/keepclient/perms_test.go
+++ b/sdk/go/keepclient/perms_test.go
@@ -20,80 +20,80 @@ const (
 	knownTimestamp     = "7fffffff"
 	knownSigHint       = "+A" + knownSignature + "@" + knownTimestamp
 	knownSignedLocator = knownLocator + knownSigHint
-	blobSigningTtl     = time.Duration(1) * time.Second
+	blobSignatureTTL     = time.Second
 )
 
 func TestSignLocator(t *testing.T) {
 	if ts, err := parseHexTimestamp(knownTimestamp); err != nil {
 		t.Errorf("bad knownTimestamp %s", knownTimestamp)
 	} else {
-		if knownSignedLocator != SignLocator(knownLocator, knownToken, ts, blobSigningTtl, []byte(knownKey)) {
+		if knownSignedLocator != SignLocator(knownLocator, knownToken, ts, blobSignatureTTL, []byte(knownKey)) {
 			t.Fail()
 		}
 	}
 }
 
 func TestVerifySignature(t *testing.T) {
-	if VerifySignature(knownSignedLocator, knownToken, blobSigningTtl, []byte(knownKey)) != nil {
+	if VerifySignature(knownSignedLocator, knownToken, blobSignatureTTL, []byte(knownKey)) != nil {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureExtraHints(t *testing.T) {
-	if VerifySignature(knownLocator+"+K at xyzzy"+knownSigHint, knownToken, blobSigningTtl, []byte(knownKey)) != nil {
+	if VerifySignature(knownLocator+"+K at xyzzy"+knownSigHint, knownToken, blobSignatureTTL, []byte(knownKey)) != nil {
 		t.Fatal("Verify cannot handle hint before permission signature")
 	}
 
-	if VerifySignature(knownLocator+knownSigHint+"+Zfoo", knownToken, blobSigningTtl, []byte(knownKey)) != nil {
+	if VerifySignature(knownLocator+knownSigHint+"+Zfoo", knownToken, blobSignatureTTL, []byte(knownKey)) != nil {
 		t.Fatal("Verify cannot handle hint after permission signature")
 	}
 
-	if VerifySignature(knownLocator+"+K at xyzzy"+knownSigHint+"+Zfoo", knownToken, blobSigningTtl, []byte(knownKey)) != nil {
+	if VerifySignature(knownLocator+"+K at xyzzy"+knownSigHint+"+Zfoo", knownToken, blobSignatureTTL, []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) {
-	if VerifySignature(knownHash+"+999999"+knownSigHint, knownToken, blobSigningTtl, []byte(knownKey)) != nil {
+	if VerifySignature(knownHash+"+999999"+knownSigHint, knownToken, blobSignatureTTL, []byte(knownKey)) != nil {
 		t.Fatal("Verify cannot handle incorrect size hint")
 	}
 
-	if VerifySignature(knownHash+knownSigHint, knownToken, blobSigningTtl, []byte(knownKey)) != nil {
+	if VerifySignature(knownHash+knownSigHint, knownToken, blobSignatureTTL, []byte(knownKey)) != nil {
 		t.Fatal("Verify cannot handle missing size hint")
 	}
 }
 
 func TestVerifySignatureBadSig(t *testing.T) {
 	badLocator := knownLocator + "+Aaaaaaaaaaaaaaaa@" + knownTimestamp
-	if VerifySignature(badLocator, knownToken, blobSigningTtl, []byte(knownKey)) != ErrSignatureMissing {
+	if VerifySignature(badLocator, knownToken, blobSignatureTTL, []byte(knownKey)) != ErrSignatureMissing {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureBadTimestamp(t *testing.T) {
 	badLocator := knownLocator + "+A" + knownSignature + "@OOOOOOOl"
-	if VerifySignature(badLocator, knownToken, blobSigningTtl, []byte(knownKey)) != ErrSignatureMissing {
+	if VerifySignature(badLocator, knownToken, blobSignatureTTL, []byte(knownKey)) != ErrSignatureMissing {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureBadSecret(t *testing.T) {
-	if VerifySignature(knownSignedLocator, knownToken, blobSigningTtl, []byte("00000000000000000000")) != ErrSignatureInvalid {
+	if VerifySignature(knownSignedLocator, knownToken, blobSignatureTTL, []byte("00000000000000000000")) != ErrSignatureInvalid {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureBadToken(t *testing.T) {
-	if VerifySignature(knownSignedLocator, "00000000", blobSigningTtl, []byte(knownKey)) != ErrSignatureInvalid {
+	if VerifySignature(knownSignedLocator, "00000000", blobSignatureTTL, []byte(knownKey)) != ErrSignatureInvalid {
 		t.Fail()
 	}
 }
 
 func TestVerifySignatureExpired(t *testing.T) {
 	yesterday := time.Now().AddDate(0, 0, -1)
-	expiredLocator := SignLocator(knownHash, knownToken, yesterday, blobSigningTtl, []byte(knownKey))
-	if VerifySignature(expiredLocator, knownToken, blobSigningTtl, []byte(knownKey)) != ErrSignatureExpired {
+	expiredLocator := SignLocator(knownHash, knownToken, yesterday, blobSignatureTTL, []byte(knownKey))
+	if VerifySignature(expiredLocator, knownToken, blobSignatureTTL, []byte(knownKey)) != ErrSignatureExpired {
 		t.Fail()
 	}
 }
diff --git a/services/api/app/models/blob.rb b/services/api/app/models/blob.rb
index 91007fc..41d5b27 100644
--- a/services/api/app/models/blob.rb
+++ b/services/api/app/models/blob.rb
@@ -49,11 +49,12 @@ class Blob
     end
     timestamp_hex = timestamp.to_s(16)
     # => "53163cb4"
+    blob_signature_ttl = Rails.configuration.blob_signature_ttl.to_s(16)
 
     # Generate a signature.
     signature =
       generate_signature((opts[:key] or Rails.configuration.blob_signing_key),
-                         blob_hash, opts[:api_token], timestamp_hex)
+                         blob_hash, opts[:api_token], timestamp_hex, blob_signature_ttl)
 
     blob_locator + '+A' + signature + '@' + timestamp_hex
   end
@@ -96,10 +97,11 @@ class Blob
     if timestamp.to_i(16) < (opts[:now] or db_current_time.to_i)
       raise Blob::InvalidSignatureError.new 'Signature expiry time has passed.'
     end
+    blob_signature_ttl = Rails.configuration.blob_signature_ttl.to_s(16)
 
     my_signature =
       generate_signature((opts[:key] or Rails.configuration.blob_signing_key),
-                         blob_hash, opts[:api_token], timestamp)
+                         blob_hash, opts[:api_token], timestamp, blob_signature_ttl)
 
     if my_signature != given_signature
       raise Blob::InvalidSignatureError.new 'Signature is invalid.'
@@ -108,11 +110,11 @@ class Blob
     true
   end
 
-  def self.generate_signature key, blob_hash, api_token, timestamp
+  def self.generate_signature key, blob_hash, api_token, timestamp, blob_signature_ttl
     OpenSSL::HMAC.hexdigest('sha1', key,
                             [blob_hash,
                              api_token,
                              timestamp,
-                             Rails.configuration.blob_signature_ttl].join('@'))
+                             blob_signature_ttl].join('@'))
   end
 end
diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index 09ce8b6..c709633 100644
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -153,9 +153,7 @@ common:
   ###
 
   # Lifetime (in seconds) of blob permission signatures generated by
-  # the API server. This will become a part of the blob signing key,
-  # and will cause clients to retry or fail if changed while they are
-  # in progres.This determines how long a client can take (after
+  # the API server. This determines how long a client can take (after
   # retrieving a collection record) to retrieve the collection data
   # from Keep. If the client needs more time than that (assuming the
   # collection still has the same content and the relevant user/token
diff --git a/services/keepstore/perms_test.go b/services/keepstore/perms_test.go
index bab5ed2..c8289c2 100644
--- a/services/keepstore/perms_test.go
+++ b/services/keepstore/perms_test.go
@@ -34,7 +34,7 @@ func TestSignLocator(t *testing.T) {
 	}
 	t0 := time.Unix(tsInt, 0)
 
-	blobSignatureTTL = time.Duration(1) * time.Second
+	blobSignatureTTL = time.Second
 
 	PermissionSecret = []byte(knownKey)
 	if x := SignLocator(knownLocator, knownToken, t0); x != knownSignedLocator {
@@ -52,7 +52,7 @@ func TestVerifyLocator(t *testing.T) {
 		PermissionSecret = b
 	}(PermissionSecret)
 
-	blobSignatureTTL = time.Duration(1) * time.Second
+	blobSignatureTTL = time.Second
 
 	PermissionSecret = []byte(knownKey)
 	if err := VerifySignature(knownSignedLocator, knownToken); err != nil {
diff --git a/tools/keep-block-check/keep-block-check.go b/tools/keep-block-check/keep-block-check.go
index ed546f0..f27a4bc 100644
--- a/tools/keep-block-check/keep-block-check.go
+++ b/tools/keep-block-check/keep-block-check.go
@@ -48,10 +48,10 @@ func doMain(args []string) error {
 		"",
 		"Block hash prefix. When a prefix is specified, only hashes listed in the file with this prefix will be checked.")
 
-	blobSigningTTL := flags.Duration(
+	blobSignatureTTL := flags.Duration(
 		"blob-signing-ttl",
-		0*time.Second,
-		"Lifetime of blob permission signatures on the keepservers. If not provided, this will be retrieved from the keepservers.")
+		0,
+		"Lifetime of blob permission signatures on the keepservers. If not provided, this will be retrieved from the API server's discovery document.")
 
 	verbose := flags.Bool(
 		"v",
@@ -73,12 +73,12 @@ func doMain(args []string) error {
 	}
 
 	// setup keepclient
-	kc, err := setupKeepClient(config, *keepServicesJSON, *blobSigningTTL)
+	kc, err := setupKeepClient(config, *keepServicesJSON, *blobSignatureTTL)
 	if err != nil {
 		return fmt.Errorf("Error configuring keepclient: %s", err.Error())
 	}
 
-	return performKeepBlockCheck(kc, *blobSigningTTL, blobSigningKey, blockLocators, *verbose)
+	return performKeepBlockCheck(kc, *blobSignatureTTL, blobSigningKey, blockLocators, *verbose)
 }
 
 type apiConfig struct {
@@ -143,7 +143,7 @@ func readConfigFromFile(filename string) (config apiConfig, blobSigningKey strin
 }
 
 // setup keepclient using the config provided
-func setupKeepClient(config apiConfig, keepServicesJSON string, blobSigningTTL time.Duration) (kc *keepclient.KeepClient, err error) {
+func setupKeepClient(config apiConfig, keepServicesJSON string, blobSignatureTTL time.Duration) (kc *keepclient.KeepClient, err error) {
 	arv := arvadosclient.ArvadosClient{
 		ApiToken:    config.APIToken,
 		ApiServer:   config.APIHost,
@@ -167,11 +167,11 @@ func setupKeepClient(config apiConfig, keepServicesJSON string, blobSigningTTL t
 		}
 	}
 
-	// Get if blobSigningTTL is not provided
-	if blobSigningTTL == 0 {
+	// Get if blobSignatureTTL is not provided
+	if blobSignatureTTL == 0 {
 		value, err := arv.Discovery("blobSignatureTtl")
 		if err == nil {
-			blobSigningTTL = time.Duration(int(value.(float64))) * time.Second
+			blobSignatureTTL = time.Duration(int(value.(float64))) * time.Second
 		} else {
 			return nil, err
 		}
@@ -206,7 +206,7 @@ func getBlockLocators(locatorFile, prefix string) (locators []string, err error)
 }
 
 // Get block headers from keep. Log any errors.
-func performKeepBlockCheck(kc *keepclient.KeepClient, blobSigningTTL time.Duration, blobSigningKey string, blockLocators []string, verbose bool) error {
+func performKeepBlockCheck(kc *keepclient.KeepClient, blobSignatureTTL time.Duration, blobSigningKey string, blockLocators []string, verbose bool) error {
 	totalBlocks := len(blockLocators)
 	notFoundBlocks := 0
 	current := 0
@@ -218,7 +218,7 @@ func performKeepBlockCheck(kc *keepclient.KeepClient, blobSigningTTL time.Durati
 		getLocator := locator
 		if blobSigningKey != "" {
 			expiresAt := time.Now().AddDate(0, 0, 1)
-			getLocator = keepclient.SignLocator(locator, kc.Arvados.ApiToken, expiresAt, blobSigningTTL, []byte(blobSigningKey))
+			getLocator = keepclient.SignLocator(locator, kc.Arvados.ApiToken, expiresAt, blobSignatureTTL, []byte(blobSigningKey))
 		}
 
 		_, _, err := kc.Ask(getLocator)
diff --git a/tools/keep-block-check/keep-block-check_test.go b/tools/keep-block-check/keep-block-check_test.go
index 9b52ded..6c77612 100644
--- a/tools/keep-block-check/keep-block-check_test.go
+++ b/tools/keep-block-check/keep-block-check_test.go
@@ -36,7 +36,7 @@ var logBuffer bytes.Buffer
 var TestHash = "aaaa09c290d0fb1ca068ffaddf22cbd0"
 var TestHash2 = "aaaac516f788aec4f30932ffb6395c39"
 
-var blobSigningTTL = time.Duration(2*7*24) * time.Hour
+var blobSignatureTTL = time.Duration(2*7*24) * time.Hour
 
 func (s *ServerRequiredSuite) SetUpSuite(c *C) {
 	arvadostest.StartAPI()
@@ -82,7 +82,7 @@ func setupKeepBlockCheck(c *C, enforcePermissions bool, keepServicesJSON string)
 
 	// setup keepclients
 	var err error
-	kc, err = setupKeepClient(config, keepServicesJSON, blobSigningTTL)
+	kc, err = setupKeepClient(config, keepServicesJSON, blobSignatureTTL)
 	c.Check(err, IsNil)
 }
 
@@ -154,7 +154,7 @@ func checkNoErrorsLogged(c *C, prefix, suffix string) {
 func (s *ServerRequiredSuite) TestBlockCheck(c *C) {
 	setupKeepBlockCheck(c, false, "")
 	allLocators := setupTestData(c)
-	err := performKeepBlockCheck(kc, blobSigningTTL, "", allLocators, true)
+	err := performKeepBlockCheck(kc, blobSignatureTTL, "", allLocators, true)
 	c.Check(err, IsNil)
 	checkNoErrorsLogged(c, "Error verifying block", "Block not found")
 }
@@ -162,7 +162,7 @@ func (s *ServerRequiredSuite) TestBlockCheck(c *C) {
 func (s *ServerRequiredSuite) TestBlockCheckWithBlobSigning(c *C) {
 	setupKeepBlockCheck(c, true, "")
 	allLocators := setupTestData(c)
-	err := performKeepBlockCheck(kc, blobSigningTTL, arvadostest.BlobSigningKey, allLocators, true)
+	err := performKeepBlockCheck(kc, blobSignatureTTL, arvadostest.BlobSigningKey, allLocators, true)
 	c.Check(err, IsNil)
 	checkNoErrorsLogged(c, "Error verifying block", "Block not found")
 }
@@ -172,7 +172,7 @@ func (s *ServerRequiredSuite) TestBlockCheck_NoSuchBlock(c *C) {
 	allLocators := setupTestData(c)
 	allLocators = append(allLocators, TestHash)
 	allLocators = append(allLocators, TestHash2)
-	err := performKeepBlockCheck(kc, blobSigningTTL, "", allLocators, true)
+	err := performKeepBlockCheck(kc, blobSignatureTTL, "", allLocators, true)
 	c.Check(err, NotNil)
 	c.Assert(err.Error(), Equals, "Block verification failed for 2 out of 7 blocks with matching prefix.")
 	checkErrorLog(c, []string{TestHash, TestHash2}, "Error verifying block", "Block not found")
@@ -187,7 +187,7 @@ func (s *ServerRequiredSuite) TestBlockCheck_NoSuchBlock_WithMatchingPrefix(c *C
 	defer os.Remove(locatorFile)
 	locators, err := getBlockLocators(locatorFile, "aaa")
 	c.Check(err, IsNil)
-	err = performKeepBlockCheck(kc, blobSigningTTL, "", locators, true)
+	err = performKeepBlockCheck(kc, blobSignatureTTL, "", locators, true)
 	c.Check(err, NotNil)
 	// Of the 7 blocks in allLocators, only two match the prefix and hence only those are checked
 	c.Assert(err.Error(), Equals, "Block verification failed for 2 out of 2 blocks with matching prefix.")
@@ -203,14 +203,14 @@ func (s *ServerRequiredSuite) TestBlockCheck_NoSuchBlock_WithPrefixMismatch(c *C
 	defer os.Remove(locatorFile)
 	locators, err := getBlockLocators(locatorFile, "999")
 	c.Check(err, IsNil)
-	err = performKeepBlockCheck(kc, blobSigningTTL, "", locators, true)
+	err = performKeepBlockCheck(kc, blobSignatureTTL, "", locators, true)
 	c.Check(err, IsNil) // there were no matching locators in file and hence nothing was checked
 }
 
 func (s *ServerRequiredSuite) TestBlockCheck_BadSignature(c *C) {
 	setupKeepBlockCheck(c, true, "")
 	setupTestData(c)
-	err := performKeepBlockCheck(kc, blobSigningTTL, "badblobsigningkey", []string{TestHash, TestHash2}, false)
+	err := performKeepBlockCheck(kc, blobSignatureTTL, "badblobsigningkey", []string{TestHash, TestHash2}, false)
 	c.Assert(err.Error(), Equals, "Block verification failed for 2 out of 2 blocks with matching prefix.")
 	checkErrorLog(c, []string{TestHash, TestHash2}, "Error verifying block", "HTTP 403")
 	// verbose logging not requested
@@ -246,7 +246,7 @@ var testKeepServicesJSON = `{
 // Expect error during performKeepBlockCheck due to unreachable keepservers.
 func (s *ServerRequiredSuite) TestErrorDuringKeepBlockCheck_FakeKeepservers(c *C) {
 	setupKeepBlockCheck(c, false, testKeepServicesJSON)
-	err := performKeepBlockCheck(kc, blobSigningTTL, "", []string{TestHash, TestHash2}, true)
+	err := performKeepBlockCheck(kc, blobSignatureTTL, "", []string{TestHash, TestHash2}, true)
 	c.Assert(err.Error(), Equals, "Block verification failed for 2 out of 2 blocks with matching prefix.")
 	checkErrorLog(c, []string{TestHash, TestHash2}, "Error verifying block", "")
 }
diff --git a/tools/keep-rsync/keep-rsync.go b/tools/keep-rsync/keep-rsync.go
index e46d368..912238c 100644
--- a/tools/keep-rsync/keep-rsync.go
+++ b/tools/keep-rsync/keep-rsync.go
@@ -60,10 +60,10 @@ func doMain() error {
 		"",
 		"Index prefix")
 
-	blobSigningTTL := flags.Duration(
+	srcBlobSignatureTTL := flags.Duration(
 		"blob-signing-ttl",
-		0*time.Second,
-		"Lifetime of blob permission signatures on source keepservers. If not provided, this will be retrieved from the keepservers.")
+		0,
+		"Lifetime of blob permission signatures on source keepservers. If not provided, this will be retrieved from the API server's discovery document.")
 
 	// Parse args; omit the first arg which is the command name
 	flags.Parse(os.Args[1:])
@@ -79,18 +79,18 @@ func doMain() error {
 	}
 
 	// setup src and dst keepclients
-	kcSrc, err := setupKeepClient(srcConfig, *srcKeepServicesJSON, false, 0, *blobSigningTTL)
+	kcSrc, err := setupKeepClient(srcConfig, *srcKeepServicesJSON, false, 0, *srcBlobSignatureTTL)
 	if err != nil {
 		return fmt.Errorf("Error configuring src keepclient: %s", err.Error())
 	}
 
-	kcDst, err := setupKeepClient(dstConfig, *dstKeepServicesJSON, true, *replications, *blobSigningTTL)
+	kcDst, err := setupKeepClient(dstConfig, *dstKeepServicesJSON, true, *replications, 0)
 	if err != nil {
 		return fmt.Errorf("Error configuring dst keepclient: %s", err.Error())
 	}
 
 	// Copy blocks not found in dst from src
-	err = performKeepRsync(kcSrc, kcDst, *blobSigningTTL, srcBlobSigningKey, *prefix)
+	err = performKeepRsync(kcSrc, kcDst, *srcBlobSignatureTTL, srcBlobSigningKey, *prefix)
 	if err != nil {
 		return fmt.Errorf("Error while syncing data: %s", err.Error())
 	}
@@ -160,7 +160,7 @@ func readConfigFromFile(filename string) (config apiConfig, blobSigningKey strin
 }
 
 // setup keepclient using the config provided
-func setupKeepClient(config apiConfig, keepServicesJSON string, isDst bool, replications int, blobSigningTTL time.Duration) (kc *keepclient.KeepClient, err error) {
+func setupKeepClient(config apiConfig, keepServicesJSON string, isDst bool, replications int, srcBlobSignatureTTL time.Duration) (kc *keepclient.KeepClient, err error) {
 	arv := arvadosclient.ArvadosClient{
 		ApiToken:    config.APIToken,
 		ApiServer:   config.APIHost,
@@ -198,11 +198,11 @@ func setupKeepClient(config apiConfig, keepServicesJSON string, isDst bool, repl
 		kc.Want_replicas = replications
 	}
 
-	// If blobSigningTTL is not provided, get it from source
-	if !isDst && blobSigningTTL == 0 {
+	// If srcBlobSignatureTTL is not provided, get it from API server discovery doc
+	if !isDst && srcBlobSignatureTTL == 0 {
 		value, err := arv.Discovery("blobSignatureTtl")
 		if err == nil {
-			blobSigningTTL = time.Duration(int(value.(float64))) * time.Second
+			srcBlobSignatureTTL = time.Duration(int(value.(float64))) * time.Second
 		} else {
 			return nil, err
 		}
@@ -213,7 +213,7 @@ func setupKeepClient(config apiConfig, keepServicesJSON string, isDst bool, repl
 
 // Get unique block locators from src and dst
 // Copy any blocks missing in dst
-func performKeepRsync(kcSrc, kcDst *keepclient.KeepClient, blobSigningTTL time.Duration, blobSigningKey, prefix string) error {
+func performKeepRsync(kcSrc, kcDst *keepclient.KeepClient, srcBlobSignatureTTL time.Duration, blobSigningKey, prefix string) error {
 	// Get unique locators from src
 	srcIndex, err := getUniqueLocators(kcSrc, prefix)
 	if err != nil {
@@ -233,7 +233,7 @@ func performKeepRsync(kcSrc, kcDst *keepclient.KeepClient, blobSigningTTL time.D
 	log.Printf("Before keep-rsync, there are %d blocks in src and %d blocks in dst. Start copying %d blocks from src not found in dst.",
 		len(srcIndex), len(dstIndex), len(toBeCopied))
 
-	err = copyBlocksToDst(toBeCopied, kcSrc, kcDst, blobSigningTTL, blobSigningKey)
+	err = copyBlocksToDst(toBeCopied, kcSrc, kcDst, srcBlobSignatureTTL, blobSigningKey)
 
 	return err
 }
@@ -269,7 +269,7 @@ func getMissingLocators(srcLocators, dstLocators map[string]bool) []string {
 }
 
 // Copy blocks from src to dst; only those that are missing in dst are copied
-func copyBlocksToDst(toBeCopied []string, kcSrc, kcDst *keepclient.KeepClient, blobSigningTTL time.Duration, blobSigningKey string) error {
+func copyBlocksToDst(toBeCopied []string, kcSrc, kcDst *keepclient.KeepClient, srcBlobSignatureTTL time.Duration, blobSigningKey string) error {
 	total := len(toBeCopied)
 
 	startedAt := time.Now()
@@ -286,7 +286,7 @@ func copyBlocksToDst(toBeCopied []string, kcSrc, kcDst *keepclient.KeepClient, b
 		getLocator := locator
 		expiresAt := time.Now().AddDate(0, 0, 1)
 		if blobSigningKey != "" {
-			getLocator = keepclient.SignLocator(getLocator, kcSrc.Arvados.ApiToken, expiresAt, blobSigningTTL, []byte(blobSigningKey))
+			getLocator = keepclient.SignLocator(getLocator, kcSrc.Arvados.ApiToken, expiresAt, srcBlobSignatureTTL, []byte(blobSigningKey))
 		}
 
 		reader, len, _, err := kcSrc.Get(getLocator)
diff --git a/tools/keep-rsync/keep-rsync_test.go b/tools/keep-rsync/keep-rsync_test.go
index 90b16b2..f1f6a5f 100644
--- a/tools/keep-rsync/keep-rsync_test.go
+++ b/tools/keep-rsync/keep-rsync_test.go
@@ -49,7 +49,7 @@ func (s *DoMainTestSuite) SetUpSuite(c *C) {
 
 var kcSrc, kcDst *keepclient.KeepClient
 var srcKeepServicesJSON, dstKeepServicesJSON, blobSigningKey string
-var blobSigningTTL = time.Duration(2*7*24) * time.Hour
+var blobSignatureTTL = time.Duration(2*7*24) * time.Hour
 
 func (s *ServerRequiredSuite) SetUpTest(c *C) {
 	// reset all variables between tests
@@ -100,7 +100,7 @@ func setupRsync(c *C, enforcePermissions bool, replications int) {
 
 	// setup keepclients
 	var err error
-	kcSrc, err = setupKeepClient(srcConfig, srcKeepServicesJSON, false, 0, blobSigningTTL)
+	kcSrc, err = setupKeepClient(srcConfig, srcKeepServicesJSON, false, 0, blobSignatureTTL)
 	c.Check(err, IsNil)
 
 	kcDst, err = setupKeepClient(dstConfig, dstKeepServicesJSON, true, replications, 0)
@@ -175,7 +175,7 @@ func testNoCrosstalk(c *C, testData string, kc1, kc2 *keepclient.KeepClient) {
 	c.Assert(err, Equals, nil)
 
 	locator = strings.Split(locator, "+")[0]
-	_, _, _, err = kc2.Get(keepclient.SignLocator(locator, kc2.Arvados.ApiToken, time.Now().AddDate(0, 0, 1), blobSigningTTL, []byte(blobSigningKey)))
+	_, _, _, err = kc2.Get(keepclient.SignLocator(locator, kc2.Arvados.ApiToken, time.Now().AddDate(0, 0, 1), blobSignatureTTL, []byte(blobSigningKey)))
 	c.Assert(err, NotNil)
 	c.Check(err.Error(), Equals, "Block not found")
 }
@@ -257,7 +257,7 @@ func testKeepRsync(c *C, enforcePermissions bool, prefix string) {
 	// setupTestData
 	setupTestData(c, prefix)
 
-	err := performKeepRsync(kcSrc, kcDst, blobSigningTTL, blobSigningKey, prefix)
+	err := performKeepRsync(kcSrc, kcDst, blobSignatureTTL, blobSigningKey, prefix)
 	c.Check(err, IsNil)
 
 	// Now GetIndex from dst and verify that all 5 from src and the 2 extra blocks are found
@@ -328,7 +328,7 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_FakeSrcKeepservers(c *C) {
 
 	setupRsync(c, false, 1)
 
-	err := performKeepRsync(kcSrc, kcDst, blobSigningTTL, "", "")
+	err := performKeepRsync(kcSrc, kcDst, blobSignatureTTL, "", "")
 	log.Printf("Err = %v", err)
 	c.Check(strings.Contains(err.Error(), "no such host"), Equals, true)
 }
@@ -340,7 +340,7 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_FakeDstKeepservers(c *C) {
 
 	setupRsync(c, false, 1)
 
-	err := performKeepRsync(kcSrc, kcDst, blobSigningTTL, "", "")
+	err := performKeepRsync(kcSrc, kcDst, blobSignatureTTL, "", "")
 	log.Printf("Err = %v", err)
 	c.Check(strings.Contains(err.Error(), "no such host"), Equals, true)
 }
@@ -355,7 +355,7 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_ErrorGettingBlockFromSrc(c *C
 	// Change blob signing key to a fake key, so that Get from src fails
 	blobSigningKey = "thisisfakeblobsigningkey"
 
-	err := performKeepRsync(kcSrc, kcDst, blobSigningTTL, blobSigningKey, "")
+	err := performKeepRsync(kcSrc, kcDst, blobSignatureTTL, blobSigningKey, "")
 	c.Check(strings.Contains(err.Error(), "HTTP 403 \"Forbidden\""), Equals, true)
 }
 
@@ -369,7 +369,7 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_ErrorPuttingBlockInDst(c *C)
 	// Increase Want_replicas on dst to result in insufficient replicas error during Put
 	kcDst.Want_replicas = 2
 
-	err := performKeepRsync(kcSrc, kcDst, blobSigningTTL, blobSigningKey, "")
+	err := performKeepRsync(kcSrc, kcDst, blobSignatureTTL, blobSigningKey, "")
 	c.Check(strings.Contains(err.Error(), "Could not write sufficient replicas"), Equals, true)
 }
 

commit aec99288fab5ceed8dc746e62efaeee33ff38a82
Merge: 6a202a2 af754a6
Author: radhika <radhika at curoverse.com>
Date:   Thu Apr 21 07:23:00 2016 -0400

    Merge branch '8936-ttl-in-signing-key-TC' into 8936-ttl-in-signing-key


commit af754a64c48e30aec75bbcb1b741a768faacb94c
Author: Tom Clegg <tom at curoverse.com>
Date:   Mon Apr 18 16:23:01 2016 -0400

    8936: Warn about disruptive effect of modifying blob_signature_ttl and blob_signing_key.

diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index 6691683..c709633 100644
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -27,6 +27,11 @@ common:
   # generate permission signatures for Keep locators. It must be
   # identical to the permission key given to Keep. IMPORTANT: This is
   # a site secret. It should be at least 50 characters.
+  #
+  # Modifying blob_signing_key will invalidate all existing
+  # signatures, which can cause programs to fail (e.g., arv-put,
+  # arv-get, and Crunch jobs).  To avoid errors, rotate keys only when
+  # no such processes are running.
   blob_signing_key: ~
 
   # These settings are provided by your OAuth2 provider (e.g.,
@@ -155,12 +160,12 @@ common:
   # still has permission) the client can retrieve the collection again
   # to get fresh signatures.
   #
-  # Datamanager considers an unreferenced block older than this to be
-  # eligible for garbage collection. Therefore, it should never be
-  # smaller than the corresponding value used by any local keepstore
-  # service (see keepstore -blob-signature-ttl flag). This rule
-  # prevents datamanager from trying to garbage-collect recently
-  # written blocks while clients are still holding valid signatures.
+  # This must be exactly equal to the -blob-signature-ttl flag used by
+  # keepstore servers.  Otherwise, reading data blocks and saving
+  # collections will fail with HTTP 403 permission errors.
+  #
+  # Modifying blob_signature_ttl invalidates existing signatures; see
+  # blob_signing_key note above.
   #
   # The default is 2 weeks.
   blob_signature_ttl: 1209600

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list