Skip to content

Commit

Permalink
auth/cephx: cap ticket validity by expiration on "next" key
Browse files Browse the repository at this point in the history
If auth_mon_ticket_ttl is increased by several times as done in
commit 522a52e ("auth/cephx: rotate auth tickets less often"),
active clients eventually get stuck because the monitor sends out an
auth ticket with a bogus validity.  The ticket is secured with the
"current" secret that is scheduled to expire according to the old TTL,
but the validity of the ticket is set to the new TTL.  As a result,
the client simply doesn't attempt to renew, letting the secrets rotate
potentially more than once.  When that happens, the client first hits
auth authorizer errors as it tries to renew service tickets and when
it finally gets to renewing the auth ticket, it hits the insecure
global_id reclaim wall.

Cap TTL by expiration of "next" key -- the "current" key may be
milliseconds away from expiration and still be used, legitimately.
Do it in KeyServerData alongside key rotation code and propagate the
capped TTL to the upper layer.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
  • Loading branch information
idryomov committed Apr 15, 2021
1 parent 3078af7 commit 198f9f6
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 28 deletions.
40 changes: 27 additions & 13 deletions src/auth/cephx/CephxKeyServer.cc
Expand Up @@ -30,7 +30,8 @@ using ceph::bufferlist;
using ceph::Formatter;

bool KeyServerData::get_service_secret(CephContext *cct, uint32_t service_id,
CryptoKey& secret, uint64_t& secret_id) const
CryptoKey& secret, uint64_t& secret_id,
double& ttl) const
{
auto iter = rotating_secrets.find(service_id);
if (iter == rotating_secrets.end()) {
Expand All @@ -45,15 +46,25 @@ bool KeyServerData::get_service_secret(CephContext *cct, uint32_t service_id,
if (secrets.secrets.size() > 1)
++riter;

if (riter->second.expiration < ceph_clock_now())
utime_t now = ceph_clock_now();
if (riter->second.expiration < now)
++riter; // "current" key has expired, use "next" key instead

secret_id = riter->first;
secret = riter->second.key;

// ttl may have just been increased by the user
// cap it by expiration of "next" key to prevent handing out a ticket
// with a bogus, possibly way into the future, validity
ttl = service_id == CEPH_ENTITY_TYPE_AUTH ?
cct->_conf->auth_mon_ticket_ttl : cct->_conf->auth_service_ticket_ttl;
ttl = min(ttl, static_cast<double>(
secrets.secrets.rbegin()->second.expiration - now));

ldout(cct, 30) << __func__ << " service "
<< ceph_entity_type_name(service_id) << " secret_id "
<< secret_id << " " << riter->second << dendl;
<< secret_id << " " << riter->second << " ttl " << ttl
<< dendl;
return true;
}

Expand Down Expand Up @@ -226,12 +237,12 @@ bool KeyServer::get_caps(const EntityName& name, const string& type,
return data.get_caps(cct, name, type, caps_info);
}

bool KeyServer::get_service_secret(uint32_t service_id,
CryptoKey& secret, uint64_t& secret_id) const
bool KeyServer::get_service_secret(uint32_t service_id, CryptoKey& secret,
uint64_t& secret_id, double& ttl) const
{
std::scoped_lock l{lock};

return data.get_service_secret(cct, service_id, secret, secret_id);
return data.get_service_secret(cct, service_id, secret, secret_id, ttl);
}

bool KeyServer::get_service_secret(uint32_t service_id,
Expand Down Expand Up @@ -403,12 +414,13 @@ bool KeyServer::get_service_caps(const EntityName& name, uint32_t service_id,

int KeyServer::_build_session_auth_info(uint32_t service_id,
const AuthTicket& parent_ticket,
CephXSessionAuthInfo& info)
CephXSessionAuthInfo& info,
double ttl)
{
info.service_id = service_id;
info.ticket = parent_ticket;
info.ticket.init_timestamps(ceph_clock_now(),
cct->_conf->auth_service_ticket_ttl);
info.ticket.init_timestamps(ceph_clock_now(), ttl);
info.validity.set_from_double(ttl);

generate_secret(info.session_key);

Expand All @@ -426,13 +438,14 @@ int KeyServer::build_session_auth_info(uint32_t service_id,
const AuthTicket& parent_ticket,
CephXSessionAuthInfo& info)
{
if (!get_service_secret(service_id, info.service_secret, info.secret_id)) {
double ttl;
if (!get_service_secret(service_id, info.service_secret, info.secret_id,
ttl)) {
return -EACCES;
}

std::scoped_lock l{lock};

return _build_session_auth_info(service_id, parent_ticket, info);
return _build_session_auth_info(service_id, parent_ticket, info, ttl);
}

int KeyServer::build_session_auth_info(uint32_t service_id,
Expand All @@ -445,6 +458,7 @@ int KeyServer::build_session_auth_info(uint32_t service_id,
info.secret_id = secret_id;

std::scoped_lock l{lock};
return _build_session_auth_info(service_id, parent_ticket, info);
return _build_session_auth_info(service_id, parent_ticket, info,
cct->_conf->auth_service_ticket_ttl);
}

10 changes: 6 additions & 4 deletions src/auth/cephx/CephxKeyServer.h
Expand Up @@ -94,7 +94,8 @@ struct KeyServerData {
}

bool get_service_secret(CephContext *cct, uint32_t service_id,
CryptoKey& secret, uint64_t& secret_id) const;
CryptoKey& secret, uint64_t& secret_id,
double& ttl) const;
bool get_service_secret(CephContext *cct, uint32_t service_id,
uint64_t secret_id, CryptoKey& secret) const;
bool get_auth(const EntityName& name, EntityAuth& auth) const;
Expand Down Expand Up @@ -199,7 +200,8 @@ class KeyServer : public KeyStore {
void _dump_rotating_secrets();
int _build_session_auth_info(uint32_t service_id,
const AuthTicket& parent_ticket,
CephXSessionAuthInfo& info);
CephXSessionAuthInfo& info,
double ttl);
bool _get_service_caps(const EntityName& name, uint32_t service_id,
AuthCapsInfo& caps) const;
public:
Expand All @@ -223,8 +225,8 @@ class KeyServer : public KeyStore {
uint64_t secret_id);

/* get current secret for specific service type */
bool get_service_secret(uint32_t service_id, CryptoKey& service_key,
uint64_t& secret_id) const;
bool get_service_secret(uint32_t service_id, CryptoKey& secret,
uint64_t& secret_id, double& ttl) const;
bool get_service_secret(uint32_t service_id, uint64_t secret_id,
CryptoKey& secret) const override;

Expand Down
23 changes: 12 additions & 11 deletions src/auth/cephx/CephxServiceHandler.cc
Expand Up @@ -218,24 +218,27 @@ int CephxServiceHandler::handle_request(
break;
}

info.ticket.init_timestamps(ceph_clock_now(),
cct->_conf->auth_mon_ticket_ttl);
double ttl;
if (!key_server->get_service_secret(CEPH_ENTITY_TYPE_AUTH,
info.service_secret, info.secret_id,
ttl)) {
ldout(cct, 0) << " could not get service secret for auth subsystem" << dendl;
ret = -EIO;
break;
}

info.service_id = CEPH_ENTITY_TYPE_AUTH;
info.ticket.name = entity_name;
info.ticket.global_id = global_id;
info.validity += cct->_conf->auth_mon_ticket_ttl;
info.ticket.init_timestamps(ceph_clock_now(), ttl);
info.validity.set_from_double(ttl);

key_server->generate_secret(session_key);

info.session_key = session_key;
if (psession_key) {
*psession_key = session_key;
}
info.service_id = CEPH_ENTITY_TYPE_AUTH;
if (!key_server->get_service_secret(CEPH_ENTITY_TYPE_AUTH, info.service_secret, info.secret_id)) {
ldout(cct, 0) << " could not get service secret for auth subsystem" << dendl;
ret = -EIO;
break;
}

vector<CephXSessionAuthInfo> info_vec;
info_vec.push_back(info);
Expand Down Expand Up @@ -296,7 +299,6 @@ int CephxServiceHandler::handle_request(
service_id,
info.ticket,
svc_info);
svc_info.validity += cct->_conf->auth_service_ticket_ttl;
info_vec.push_back(svc_info);
}
}
Expand Down Expand Up @@ -366,7 +368,6 @@ int CephxServiceHandler::handle_request(
service_err = r;
continue;
}
info.validity += cct->_conf->auth_service_ticket_ttl;
info_vec.push_back(info);
++found_services;
}
Expand Down

0 comments on commit 198f9f6

Please sign in to comment.