mon/MonClient: preserve auth state on reconnects

Commit a2eb6ae3fb57 ("mon/monclient: hunt for multiple monitor in
parallel") introduced a regression where auth state (global_id and
AuthClientHandler) was no longer preserved on reconnects.  The ensuing
breakage was quickly noticed and prompted a follow-on fix 8bb6193c8f53
("mon/MonClient: persist global_id across re-connecting").

However, as evident from the subject, the follow-on fix only took
care of the global_id part.  AuthClientHandler is still destroyed
and all cephx tickets are discarded.  A new from-scratch instance
is created for each MonConnection and CEPHX_GET_AUTH_SESSION_KEY
requests end up with CephXAuthenticate::old_ticket not populated.
The bug is in MonClient, so both msgr1 and msgr2 are affected.

This should have resulted in a similar sort of breakage but didn't
because of a much larger bug.  The monitor should have denied the
attempt to reclaim global_id with no valid ticket proving previous
possession of that global_id presented.  Alas, it appears that this
aspect of the cephx protocol has never been enforced.  This is dealt
with in the next patch.

To fix the issue at hand, clone AuthClientHandler into each
MonConnection so that each respective CEPHX_GET_AUTH_SESSION_KEY
request gets a copy of the current auth ticket.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit 236b536b28482ec9d8b872de03da7d702ce4787b)

Conflicts:
	src/mon/MonClient.cc [ commit 1e9b18008c5e ("mon: set
	  MonClient::_add_conn return type to void") not in nautilus ]

diff --git a/src/auth/AuthClientHandler.h b/src/auth/AuthClientHandler.h
index e0f3a7de952a6822f96ec2b9d3159f90d07ccf71..8ae882bc1f667d4fdac2581572acbb8a37bb64fd 100644 (file)
--- a/src/auth/AuthClientHandler.h
+++ b/src/auth/AuthClientHandler.h
@@ -37,6 +37,8 @@ public:
   {}
   virtual ~AuthClientHandler() {}
 
+  virtual AuthClientHandler* clone() const = 0;
+
   void init(const EntityName& n) { name = n; }
   
   void set_want_keys(__u32 keys) {

diff --git a/src/auth/cephx/CephxClientHandler.h b/src/auth/cephx/CephxClientHandler.h
index cd1196a689ce2cae6abea8fc3acbdb802d2f558c..b99691af7f0c7d04bc61e0663303129e2749eb59 100644 (file)
--- a/src/auth/cephx/CephxClientHandler.h
+++ b/src/auth/cephx/CephxClientHandler.h
@@ -48,6 +48,10 @@ public:
     reset();
   }
 
+  CephxClientHandler* clone() const override {
+    return new CephxClientHandler(*this);
+  }
+
   void reset() override;
   void prepare_build_request() override;
   int build_request(bufferlist& bl) const override;

diff --git a/src/auth/krb/KrbClientHandler.hpp b/src/auth/krb/KrbClientHandler.hpp
index a8960a8898cbd7f372c658267ddb84410437e479..f1546dbb338c8ea7f7ccaa152be3f7d9c5c9aac5 100644 (file)
--- a/src/auth/krb/KrbClientHandler.hpp
+++ b/src/auth/krb/KrbClientHandler.hpp
@@ -39,7 +39,11 @@ class KrbClientHandler : public AuthClientHandler {
       reset();
     }
     ~KrbClientHandler() override;
-    
+
+    KrbClientHandler* clone() const override {
+      return new KrbClientHandler(*this);
+    }
+
     int get_protocol() const override { return CEPH_AUTH_GSS; }
     void reset() override {
       m_gss_client_name = GSS_C_NO_NAME; 

diff --git a/src/auth/none/AuthNoneClientHandler.h b/src/auth/none/AuthNoneClientHandler.h
index eb3ef8f552171460d7b54358e8ffa8d332ea765f..4cc4c8a0c933db68c21be3b4a9a395d03046b41a 100644 (file)
--- a/src/auth/none/AuthNoneClientHandler.h
+++ b/src/auth/none/AuthNoneClientHandler.h
@@ -26,6 +26,10 @@ public:
   AuthNoneClientHandler(CephContext *cct_)
     : AuthClientHandler(cct_) {}
 
+  AuthNoneClientHandler* clone() const override {
+    return new AuthNoneClientHandler(*this);
+  }
+
   void reset() override { }
 
   void prepare_build_request() override {}

diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc
index 96bd816f7dceb0d7c0d22269cace24a5fa867e52..d57075f5797a4041916f345b73b506197f556051 100644 (file)
--- a/src/mon/MonClient.cc
+++ b/src/mon/MonClient.cc
@@ -681,6 +681,9 @@ MonConnection& MonClient::_add_conn(unsigned rank, uint64_t global_id)
   auto peer = monmap.get_addrs(rank);
   auto conn = messenger->connect_to_mon(peer);
   MonConnection mc(cct, conn, global_id, &auth_registry);
+  if (auth) {
+    mc.get_auth().reset(auth->clone());
+  }
   auto inserted = pending_cons.insert(make_pair(peer, move(mc)));
   ldout(cct, 10) << "picked mon." << monmap.get_name(rank)
                  << " con " << conn
@@ -1552,9 +1555,6 @@ int MonConnection::get_auth_request(
     return -EACCES;
   }
 
-  if (auth) {
-    auth.reset();
-  }
   int r = _init_auth(*method, entity_name, want_keys, keyring, true);
   ceph_assert(r == 0);
 
@@ -1671,12 +1671,6 @@ int MonConnection::_negotiate(MAuthReply *m,
                              uint32_t want_keys,
                              RotatingKeyRing* keyring)
 {
-  if (auth && (int)m->protocol == auth->get_protocol()) {
-    // good, negotiation completed
-    auth->reset();
-    return 0;
-  }
-
   int r = _init_auth(m->protocol, entity_name, want_keys, keyring, false);
   if (r == -ENOTSUP) {
     if (m->result == -ENOTSUP) {
@@ -1695,9 +1689,15 @@ int MonConnection::_init_auth(
   RotatingKeyRing* keyring,
   bool msgr2)
 {
-  ldout(cct,10) << __func__ << " method " << method << dendl;
-  auth.reset(
-    AuthClientHandler::create(cct, method, keyring));
+  ldout(cct, 10) << __func__ << " method " << method << dendl;
+  if (auth && auth->get_protocol() == (int)method) {
+    ldout(cct, 10) << __func__ << " already have auth, reseting" << dendl;
+    auth->reset();
+    return 0;
+  }
+
+  ldout(cct, 10) << __func__ << " creating new auth" << dendl;
+  auth.reset(AuthClientHandler::create(cct, method, keyring));
   if (!auth) {
     ldout(cct, 10) << " no handler for protocol " << method << dendl;
     return -ENOTSUP;