mon: AuthMonitor: change 'auth add' behaviour

If an entity already existed, 'auth add' would smash its key and caps
with whatever was on the supplied keyring file; if a keyring weren't
specified, we would simply generate a new key and destroy all existing
caps (unless caps were specified and happened to be different from the
already in-place caps).  This behaviour is obviously sketchy.

With this patch we now enforce the following behaviour:

- if the command reproduces the current state (same key, same caps), we
  return 0; else,
- if entity exists and supplied key OR caps are different, return -EINVAL
- if entity does not exist in current state, check if we are about to
  create it (by checking the pending state); if so, wait for the new state
  to be committed and re-handle the command then
- else create a new entity.

Signed-off-by: Joao Eduardo Luis <joao.luis@inktank.com>

1 files changed, 101 insertions, 20 deletions

diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc
index 63bcbb1ef03..22301099c41 100644
--- a/src/mon/AuthMonitor.cc
+++ b/src/mon/AuthMonitor.cc
@@ -738,37 +738,118 @@ bool AuthMonitor::prepare_command(MMonCommand *m)
     wait_for_finished_proposal(new Monitor::C_Command(mon, m, 0, rs, get_last_committed()));
     //paxos->wait_for_commit(new Monitor::C_Command(mon, m, 0, rs, get_last_committed()));
     return true;
-  } else if (prefix == "auth add") {
+  } else if (prefix == "auth add" && !entity_name.empty()) {
+    /* expected behavior:
+     *  - if command reproduces current state, return 0.
+     *  - if command adds brand new entity, handle it.
+     *  - if command adds new state to existing entity, return error.
+     */
     KeyServerData::Incremental auth_inc;
     auth_inc.name = entity;
     bufferlist bl = m->get_data();
-    dout(10) << "AuthMonitor::prepare_command bl.length()=" << bl.length() << dendl;
-    if (bl.length()) {
+    bool has_keyring = (bl.length() > 0);
+    map<string,bufferlist> new_caps;
+
+    KeyRing new_keyring;
+    if (has_keyring) {
       bufferlist::iterator iter = bl.begin();
-      KeyRing keyring;
       try {
-	::decode(keyring, iter);
+        ::decode(new_keyring, iter);
       } catch (const buffer::error &ex) {
-	ss << "error decoding keyring";
-	err = -EINVAL;
-	goto done;
+        ss << "error decoding keyring";
+        err = -EINVAL;
+        goto done;
+      }
+    }
+
+    // build new caps from provided arguments (if available)
+    for (vector<string>::iterator it = caps_vec.begin();
+        it != caps_vec.end(); it += 2) {
+      string sys = *it;
+      bufferlist cap;
+      ::encode(*(it+1), cap);
+      new_caps[sys] = cap;
+    }
+
+    if (mon->key_server.get_auth(auth_inc.name, auth_inc.auth)) {
+      // we already have this entity; let us check if the key match
+      if (has_keyring) {
+        EntityAuth new_inc;
+        if (!new_keyring.get_auth(auth_inc.name, new_inc)) {
+          ss << "key for " << auth_inc.name
+             << " not found in provided keyring";
+          err = -EINVAL;
+          goto done;
+        }
+
+        if (auth_inc.auth.key.get_secret().cmp(new_inc.key.get_secret())
+            || (new_inc.caps.size() > 0
+              && new_inc.caps.size() != auth_inc.auth.caps.size()))
+        {
+          // key exists but does not match
+          ss << "entity " << auth_inc.name
+             << " exists but does not match the provided keyring";
+          err = -EINVAL;
+          goto done;
+         } else if (new_inc.caps.size() > 0) {
+           // add keyring's caps to the new_caps map
+           for (map<string,bufferlist>::iterator it = new_inc.caps.begin();
+                it != new_inc.caps.end(); ++it) {
+             new_caps[it->first] = it->second;
+           }
+         }
       }
-      if (!keyring.get_auth(auth_inc.name, auth_inc.auth)) {
-	ss << "key for " << auth_inc.name << " not found in provided keyring";
-	err = -EINVAL;
-	goto done;
+
+      // check if caps match.  If so, return 0; otherwise return -EINVAL
+      for (map<string,bufferlist>::iterator it = new_caps.begin();
+          it != new_caps.end(); ++it) {
+        string sys = it->first;
+        bufferlist &cap = it->second;
+
+        if (auth_inc.auth.caps.count(sys) == 0
+            || !auth_inc.auth.caps[sys].contents_equal(cap)) {
+          ss << "entity " << auth_inc.name << " exists but cap "
+            << sys << " does not match";
+          err = -EINVAL;
+          goto done;
+        }
       }
-    } else {
-      // generate a new random key
-      dout(10) << "AuthMonitor::prepare_command generating random key for " << auth_inc.name << dendl;
-      auth_inc.auth.key.create(g_ceph_context, CEPH_CRYPTO_AES);
+      // caps match; return.
+      err = 0;
+      goto done;
     }
 
-    auth_inc.op = KeyServerData::AUTH_INC_ADD;
+    // are we about to have it?
+    for (vector<Incremental>::iterator p = pending_auth.begin();
+        p != pending_auth.end();
+        ++p) {
+      if (p->inc_type == AUTH_DATA) {
+        KeyServerData::Incremental inc;
+        bufferlist::iterator q = p->auth_data.begin();
+        ::decode(inc, q);
+        if (inc.op == KeyServerData::AUTH_INC_ADD &&
+            inc.name == entity) {
+          wait_for_finished_proposal(
+              new Monitor::C_Command(mon, m, 0, rs, get_last_committed()));
+          return true;
+        }
+      }
+    }
 
-    for (vector<string>::iterator it = caps_vec.begin();
-	 it != caps_vec.end(); it += 2)
-      ::encode(*(it+1), auth_inc.auth.caps[*it]);
+    // okay, add it.
+    auth_inc.op = KeyServerData::AUTH_INC_ADD;
+    for (map<string,bufferlist>::iterator it = new_caps.begin();
+         it != new_caps.end(); ++it) {
+      auth_inc.auth.caps[it->first] = it->second;
+    }
+    // if a key was not provided in the keyring, then generate a new one
+    if (!has_keyring) {
+      dout(10) << "AuthMonitor::prepare_command generating random key for "
+               << auth_inc.name << dendl;
+      auth_inc.auth.key.create(g_ceph_context, CEPH_CRYPTO_AES);
+    } else {
+      assert(new_keyring.get_auth(auth_inc.name, auth_inc.auth));
+    }
 
     dout(10) << " importing " << auth_inc.name << dendl;
     dout(30) << "    " << auth_inc.auth << dendl;