[ARVADOS] created: 3564cd933a31384c7b559f102cc9a48ef6122e84

git at public.curoverse.com git at public.curoverse.com
Mon Jun 8 15:41:28 EDT 2015


        at  3564cd933a31384c7b559f102cc9a48ef6122e84 (commit)


commit 3564cd933a31384c7b559f102cc9a48ef6122e84
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Jun 6 16:29:48 2015 -0400

    6222: Precompile all regexps. Remove wasted effort in GetBlockHandler.

diff --git a/services/keepstore/handler_test.go b/services/keepstore/handler_test.go
index 5bd14be..074e499 100644
--- a/services/keepstore/handler_test.go
+++ b/services/keepstore/handler_test.go
@@ -195,7 +195,7 @@ func TestPutHandler(t *testing.T) {
 		"Authenticated PUT, signed locator, with server key",
 		http.StatusOK, response)
 	response_locator := strings.TrimSpace(response.Body.String())
-	if !VerifySignature(response_locator, known_token) {
+	if VerifySignature(response_locator, known_token) != nil {
 		t.Errorf("Authenticated PUT, signed locator, with server key:\n"+
 			"response '%s' does not contain a valid signature",
 			response_locator)
@@ -788,7 +788,7 @@ func ExpectStatusCode(
 	expected_status int,
 	response *httptest.ResponseRecorder) {
 	if response.Code != expected_status {
-		t.Errorf("%s: expected status %s, got %+v",
+		t.Errorf("%s: expected status %d, got %+v",
 			testname, expected_status, response)
 	}
 }
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index 8930b79..2b437e7 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -20,7 +20,6 @@ import (
 	"os"
 	"regexp"
 	"strconv"
-	"strings"
 	"syscall"
 	"time"
 )
@@ -66,51 +65,15 @@ func BadRequestHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func GetBlockHandler(resp http.ResponseWriter, req *http.Request) {
-	hash := mux.Vars(req)["hash"]
-
-	hints := mux.Vars(req)["hints"]
-
-	// Parse the locator string and hints from the request.
-	// TODO(twp): implement a Locator type.
-	var signature, timestamp string
-	if hints != "" {
-		signature_pat, _ := regexp.Compile("^A([[:xdigit:]]+)@([[:xdigit:]]{8})$")
-		for _, hint := range strings.Split(hints, "+") {
-			if match, _ := regexp.MatchString("^[[:digit:]]+$", hint); match {
-				// Server ignores size hints
-			} else if m := signature_pat.FindStringSubmatch(hint); m != nil {
-				signature = m[1]
-				timestamp = m[2]
-			} else if match, _ := regexp.MatchString("^[[:upper:]]", hint); match {
-				// Any unknown hint that starts with an uppercase letter is
-				// presumed to be valid and ignored, to permit forward compatibility.
-			} else {
-				// Unknown format; not a valid locator.
-				http.Error(resp, BadRequestError.Error(), BadRequestError.HTTPCode)
-				return
-			}
-		}
-	}
-
-	// If permission checking is in effect, verify this
-	// request's permission signature.
 	if enforce_permissions {
-		if signature == "" || timestamp == "" {
-			http.Error(resp, PermissionError.Error(), PermissionError.HTTPCode)
-			return
-		} else if IsExpired(timestamp) {
-			http.Error(resp, ExpiredError.Error(), ExpiredError.HTTPCode)
+		locator := req.URL.Path[1:] // strip leading slash
+		if err := VerifySignature(locator, GetApiToken(req)); err != nil {
+			http.Error(resp, err.Error(), err.(*KeepError).HTTPCode)
 			return
-		} else {
-			req_locator := req.URL.Path[1:] // strip leading slash
-			if !VerifySignature(req_locator, GetApiToken(req)) {
-				http.Error(resp, PermissionError.Error(), PermissionError.HTTPCode)
-				return
-			}
 		}
 	}
 
-	block, err := GetBlock(hash, false)
+	block, err := GetBlock(mux.Vars(req)["hash"], false)
 	if err != nil {
 		// This type assertion is safe because the only errors
 		// GetBlock can return are DiskHashError or NotFoundError.
@@ -630,28 +593,25 @@ func PutBlock(block []byte, hash string) error {
 	}
 }
 
+var validLocatorRe = regexp.MustCompile(`^[0-9a-f]{32}$`)
+
 // IsValidLocator
 //     Return true if the specified string is a valid Keep locator.
 //     When Keep is extended to support hash types other than MD5,
 //     this should be updated to cover those as well.
 //
 func IsValidLocator(loc string) bool {
-	match, err := regexp.MatchString(`^[0-9a-f]{32}$`, loc)
-	if err == nil {
-		return match
-	}
-	log.Printf("IsValidLocator: %s\n", err)
-	return false
+	return validLocatorRe.MatchString(loc)
 }
 
+var authRe = regexp.MustCompile(`^OAuth2\s+(.*)`)
+
 // GetApiToken returns the OAuth2 token from the Authorization
 // header of a HTTP request, or an empty string if no matching
 // token is found.
 func GetApiToken(req *http.Request) string {
 	if auth, ok := req.Header["Authorization"]; ok {
-		if pat, err := regexp.Compile(`^OAuth2\s+(.*)`); err != nil {
-			log.Println(err)
-		} else if match := pat.FindStringSubmatch(auth[0]); match != nil {
+		if match := authRe.FindStringSubmatch(auth[0]); match != nil {
 			return match[1]
 		}
 	}
diff --git a/services/keepstore/perms.go b/services/keepstore/perms.go
index 1048f53..65160b1 100644
--- a/services/keepstore/perms.go
+++ b/services/keepstore/perms.go
@@ -82,22 +82,29 @@ func SignLocator(blob_locator string, api_token string, expiry time.Time) string
 
 var signedLocatorRe = regexp.MustCompile(`^([[:xdigit:]]{32}).*\+A([[:xdigit:]]{40})@([[:xdigit:]]{8})`)
 
-// VerifySignature returns true if the signature on the signed_locator
-// can be verified using the given api_token.
-func VerifySignature(signed_locator string, api_token string) bool {
+// VerifySignature returns nil if the signature on the signed_locator
+// can be verified using the given api_token. Otherwise it returns
+// either ExpiredError (if the timestamp has expired, which is
+// something the client could have figured out independently) or
+// PermissionError.
+func VerifySignature(signed_locator string, api_token string) error {
 	matches := signedLocatorRe.FindStringSubmatch(signed_locator)
 	if matches == nil {
 		// Could not find a permission signature at all
-		return false
+		return PermissionError
 	}
 	blob_hash := matches[1]
 	sig_hex := matches[2]
 	exp_hex := matches[3]
-	if exp_time, err := ParseHexTimestamp(exp_hex); err != nil || exp_time.Before(time.Now()) {
-		// Signature is expired, or timestamp is unparseable
-		return false
+	if exp_time, err := ParseHexTimestamp(exp_hex); err != nil {
+		return PermissionError
+	} else if exp_time.Before(time.Now()) {
+		return ExpiredError
 	}
-	return sig_hex == MakePermSignature(blob_hash, api_token, exp_hex)
+	if sig_hex != MakePermSignature(blob_hash, api_token, exp_hex) {
+		return PermissionError
+	}
+	return nil
 }
 
 func ParseHexTimestamp(timestamp_hex string) (ts time.Time, err error) {
diff --git a/services/keepstore/perms_test.go b/services/keepstore/perms_test.go
index 7367dbf..85883b0 100644
--- a/services/keepstore/perms_test.go
+++ b/services/keepstore/perms_test.go
@@ -39,7 +39,7 @@ func TestVerifySignature(t *testing.T) {
 	PermissionSecret = []byte(known_key)
 	defer func() { PermissionSecret = nil }()
 
-	if !VerifySignature(known_signed_locator, known_token) {
+	if VerifySignature(known_signed_locator, known_token) != nil {
 		t.Fail()
 	}
 }
@@ -48,15 +48,15 @@ func TestVerifySignatureExtraHints(t *testing.T) {
 	PermissionSecret = []byte(known_key)
 	defer func() { PermissionSecret = nil }()
 
-	if !VerifySignature(known_locator+"+K at xyzzy"+known_sig_hint, known_token) {
+	if VerifySignature(known_locator+"+K at xyzzy"+known_sig_hint, known_token) != nil{
 		t.Fatal("Verify cannot handle hint before permission signature")
 	}
 
-	if !VerifySignature(known_locator+known_sig_hint+"+Zfoo", known_token) {
+	if VerifySignature(known_locator+known_sig_hint+"+Zfoo", known_token) != nil {
 		t.Fatal("Verify cannot handle hint after permission signature")
 	}
 
-	if !VerifySignature(known_locator+"+K at xyzzy"+known_sig_hint+"+Zfoo", known_token) {
+	if VerifySignature(known_locator+"+K at xyzzy"+known_sig_hint+"+Zfoo", known_token) != nil {
 		t.Fatal("Verify cannot handle hints around permission signature")
 	}
 }
@@ -66,11 +66,11 @@ func TestVerifySignatureWrongSize(t *testing.T) {
 	PermissionSecret = []byte(known_key)
 	defer func() { PermissionSecret = nil }()
 
-	if !VerifySignature(known_hash+"+999999"+known_sig_hint, known_token) {
+	if VerifySignature(known_hash+"+999999"+known_sig_hint, known_token) != nil {
 		t.Fatal("Verify cannot handle incorrect size hint")
 	}
 
-	if !VerifySignature(known_hash+known_sig_hint, known_token) {
+	if VerifySignature(known_hash+known_sig_hint, known_token) != nil {
 		t.Fatal("Verify cannot handle missing size hint")
 	}
 }
@@ -80,7 +80,7 @@ func TestVerifySignatureBadSig(t *testing.T) {
 	defer func() { PermissionSecret = nil }()
 
 	bad_locator := known_locator + "+Aaaaaaaaaaaaaaaa@" + known_timestamp
-	if VerifySignature(bad_locator, known_token) {
+	if VerifySignature(bad_locator, known_token) != PermissionError {
 		t.Fail()
 	}
 }
@@ -89,8 +89,8 @@ func TestVerifySignatureBadTimestamp(t *testing.T) {
 	PermissionSecret = []byte(known_key)
 	defer func() { PermissionSecret = nil }()
 
-	bad_locator := known_locator + "+A" + known_signature + "@00000000"
-	if VerifySignature(bad_locator, known_token) {
+	bad_locator := known_locator + "+A" + known_signature + "@OOOOOOOl"
+	if VerifySignature(bad_locator, known_token) != PermissionError {
 		t.Fail()
 	}
 }
@@ -99,7 +99,7 @@ func TestVerifySignatureBadSecret(t *testing.T) {
 	PermissionSecret = []byte("00000000000000000000")
 	defer func() { PermissionSecret = nil }()
 
-	if VerifySignature(known_signed_locator, known_token) {
+	if VerifySignature(known_signed_locator, known_token) != PermissionError {
 		t.Fail()
 	}
 }
@@ -108,7 +108,7 @@ func TestVerifySignatureBadToken(t *testing.T) {
 	PermissionSecret = []byte(known_key)
 	defer func() { PermissionSecret = nil }()
 
-	if VerifySignature(known_signed_locator, "00000000") {
+	if VerifySignature(known_signed_locator, "00000000") != PermissionError {
 		t.Fail()
 	}
 }
@@ -119,7 +119,7 @@ func TestVerifySignatureExpired(t *testing.T) {
 
 	yesterday := time.Now().AddDate(0, 0, -1)
 	expired_locator := SignLocator(known_hash, known_token, yesterday)
-	if VerifySignature(expired_locator, known_token) {
+	if VerifySignature(expired_locator, known_token) != ExpiredError {
 		t.Fail()
 	}
 }

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list