Arvados

Review @ f39807a3c:

services/keepstore/perms.go:

The API server generates signatures on hash+size, so VerifySignatures needs to include the size hint when verifying.

^([[:xdigit:]]{32}).*\+A([[:xdigit:]]{40})@([[:xdigit:]]{8})

should be:

^([[:xdigit:]]{32}(\+[0-9]+)?).*\+A([[:xdigit:]]{40})@([[:xdigit:]]{8})

Consequently:

blob_hash := matches[1]
    sig_hex := matches[3]
    exp_hex := matches[4]

We should make sure the tests use hash+size for signing locators.

Also, some time ago Peter found regexp.MustCompile to compile a regex or panic if it can't. I don't know why I missed it when I was starting out. We might as well use it here to make everything simpler.

The API server generates signatures on hash+size, so VerifySignatures needs to include the size hint when verifying.

I think you'll find on closer inspection that it doesn't.

Also, some time ago Peter found regexp.MustCompile to compile a regex or panic if it can't. I don't know why I missed it when I was starting out. We might as well use it here to make everything simpler.

Ah yes, and now it can be a global, so we don't compile it N times! → 5e2eeea

LGTM. But can someone please explain to me what size hints are for in the first place? :-)

P.S. we should include tests for validating signatures both with and without size hints, just in case I screw this up again. I should have included them in the first place. You already have an LGTM so feel free to merge this as-is, I'll file a story to add the appropriate tests.

Tim Pierce wrote:

P.S. we should include tests for validating signatures both with and without size hints, just in case I screw this up again. I should have included them in the first place. You already have an LGTM so feel free to merge this as-is, I'll file a story to add the appropriate tests.

Added in 1d0d4b1

LGTM. But can someone please explain to me what size hints are for in the first place? :-)

Seeking into >64MiB files in manifests.