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 <sage@newdream.net>

diff --git a/src/auth/cephx/CephxKeyServer.cc b/src/auth/cephx/CephxKeyServer.cc
index 17c6e091887835f228b19b3728bd10f73efc9642..86ccc1ca2fbb78e8c3da6da5f4597df742326ba4 100644 (file)
--- 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 68c40cc01b075a165e1bcfd2a60d6d9a7510488b..945a7f4dcd8979f64643e6924173c22104c5d687 100644 (file)
--- 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 ebe99c6724264a25973a859659265943636848ea..f6b035fb2eb35e70222482af0b5176c31e2ff32c 100644 (file)
--- 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 048fc0c08ef4d3ec9015bfce0cab890c51752bdd..4312b56071f4b7503ddee6468c58a9a616efbdf7 100644 (file)
--- a/src/mon/AuthMonitor.h
+++ b/src/mon/AuthMonitor.h
@@ -97,7 +97,6 @@ public:
 
 private:
   std::vector<Incremental> 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)
   {}