auth/cephx: option to disallow unauthorized global_id (re)use

global_id is a cluster-wide unique id that must remain stable for the
lifetime of the client instance.  The cephx protocol has a facility to
allow clients to preserve their global_id across reconnects:

(1) the client should provide its global_id in the initial handshake
    message/frame and later include its auth ticket proving previous
    possession of that global_id in CEPHX_GET_AUTH_SESSION_KEY request

(2) the monitor should verify that the included auth ticket is valid
    and has the same global_id and, if so, allow the reclaim

(3) if the reclaim is allowed, the new auth ticket should be
    encrypted with the session key of the included auth ticket to
    ensure authenticity of the client performing reclaim.  (The
    included auth ticket could have been snooped when the monitor
    originally shared it with the client or any time the client
    provided it back to the monitor as part of requesting service
    tickets, but only the genuine client would have its session key
    and be able to decrypt.)

Unfortunately, all (1), (2) and (3) have been broken for a while:

- (1) was broken in 2016 by commit a2eb6ae3fb57 ("mon/monclient:
  hunt for multiple monitor in parallel") and is addressed in patch
  "mon/MonClient: preserve auth state on reconnects"

- it turns out that (2) has never been enforced.  When cephx was
  being designed and implemented in 2009, two changes to the protocol
  raced with each other pulling it in different directions: commits
  0669ca21f4f7 ("auth: reuse global_id when requesting tickets")
  and fec31964a12b ("auth: when renewing session, encrypt ticket")
  added the reclaim mechanism based strictly on auth tickets, while
  commit 5eeb711b6b2b ("auth: change server side negotiation a bit")
  allowed the client to provide global_id in the initial handshake.
  These changes didn't get reconciled and as a result a malicious
  client can assign itself any global_id of its choosing by simply
  passing something other than 0 in MAuth message or AUTH_REQUEST
  frame and not even bother supplying any ticket.  This includes
  getting a global_id that is being used by another client.

- (3) was broken in 2019 with addition of support for msgr2, where
  the new auth ticket ends up being shared unencrypted.  However the
  root cause is deeper and a malicious client can coerce msgr1 into
  the same.  This also goes back to 2009 and is addressed in patch
  "auth/cephx: ignore CEPH_ENTITY_TYPE_AUTH in requested keys".

Because (2) has never been enforced, no one noticed when (1) got
broken and we began to rely on this flaw for normal operation in
the face of reconnects due to network hiccups or otherwise.  As of
today, only pre-luminous userspace clients and kernel clients are
not exercising it on a daily basis.

Bump CephXAuthenticate version and use a dummy v3 to distinguish
between legacy clients that don't (may not) include their auth ticket
and new clients.  For new clients, unconditionally disallow claiming
global_id without a corresponding auth ticket.  For legacy clients,
introduce a choice between permissive (current behavior, default for
the foreseeable future) and enforcing mode.

If the reclaim is disallowed, return EACCES.  While MonClient does
have some provision for global_id changes and we could conceivably
implement enforcement by handing out a fresh global_id instead of
the provided one, those code paths have never been tested and there
are too many ways a sudden global_id change could go wrong.

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

Conflicts:
	src/auth/cephx/CephxProtocol.h [ bufferlist vs
	  ceph::buffer::list ]
	src/auth/cephx/CephxServiceHandler.h [ ditto ]
	src/auth/none/AuthNoneServiceHandler.h [ ditto ]

diff --git a/src/auth/AuthServiceHandler.cc b/src/auth/AuthServiceHandler.cc
index 6e70de41dbd322eed289fe114bf4cb31e3784ea8..07e71298da69b3fff3fb7caaf1572d55cb025586 100644 (file)
--- a/src/auth/AuthServiceHandler.cc
+++ b/src/auth/AuthServiceHandler.cc
@@ -29,7 +29,8 @@ int AuthServiceHandler::start_session(const EntityName& entity_name,
                                      ceph::buffer::list *result,
                                      AuthCapsInfo *caps)
 {
-  ceph_assert(!this->entity_name.get_type() && !this->global_id);
+  ceph_assert(!this->entity_name.get_type() && !this->global_id &&
+             global_id_status == global_id_status_t::NONE);
 
   ldout(cct, 10) << __func__ << " entity_name=" << entity_name
                 << " global_id=" << global_id << " is_new_global_id="

diff --git a/src/auth/AuthServiceHandler.h b/src/auth/AuthServiceHandler.h
index 89619d17cd825ed35ae88e95074062747e46d8f5..e10c69491f7e1c40284c12c576d4f70d049e998b 100644 (file)
--- a/src/auth/AuthServiceHandler.h
+++ b/src/auth/AuthServiceHandler.h
@@ -25,11 +25,28 @@ class KeyServer;
 class CryptoKey;
 struct AuthCapsInfo;
 
+enum class global_id_status_t {
+  NONE,
+  // fresh client (global_id == 0); waiting for CephXAuthenticate
+  NEW_PENDING,
+  // connected client; new enough to correctly reclaim global_id
+  NEW_OK,
+  // connected client; unknown whether it can reclaim global_id correctly
+  NEW_NOT_EXPOSED,
+  // reconnecting client (global_id != 0); waiting for CephXAuthenticate
+  RECLAIM_PENDING,
+  // reconnected client; correctly reclaimed global_id
+  RECLAIM_OK,
+  // reconnected client; did not properly prove prior global_id ownership
+  RECLAIM_INSECURE
+};
+
 struct AuthServiceHandler {
 protected:
   CephContext *cct;
   EntityName entity_name;
   uint64_t global_id = 0;
+  global_id_status_t global_id_status = global_id_status_t::NONE;
 
 public:
   explicit AuthServiceHandler(CephContext *cct_) : cct(cct_) {}
@@ -44,13 +61,13 @@ public:
   virtual int handle_request(ceph::buffer::list::const_iterator& indata,
                             size_t connection_secret_required_length,
                             ceph::buffer::list *result,
-                            uint64_t *global_id,
                             AuthCapsInfo *caps,
                             CryptoKey *session_key,
                             std::string *connection_secret) = 0;
 
   const EntityName& get_entity_name() { return entity_name; }
   uint64_t get_global_id() { return global_id; }
+  global_id_status_t get_global_id_status() { return global_id_status; }
 
 private:
   virtual int do_start_session(bool is_new_global_id,

diff --git a/src/auth/cephx/CephxProtocol.h b/src/auth/cephx/CephxProtocol.h
index c8f9f92dd39860f53a58ce149c95c3c7e770424c..f9a8e9c1a7bb96c91c0008ee15e28f37b81cbff7 100644 (file)
--- a/src/auth/cephx/CephxProtocol.h
+++ b/src/auth/cephx/CephxProtocol.h
@@ -123,9 +123,11 @@ struct CephXAuthenticate {
   CephXTicketBlob old_ticket;
   uint32_t other_keys = 0;  // replaces CephXServiceTicketRequest
 
+  bool old_ticket_may_be_omitted;
+
   void encode(bufferlist& bl) const {
     using ceph::encode;
-    __u8 struct_v = 2;
+    __u8 struct_v = 3;
     encode(struct_v, bl);
     encode(client_challenge, bl);
     encode(key, bl);
@@ -142,6 +144,13 @@ struct CephXAuthenticate {
     if (struct_v >= 2) {
       decode(other_keys, bl);
     }
+
+    // v2 and v3 encodings are the same, but:
+    // - some clients that send v1 or v2 don't populate old_ticket
+    //   on reconnects (but do on renewals)
+    // - any client that sends v3 or later is expected to populate
+    //   old_ticket both on reconnects and renewals
+    old_ticket_may_be_omitted = struct_v < 3;
   }
 };
 WRITE_CLASS_ENCODER(CephXAuthenticate)

diff --git a/src/auth/cephx/CephxServiceHandler.cc b/src/auth/cephx/CephxServiceHandler.cc
index b8b6d608c482330d83118370af4db16c9016eb0c..78c91a8f79e23e9acb41a0ac67276e9c372221a6 100644 (file)
--- a/src/auth/cephx/CephxServiceHandler.cc
+++ b/src/auth/cephx/CephxServiceHandler.cc
@@ -32,6 +32,9 @@ int CephxServiceHandler::do_start_session(
   bufferlist *result_bl,
   AuthCapsInfo *caps)
 {
+  global_id_status = is_new_global_id ? global_id_status_t::NEW_PENDING :
+                                       global_id_status_t::RECLAIM_PENDING;
+
   uint64_t min = 1; // always non-zero
   uint64_t max = std::numeric_limits<uint64_t>::max();
   server_challenge = ceph::util::generate_random_number<uint64_t>(min, max);
@@ -44,11 +47,90 @@ int CephxServiceHandler::do_start_session(
   return 0;
 }
 
+int CephxServiceHandler::verify_old_ticket(
+  const CephXAuthenticate& req,
+  CephXServiceTicketInfo& old_ticket_info,
+  bool& should_enc_ticket)
+{
+  ldout(cct, 20) << " checking old_ticket: secret_id="
+                << req.old_ticket.secret_id
+                << " len=" << req.old_ticket.blob.length()
+                << ", old_ticket_may_be_omitted="
+                << req.old_ticket_may_be_omitted << dendl;
+  ceph_assert(global_id_status != global_id_status_t::NONE);
+  if (global_id_status == global_id_status_t::NEW_PENDING) {
+    // old ticket is not needed
+    if (req.old_ticket.blob.length()) {
+      ldout(cct, 0) << " superfluous ticket presented" << dendl;
+      return -EINVAL;
+    }
+    if (req.old_ticket_may_be_omitted) {
+      ldout(cct, 10) << " new global_id " << global_id
+                    << " (unexposed legacy client)" << dendl;
+      global_id_status = global_id_status_t::NEW_NOT_EXPOSED;
+    } else {
+      ldout(cct, 10) << " new global_id " << global_id << dendl;
+      global_id_status = global_id_status_t::NEW_OK;
+    }
+    return 0;
+  }
+
+  if (!req.old_ticket.blob.length()) {
+    // old ticket is needed but not presented
+    if (cct->_conf->auth_allow_insecure_global_id_reclaim &&
+       req.old_ticket_may_be_omitted) {
+      ldout(cct, 10) << " allowing reclaim of global_id " << global_id
+                    << " with no ticket presented (legacy client, auth_allow_insecure_global_id_reclaim=true)"
+                    << dendl;
+      global_id_status = global_id_status_t::RECLAIM_INSECURE;
+      return 0;
+    }
+    ldout(cct, 0) << " attempt to reclaim global_id " << global_id
+                 << " without presenting ticket" << dendl;
+    return -EACCES;
+  }
+
+  if (!cephx_decode_ticket(cct, key_server, CEPH_ENTITY_TYPE_AUTH,
+                          req.old_ticket, old_ticket_info)) {
+    if (cct->_conf->auth_allow_insecure_global_id_reclaim &&
+       req.old_ticket_may_be_omitted) {
+      ldout(cct, 10) << " allowing reclaim of global_id " << global_id
+                    << " using bad ticket (legacy client, auth_allow_insecure_global_id_reclaim=true)"
+                    << dendl;
+      global_id_status = global_id_status_t::RECLAIM_INSECURE;
+      return 0;
+    }
+    ldout(cct, 0) << " attempt to reclaim global_id " << global_id
+                 << " using bad ticket" << dendl;
+    return -EACCES;
+  }
+  ldout(cct, 20) << " decoded old_ticket: global_id="
+                << old_ticket_info.ticket.global_id << dendl;
+  if (global_id != old_ticket_info.ticket.global_id) {
+    if (cct->_conf->auth_allow_insecure_global_id_reclaim &&
+       req.old_ticket_may_be_omitted) {
+      ldout(cct, 10) << " allowing reclaim of global_id " << global_id
+                    << " using mismatching ticket (legacy client, auth_allow_insecure_global_id_reclaim=true)"
+                    << dendl;
+      global_id_status = global_id_status_t::RECLAIM_INSECURE;
+      return 0;
+    }
+    ldout(cct, 0) << " attempt to reclaim global_id " << global_id
+                 << " using mismatching ticket" << dendl;
+    return -EACCES;
+  }
+  ldout(cct, 10) << " allowing reclaim of global_id " << global_id
+                << " (valid ticket presented, will encrypt new ticket)"
+                << dendl;
+  global_id_status = global_id_status_t::RECLAIM_OK;
+  should_enc_ticket = true;
+  return 0;
+}
+
 int CephxServiceHandler::handle_request(
   bufferlist::const_iterator& indata,
   size_t connection_secret_required_len,
   bufferlist *result_bl,
-  uint64_t *global_id,
   AuthCapsInfo *caps,
   CryptoKey *psession_key,
   std::string *pconnection_secret)
@@ -120,22 +202,18 @@ int CephxServiceHandler::handle_request(
        ret = -EACCES;
        break;
       }
-      CephXServiceTicketInfo old_ticket_info;
 
-      if (cephx_decode_ticket(cct, key_server, CEPH_ENTITY_TYPE_AUTH,
-                             req.old_ticket, old_ticket_info)) {
-        *global_id = old_ticket_info.ticket.global_id;
-        ldout(cct, 10) << "decoded old_ticket with global_id=" << *global_id
-                      << dendl;
-        should_enc_ticket = true;
+      CephXServiceTicketInfo old_ticket_info;
+      ret = verify_old_ticket(req, old_ticket_info, should_enc_ticket);
+      if (ret) {
+       ldout(cct, 0) << " could not verify old ticket" << dendl;
+       break;
       }
 
-      ldout(cct,10) << __func__ << " auth ticket global_id " << *global_id
-                   << dendl;
       info.ticket.init_timestamps(ceph_clock_now(),
                                  cct->_conf->auth_mon_ticket_ttl);
       info.ticket.name = entity_name;
-      info.ticket.global_id = *global_id;
+      info.ticket.global_id = global_id;
       info.validity += cct->_conf->auth_mon_ticket_ttl;
 
       key_server->generate_secret(session_key);

diff --git a/src/auth/cephx/CephxServiceHandler.h b/src/auth/cephx/CephxServiceHandler.h
index 024fd1bdc939723a25a58d963b9237919a4e47c4..28d24f1ecad375a8895cf028552e2e3d8df2d234 100644 (file)
--- a/src/auth/cephx/CephxServiceHandler.h
+++ b/src/auth/cephx/CephxServiceHandler.h
@@ -19,6 +19,8 @@
 #include "auth/Auth.h"
 
 class KeyServer;
+struct CephXAuthenticate;
+struct CephXServiceTicketInfo;
 
 class CephxServiceHandler  : public AuthServiceHandler {
   KeyServer *key_server;
@@ -33,7 +35,6 @@ public:
     bufferlist::const_iterator& indata,
     size_t connection_secret_required_length,
     bufferlist *result_bl,
-    uint64_t *global_id,
     AuthCapsInfo *caps,
     CryptoKey *session_key,
     std::string *connection_secret) override;
@@ -43,6 +44,9 @@ private:
                       bufferlist *result_bl,
                       AuthCapsInfo *caps) override;
 
+  int verify_old_ticket(const CephXAuthenticate& req,
+                       CephXServiceTicketInfo& old_ticket_info,
+                       bool& should_enc_ticket);
   void build_cephx_response_header(int request_type, int status,
                                   bufferlist& bl);
 };

diff --git a/src/auth/krb/KrbServiceHandler.cpp b/src/auth/krb/KrbServiceHandler.cpp
index 0b51097499c9ec13db5b95b950850b21556581ba..c2ca3bbf28ccd04daebc4348fc00d5164c25a43f 100644 (file)
--- a/src/auth/krb/KrbServiceHandler.cpp
+++ b/src/auth/krb/KrbServiceHandler.cpp
@@ -30,7 +30,6 @@ int KrbServiceHandler::handle_request(
   bufferlist::const_iterator& indata,
   size_t connection_secret_required_length,
   bufferlist *buff_list,
-  uint64_t *global_id,
   AuthCapsInfo *caps,
   CryptoKey *session_key,
   std::string *connection_secret)

diff --git a/src/auth/krb/KrbServiceHandler.hpp b/src/auth/krb/KrbServiceHandler.hpp
index a7c467e484c6c56b92532e2a9e5e441e2611d87d..ee91baa5532f49e5bdaefefca6957564fa83a947 100644 (file)
--- a/src/auth/krb/KrbServiceHandler.hpp
+++ b/src/auth/krb/KrbServiceHandler.hpp
@@ -40,7 +40,6 @@ class KrbServiceHandler : public AuthServiceHandler {
     int handle_request(bufferlist::const_iterator& indata,
                       size_t connection_secret_required_length,
                       bufferlist *buff_list,
-                       uint64_t *global_id,
                        AuthCapsInfo *caps,
                       CryptoKey *session_key,
                       std::string *connection_secret) override;

diff --git a/src/auth/none/AuthNoneServiceHandler.h b/src/auth/none/AuthNoneServiceHandler.h
index a419eb5f4e80c5f4a238f6f151caedff8900124c..9a8a38d2260be56a19b7fe5129a64e7996ca9ea0 100644 (file)
--- a/src/auth/none/AuthNoneServiceHandler.h
+++ b/src/auth/none/AuthNoneServiceHandler.h
@@ -28,7 +28,6 @@ public:
   int handle_request(bufferlist::const_iterator& indata,
                     size_t connection_secret_required_length,
                     bufferlist *result_bl,
-                    uint64_t *global_id,
                     AuthCapsInfo *caps,
                     CryptoKey *session_key,
                     std::string *connection_secret) override {

diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h
index 6aa45b7e4cb118e4de7efd8f03ed16fca90ca618..b543d256daf33e7695d11a4c5cb518a6b31d29b5 100644 (file)
--- a/src/common/legacy_config_opts.h
+++ b/src/common/legacy_config_opts.h
@@ -330,6 +330,7 @@ OPTION(cephx_service_require_version, OPT_INT)
 OPTION(cephx_sign_messages, OPT_BOOL)  // Default to signing session messages if supported
 OPTION(auth_mon_ticket_ttl, OPT_DOUBLE)
 OPTION(auth_service_ticket_ttl, OPT_DOUBLE)
+OPTION(auth_allow_insecure_global_id_reclaim, OPT_BOOL)
 OPTION(auth_debug, OPT_BOOL)          // if true, assert when weird things happen
 OPTION(mon_client_hunt_parallel, OPT_U32)   // how many mons to try to connect to in parallel during hunt
 OPTION(mon_client_hunt_interval, OPT_DOUBLE)   // try new mon every N seconds until we connect

diff --git a/src/common/options.cc b/src/common/options.cc
index f74aa788a73b0a2b9b36995b116671f183344a18..2e53f3f573cd96ed5f2bb924bcbe8d193ad9f85c 100644 (file)
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -2264,6 +2264,11 @@ std::vector<Option> get_global_options() {
     .set_default(1_hr)
     .set_description(""),
 
+    Option("auth_allow_insecure_global_id_reclaim", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
+    .set_default(true)
+    .set_description("Allow reclaiming global_id without presenting a valid ticket proving previous possession of that global_id")
+    .set_long_description("Allowing unauthorized global_id (re)use poses a security risk.  Unfortunately, older clients may omit their ticket on reconnects and therefore rely on this being allowed for preserving their global_id for the lifetime of the client instance."),
+
     Option("auth_debug", Option::TYPE_BOOL, Option::LEVEL_DEV)
     .set_default(false)
     .set_description(""),

diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc
index 96b40a5c41e4d913f0d85d2ece372e87f994276b..f189918bec1cc048503d33b3e855e9ca15909c75 100644 (file)
--- a/src/mon/AuthMonitor.cc
+++ b/src/mon/AuthMonitor.cc
@@ -725,7 +725,6 @@ bool AuthMonitor::prep_auth(MonOpRequestRef op, bool paxos_writable)
        indata,
        0, // no connection_secret needed
        &response_bl,
-       &s->con->peer_global_id,
        &s->con->peer_caps_info,
        nullptr, nullptr);
     }

diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index 345515a59420ae657c38b7a0da40e553d54e454b..4f09e21dae746afbcf6a51a6a213ec43059f1215 100644 (file)
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -6285,7 +6285,6 @@ int Monitor::handle_auth_request(
       p,
       auth_meta->get_connection_secret_length(),
       reply,
-      &con->peer_global_id,
       &con->peer_caps_info,
       &auth_meta->session_key,
       &auth_meta->connection_secret);