From: Sage Weil Date: Wed, 29 Sep 2021 20:28:07 +0000 (-0400) Subject: mon,auth: fix proposal of rotating keys X-Git-Tag: v17.1.0~745^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=18864380cc3289e20fc1cbbfaefa34004265c774;p=ceph.git mon,auth: fix proposal of rotating keys Instead of updating the live CephxKeyServer's rotating_keys and also including them in a paxos proposal, propose new keys only in the proposal, and only make them live once they are committed. This keeps mons fully in sync and avoids any inconsistency between the live behavior and committed state (e.g., stale or divergent keys being applied and passed out to daemons). Signed-off-by: Sage Weil --- diff --git a/src/auth/cephx/CephxKeyServer.cc b/src/auth/cephx/CephxKeyServer.cc index 17c6e0918878..86ccc1ca2fbb 100644 --- a/src/auth/cephx/CephxKeyServer.cc +++ b/src/auth/cephx/CephxKeyServer.cc @@ -149,7 +149,6 @@ int KeyServer::start_server() { std::scoped_lock l{lock}; - _check_rotating_secrets(); _dump_rotating_secrets(); return 0; } @@ -159,30 +158,6 @@ void KeyServer::dump() _dump_rotating_secrets(); } -bool KeyServer::_check_rotating_secrets() -{ - ldout(cct, 10) << "_check_rotating_secrets" << dendl; - - int added = 0; - added += _rotate_secret(CEPH_ENTITY_TYPE_AUTH); - added += _rotate_secret(CEPH_ENTITY_TYPE_MON); - added += _rotate_secret(CEPH_ENTITY_TYPE_OSD); - added += _rotate_secret(CEPH_ENTITY_TYPE_MDS); - added += _rotate_secret(CEPH_ENTITY_TYPE_MGR); - - if (added) { - data.rotating_ver++; - ldout(cct, 10) << __func__ << " added " << added - << ", rotating_ver=" << data.rotating_ver - << dendl; - //data.next_rotating_time = ceph_clock_now(cct); - //data.next_rotating_time += std::min(cct->_conf->auth_mon_ticket_ttl, cct->_conf->auth_service_ticket_ttl); - _dump_rotating_secrets(); - return true; - } - return false; -} - void KeyServer::_dump_rotating_secrets() { ldout(cct, 30) << "_dump_rotating_secrets" << dendl; @@ -199,9 +174,9 @@ void KeyServer::_dump_rotating_secrets() } } -int KeyServer::_rotate_secret(uint32_t service_id) +int KeyServer::_rotate_secret(uint32_t service_id, KeyServerData &pending_data) { - RotatingSecrets& r = data.rotating_secrets[service_id]; + RotatingSecrets& r = pending_data.rotating_secrets[service_id]; int added = 0; utime_t now = ceph_clock_now(); double ttl = service_id == CEPH_ENTITY_TYPE_AUTH ? cct->_conf->auth_mon_ticket_ttl : cct->_conf->auth_service_ticket_ttl; @@ -366,26 +341,30 @@ void KeyServer::encode_plaintext(bufferlist &bl) bl.append(os.str()); } -bool KeyServer::updated_rotating(bufferlist& rotating_bl, version_t& rotating_ver) +bool KeyServer::prepare_rotating_update(bufferlist& rotating_bl) { std::scoped_lock l{lock}; ldout(cct, 20) << __func__ << " before: data.rotating_ver=" << data.rotating_ver - << " vs rotating_ver " << rotating_ver << dendl; + << dendl; - bool r = _check_rotating_secrets(); - - ldout(cct, 20) << __func__ << " after: data.rotating_ver=" << data.rotating_ver - << " vs rotating_ver " << rotating_ver << dendl; + KeyServerData pending_data(nullptr); + pending_data.rotating_ver = data.rotating_ver + 1; + pending_data.rotating_secrets = data.rotating_secrets; - if (data.rotating_ver <= rotating_ver) { - ceph_assert(!r); + int added = 0; + added += _rotate_secret(CEPH_ENTITY_TYPE_AUTH, pending_data); + added += _rotate_secret(CEPH_ENTITY_TYPE_MON, pending_data); + added += _rotate_secret(CEPH_ENTITY_TYPE_OSD, pending_data); + added += _rotate_secret(CEPH_ENTITY_TYPE_MDS, pending_data); + added += _rotate_secret(CEPH_ENTITY_TYPE_MGR, pending_data); + if (!added) { return false; } - - data.encode_rotating(rotating_bl); - - rotating_ver = data.rotating_ver; + ldout(cct, 20) << __func__ << " after: pending_data.rotating_ver=" + << pending_data.rotating_ver + << dendl; + pending_data.encode_rotating(rotating_bl); return true; } diff --git a/src/auth/cephx/CephxKeyServer.h b/src/auth/cephx/CephxKeyServer.h index 68c40cc01b07..945a7f4dcd89 100644 --- a/src/auth/cephx/CephxKeyServer.h +++ b/src/auth/cephx/CephxKeyServer.h @@ -195,8 +195,7 @@ class KeyServer : public KeyStore { KeyServerData data; mutable ceph::mutex lock; - int _rotate_secret(uint32_t service_id); - bool _check_rotating_secrets(); + int _rotate_secret(uint32_t service_id, KeyServerData &pending_data); void _dump_rotating_secrets(); int _build_session_auth_info(uint32_t service_id, const AuthTicket& parent_ticket, @@ -299,7 +298,7 @@ public: } } - bool updated_rotating(ceph::buffer::list& rotating_bl, version_t& rotating_ver); + bool prepare_rotating_update(ceph::buffer::list& rotating_bl); bool get_rotating_encrypted(const EntityName& name, ceph::buffer::list& enc_bl) const; diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc index ebe99c672426..f6b035fb2eb3 100644 --- a/src/mon/AuthMonitor.cc +++ b/src/mon/AuthMonitor.cc @@ -83,11 +83,12 @@ bool AuthMonitor::check_rotate() { KeyServerData::Incremental rot_inc; rot_inc.op = KeyServerData::AUTH_INC_SET_ROTATING; - if (!mon.key_server.updated_rotating(rot_inc.rotating_bl, last_rotating_ver)) - return false; - dout(10) << __func__ << " updated rotating" << dendl; - push_cephx_inc(rot_inc); - return true; + if (mon.key_server.prepare_rotating_update(rot_inc.rotating_bl)) { + dout(10) << __func__ << " updating rotating" << dendl; + push_cephx_inc(rot_inc); + return true; + } + return false; } /* @@ -139,16 +140,26 @@ void AuthMonitor::on_active() if (!mon.is_leader()) return; + mon.key_server.start_server(); - bool increase; - { - std::lock_guard l(mon.auth_lock); - increase = _should_increase_max_global_id(); - } - if (is_writeable() && increase) { - increase_max_global_id(); - propose_pending(); + if (is_writeable()) { + bool propose = false; + if (check_rotate()) { + propose = true; + } + bool increase; + { + std::lock_guard l(mon.auth_lock); + increase = _should_increase_max_global_id(); + } + if (increase) { + increase_max_global_id(); + propose = true; + } + if (propose) { + propose_pending(); + } } } @@ -241,7 +252,6 @@ void AuthMonitor::create_initial() // initialize rotating keys mon.key_server.clear_secrets(); - last_rotating_ver = 0; check_rotate(); ceph_assert(pending_auth.size() == 1); @@ -358,7 +368,6 @@ void AuthMonitor::update_from_paxos(bool *need_bootstrap) dout(10) << __func__ << " max_global_id=" << max_global_id << " format_version " << format_version - << ", last_rotating_ver " << last_rotating_ver << dendl; mon.key_server.dump(); diff --git a/src/mon/AuthMonitor.h b/src/mon/AuthMonitor.h index 048fc0c08ef4..4312b56071f4 100644 --- a/src/mon/AuthMonitor.h +++ b/src/mon/AuthMonitor.h @@ -97,7 +97,6 @@ public: private: std::vector pending_auth; - version_t last_rotating_ver; uint64_t max_global_id; uint64_t last_allocated_id; @@ -186,7 +185,6 @@ private: public: AuthMonitor(Monitor &mn, Paxos &p, const std::string& service_name) : PaxosService(mn, p, service_name), - last_rotating_ver(0), max_global_id(0), last_allocated_id(0) {}