cephx: fix up key rotation

src/auth/Auth.h

@@ -213,7 +213,7 @@ struct AuthAuthorizer {
 /*
  * Key management
  */ 
-#define KEY_ROTATE_NUM 3
+#define KEY_ROTATE_NUM 3   /* prev, current, next */
 
 struct ExpiringCryptoKey {
   CryptoKey key;
@@ -258,15 +258,34 @@ struct RotatingSecrets {
     ::decode(max_ver, bl);
   }
   
-  void add(ExpiringCryptoKey& key) {
+  uint64_t add(ExpiringCryptoKey& key) {
     secrets[++max_ver] = key;
     while (secrets.size() > KEY_ROTATE_NUM)
       secrets.erase(secrets.begin());
+    return max_ver;
   }
   
   bool need_new_secrets() {
     return secrets.size() < KEY_ROTATE_NUM;
   }
+  bool need_new_secrets(utime_t now) {
+    return secrets.size() < KEY_ROTATE_NUM || current().expiration <= now;
+  }
+
+  ExpiringCryptoKey& previous() {
+    return secrets.begin()->second;
+  }
+  ExpiringCryptoKey& current() {
+    map<uint64_t, ExpiringCryptoKey>::iterator p = secrets.begin();
+    p++;
+    return p->second;
+  }
+  ExpiringCryptoKey& next() {
+    return secrets.rbegin()->second;
+  }
+  bool empty() {
+    return secrets.empty();
+  }
 
   void dump();
 };

src/auth/RotatingKeyRing.cc

@@ -18,6 +18,11 @@ bool RotatingKeyRing::need_new_secrets()
   Mutex::Locker l(lock);
   return secrets.need_new_secrets();
 }
+bool RotatingKeyRing::need_new_secrets(utime_t now)
+{
+  Mutex::Locker l(lock);
+  return secrets.need_new_secrets(now);
+}
 
 void RotatingKeyRing::set_secrets(RotatingSecrets& s)
 {

src/auth/RotatingKeyRing.h

@@ -39,6 +39,7 @@ public:
     lock("RotatingKeyRing::lock") {}
 
   bool need_new_secrets();
+  bool need_new_secrets(utime_t now);
   void set_secrets(RotatingSecrets& s);
   void dump_rotating();
   bool get_secret(EntityName& name, CryptoKey& secret);

src/auth/cephx/CephxKeyServer.cc

@@ -65,8 +65,15 @@ bool KeyServerData::get_service_secret(uint32_t service_id, uint64_t secret_id,
   RotatingSecrets& secrets = iter->second;
   map<uint64_t, ExpiringCryptoKey>::iterator riter = secrets.secrets.find(secret_id);
 
-  if (riter == secrets.secrets.end())
+  if (riter == secrets.secrets.end()) {
+    dout(10) << "get_service_secret service " << ceph_entity_type_name(service_id) << " secret " << secret_id
+            << " not found; i have:" << dendl;
+    for (map<uint64_t, ExpiringCryptoKey>::iterator iter = secrets.secrets.begin();
+        iter != secrets.secrets.end();
+        ++iter)
+      dout(10) << " id " << iter->first << " " << iter->second << dendl;
     return false;
+  }
 
   secret = riter->second.key;
 
@@ -111,35 +118,38 @@ KeyServer::KeyServer() : lock("KeyServer::lock")
 {
 }
 
-int KeyServer::start_server(bool init)
+int KeyServer::start_server()
 {
   Mutex::Locker l(lock);
 
-  if (init) {
-    _generate_all_rotating_secrets(init);
-  }
+  _check_rotating_secrets();
+  _dump_rotating_secrets();
   return 0;
 }
 
-void KeyServer::_generate_all_rotating_secrets(bool init)
+bool KeyServer::_check_rotating_secrets()
 {
-  data.rotating_ver++;
-  data.next_rotating_time = g_clock.now();
-  data.next_rotating_time += g_conf.auth_mon_ticket_ttl;
-  dout(10) << "generate_all_rotating_secrets" << dendl;
+  dout(10) << "_check_rotating_secrets" << dendl;
 
-  int i = KEY_ROTATE_NUM;
+  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);
 
-  if (init)
-    i = 1;
-
-  for (; i <= KEY_ROTATE_NUM; i++) {
-    _rotate_secret(CEPH_ENTITY_TYPE_AUTH, i);
-    _rotate_secret(CEPH_ENTITY_TYPE_MON, i);
-    _rotate_secret(CEPH_ENTITY_TYPE_OSD, i);
-    _rotate_secret(CEPH_ENTITY_TYPE_MDS, i);
+  if (added) {
+    data.rotating_ver++;
+    //data.next_rotating_time = g_clock.now();
+    //data.next_rotating_time += MIN(g_conf.auth_mon_ticket_ttl, g_conf.auth_service_ticket_ttl);
+    _dump_rotating_secrets();
+    return true;
   }
+  return false;
+}
 
+void KeyServer::_dump_rotating_secrets()
+{
+  dout(10) << "_dump_rotating_secrets" << dendl;
   for (map<uint32_t, RotatingSecrets>::iterator iter = data.rotating_secrets.begin();
        iter != data.rotating_secrets.end();
        ++iter) {
@@ -154,24 +164,28 @@ void KeyServer::_generate_all_rotating_secrets(bool init)
   }
 }
 
-void KeyServer::_rotate_secret(uint32_t service_id, int factor)
+int KeyServer::_rotate_secret(uint32_t service_id)
 {
-  ExpiringCryptoKey ek;
-  generate_secret(ek.key);
-  ek.expiration = g_clock.now();
-  ek.expiration += (g_conf.auth_mon_ticket_ttl * factor);
-  
-  data.add_rotating_secret(service_id, ek);
-}
+  RotatingSecrets& r = data.rotating_secrets[service_id];
+  int added = 0;
+  utime_t now = g_clock.now();
+  double ttl = service_id == CEPH_ENTITY_TYPE_AUTH ? g_conf.auth_mon_ticket_ttl : g_conf.auth_service_ticket_ttl;
 
-bool KeyServer::_check_rotate()
-{
-  if (g_clock.now() > data.next_rotating_time) {
-    dout(0) << "KeyServer::check_rotate: need to rotate keys" << dendl;
-    _generate_all_rotating_secrets(false);
-    return true;
+  while (r.need_new_secrets(now)) {
+    ExpiringCryptoKey ek;
+    generate_secret(ek.key);
+    if (r.empty())
+      ek.expiration = now;
+    else
+      ek.expiration = MAX(now, r.next().expiration);
+    ek.expiration += ttl;
+    uint64_t secret_id = r.add(ek);
+    dout(10) << "_rotate_secret adding " << ceph_entity_type_name(service_id)
+	     << " id " << secret_id << " " << ek
+	     << dendl;
+    added++;
   }
-  return false;
+  return added;
 }
 
 bool KeyServer::get_secret(EntityName& name, CryptoKey& secret)
@@ -279,7 +293,7 @@ bool KeyServer::updated_rotating(bufferlist& rotating_bl, version_t& rotating_ve
 {
   Mutex::Locker l(lock);
 
-  _check_rotate(); 
+  _check_rotating_secrets(); 
 
   if (data.rotating_ver <= rotating_ver)
     return false;

src/auth/cephx/CephxKeyServer.h

@@ -23,13 +23,12 @@
 
 struct KeyServerData {
   version_t version;
-  version_t rotating_ver;
-  utime_t next_rotating_time;
 
   /* for each entity */
   map<EntityName, EntityAuth> secrets;
 
   /* for each service type */
+  version_t rotating_ver;
   map<uint32_t, RotatingSecrets> rotating_secrets;
 
   KeyServerData() : version(0), rotating_ver(0) {}
@@ -39,7 +38,6 @@ struct KeyServerData {
     ::encode(struct_v, bl);
     ::encode(version, bl);
     ::encode(rotating_ver, bl);
-    ::encode(next_rotating_time, bl);
     ::encode(secrets, bl);
     ::encode(rotating_secrets, bl);
   }
@@ -48,7 +46,6 @@ struct KeyServerData {
     ::decode(struct_v, bl);
     ::decode(version, bl);
     ::decode(rotating_ver, bl);
-    ::decode(next_rotating_time, bl);
     ::decode(secrets, bl);
     ::decode(rotating_secrets, bl);
   }
@@ -82,10 +79,6 @@ struct KeyServerData {
     secrets.erase(iter);
   }
 
-  void add_rotating_secret(uint32_t service_id, ExpiringCryptoKey& key) {
-    rotating_secrets[service_id].add(key);
-  }
-
   bool get_service_secret(uint32_t service_id, ExpiringCryptoKey& secret, uint64_t& secret_id);
   bool get_service_secret(uint32_t service_id, CryptoKey& secret, uint64_t& secret_id);
   bool get_service_secret(uint32_t service_id, uint64_t secret_id, CryptoKey& secret);
@@ -173,9 +166,9 @@ class KeyServer : public KeyStore {
 
   Mutex lock;
 
-  void _rotate_secret(uint32_t service_id, int factor);
-  void _generate_all_rotating_secrets(bool init);
-  bool _check_rotate();
+  int _rotate_secret(uint32_t service_id);
+  bool _check_rotating_secrets();
+  void _dump_rotating_secrets();
   int _build_session_auth_info(uint32_t service_id, CephXServiceTicketInfo& auth_ticket_info, CephXSessionAuthInfo& info);
   bool _get_service_caps(EntityName& name, uint32_t service_id, AuthCapsInfo& caps);
 public:
@@ -186,7 +179,7 @@ public:
   bool get_secret(EntityName& name, CryptoKey& secret);
   bool get_caps(EntityName& name, string& type, AuthCapsInfo& caps);
   bool get_active_rotating_secret(EntityName& name, CryptoKey& secret);
-  int start_server(bool init);
+  int start_server();
   void rotate_timeout(double timeout);
 
   int build_session_auth_info(uint32_t service_id, CephXServiceTicketInfo& auth_ticket_info, CephXSessionAuthInfo& info);
@@ -232,10 +225,11 @@ public:
     data.remove_secret(name);
   }
 
-  void add_rotating_secret(uint32_t service_id, ExpiringCryptoKey& key) {
+  /*void add_rotating_secret(uint32_t service_id, ExpiringCryptoKey& key) {
     Mutex::Locker l(lock);
     data.add_rotating_secret(service_id, key);
   }
+  */
   void clone_to(KeyServerData& dst) {
     Mutex::Locker l(lock);
     dst = data;

src/mon/AuthMonitor.cc

@@ -84,7 +84,7 @@ void AuthMonitor::on_active()
 
   if (!mon->is_leader())
     return;
-  mon->key_server.start_server(true);
+  mon->key_server.start_server();
 /*
   check_rotate();
 */

src/mon/MonClient.cc

@@ -505,7 +505,7 @@ int MonClient::_check_auth_rotating()
     return 0;
   }
 
-  if (!rotating_secrets->need_new_secrets()) {
+  if (!rotating_secrets->need_new_secrets(g_clock.now())) {
     dout(20) << "_check_auth_rotating have uptodate secrets" << dendl;
     rotating_secrets->dump_rotating();
     return 0;