Incorrect calculation of Host header value for non-standard ports

[DocOtak]

This might be a bit of an edge case. I work on an build systems that will often be remotely deployed without internet connectivity. As such, I was doing some testing with the S3 compatible server Minio. The idea being being normal AWS S3 on shore, and Minio while remote.

After I got my signing server set up, I was still getting "Signature Does not Match" errors from Minio which was running on http://localhost:9000. I eventually figured out that was due to a mismatch in the Host header that Evaporate sent to the sining uri which did not include the non standard port, and the Host header that was being sent by the eventual XHR request to the Minio server which DID include the port.

I think the problem is with this line which grabs the hostname rather than the host: https://github.com/TTLabs/EvaporateJS/blob/939d716ed81914472d1fae77d8343e5d410ed4b8/evaporate.js#L1014

In every browser I had access to (Safari 10, Firefox 51, Chrome 56) the host attribute would drop the port number if it was the standard port for the protocol, so it would be the same as the hostname attribute in the vast majority of cases. This was even the case if the standard port was specified explicitly in the href (e.g. http://example.com:80).

[bikeath1337]

Thanks @DocOtak ! This seems like something that can be added without breaking backward compatibility. Feel free to submit an enhancement PR with specs!

[DocOtak]

Some additional information: I downloaded the IE VMs to check their behavior regarding the host vs hostname parameter on the <a> element. I checked IE10, 11 and Edge.

Edge has the behavior exhibited in the other modern browsers, where the host attribute would not contain the default port numbers for the protocol even when included in the href string.

IE10 and 11 however, always include the port number in the host attribute even when NOT specified explicitly in the href value (example console output for IE11):

a = document.createElement('a')
--> <a></a>
a.href = "http://google.com"
--> "http://google.com"
a.host
--> "google.com:80"

In IE11 (I didn't check 10), when the XHR is sent, the Host header does not include the port number for standard ports.

All the various specification documents I could find have the port being optional if it is the port is standard. That is, the behavior of IE10 and 11 are not violations of the spec for what the value of the Host should be.

Based on the above, I've concluded that my original request is indeed breaking backward compatibility.

For my own purposes, I've utilized the sendCanonicalRequestToSignerUrl option to send the request headers to the signer, I'm then modifying the host parameter on the sining side and recalculating the hash. I think the most that should be done is documenting this edge case and a potential solution, rather than changing how the host is header is determined since for actual AWS, the hostname property will always be correct in all apparent supported browsers.

[bikeath1337]

@DocOtak one option would be to enable the behavior through an option. If we default that option to false, the port logic wouldn't apply and we'd be backward compatible.

[DocOtak]

@bikeath1337 A suggestion for the config name of awsHost, as that is what it is called in the SignedS3AWSRequest.

I was thinking about what the possible input values might be, it might be nice to allow for a string override of the awsHost value:

  awsHost: 'example.com:3000',

But it could also be useful to have a function where the default implementation is something like:

function awsHost(uriObject){
  return uriObject.hostname;
}

where uriObject is what is returned by the uri() function. The idea being someone could get fancy with UA sniffing if they wanted.

I think I could put together a PR for the implementation, but I'm not sure I would be able to write a spec without some guidance... I'm rather new to JS best practices.

[bikeath1337]

@DocOtak or alternatively, we could add a port config option that we inject into the url, regardless, couldn't we?

[DocOtak]

@bikeath1337 having a port option could work and would remove some of the redundant information in the config object. The problem (I think) is that we can only try to guess what the user agent will actually send when it finally does send the request, so the ability to accept a function for figuring out if the port should be included might still be desirable.

Let me know what direction you want to go I can try to get an implementation written.

[jfcalvo]

@DocOtak I'm running minio on port 80 on localhost but still getting a Signature does not match response. Should it works assuming that the signing is correctly done?.

[sagikazarmark]

I also see this error response, but I create the signature client side using the "Use unsafe JavaScript custom auth method" from the example.

[DocOtak]

@jfcalvo Running minio on port 80 was the only way I could get the upload work "out of the box", you still need the signing server to do its thing correctly. I was using the python example converted to flask. In the hope that it may be useful, here is the code I used to recalculate the canonical request with non standard ports, note that you need to have the sendCanonicalRequestToSignerUrl option set to true in EvaporateJS:

from hashlib import sha256
import datetime
import hashlib
import hmac

from flask import Flask

from flask import request, make_response

app = Flask(__name__)

aws_host = "example.com:9000" # if non standard port, the canonical request needs to be modified
aws_secret = "secret key here" # minio calls this "SecretKey"
region = 'us-east-1'
service = 's3'

@app.route('/sign')
def sign():
    to_sign = request.args.get("to_sign")
    canonical_request = request.args.get('canonical_request')
    current_canonical_signature = sha256(canonical_request.encode('utf-8')).hexdigest()

    new_canonical_request = canonical_request.replace('host:example.com', "host:"+aws_host)
    new_canonical_signature = sha256(new_canonical_request.encode('utf-8')).hexdigest()

    to_sign = to_sign.replace(current_canonical_signature, new_canonical_signature)

    date_stamp = datetime.datetime.strptime(request.args.get('datetime'), '%Y%m%dT%H%M%SZ').strftime('%Y%m%d')

    def sign(key, msg):
        return hmac.new(key, msg.encode("utf-8"), hashlib.sha256).digest()

    def getSignatureKey(key, date_stamp, regionName, serviceName):
        kDate = sign(('AWS4' + key).encode('utf-8'), date_stamp)
        kRegion = sign(kDate, regionName)
        kService = sign(kRegion, serviceName)
        kSigning = sign(kService, 'aws4_request')
        return kSigning

    signing_key = getSignatureKey(aws_secret, date_stamp, region, service)

    # Sign to_sign using the signing_key
    signature = hmac.new(
        signing_key,
        to_sign.encode('utf-8'),
        hashlib.sha256
    ).hexdigest()

    r = make_response(signature)
    r.headers['Access-Control-Allow-Origin'] = "*"

    return r

if __name__ == "__main__":
    app.run('0.0.0.0')

[DocOtak]

@bikeath1337 Sorry I haven't looked at this for a while, been distracted by other things at work.

The use case I had opened this issue for is no longer a requirement of the systems we are building, i.e. we only need to be able to download things from a "fake" s3 service, uploads aren't necessary. It would probably be better to avoid the added complexity of supporting non AWS services in EvaporateJS. I'll leave it up to you if you feel this issue should remain open or not.

[jfcalvo]

Thank you very much @DocOtak I have finally make it works on port 80.