msg/async/ProtocolV2: refuse incoming connection intended for someone else

If we get a client_ident frame, and they are trying to talk to someone
else, drop the connection.

This is an inelegant workaround to http://tracker.ceph.com/issues/38247.
A nicer fix would be to restructure the protocol so that the client knows
who they connected to before they try to open a session.  That is a bigger
change that can follow...

Fixes: http://tracker.ceph.com/issues/38247
Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/doc/dev/msgr2.rst b/doc/dev/msgr2.rst
index 5e47b0f48aacd1cf66a522c0fea8f306799ee6ea..181f347f33f625010d70c86b7f172aa18fda6471 100644 (file)
--- a/doc/dev/msgr2.rst
+++ b/doc/dev/msgr2.rst
@@ -250,6 +250,7 @@ an established session.
 
     __le32 num_addrs
     entity_addrvec_t*num_addrs entity addrs
+    entity_addr_t target entity addr
     __le64 gid (numeric part of osd.0, client.123456, ...)
     __le64 global_seq
     __le64 features supported (CEPH_FEATURE_* bitmask)
@@ -258,6 +259,9 @@ an established session.
 
   - client will send first, server will reply with same.  if this is a
     new session, the client and server can proceed to the message exchange.
+  - the target addr is who the client is trying to connect *to*, so
+    that the server side can close the connection if the client is
+    talking to the wrong daemon.
   - type.gid (entity_name_t) is set here, by combinging the type shared in the hello
     frame with the gid here.  this means we don't need it
     in the header of every message.  it also means that we can't send

diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc
index 1b8522c2270a7c663437f72dfb85204224fdcb20..4fca73d544a38b16d5024a86e08e9dea2f9e9463 100644 (file)
--- a/src/msg/async/ProtocolV2.cc
+++ b/src/msg/async/ProtocolV2.cc
@@ -309,17 +309,20 @@ struct SignedEncryptedFrame : public PayloadFrame<T, Args..., signature_t> {
 };
 
 struct ClientIdentFrame
-    : public SignedEncryptedFrame<ClientIdentFrame, entity_addrvec_t, int64_t,
+    : public SignedEncryptedFrame<ClientIdentFrame, entity_addrvec_t,
+                                 entity_addr_t,
+                                 int64_t,
                                   uint64_t, uint64_t, uint64_t, uint64_t> {
   const ProtocolV2::Tag tag = ProtocolV2::Tag::CLIENT_IDENT;
   using SignedEncryptedFrame::SignedEncryptedFrame;
 
   inline entity_addrvec_t &addrs() { return get_val<0>(); }
-  inline int64_t &gid() { return get_val<1>(); }
-  inline uint64_t &global_seq() { return get_val<2>(); }
-  inline uint64_t &supported_features() { return get_val<3>(); }
-  inline uint64_t &required_features() { return get_val<4>(); }
-  inline uint64_t &flags() { return get_val<5>(); }
+  inline entity_addr_t &target_addr() { return get_val<1>(); }
+  inline int64_t &gid() { return get_val<2>(); }
+  inline uint64_t &global_seq() { return get_val<3>(); }
+  inline uint64_t &supported_features() { return get_val<4>(); }
+  inline uint64_t &required_features() { return get_val<5>(); }
+  inline uint64_t &flags() { return get_val<6>(); }
 };
 
 struct ServerIdentFrame
@@ -2229,6 +2232,7 @@ CtPtr ProtocolV2::send_client_ident() {
   }
 
   ClientIdentFrame client_ident(this, messenger->get_myaddrs(),
+                               connection->target_addr,
                                 messenger->get_myname().num(), global_seq,
                                 connection->policy.features_supported,
                                 connection->policy.features_required | msgr2_required,
@@ -2505,6 +2509,7 @@ CtPtr ProtocolV2::handle_client_ident(char *payload, uint32_t length) {
 
   ldout(cct, 5) << __func__ << " received client identification: "
                 << "addrs=" << client_ident.addrs()
+               << " target=" << client_ident.target_addr()
                 << " gid=" << client_ident.gid()
                 << " global_seq=" << client_ident.global_seq()
                 << " features_supported=" << std::hex
@@ -2515,6 +2520,13 @@ CtPtr ProtocolV2::handle_client_ident(char *payload, uint32_t length) {
       client_ident.addrs().front() == entity_addr_t()) {
     return _fault();  // a v2 peer should never do this
   }
+  if (!messenger->get_myaddrs().contains(client_ident.target_addr())) {
+    ldout(cct,5) << __func__ << " peer is trying to reach "
+                << client_ident.target_addr()
+                << " which is not us (" << messenger->get_myaddrs() << ")"
+                << dendl;
+    return _fault();
+  }
 
   connection->set_peer_addrs(client_ident.addrs());
   connection->target_addr = connection->_infer_target_addr(client_ident.addrs());