Keystone OAuth1 doesn't handle invalid request properly

[Dave Chen]

For the access token request,

- If the signature is not valid, it will raise TypeError exception.

2016-08-23 16:45:19.705 5202 TRACE keystone.common.wsgi File "./keystone/common/wsgi.py", line 227, in __call__
2016-08-23 16:45:19.705 5202 TRACE keystone.common.wsgi result = method(req, **params)
2016-08-23 16:45:19.705 5202 TRACE keystone.common.wsgi File "./keystone/oauth1/controllers.py", line 309, in create_access_token
2016-08-23 16:45:19.705 5202 TRACE keystone.common.wsgi params = oauth1.extract_non_oauth_params(b)
2016-08-23 16:45:19.705 5202 TRACE keystone.common.wsgi File "./keystone/oauth1/core.py", line 108, in extract_non_oauth_params
2016-08-23 16:45:19.705 5202 TRACE keystone.common.wsgi return {k: v for k, v in params if not k.startswith('oauth_')}
2016-08-23 16:45:19.705 5202 TRACE keystone.common.wsgi TypeError: 'NoneType' object is not iterable
2016-08-23 16:45:19.705 5202 TRACE keystone.common.wsgi

- If the provided consumer does not exist, it will throw NotImplementedError exception to show that dummy_client is not implemented.

All these exception is not properly handled, end user doens't know anything from these exception message. It should be Unauthorized exception raised.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/374043

[OpenStack Infra (hudson-openstack)]

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

commit be5385c5389aa9c4879647c9b9e4327cc73189a2
Author: Dave Chen <email address hidden>
Date: Wed Aug 24 18:54:14 2016 +0800

    Handle the exception from creating access token properly

    If there is any request from client with any invalid request
    parameters, invalid signature for example, keystone should
    capture that and raise the exception.

    It was `NotImplementedError`, `TypeError` thrown out and
    presented directly to end user, and nothing helpful message
    is given.

    This patch fix that and show as many exception message that
    is helpful for diagnosis as possible.

    Change-Id: I112d0cd0c8a460c7b4d8d0e1c0b9c742aab9fde7
    Closes-Bug: #1616424

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/361087
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=54e41a310c4a96ebb2ff835e69c3d1353b95eb42
Submitter: Jenkins
Branch: master

commit 54e41a310c4a96ebb2ff835e69c3d1353b95eb42
Author: Dave Chen <email address hidden>
Date: Fri Aug 26 18:57:14 2016 +0800

    Handle the exception from creating request token properly

    The status code returned larger than 399 not indicate the
    signature is invalid, only an empty body implies.

    For other reasons which cause the failure of creating request
    token this patch show the detail message, so that give us
    some clue on where is incorrect.

    Related-Bug: #1616424
    Change-Id: Id3f0b806630697436340ab97f328743d7f811a14

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/374043
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=3c7e14091d6aa375dd01e41224f3272db7f74ddf
Submitter: Jenkins
Branch: master

commit 3c7e14091d6aa375dd01e41224f3272db7f74ddf
Author: Dave Chen <email address hidden>
Date: Wed Sep 21 18:56:16 2016 +0800

    Consolidate the common code into one method

    This is following up the comments from review [1], and take chance
    to update the exception where it's failed at the validation of
    payload in headers.

    [1] https://review.openstack.org/#/c/361087/
    Related-Bug: #1616424
    Change-Id: Ibb97a141b5cbb839b4fce062ad3470032e26f67a

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 11.0.0.0b1 development milestone.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/483588

[OpenStack Infra (hudson-openstack)]

Change abandoned by Lance Bragstad (<email address hidden>) on branch: stable/newton
Review: https://review.openstack.org/483588
Reason: I've merged this with the patch after it so that it will pass the gate [0]. I've also added a section to the commit message explaining why we had to do this.

[0] https://review.openstack.org/#/c/483589/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/483589
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=48a5336d3d4b54f954a0100ab864a5c3f6a71380
Submitter: Jenkins
Branch: stable/newton

commit 48a5336d3d4b54f954a0100ab864a5c3f6a71380
Author: Dave Chen <email address hidden>
Date: Wed Aug 24 18:54:14 2016 +0800

    Handle token exception and use proper url for verification

    This commit is a product of two separate commits in order to unwedge
    the stable/newton gate. The first commit is a oauth refactor to
    properly handle token exceptions. The second is a patch to that
    uses the proper url when verifying an oauth request token. The
    problem is that the second patch can't be applied due to the
    refactor from the first. This commit merges the two commits
    together so that their isn't a merge conflict and it passes the
    currently broken gate.

    The first commit is:

    Handle the exception from creating access token properly

    If there is any request from client with any invalid request
    parameters, invalid signature for example, keystone should
    capture that and raise the exception.

    It was `NotImplementedError`, `TypeError` thrown out and
    presented directly to end user, and nothing helpful message
    is given.

    This patch fix that and show as many exception message that
    is helpful for diagnosis as possible.

    Change-Id: I112d0cd0c8a460c7b4d8d0e1c0b9c742aab9fde7
    Closes-Bug: #1616424
    (cherry picked from commit be5385c5389aa9c4879647c9b9e4327cc73189a2)

    This is the second commit

    Change url passed to oauth signature verifier to request url

    OAUTH signature verification should happen with the same URL used for signing.
    Typically at the user end it should be signed with the request URL and hence it
    should be verified with the same.
    Currently keystone uses public endpoint URL for signature verification.

    Modified the URL passed to oauth signature verification to request URL.

    Change-Id: I28059a43cb0088c2952c19f696042ebec54d26c9
    Partial-Bug: #1687593
    (cherry picked from commit 926685c5a4823d7e3ab3879bae1529052fff7d68)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 10.0.3 release.