[ARVADOS] created: 521457373a8f1e46f44a43311be2d9242ad5d0a9
git at public.curoverse.com
git at public.curoverse.com
Fri May 23 17:27:12 EDT 2014
at 521457373a8f1e46f44a43311be2d9242ad5d0a9 (commit)
commit 521457373a8f1e46f44a43311be2d9242ad5d0a9
Author: Tim Pierce <twp at curoverse.com>
Date: Wed May 21 10:58:18 2014 -0400
2755: add support for signed locators in the Python SDK.
* arvados.Keep.put() saves the response body (which may contain a
signed locator) and returns it to the caller.
* arvados.Keep.get() passes the full signed locator to the remote Keep
server. The bare MD5 hash is still used for caching and for
shuffled_service_roots
* run_test_server.run_keep() takes arguments 'blob_signing_key' and
'enforce_permissions', for testing permission signatures in unit
tests.
* test_keep_client: new unit tests for permissions:
- with --enforce-permissions=true:
- GET with a signed locator works
- GET with an unsigned locator fails
- unauthenticated GET fails
- with --enforce-permissions=false:
- GET with a signed locator works
- GET with an unsigned locator works
- unauthenticated GET works
Bug fixes to permission handling in the Keep server:
* Locator hints may appear in any order; be flexible. Parse them in
GetBlockHandler rather than in the REST router.
* Returned locators are terminated with newline (consistent with
Warehouse, and more friendly for human debugging).
* The locator returned from a PUT request always has a size hint.
* The correct Authorization header keyword is "OAuth2", not
"OAuth". D'oh.
* Updated unit tests to accommodate newlines, size hints and OAuth2.
Refs #2755.
diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py
index e414d26..4e800f7 100644
--- a/sdk/python/arvados/keep.py
+++ b/sdk/python/arvados/keep.py
@@ -55,6 +55,7 @@ class KeepClient(object):
def __init__(self, todo):
self._todo = todo
self._done = 0
+ self._response = None
self._todo_lock = threading.Semaphore(todo)
self._done_lock = threading.Lock()
@@ -73,12 +74,23 @@ class KeepClient(object):
with self._done_lock:
return (self._done < self._todo)
- def increment_done(self):
+ def save_response(self, response_body):
"""
- Report that the current thread was successful.
+ Records a response body (a locator, possibly signed) returned by
+ the Keep server. It is not necessary to save more than
+ one response, since we presume that any locator returned
+ in response to a successful request is valid.
"""
with self._done_lock:
self._done += 1
+ self._response = response_body
+
+ def response(self):
+ """
+ Returns the body from the response to a PUT request.
+ """
+ with self._done_lock:
+ return self._response
def done(self):
"""
@@ -89,9 +101,9 @@ class KeepClient(object):
class KeepWriterThread(threading.Thread):
"""
- Write a blob of data to the given Keep server. Call
- increment_done() of the given ThreadLimiter if the write
- succeeds.
+ Write a blob of data to the given Keep server. On success, call
+ save_response() of the given ThreadLimiter to save the returned
+ locator.
"""
def __init__(self, **kwargs):
super(KeepClient.KeepWriterThread, self).__init__()
@@ -129,7 +141,7 @@ class KeepClient(object):
(str(threading.current_thread()),
self.args['data_hash'],
self.args['service_root']))
- return limiter.increment_done()
+ return limiter.save_response(content.strip())
logging.warning("Request fail: PUT %s => %s %s" %
(url, resp['status'], content))
except (httplib2.HttpLib2Error, httplib.HTTPException) as e:
@@ -277,7 +289,7 @@ class KeepClient(object):
try:
for service_root in self.shuffled_service_roots(expect_hash):
- url = service_root + expect_hash
+ url = service_root + locator
api_token = config.get('ARVADOS_API_TOKEN')
headers = {'Authorization': "OAuth2 %s" % api_token,
'Accept': 'application/octet-stream'}
@@ -289,7 +301,7 @@ class KeepClient(object):
for location_hint in re.finditer(r'\+K@([a-z0-9]+)', locator):
instance = location_hint.group(1)
- url = 'http://keep.' + instance + '.arvadosapi.com/' + expect_hash
+ url = 'http://keep.' + instance + '.arvadosapi.com/' + locator
blob = self.get_url(url, {}, expect_hash)
if blob:
slot.set(blob)
@@ -348,8 +360,9 @@ class KeepClient(object):
for t in threads:
t.join()
have_copies = thread_limiter.done()
+ # If we're done, return the response from Keep
if have_copies == want_copies:
- return (data_hash + '+' + str(len(data)))
+ return thread_limiter.response()
raise arvados.errors.KeepWriteError(
"Write fail for %s: wanted %d but wrote %d" %
(data_hash, want_copies, have_copies))
diff --git a/sdk/python/run_test_server.py b/sdk/python/run_test_server.py
index dbb4ff0..4b65d72 100644
--- a/sdk/python/run_test_server.py
+++ b/sdk/python/run_test_server.py
@@ -123,15 +123,22 @@ def stop():
os.chdir(cwd)
-def _start_keep(n):
+def _start_keep(n, keep_args):
keep0 = tempfile.mkdtemp()
- kp0 = subprocess.Popen(["bin/keep", "-volumes={}".format(keep0), "-listen=:{}".format(25107+n)])
+ keep_cmd = ["bin/keep",
+ "-volumes={}".format(keep0),
+ "-listen=:{}".format(25107+n)]
+
+ for arg, val in keep_args.iteritems():
+ keep_cmd.append("{}={}".format(arg, val))
+
+ kp0 = subprocess.Popen(keep_cmd)
with open("tmp/keep{}.pid".format(n), 'w') as f:
f.write(str(kp0.pid))
with open("tmp/keep{}.volume".format(n), 'w') as f:
f.write(keep0)
-def run_keep():
+def run_keep(blob_signing_key=None, enforce_permissions=False):
stop_keep()
cwd = os.getcwd()
@@ -146,8 +153,16 @@ def run_keep():
if not os.path.exists("tmp"):
os.mkdir("tmp")
- _start_keep(0)
- _start_keep(1)
+ keep_args = {}
+ if blob_signing_key:
+ with open("tmp/keep.blob_signing_key", "w") as f:
+ f.write(blob_signing_key)
+ keep_args['--permission-key-file'] = 'tmp/keep.blob_signing_key'
+ if enforce_permissions:
+ keep_args['--enforce-permissions'] = 'true'
+
+ _start_keep(0, keep_args)
+ _start_keep(1, keep_args)
os.environ["ARVADOS_API_HOST"] = "127.0.0.1:3001"
@@ -172,6 +187,8 @@ def _stop_keep(n):
if os.path.exists("tmp/keep{}.volume".format(n)):
with open("tmp/keep{}.volume".format(n), 'r') as r:
shutil.rmtree(r.read(), True)
+ if os.path.exists("tmp/keep.blob_signing_key"):
+ os.remove("tmp/keep.blob_signing_key")
def stop_keep():
cwd = os.getcwd()
diff --git a/sdk/python/test_keep_client.py b/sdk/python/test_keep_client.py
index aa79b0d..8fac34d 100644
--- a/sdk/python/test_keep_client.py
+++ b/sdk/python/test_keep_client.py
@@ -65,3 +65,105 @@ class KeepTestCase(unittest.TestCase):
self.assertEqual(arvados.Keep.get(blob_locator),
blob_str,
'wrong content from Keep.get(md5(<binarydata>))')
+
+class KeepPermissionTestCase(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ try:
+ del os.environ['KEEP_LOCAL_STORE']
+ except KeyError:
+ pass
+ run_test_server.run()
+ run_test_server.run_keep(blob_signing_key='abcdefghijk0123456789',
+ enforce_permissions=True)
+
+ @classmethod
+ def tearDownClass(cls):
+ run_test_server.stop()
+ run_test_server.stop_keep()
+
+ def test_KeepBasicRWTest(self):
+ run_test_server.authorize_with('active')
+ foo_locator = arvados.Keep.put('foo')
+ self.assertRegexpMatches(
+ foo_locator,
+ r'^acbd18db4cc2f85cedef654fccc4a4d8\+3\+A[a-f0-9]+@[a-f0-9]+$',
+ 'invalid locator from Keep.put("foo"): ' + foo_locator)
+ self.assertEqual(arvados.Keep.get(foo_locator),
+ 'foo',
+ 'wrong content from Keep.get(md5("foo"))')
+
+ # With Keep permissions enabled, a GET request without a locator will fail.
+ bar_locator = arvados.Keep.put('bar')
+ self.assertRegexpMatches(
+ bar_locator,
+ r'^37b51d194a7513e45b56f6524f2d51f2\+3\+A[a-f0-9]+@[a-f0-9]+$',
+ 'invalid locator from Keep.put("bar"): ' + bar_locator)
+ self.assertRaises(arvados.errors.NotFoundError,
+ arvados.Keep.get,
+ "37b51d194a7513e45b56f6524f2d51f2")
+
+ # A request without an API token will also fail.
+ del arvados.config.settings()["ARVADOS_API_TOKEN"]
+ self.assertRaises(arvados.errors.NotFoundError,
+ arvados.Keep.get,
+ bar_locator)
+
+# KeepOptionalPermission: starts Keep with --permission-key-file
+# but not --enforce-permissions (i.e. generate signatures on PUT
+# requests, but do not require them for GET requests)
+#
+class KeepOptionalPermission(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ try:
+ del os.environ['KEEP_LOCAL_STORE']
+ except KeyError:
+ pass
+ run_test_server.run()
+ run_test_server.run_keep(blob_signing_key='abcdefghijk0123456789',
+ enforce_permissions=False)
+
+ @classmethod
+ def tearDownClass(cls):
+ run_test_server.stop()
+ run_test_server.stop_keep()
+
+ def test_KeepBasicRWTest(self):
+ run_test_server.authorize_with('active')
+ foo_locator = arvados.Keep.put('foo')
+ self.assertRegexpMatches(
+ foo_locator,
+ r'^acbd18db4cc2f85cedef654fccc4a4d8\+3\+A[a-f0-9]+@[a-f0-9]+$',
+ 'invalid locator from Keep.put("foo"): ' + foo_locator)
+ self.assertEqual(arvados.Keep.get(foo_locator),
+ 'foo',
+ 'wrong content from Keep.get(md5("foo"))')
+
+ def test_KeepUnsignedLocatorTest(self):
+ # Since --enforce-permissions is not in effect, GET requests
+ # do not require signatures.
+ run_test_server.authorize_with('active')
+ foo_locator = arvados.Keep.put('foo')
+ self.assertRegexpMatches(
+ foo_locator,
+ r'^acbd18db4cc2f85cedef654fccc4a4d8\+3\+A[a-f0-9]+@[a-f0-9]+$',
+ 'invalid locator from Keep.put("foo"): ' + foo_locator)
+ self.assertEqual(arvados.Keep.get("acbd18db4cc2f85cedef654fccc4a4d8"),
+ 'foo',
+ 'wrong content from Keep.get(md5("foo"))')
+
+ def test_KeepUnauthenticatedTest(self):
+ # Since --enforce-permissions is not in effect, GET requests
+ # need not be authenticated.
+ run_test_server.authorize_with('active')
+ foo_locator = arvados.Keep.put('foo')
+ self.assertRegexpMatches(
+ foo_locator,
+ r'^acbd18db4cc2f85cedef654fccc4a4d8\+3\+A[a-f0-9]+@[a-f0-9]+$',
+ 'invalid locator from Keep.put("foo"): ' + foo_locator)
+
+ del arvados.config.settings()["ARVADOS_API_TOKEN"]
+ self.assertEqual(arvados.Keep.get("acbd18db4cc2f85cedef654fccc4a4d8"),
+ 'foo',
+ 'wrong content from Keep.get(md5("foo"))')
diff --git a/services/keep/src/keep/handler_test.go b/services/keep/src/keep/handler_test.go
index 8e7bfea..c96086f 100644
--- a/services/keep/src/keep/handler_test.go
+++ b/services/keep/src/keep/handler_test.go
@@ -15,6 +15,7 @@ import (
"net/http"
"net/http/httptest"
"regexp"
+ "strings"
"testing"
"time"
)
@@ -159,7 +160,9 @@ func TestPutHandler(t *testing.T) {
ExpectStatusCode(t,
"Unauthenticated request, no server key", http.StatusOK, response)
- ExpectBody(t, "Unauthenticated request, no server key", TEST_HASH, response)
+ ExpectBody(t,
+ "Unauthenticated request, no server key",
+ TEST_HASH_PUT_RESPONSE, response)
// ------------------
// With a server key.
@@ -183,10 +186,11 @@ func TestPutHandler(t *testing.T) {
ExpectStatusCode(t,
"Authenticated PUT, signed locator, with server key",
http.StatusOK, response)
- if !VerifySignature(response.Body.String(), known_token) {
+ response_locator := strings.TrimSpace(response.Body.String())
+ if !VerifySignature(response_locator, known_token) {
t.Errorf("Authenticated PUT, signed locator, with server key:\n"+
"response '%s' does not contain a valid signature",
- response.Body.String())
+ response_locator)
}
// Unauthenticated PUT, unsigned locator
@@ -203,7 +207,7 @@ func TestPutHandler(t *testing.T) {
http.StatusOK, response)
ExpectBody(t,
"Unauthenticated PUT, unsigned locator, with server key",
- TEST_HASH, response)
+ TEST_HASH_PUT_RESPONSE, response)
}
// Test /index requests:
@@ -407,7 +411,7 @@ func IssueRequest(router *mux.Router, rt *RequestTester) *httptest.ResponseRecor
body := bytes.NewReader(rt.request_body)
req, _ := http.NewRequest(rt.method, rt.uri, body)
if rt.api_token != "" {
- req.Header.Set("Authorization", "OAuth "+rt.api_token)
+ req.Header.Set("Authorization", "OAuth2 "+rt.api_token)
}
router.ServeHTTP(response, req)
return response
diff --git a/services/keep/src/keep/keep.go b/services/keep/src/keep/keep.go
index d65e445..1f80678 100644
--- a/services/keep/src/keep/keep.go
+++ b/services/keep/src/keep/keep.go
@@ -271,7 +271,7 @@ func MakeRESTRouter() *mux.Router {
rest.HandleFunc(
`/{hash:[0-9a-f]{32}}`, GetBlockHandler).Methods("GET", "HEAD")
rest.HandleFunc(
- `/{hash:[0-9a-f]{32}}+A{signature:[0-9a-f]+}@{timestamp:[0-9a-f]+}`,
+ `/{hash:[0-9a-f]{32}}+{hints}`,
GetBlockHandler).Methods("GET", "HEAD")
rest.HandleFunc(`/{hash:[0-9a-f]{32}}`, PutBlockHandler).Methods("PUT")
@@ -330,8 +330,28 @@ func GetBlockHandler(resp http.ResponseWriter, req *http.Request) {
log.Printf("%s %s", req.Method, hash)
- signature := mux.Vars(req)["signature"]
- timestamp := mux.Vars(req)["timestamp"]
+ 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 match, _ := regexp.MatchString("^K([[:alnum:]]+)$", hint); match {
+ // Server ignores location hints
+ } else if m := signature_pat.FindStringSubmatch(hint); m != nil {
+ signature = m[1]
+ timestamp = m[2]
+ } else {
+ // Not a valid locator: return 404
+ http.Error(resp, NotFoundError.Error(), NotFoundError.HTTPCode)
+ return
+ }
+ }
+ }
// If permission checking is in effect, verify this
// request's permission signature.
@@ -343,8 +363,8 @@ func GetBlockHandler(resp http.ResponseWriter, req *http.Request) {
http.Error(resp, ExpiredError.Error(), ExpiredError.HTTPCode)
return
} else {
- validsig := MakePermSignature(hash, GetApiToken(req), timestamp)
- if signature != validsig {
+ req_locator := req.URL.Path[1:] // strip leading slash
+ if !VerifySignature(req_locator, GetApiToken(req)) {
http.Error(resp, PermissionError.Error(), PermissionError.HTTPCode)
return
}
@@ -384,11 +404,15 @@ func PutBlockHandler(resp http.ResponseWriter, req *http.Request) {
//
if buf, err := ReadAtMost(req.Body, BLOCKSIZE); err == nil {
if err := PutBlock(buf, hash); err == nil {
- // Success; sign the locator and return it to the client.
+ // Success; add a size hint, sign the locator if
+ // possible, and return it to the client.
+ return_hash := fmt.Sprintf("%s+%d", hash, len(buf))
api_token := GetApiToken(req)
- expiry := time.Now().Add(permission_ttl)
- signed_loc := SignLocator(hash, api_token, expiry)
- resp.Write([]byte(signed_loc))
+ if PermissionSecret != nil && api_token != "" {
+ expiry := time.Now().Add(permission_ttl)
+ return_hash = SignLocator(return_hash, api_token, expiry)
+ }
+ resp.Write([]byte(return_hash + "\n"))
} else {
ke := err.(*KeepError)
http.Error(resp, ke.Error(), ke.HTTPCode)
@@ -650,13 +674,15 @@ func IsValidLocator(loc string) bool {
return false
}
-// GetApiToken returns the OAuth token from the Authorization
+// 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 strings.HasPrefix(auth[0], "OAuth ") {
- return auth[0][6:]
+ if pat, err := regexp.Compile(`^OAuth2\s+(.*)`); err != nil {
+ log.Println(err)
+ } else if match := pat.FindStringSubmatch(auth[0]); match != nil {
+ return match[1]
}
}
return ""
diff --git a/services/keep/src/keep/keep_test.go b/services/keep/src/keep/keep_test.go
index 6642c72..7a787c1 100644
--- a/services/keep/src/keep/keep_test.go
+++ b/services/keep/src/keep/keep_test.go
@@ -12,6 +12,7 @@ import (
var TEST_BLOCK = []byte("The quick brown fox jumps over the lazy dog.")
var TEST_HASH = "e4d909c290d0fb1ca068ffaddf22cbd0"
+var TEST_HASH_PUT_RESPONSE = "e4d909c290d0fb1ca068ffaddf22cbd0+44\n"
var TEST_BLOCK_2 = []byte("Pack my box with five dozen liquor jugs.")
var TEST_HASH_2 = "f15ac516f788aec4f30932ffb6395c39"
diff --git a/services/keep/src/keep/perms.go b/services/keep/src/keep/perms.go
index 0d1b091..f7caca4 100644
--- a/services/keep/src/keep/perms.go
+++ b/services/keep/src/keep/perms.go
@@ -83,10 +83,10 @@ func SignLocator(blob_locator string, api_token string, expiry time.Time) string
// 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 {
- if re, err := regexp.Compile(`^(.*)\+A(.*)@(.*)$`); err == nil {
+ if re, err := regexp.Compile(`^([a-f0-9]+(\+[0-9]+)?)\+A(.*)@(.*)$`); err == nil {
if matches := re.FindStringSubmatch(signed_locator); matches != nil {
blob_locator := matches[1]
- timestamp_hex := matches[3]
+ timestamp_hex := matches[4]
if expire_ts, err := ParseHexTimestamp(timestamp_hex); err == nil {
// Fail signatures with expired timestamps.
if expire_ts.Before(time.Now()) {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list