Allow remote signing by introducing Signer interface

[guymguym]

Now s3util can take a more generic Signer interface instead of the fixed s3util.Config, which is needed to allow uploads from clients that shouldn't have the secret key, and therefore requests a web server to sign its requests.

[guymguym]

Here is the code that I used to implement a remote signer:

type SignInfo struct {
    Method  string
    Url     string
    Headers http.Header
}

type SignerClient struct {
    SignerServerUrl string
    // The http client to use. if nil, will use http.DefaultClient.
    Client *http.Client
}

// implement teh s3.Signer interface
func (s *SignerClient) Sign(r *http.Request) {
    client := s.Client
    if client == nil {
        client = http.DefaultClient
    }

    // Take method, url and most importantly the headers and encode as json buffer
    sign_info := SignInfo{
        Method:  r.Method,
        Url:     r.URL.String(),
        Headers: r.Header,
    }
    data, err := json.Marshal(sign_info)
    if err != nil {
        log.Println(err)
        return
    }

    // make a post request with the info
    sign_req, err := http.NewRequest("POST", s.SignerServerUrl, bytes.NewBuffer(data))
    if err != nil {
        log.Println(err)
        return
    }

    // send the request
    res, err := client.Do(sign_req)
    if err != nil {
        log.Println(err)
        return
    }
    defer res.Body.Close()
    if err = httpStatus(res); err != nil {
        log.Println(err)
        return
    }

    // decode the reply into the info again which will add headers to r
    // since the headers map is shared between r and sign_info
    err = json.NewDecoder(res.Body).Decode(&sign_info)
    if err != nil {
        log.Println(err)
        return
    }
}

// Server for remote sign requests from SignerClient
type SignerServer struct {
    Signer   s3.Signer
    Approver func(SignInfo) error
}

func (s *SignerServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    // read the sign info json from request body
    var sign_info SignInfo
    err := json.NewDecoder(r.Body).Decode(&sign_info)
    if httpError(err, w, http.StatusInternalServerError) {
        return
    }

    // validatinh that the info is indeed approved
    if s.Approver != nil {
        err = s.Approver(sign_info)
        if httpError(err, w, http.StatusForbidden) {
            return
        }
    }

    // create a dummy request to hold the info, since the sign requires a request
    sign_req, err := http.NewRequest(sign_info.Method, sign_info.Url, nil)
    if httpError(err, w, http.StatusInternalServerError) {
        return
    }
    sign_req.Header = sign_info.Headers

    // sing the request
    s.Signer.Sign(sign_req)

    // send back the info which still points to the same headers
    // that the sign modified.
    err = json.NewEncoder(w).Encode(&sign_info)
    if httpError(err, w, http.StatusInternalServerError) {
        return
    }
}

func httpError(err error, w http.ResponseWriter, code int, info ...interface{}) bool {
    if err != nil {
        log.Println("ERROR:", err, info)
        debug.PrintStack()
        http.Error(w, "\""+err.Error()+"\"", code)
        return true
    }
    return false
}

func httpStatus(res *http.Response) error {
    switch res.StatusCode {
    case http.StatusOK,
        http.StatusCreated,
        http.StatusAccepted,
        // http.StatusNonAuthoritativeInfo,
        http.StatusNoContent,
        // http.StatusResetContent,
        http.StatusPartialContent:
        return nil
    }
    data, err := ioutil.ReadAll(res.Body)
    if err != nil {
        return errors.New(res.Status + ": " + err.Error())
    }
    return errors.New(res.Status + ": " + string(data))
}

[kr]

This is very cool. Thanks for sending the patch!

It just needs a bit of cleanup before I can merge it.

• Please don't change the import paths (unless you want to maintain a fork indefinitely!) :)
• I think the Signer interface should be defined in package s3util.

[guymguym]

Hi. Thanks for the entire library :)

Sorry about the imports, but when I pulled my version to another project I had to fix them or it wouldn't compile... I saw some other fork of this library where someone changed all the inner imports to be relative. Is that a good option?

One thing to notice is that currently Signer.Sign() function does not return an error, so if something fails in the communication it will only log, but then will still try to send the un-signed request to amazon, which is not really terrible, but not what we would expect a library to do...

However I didn't want to change the existing paths that call Sign() without error checking and find how to propagate/ignore the errors. But it seems a better approach to return an error, and let the calling code decide. This bring back exception-chaos nostalgia moments :)

[kr]

Returning an error is the right thing to do. It's only a coincidence that the existing Sign function never fails. Error handling should be easy for this patch: if Sign returns an error, just return it. All necessary error handling (except for abort; see the TODO comment) is already in place.

Relative package paths exist only for convenient go tool commands (such as go test ./...). They should never be used in import statements.

I understand about the inconvenience if you want to distribute patches on github and use them in other projects. There's no good way to do that, except to change the import paths in your fork, but leave the original imports in patches you send upstream. (If you're only doing local development, you never have to change the imports at all.)