Remove padding from Fernet tokens

Bug #1491926 reported by Lance Bragstad on 2015-09-03
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Medium
Lance Bragstad
Kilo
Medium
Lance Bragstad

Bug Description

In bug 1433372, we determined that we should percent encode Fernet tokens, because the padding characters (=) aren't considered URL safe by some RFCs.

We also fail some tempest tests because clients sometimes decode or encode responses [0]. We should just remove the padding, that way clients don't have to worry about it. When we go to validate a token, we can determine what the padding is based on the length of the token:

missing_padding = 4 - len(token) % 4
if missing_padding:
    token += b'=' * missing_padding

A patch can be proposed to master, stable/liberty, and stable/kilo to ensure that Fernet tokens can be validated regardless of padding. This is important to consider when upgrading from Kilo to Liberty or Kilo to Mitaka.

[0] http://cdn.pasteraw.com/es3j52dpfgem4nom62e7vktk7g5u2j1

tags: added: fernet
Changed in keystone:
assignee: nobody → Tom Cocozzello (tjcocozz)
Dolph Mathews (dolph) on 2015-09-03
Changed in keystone:
importance: Undecided → Medium
status: New → Triaged
Dolph Mathews (dolph) wrote :

Tom, Lance opened this bug as he already had an patch in progress to remove the padding. He should have assigned it to himself *glares at Lance*. Appreciate the eagerness, though! :)

Changed in keystone:
assignee: Tom Cocozzello (tjcocozz) → Lance Bragstad (lbragstad)
Lance Bragstad (lbragstad) wrote :

Patch on the way!

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

Changed in keystone:
status: Triaged → In Progress

I am sorry lance. The bug is all yours. :p

Lance Bragstad (lbragstad) wrote :

No worries Tom! I should have assigned it to myself when I created it. Awesome to see the eagerness though :)

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

commit f3e3a653f9c9ce0f9a7ba842eff118e5887eb388
Author: Lance Bragstad <email address hidden>
Date: Thu Sep 3 16:09:03 2015 +0000

    Remove padding from Fernet tokens

    Fernet tokens were previously percent encoded. This can cause issues with
    clients doing their own encoding or not. By removing the padding and then
    re-establishing it when we validate the token, we don't present that problem to
    clients. This also shortens the length of a Fernet token.

    Change-Id: I674bad86ccc9027ac3b365c10b3b142fc9d73c17
    Related-Bug: 1433372
    Closes-Bug: 1491926

Changed in keystone:
status: In Progress → Fix Committed
Dolph Mathews (dolph) on 2015-09-09
tags: added: kilo-backport-potential
Dolph Mathews (dolph) on 2015-09-09
tags: removed: kilo-backport-potential
Dolph Mathews (dolph) wrote :

Re-opening this bug so that we can eliminate any misbehaviors of the above patch during upgrades (existing tokens with the legacy encoding still need to be validate correctly).

Changed in keystone:
status: Fix Committed → In Progress
Dolph Mathews (dolph) on 2015-09-15
Changed in keystone:
milestone: none → liberty-rc1
Changed in keystone:
milestone: liberty-rc1 → none

Change abandoned by Lance Bragstad (<email address hidden>) on branch: master
Review: https://review.openstack.org/221786
Reason: This should be reviewed instead - https://review.openstack.org/#/c/231022/

Changed in keystone:
milestone: none → liberty-rc2
description: updated

Change abandoned by Lance Bragstad (<email address hidden>) on branch: stable/liberty
Review: https://review.openstack.org/231022
Reason: Review this instead! https://review.openstack.org/#/c/231051/

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

commit 90aca980587283d26fcfd20ae6be86ca1bbe2788
Author: Lance Bragstad <email address hidden>
Date: Wed Sep 9 14:09:06 2015 +0000

    Ensure token validation works irrespective of padding

    In change I674bad86ccc9027ac3b365c10b3b142fc9d73c17 we removed the padding from
    Fernet tokens. This commit makes it so that we can validate Fernet tokens that
    come in with and without padding.

    Change-Id: I60e52af6c4cb39f2d136540a487bba8505a2c6b4
    Closes-Bug: 1491926

Changed in keystone:
status: In Progress → Fix Committed

Reviewed: https://review.openstack.org/231051
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=8dcd82fb9c76d43f26338bee293b32f4af473ad9
Submitter: Jenkins
Branch: stable/liberty

commit 8dcd82fb9c76d43f26338bee293b32f4af473ad9
Author: Lance Bragstad <email address hidden>
Date: Wed Sep 9 14:09:06 2015 +0000

    Ensure token validation works irrespective of padding

    In change I674bad86ccc9027ac3b365c10b3b142fc9d73c17 we removed the padding from
    Fernet tokens. This commit makes it so that we can validate Fernet tokens that
    come in with and without padding.

    Change-Id: I60e52af6c4cb39f2d136540a487bba8505a2c6b4
    Closes-Bug: 1491926
    (cherry picked from commit 90aca980587283d26fcfd20ae6be86ca1bbe2788)

tags: added: in-stable-liberty
Thierry Carrez (ttx) on 2015-10-06
Changed in keystone:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2015-10-15
Changed in keystone:
milestone: liberty-rc2 → 8.0.0

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

commit 8dcd82fb9c76d43f26338bee293b32f4af473ad9
Author: Lance Bragstad <email address hidden>
Date: Wed Sep 9 14:09:06 2015 +0000

    Ensure token validation works irrespective of padding

    In change I674bad86ccc9027ac3b365c10b3b142fc9d73c17 we removed the padding from
    Fernet tokens. This commit makes it so that we can validate Fernet tokens that
    come in with and without padding.

    Change-Id: I60e52af6c4cb39f2d136540a487bba8505a2c6b4
    Closes-Bug: 1491926
    (cherry picked from commit 90aca980587283d26fcfd20ae6be86ca1bbe2788)

commit 4a239f23e3e599b55f6553f3b4f3290773d2ded7
Author: Tony Wang <email address hidden>
Date: Fri Aug 21 23:41:32 2015 -0400

    Show v3 endpoints in v2 endpoint list

    v3 endpoints that are not migrated from v2 endpoints, were not shown in
    v2 endpoint list. While it was more clear, separating v2 and v3
    endpoints, it was not easy for tools or services which are still using
    v2 APIs to get the endpoints created by v3 API.
    Since it won't do harm to return more information, this commit is to
    show v3 endpoints in API call to list v2 endpoints.
    NOTE: public interface is required in a v2 endpoints, so only v3
    endpoints with public interface are added into the list.

    Change-Id: Id9b3822f87e973b6eb7dd9fc715c2dba76b9fe04
    Closes-Bug: #1480270
    (cherry picked from commit 0d13a85e93737df8c9c130f194839b27d0781890)

commit ec3ceb5956aa2bab5fe03107849a7ccde27ef87c
Author: OpenStack Proposal Bot <email address hidden>
Date: Thu Oct 1 06:09:29 2015 +0000

    Imported Translations from Zanata

    For more information about this automatic import see:
    https://wiki.openstack.org/wiki/Translations/Infrastructure

    Change-Id: I621480e00e624575dd01138de260f246570aceee

commit 92f7085cd32d168771ca667fe2503222ef885902
Author: Marek Denis <email address hidden>
Date: Tue Jul 28 16:23:07 2015 +0200

    Skip rows with empty remote_ids

    Federation migration scripts are trying to migrate
    identity_provider.remote_id columns to a separate table remote_idps and
    so 1:N relations are possible. We should, however do any DB operations
    on identity_providers's values that have non null remote_id values.

    Change-Id: Ifbb80896969986bafedf1c879bc7474832afca60
    Closes-Bug: #1478961

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

Change abandoned by Steve Martinelli (<email address hidden>) on branch: stable/kilo
Review: https://review.openstack.org/221799
Reason: no change in 60 days after a negative review, marking this as abandoned, please feel free to restore

Change abandoned by Steve Martinelli (<email address hidden>) on branch: stable/kilo
Review: https://review.openstack.org/231057
Reason: no change in 60 days after a negative review, marking this as abandoned, please feel free to restore

Morgan Fainberg (mdrnstm) wrote :

Kilo is EOL

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers