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>
(cherry picked from commit 18864380cc3289e20fc1cbbfaefa34004265c774)

diff --git a/src/auth/cephx/CephxKeyServer.cc b/src/auth/cephx/CephxKeyServer.cc
index 2961615b13eea5ee29209ae92ab331f408277e3b..ac83b7f3ee8f6e7512b35a0ff85b25e906c5812d 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 a4ae8c82f464df72e3c6df0f2f03e324d1a9e1b2..f08608c6133d6e44d2306792587dd10d2aa37fdd 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)
   {}