[ARVADOS] updated: 7c0924d91aef5da1f69bd5f88b61880915afae4a
git at public.curoverse.com
git at public.curoverse.com
Tue Jun 16 16:09:43 EDT 2015
Summary of changes:
services/keepstore/handler_test.go | 4 +--
services/keepstore/handlers.go | 60 +++++++-------------------------------
services/keepstore/perms.go | 23 ++++++++++-----
services/keepstore/perms_test.go | 24 +++++++--------
4 files changed, 39 insertions(+), 72 deletions(-)
via 7c0924d91aef5da1f69bd5f88b61880915afae4a (commit)
via 37ffea7224fa4af8af63de2882e4cf9592870822 (commit)
from 912d5c4e0b616d9e18332b33a05395e3f6eb77b9 (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 7c0924d91aef5da1f69bd5f88b61880915afae4a
Merge: 912d5c4 37ffea7
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Jun 16 16:09:04 2015 -0400
Merge branch '6222-precompile-regexps' refs #6222
commit 37ffea7224fa4af8af63de2882e4cf9592870822
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