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>

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;
 }
 

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;
 

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();

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)
   {}