check_signature syntax error

[Xurong Yang]

    credentials is a dictionary,credentials.signature syntax error
    def check_signature(self, creds_ref, credentials):
        signer = ec2_utils.Ec2Signer(creds_ref['secret'])
        signature = signer.generate(credentials)
        if utils.auth_str_equal(credentials['signature'], signature):
            return
        # NOTE(vish): Some libraries don't use the port when signing
        # requests, so try again without port.
        elif ':' in credentials['signature']:
            hostname, _port = credentials['host'].split(':')
            credentials['host'] = hostname
            signature = signer.generate(credentials)
            if not utils.auth_str_equal(credentials.signature, signature):
                raise exception.Unauthorized(message='Invalid EC2 signature.')
        else:

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/78850

[Morgan Fainberg (mdrnstm)]

Unassigning due to inactivity.

[Dave Chen (wei-d-chen)]

The proposal for fixing this bug is submitted here: https://review.openstack.org/#/c/143772/

The previous check logic has a lot of issue with it, here are some explanation for the patch.

https://github.com/openstack/keystone/blob/master/keystone/contrib/ec2/controllers.py#L63 is not make sense, I think the intention of author would want to check whether the colon and port is existed in credentials['host'], the colon existed in credentials['signature'] and check against this sound absurd.

It's common to see the colon and port are ommited in the request header, but when they are presented there and the signature method is sanity a little bit, there is no case and no reason at all why they are split from the sign request. so the needless check against this will be never get effective.

[Adam Young (ayoung)]

Please rewrite the bug description to exzplain what the problem is. It sounds like the function is not properly documented on how to pass in the credentials. This API is designed to mimic something from Amazon, so it is possible it has a non-standard set of assumptions.

[Lance Bragstad (lbragstad)]

Looks like there was another patch that was submitted to do the same thing:

https://review.openstack.org/#/c/78837/

But, it's been MIA for a while.

[Dave Chen (wei-d-chen)]

Yes, that patch has been abandoned for an while.

[Dave Chen (wei-d-chen)]

@Adam, I opened a new bug, bug #1434599 to describe other issue beside this, and give some comment in the patch to make it looks more clear, thanks.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/143772
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=7ace25e25fafef7809789dc3b799c1943a45301d
Submitter: Jenkins
Branch: master

commit 7ace25e25fafef7809789dc3b799c1943a45301d
Author: Dave Chen <email address hidden>
Date: Wed Dec 24 20:41:20 2014 +0800

    Fix errors in ec2 signature logic checking

    - Check for colon in credentials['host'] instead of
    credentials['signature'].
    - Fix the syntax issue when trying to get the signature
    from the dict.
    - credentials['signature'] is not guaranteed to existed,
    so check it explicitly.
    - Need to reinitialize 'signer' to avoid contaminated status
    of signature.

    Closes-Bug: #1289115
    Closes-Bug: #1434599
    Change-Id: Idb5d97c30a20872fdaafea786bcea2631d70858c