New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rgw: swift: disable revocation thread if sleep == 0 #14501

Merged
merged 1 commit into from Apr 21, 2017

Conversation

Projects
None yet
4 participants
@mdw-at-linuxbox
Contributor

mdw-at-linuxbox commented Apr 13, 2017

Keystone tokens can be revoked. This causes them to fail
validation. However, in ceph, we cache them. As long as
they're in the cache we trust them. To find revoked tokens
there's a call OSI-PKI/revoked but that's only useful for
pki tokens. Installations using fernet/uuid may not even
have the proper credentials to support the call, in which
case the call blows up in various ways filling up logs
with complaints.

This code makes the revocation thread optional; by disabling it,
the complaints go away. A further fix is in the works
to use other more modern calls available in modern keystone
installations to properly deal with non-PKI/PKIZ tokens.

To disable the revocation thread, set
rgw_keystone_revocation_interval = 0
You may also want to set
rgw_keystone_token_cache_size = 0
to cause tokens to be validate on every call.

Fixes: http://tracker.ceph.com/issues/9493
Fixes: http://tracker.ceph.com/issues/19499

Signed-off-by: Marcus Watts mwatts@redhat.com

/* The thread name has been kept for backward compliance. */
revocator.create("rgw_swift_k_rev");
revocator.create("rgw_swift_k_rev");

This comment has been minimized.

@rzarzynski

rzarzynski Apr 13, 2017

Contributor

It would be nice to have the 2-space indentation for this changed line as well as the comment that became a part of the if block.

@rzarzynski rzarzynski self-assigned this Apr 13, 2017

rgw: swift: disable revocation thread if sleep == 0 || cache_size == 0
Keystone tokens can be revoked.  This causes them to fail
validation.  However, in ceph, we cache them.  As long as
they're in the cache we trust them.  To find revoked tokens
there's a call OSI-PKI/revoked but that's only useful for
pki tokens.  Installations using fernet/uuid may not even
have the proper credentials to support the call, in which
case the call blows up in various ways filling up logs
with complaints.

This code makes the revocation thread optional; by disabling it,
the complaints go away.  A further fix is in the works
to use other more modern calls available in modern keystone
installations to properly deal with non-PKI/PKIZ tokens.

To disable the revocation thread, use at least one of these:
        rgw_keystone_token_cache_size = 0
		using this will cause tokens to be validated on every call.
You may instead want to set
        rgw_keystone_revocation_interval = 0
		using just this will disable the revocation thread,
		but leaves the cache in use.  That avoids the extra
		validation overhead, but means token revocation won't
		work very well.

Fixes: http://tracker.ceph.com/issues/9493
Fixes: http://tracker.ceph.com/issues/19499

Signed-off-by: Marcus Watts <mwatts@redhat.com>
@mdw-at-linuxbox

This comment has been minimized.

Contributor

mdw-at-linuxbox commented Apr 14, 2017

I've pushed a new version. Slight formatting changes, plus one change: now possible to disable thread by setting either of rgw_keystone_revocation_interval or rgw_keystone_token_cache_size to 0. Setting cache size = 0 makes revocation work at the cost of doing token validation on every use. Setting revocation_interval = 0 allows the cache to be useful, but makes token revocation iffy.

@rzarzynski

LGTM.

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Apr 17, 2017

@mdw-at-linuxbox: I'm not sure whether the Fixes: http://tracker.ceph.com/issues/19499 (the OS-REVOKE extension of Identity API v3) is accurate. The patch is rather related to it.

Anyway, it's very minor regarding solely the commit message, not the code. That doesn't prevent testing.

@mdw-at-linuxbox mdw-at-linuxbox merged commit 22d455c into ceph:master Apr 21, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment