crimson/net: audit peer_name(type, id) during handshake

Allow connect to specific peer with entity_name_t, with required
internal validation during handshake in v2.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/net/Connection.h b/src/crimson/net/Connection.h
index 04c57cb120fc4e920a811b0f2c5652911879476e..25b3f5af562d43cd81ea1c865aa241ecd43c167e 100644 (file)
--- a/src/crimson/net/Connection.h
+++ b/src/crimson/net/Connection.h
@@ -29,7 +29,7 @@ class Interceptor;
 using seq_num_t = uint64_t;
 
 class Connection : public seastar::enable_shared_from_this<Connection> {
-  entity_name_t peer_name = {0, -1};
+  entity_name_t peer_name = {0, entity_name_t::NEW};
 
  protected:
   entity_addr_t peer_addr;
@@ -42,9 +42,34 @@ class Connection : public seastar::enable_shared_from_this<Connection> {
   clock_t::time_point last_keepalive;
   clock_t::time_point last_keepalive_ack;
 
-  void set_peer_type(entity_type_t peer_type) { peer_name._type = peer_type; }
-  void set_peer_id(int64_t peer_id) { peer_name._num = peer_id; }
-  void set_peer_name(entity_name_t name) { peer_name = name; }
+  void set_peer_type(entity_type_t peer_type) {
+    // it is not allowed to assign an unknown value when the current
+    // value is known
+    assert(!(peer_type == 0 &&
+             peer_name.type() != 0));
+    // it is not allowed to assign a different known value when the
+    // current value is also known.
+    assert(!(peer_type != 0 &&
+             peer_name.type() != 0 &&
+             peer_type != peer_name.type()));
+    peer_name._type = peer_type;
+  }
+  void set_peer_id(int64_t peer_id) {
+    // it is not allowed to assign an unknown value when the current
+    // value is known
+    assert(!(peer_id == entity_name_t::NEW &&
+             peer_name.num() != entity_name_t::NEW));
+    // it is not allowed to assign a different known value when the
+    // current value is also known.
+    assert(!(peer_id != entity_name_t::NEW &&
+             peer_name.num() != entity_name_t::NEW &&
+             peer_id != peer_name.num()));
+    peer_name._num = peer_id;
+  }
+  void set_peer_name(entity_name_t name) {
+    set_peer_type(name.type());
+    set_peer_id(name.num());
+  }
 
  public:
   uint64_t peer_global_id = 0;

diff --git a/src/crimson/net/Messenger.h b/src/crimson/net/Messenger.h
index 0ad93ea35c89fca8dde43654b0440510b577dd2b..6423f89d967c3128b269f7d88b04426284114cbd 100644 (file)
--- a/src/crimson/net/Messenger.h
+++ b/src/crimson/net/Messenger.h
@@ -78,7 +78,13 @@ public:
   /// or a new pending connection
   virtual ConnectionRef
   connect(const entity_addr_t& peer_addr,
-          const entity_type_t& peer_type) = 0;
+          const entity_name_t& peer_name) = 0;
+
+  ConnectionRef
+  connect(const entity_addr_t& peer_addr,
+          const entity_type_t& peer_type) {
+    return connect(peer_addr, entity_name_t(peer_type, -1));
+  }
 
   // wait for messenger shutdown
   virtual seastar::future<> wait() = 0;

diff --git a/src/crimson/net/Protocol.h b/src/crimson/net/Protocol.h
index b765b23539d63fd7ed03345e9bba41a31a4392a5..a6c922c33f1314eba7e3490bf4388346f4c78859 100644 (file)
--- a/src/crimson/net/Protocol.h
+++ b/src/crimson/net/Protocol.h
@@ -41,7 +41,7 @@ class Protocol {
   }
 
   virtual void start_connect(const entity_addr_t& peer_addr,
-                             const entity_type_t& peer_type) = 0;
+                             const entity_name_t& peer_name) = 0;
 
   virtual void start_accept(SocketRef&& socket,
                             const entity_addr_t& peer_addr) = 0;

diff --git a/src/crimson/net/ProtocolV1.cc b/src/crimson/net/ProtocolV1.cc
index b22ce30653a3f18926d4950d0de7ec6c5690caf7..a5a813a81a6f17544de897f2f628f298cf63ccbb 100644 (file)
--- a/src/crimson/net/ProtocolV1.cc
+++ b/src/crimson/net/ProtocolV1.cc
@@ -302,7 +302,7 @@ ProtocolV1::repeat_connect()
 }
 
 void ProtocolV1::start_connect(const entity_addr_t& _peer_addr,
-                               const entity_type_t& _peer_type)
+                               const entity_name_t& _peer_name)
 {
   ceph_assert(state == state_t::none);
   logger().trace("{} trigger connecting, was {}", conn, static_cast<int>(state));
@@ -312,8 +312,8 @@ void ProtocolV1::start_connect(const entity_addr_t& _peer_addr,
   ceph_assert(!socket);
   conn.peer_addr = _peer_addr;
   conn.target_addr = _peer_addr;
-  conn.set_peer_type(_peer_type);
-  conn.policy = messenger.get_policy(_peer_type);
+  conn.set_peer_name(_peer_name);
+  conn.policy = messenger.get_policy(_peer_name.type());
   messenger.register_conn(
     seastar::static_pointer_cast<SocketConnection>(conn.shared_from_this()));
   gated_dispatch("start_connect", [this] {
@@ -534,6 +534,13 @@ seastar::future<stop_t> ProtocolV1::repeat_handle_connect()
     .then([this](bufferlist bl) {
       auto p = bl.cbegin();
       ::decode(h.connect, p);
+      if (conn.get_peer_type() != 0 &&
+          conn.get_peer_type() != h.connect.host_type) {
+        logger().error("{} repeat_handle_connect(): my peer type does not match"
+                       " what peer advertises {} != {}",
+                       conn, conn.get_peer_type(), h.connect.host_type);
+        throw std::system_error(make_error_code(error::protocol_aborted));
+      }
       conn.set_peer_type(h.connect.host_type);
       conn.policy = messenger.get_policy(h.connect.host_type);
       if (!conn.policy.lossy && !conn.policy.server && conn.target_addr.get_port() <= 0) {

diff --git a/src/crimson/net/ProtocolV1.h b/src/crimson/net/ProtocolV1.h
index 2ebfcc9ca929046d806077da9af88ea8d6704740..31a6ddc2eae213168c55b2aae7653c545fdacad8 100644 (file)
--- a/src/crimson/net/ProtocolV1.h
+++ b/src/crimson/net/ProtocolV1.h
@@ -19,7 +19,7 @@ class ProtocolV1 final : public Protocol {
 
  private:
   void start_connect(const entity_addr_t& peer_addr,
-                     const entity_type_t& peer_type) override;
+                     const entity_name_t& peer_name) override;
 
   void start_accept(SocketRef&& socket,
                     const entity_addr_t& peer_addr) override;

diff --git a/src/crimson/net/ProtocolV2.cc b/src/crimson/net/ProtocolV2.cc
index b6d35514d5940f1a5551802229fad6b93605ac4b..7f5ff573b8242833ea3caad8075d78411323269b 100644 (file)
--- a/src/crimson/net/ProtocolV2.cc
+++ b/src/crimson/net/ProtocolV2.cc
@@ -154,18 +154,18 @@ ProtocolV2::ProtocolV2(Dispatcher& dispatcher,
 ProtocolV2::~ProtocolV2() {}
 
 void ProtocolV2::start_connect(const entity_addr_t& _peer_addr,
-                               const entity_type_t& _peer_type)
+                               const entity_name_t& _peer_name)
 {
   ceph_assert(state == state_t::NONE);
   ceph_assert(!socket);
   conn.peer_addr = _peer_addr;
   conn.target_addr = _peer_addr;
-  conn.set_peer_type(_peer_type);
-  conn.policy = messenger.get_policy(_peer_type);
+  conn.set_peer_name(_peer_name);
+  conn.policy = messenger.get_policy(_peer_name.type());
   client_cookie = generate_client_cookie();
-  logger().info("{} ProtocolV2::start_connect(): peer_addr={}, peer_type={}, cc={}"
+  logger().info("{} ProtocolV2::start_connect(): peer_addr={}, peer_name={}, cc={}"
                 " policy(lossy={}, server={}, standby={}, resetcheck={})",
-                conn, _peer_addr, ceph_entity_type_name(_peer_type), client_cookie,
+                conn, _peer_addr, _peer_name, client_cookie,
                 conn.policy.lossy, conn.policy.server,
                 conn.policy.standby, conn.policy.resetcheck);
   messenger.register_conn(
@@ -744,6 +744,13 @@ ProtocolV2::client_connect()
             throw std::system_error(
                 make_error_code(crimson::net::error::bad_peer_address));
           }
+          if (conn.get_peer_id() != entity_name_t::NEW &&
+              conn.get_peer_id() != server_ident.gid()) {
+            logger().error("{} connection peer id ({}) does not match "
+                           "what it should be ({}) during connecting, close",
+                            conn, server_ident.gid(), conn.get_peer_id());
+            abort_in_close(*this, true);
+          }
           conn.set_peer_id(server_ident.gid());
           conn.set_features(server_ident.supported_features() &
                             conn.policy.features_supported);
@@ -1075,6 +1082,20 @@ seastar::future<> ProtocolV2::server_auth()
   });
 }
 
+bool ProtocolV2::validate_peer_name(const entity_name_t& peer_name) const
+{
+  auto my_peer_name = conn.get_peer_name();
+  if (my_peer_name.type() != peer_name.type()) {
+    return false;
+  }
+  if (my_peer_name.num() != entity_name_t::NEW &&
+      peer_name.num() != entity_name_t::NEW &&
+      my_peer_name.num() != peer_name.num()) {
+    return false;
+  }
+  return true;
+}
+
 seastar::future<ProtocolV2::next_step_t>
 ProtocolV2::send_wait()
 {
@@ -1131,6 +1152,12 @@ ProtocolV2::handle_existing_connection(SocketConnectionRef existing_conn)
                  existing_proto->client_cookie,
                  existing_proto->server_cookie);
 
+  if (!validate_peer_name(existing_conn->get_peer_name())) {
+    logger().error("{} server_connect: my peer_name doesn't match"
+                   " the existing connection {}, abort", conn, existing_conn);
+    abort_in_fault();
+  }
+
   if (existing_proto->state == state_t::REPLACING) {
     logger().warn("{} server_connect: racing replace happened while"
                   " replacing existing connection {}, send wait.",
@@ -1252,6 +1279,13 @@ ProtocolV2::server_connect()
           make_error_code(crimson::net::error::bad_peer_address));
     }
 
+    if (conn.get_peer_id() != entity_name_t::NEW &&
+        conn.get_peer_id() != client_ident.gid()) {
+      logger().error("{} client_ident peer_id ({}) does not match"
+                     " what it should be ({}) during accepting, abort",
+                      conn, client_ident.gid(), conn.get_peer_id());
+      abort_in_fault();
+    }
     conn.set_peer_id(client_ident.gid());
     client_cookie = client_ident.cookie();
 
@@ -1409,6 +1443,12 @@ ProtocolV2::server_reconnect()
                    existing_proto->client_cookie,
                    existing_proto->server_cookie);
 
+    if (!validate_peer_name(existing_conn->get_peer_name())) {
+      logger().error("{} server_reconnect: my peer_name doesn't match"
+                     " the existing connection {}, abort", conn, existing_conn);
+      abort_in_fault();
+    }
+
     if (existing_proto->state == state_t::REPLACING) {
       logger().warn("{} server_reconnect: racing replace happened while "
                     " replacing existing connection {}, retry global.",
@@ -1763,7 +1803,10 @@ void ProtocolV2::trigger_replacing(bool reconnect,
         return write_frame(reconnect_ok);
       } else {
         client_cookie = new_client_cookie;
-        conn.set_peer_name(new_peer_name);
+        assert(conn.get_peer_type() == new_peer_name.type());
+        if (conn.get_peer_id() == entity_name_t::NEW) {
+          conn.set_peer_id(new_peer_name.num());
+        }
         connection_features = new_conn_features;
         return send_server_ident();
       }

diff --git a/src/crimson/net/ProtocolV2.h b/src/crimson/net/ProtocolV2.h
index f98bf3d4366f263b3f4dff6ad37413a1ecca4bb4..5d3a6742181205500dc98131647190cafdee6b30 100644 (file)
--- a/src/crimson/net/ProtocolV2.h
+++ b/src/crimson/net/ProtocolV2.h
@@ -20,7 +20,7 @@ class ProtocolV2 final : public Protocol {
 
  private:
   void start_connect(const entity_addr_t& peer_addr,
-                     const entity_type_t& peer_type) override;
+                     const entity_name_t& peer_name) override;
 
   void start_accept(SocketRef&& socket,
                     const entity_addr_t& peer_addr) override;
@@ -159,6 +159,7 @@ class ProtocolV2 final : public Protocol {
   seastar::future<> _handle_auth_request(bufferlist& auth_payload, bool more);
   seastar::future<> server_auth();
 
+  bool validate_peer_name(const entity_name_t& peer_name) const;
   seastar::future<next_step_t> send_wait();
   seastar::future<next_step_t> reuse_connection(ProtocolV2* existing_proto,
                                                 bool do_reset=false,

diff --git a/src/crimson/net/SocketConnection.cc b/src/crimson/net/SocketConnection.cc
index 6d84acf1469246ab6ef3893a863b7ade3f386932..4e80b8104d174520e154f961658c31eec8f968be 100644 (file)
--- a/src/crimson/net/SocketConnection.cc
+++ b/src/crimson/net/SocketConnection.cc
@@ -114,9 +114,9 @@ bool SocketConnection::update_rx_seq(seq_num_t seq)
 
 void
 SocketConnection::start_connect(const entity_addr_t& _peer_addr,
-                                const entity_type_t& _peer_type)
+                                const entity_name_t& _peer_name)
 {
-  protocol->start_connect(_peer_addr, _peer_type);
+  protocol->start_connect(_peer_addr, _peer_name);
 }
 
 void

diff --git a/src/crimson/net/SocketConnection.h b/src/crimson/net/SocketConnection.h
index 358e8e00807f0245d2777b0f5e077810daeaae42..90b84181995bd1a63a4dab60b4fb87176a8955a0 100644 (file)
--- a/src/crimson/net/SocketConnection.h
+++ b/src/crimson/net/SocketConnection.h
@@ -83,7 +83,7 @@ class SocketConnection : public Connection {
   /// start a handshake from the client's perspective,
   /// only call when SocketConnection first construct
   void start_connect(const entity_addr_t& peer_addr,
-                     const entity_type_t& peer_type);
+                     const entity_name_t& peer_name);
   /// start a handshake from the server's perspective,
   /// only call when SocketConnection first construct
   void start_accept(SocketRef&& socket,

diff --git a/src/crimson/net/SocketMessenger.cc b/src/crimson/net/SocketMessenger.cc
index c11e2b32753e2be116356ecefcb11f89c4e95a2b..d6cab60973491b2575b97e6371943f0d001cdd74 100644 (file)
--- a/src/crimson/net/SocketMessenger.cc
+++ b/src/crimson/net/SocketMessenger.cc
@@ -132,7 +132,7 @@ seastar::future<> SocketMessenger::start(Dispatcher *disp) {
 }
 
 crimson::net::ConnectionRef
-SocketMessenger::connect(const entity_addr_t& peer_addr, const entity_type_t& peer_type)
+SocketMessenger::connect(const entity_addr_t& peer_addr, const entity_name_t& peer_name)
 {
   assert(seastar::engine().cpu_id() == master_sid);
 
@@ -146,7 +146,7 @@ SocketMessenger::connect(const entity_addr_t& peer_addr, const entity_type_t& pe
   }
   SocketConnectionRef conn = seastar::make_shared<SocketConnection>(
       *this, *dispatcher, peer_addr.is_msgr2());
-  conn->start_connect(peer_addr, peer_type);
+  conn->start_connect(peer_addr, peer_name);
   return conn->shared_from_this();
 }
 

diff --git a/src/crimson/net/SocketMessenger.h b/src/crimson/net/SocketMessenger.h
index d1b86e16a2abada7b1640e19b979922313f9bca2..9419e0a15c3d8c9b68f9f3523520809b0064418c 100644 (file)
--- a/src/crimson/net/SocketMessenger.h
+++ b/src/crimson/net/SocketMessenger.h
@@ -66,7 +66,7 @@ class SocketMessenger final : public Messenger {
   seastar::future<> start(Dispatcher *dispatcher) override;
 
   ConnectionRef connect(const entity_addr_t& peer_addr,
-                        const entity_type_t& peer_type) override;
+                        const entity_name_t& peer_name) override;
   // can only wait once
   seastar::future<> wait() override {
     assert(seastar::engine().cpu_id() == master_sid);