Skip to content

Conversation

@mfine
Copy link

@mfine mfine commented Jul 14, 2013

Pass the signature as a query parameter. Takes a time.Time parameter. Don't love the SignURL name - SignQuery?

Duplicating writeSigData with writeSigURLData - could fold these in to the same routine, and check for whether a time value got passed in and the either do data' orexpires`.

Not sure if this works. Also needs tests.

@mfine mfine mentioned this pull request Jul 14, 2013
@mfine
Copy link
Author

mfine commented Jul 14, 2013

Hmm, overwriting the query is pretty messy. And that conversion on the timestamp doesn't work. Hmm, maybe I'll just pass in an expires string for 'date' and then pull out the auth (rough code):

 func s3Url(bucket, key string) string {
    return fmt.Sprintf("https://s3-external-1.amazonaws.com/%s/%s/%s", bucket, domain, key)
}

func s3SignedUrl(method, bucket, key string) (string, error) {
    req, err := http.NewRequest(method, s3Url(bucket, key), nil)
    if err != nil {
        return "", err
    }

    expires := strconv.Itoa(int(time.Now().Unix())+3600)
    req.Header.Set("Date", expires)
    s3.Sign(req, keys)
    auth := strings.Split(req.Header.Get("Authorization"), ":")
    url := s3Url(bucket, key)+"?Signature="+auth[1]+"&Expires="+expires+"&AWSAccessKeyId="+keys.AccessKey

    return url, nil
}

@kr
Copy link
Owner

kr commented Jul 14, 2013

You referring to string(t.Unix()) not working? Yeah, it needs to
be something like strconv.FormatInt(t.Unix(), 10). (The
expression string(n) gives you the unicode codepoint n as
a utf8 string. For example http://play.golang.org/p/c6NKU_B8Q0)

I feel like this function should actually not modify the request,
but instead return a new URL. The user doesn't need to actually
use the request right away, since presumably they just want
to share the url with some other process. That's why my initial
suggested name was a noun phrase rather than a verb
(SignedURL instead of SignWhatever).

I'm not worried about duplicating code in writeSigURLData. It can
always be factored out later. As long as the public interface is good
and we have some test coverage.

@kr
Copy link
Owner

kr commented Jul 14, 2013

Oh wow, does that work to pull out the auth header like that?
I would think the string-to-sign would always be different no
matter what between a header signature and a query param
signature.

@kr
Copy link
Owner

kr commented Jul 14, 2013

Guess I didn't ready the spec closely enough, I see, the date
parameter in the string to sign doesn't include the word "date"
(or "expires"), it's just the actual date. Nice.

Your code snippet in the comment above suggests it would be
nice to expose a slightly lower-level function, like

func Signature(r *http.Request, t time.Time, k Keys) string

That would be used by the existing Sign and would also
work for a signed URL.

@mfine
Copy link
Author

mfine commented Jul 15, 2013

@kr yeah, I think that would be great. I've been going back and forth whether it would actually be cool to expose a Signer that's independent of the request - right now, I'm building the request essentially just to pass headers. Is there a way to just capture the the signing as simply as possible?

base64(hmac-sha1(VERB + "\n" + CONTENT-MD5 + "\n" + CONTENT-TYPE + "\n" + (DATE|Expires) + "\n" + CanonicalizedAmzHeaders + "\n" + CanonicalizedResource))

@kr
Copy link
Owner

kr commented Jul 15, 2013

Yeah good question. The string to sign has all these
weird fiddly parts, it could be derived from an http.Header,
but you also need the URL, and the method, and the host
(which is either part of the header or the url). I feel like
instead of supplying these things as separate parameters
it's more uniform to put them in an http.Request, which is
essentially the standard way to represent an http request.

If you already have those parts sitting around separately,
then it's the difference between

Signature(url, method, header, time, keys)

// vs

Signature(&http.Request{URL:url, Method:method, Header:header}, time, keys)

Even though it's more code, I prefer the second one because
it's more in line with the amazon docs, which talk about signing
a "request" regardless of whether you're putting the signature
in a header or the query params.

(And if you have no headers to add, it could be a little shorter:)

Signature(&http.Request{URL:url, Method:method}, time, keys)

(This wouldn't work currently because package s3 assumes the
header is not nil, but that's fixable.)

@matope matope mentioned this pull request Dec 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants