With fernet tokens, validate token loses the ms on 'expires' value

Bug #1459790 reported by Kim Jensen on 2015-05-28
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Medium
Roxana Gherle
Kilo
Medium
Dolph Mathews

Bug Description

With fernet tokens, the expires ms value is 0 when the token is validated. So the 'expires' on the post token and the get token are different; this is not the case with uuid tokens.

$ curl -s \
 -H "Content-Type: application/json" \
 -d '{ "auth":{ "tenantName":"testTenantName", "passwordCredentials":{ "username":"testUserName", "password":"password" }}}' \
-X POST $KEYSTONE_ENDPOINT:5000/v2.0/tokens | python -mjson.tool

post token portion of the response contains 'expires' with a ms value :

        "token": {
            "audit_ids": [
                "eZtfF60tR7y5oAuL4LSr4w"
            ],
            "expires": "2015-05-28T20:50:56.015102Z",
            "id": "gAAAAABVZ2OQu3OunvR6FKklDdNWj95Aq-ju_sIhB9o0KRin2SpLRUa0C3H_XiV_RWN409Ma-Q7lIkA_S6mY3bnxgboJZ_qxUiTdzUscG5y_fSCUW5sQqmB2AI1rlmMetvTl6AnnRKzVHVlJlDKQNHuk0MzHM3IVr4-ysJ2AHBtmDfkdpRZCrFo%3D",
            "issued_at": "2015-05-28T18:50:56.015211Z",
            "tenant": {
                "description": "Test tenant ...",
                "enabled": true,
                "id": "1c6e0d2ac4bf4cd5bc7666d86b28aee0",
                "name": "testTenantName",
                "parent_id": null
            }
        },

If this token is validated, the expires ms now show as 000000Z

$ curl -s \
 -H "Content-Type: application/json" \
 -H "X-Auth-Token: $ADMIN_TOKEN" \
-X GET $KEYSTONE_ENDPOINT:35357/v2.0/tokens/$USER_TOKEN | python -mjson.tool

get token portion of the response contains 'expires' with ms = 000000Z

],
        "token": {
            "audit_ids": [
                "lZwaM7oaShCZGQt0A9FaKA"
            ],
            "expires": "2015-05-28T20:27:24.000000Z",
            "id": "gAAAAABVZ14MKoaOBq4WBHaF1fqEKrN_nTrYYhwi8xrAisWmyJ52DJOrVlyxAoUuL_tfrGhslYVffRTosF5FqQVYlNq6hqU-qGzhueC4xVJZL8oitv0PfOdGfLgAWM1pciuiIdDLnWb-6oNrgZ9l1lHqn1kyuO0JVmS_YJfYI4YOt0o7ZfJhzFQ=",
            "issued_at": "2015-05-28T18:27:24.000000Z",
            "tenant": {
                "description": "Test tenant ...",
                "enabled": true,
                "id": "1c6e0d2ac4bf4cd5bc7666d86b28aee0",
                "name": "testTenantName",
                "parent_id": null
            }
        },

Dolph Mathews (dolph) wrote :

The report is accurate, but the assertion about which timestamp is "correct" is actually backward from what was intended.

The value returned on validation is correct, whereas the value returned during token creation mistakenly includes a decimal. The decimal portion of the expiration is never actually stored as a float in the token, so it should not be rendered during token creation with a zero decimal either.

tags: added: fernet
Changed in keystone:
status: New → Triaged
importance: Undecided → Low
Changed in keystone:
assignee: nobody → Deepti Ramakrishna (dramakri)
Dolph Mathews (dolph) wrote :

Deepti, do you already have a fix in progress for this? If not, I have a solution that I wrote while I was increasing Fernet's test coverage.

Deepti Ramakrishna (dramakri) wrote :

Dolph, feel free to assign it to yourself.

Dolph Mathews (dolph) wrote :

Thanks, Deepti! This patch is very much a mess of WIP (as of patchset 1), but the fix is included:

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

Changed in keystone:
assignee: Deepti Ramakrishna (dramakri) → Dolph Mathews (dolph)

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

commit a8c57027dad13389e1aebc242e18f9e05726b349
Author: Dolph Mathews <email address hidden>
Date: Fri Jul 17 19:33:22 2015 +0000

    Additional Fernet test coverage

    This expands existing test coverage to include Fernet tokens, a few of
    which expose a couple issues (see related bugs below).

    Change-Id: I53374d41e4e5453817b6635aee56df625073d015
    Related-Bug: 1459790
    Related-Bug: 1475762

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

Changed in keystone:
status: Triaged → In Progress
Morgan Fainberg (mdrnstm) wrote :

there are already cases (non-fernet) where we can have / not have microseconds in both v2 and v3. I would rather drive to microseconds everywhere rather than special casing. See -1 comment on proposed fix.

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

Changed in keystone:
assignee: Dolph Mathews (dolph) → Roxana Gherle (roxana-gherle)
Roxana Gherle (roxana-gherle) wrote :

Hi

Was there already a fix for this? I only saw the added test cases, but I didn't find an actual fix for it.

I went ahead and uploaded a patch which preserves milliseconds at validation time. Let me know what you think.

Thanks,
Roxana

Roxana Gherle (roxana-gherle) wrote :

Oh, I just saw the other patch submitted for review, ups..

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

Changed in keystone:
assignee: Roxana Gherle (roxana-gherle) → Dolph Mathews (dolph)

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

Dolph Mathews (dolph) wrote :

No worries, Roxana! I like your fix far better (assuming it's equivalent), but want to understand that test failure before proceeding.

Changed in keystone:
assignee: Dolph Mathews (dolph) → Roxana Gherle (roxana-gherle)

Change abandoned by Lance Bragstad (<email address hidden>) on branch: master
Review: https://review.openstack.org/210068
Reason: Abandoning per Roxana's message, please review https://review.openstack.org/#/c/210037/ instead.

Change abandoned by Dolph Mathews (<email address hidden>) on branch: master
Review: https://review.openstack.org/210049

Change abandoned by Dolph Mathews (<email address hidden>) on branch: master
Review: https://review.openstack.org/208021
Reason: Abandoning in favor of https://review.openstack.org/#/c/210037/

Dolph Mathews (dolph) on 2015-08-07
tags: added: kilo-backport-potential

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

commit 9391f9307e834f437c6712797c903921881f2977
Author: Roxana Gherle <email address hidden>
Date: Thu Aug 6 09:46:58 2015 -0700

    Fernet 'expires' value loses 'ms' after validation

    When we create a Fernet token we get a token with an expiration
    time which contains microseconds. When we validate a Fernet token,
    the expiration time changes and comes back missing the microseconds.
    We need to make the expiration time the same between creation and
    validation time, having microseconds in both cases. This applies to
    both v2 and v3 API.

    Closes-Bug: #1459790
    Change-Id: Ic6adbc2d69af03519a2c4ef66d558bcff9022049

Changed in keystone:
status: In Progress → Fix Committed
Dolph Mathews (dolph) on 2015-08-13
Changed in keystone:
importance: Low → Medium

Reviewed: https://review.openstack.org/212947
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=a1e20fbbb5d739b878b00638769ff81466f3a0b8
Submitter: Jenkins
Branch: stable/kilo

commit a1e20fbbb5d739b878b00638769ff81466f3a0b8
Author: Dolph Mathews <email address hidden>
Date: Fri Jul 17 19:33:22 2015 +0000

    Additional Fernet test coverage

    This expands existing test coverage to include Fernet tokens, a few of
    which expose a couple issues (see related bugs below).

    NOTE: The config fixture in keystone/tests/unit/test_v3_auth.py L536
    needed to be revised to use a full class path, rather than the
    stevedore-based entry point loading introduced to keystone during the
    liberty release cycle.

    Change-Id: I53374d41e4e5453817b6635aee56df625073d015
    Related-Bug: 1459790
    Related-Bug: 1475762
    (cherry picked from commit a8c57027dad13389e1aebc242e18f9e05726b349)

tags: added: in-stable-kilo

Reviewed: https://review.openstack.org/212953
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=aa3354e73bda53bbe8c354b644ed37e8b100a7d8
Submitter: Jenkins
Branch: stable/kilo

commit aa3354e73bda53bbe8c354b644ed37e8b100a7d8
Author: Roxana Gherle <email address hidden>
Date: Thu Aug 6 09:46:58 2015 -0700

    Fernet 'expires' value loses 'ms' after validation

    When we create a Fernet token we get a token with an expiration
    time which contains microseconds. When we validate a Fernet token,
    the expiration time changes and comes back missing the microseconds.
    We need to make the expiration time the same between creation and
    validation time, having microseconds in both cases. This applies to
    both v2 and v3 API.

    Conflicts:
        keystone/tests/unit/test_v3_auth.py
        keystone/tests/unit/token/test_fernet_provider.py
        keystone/token/providers/fernet/core.py
        keystone/token/providers/fernet/token_formatters.py

    NOTE: This backport differs from the patch to master to handle the
    difference in oslo.timeutils versus the move to keystone's own
    implementation of isotime in master, and to move the fix in
    keystone/token/providers/fernet/core.py to avoid having to backport a
    significant refactor Fernet's implementation.

    Closes-Bug: #1459790
    Change-Id: Ic6adbc2d69af03519a2c4ef66d558bcff9022049
    (cherry picked from commit 9391f9307e834f437c6712797c903921881f2977)

Dolph Mathews (dolph) on 2015-08-25
tags: removed: kilo-backport-potential
Changed in keystone:
milestone: none → liberty-3
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2015-10-15
Changed in keystone:
milestone: liberty-3 → 8.0.0
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers