From e399fc75141cd282182a1da381798c118ffcbb41 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Mon, 11 Feb 2019 09:29:30 -0600 Subject: [PATCH] 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 --- doc/dev/msgr2.rst | 4 ++++ src/msg/async/ProtocolV2.cc | 24 ++++++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/doc/dev/msgr2.rst b/doc/dev/msgr2.rst index 5e47b0f48aacd..181f347f33f62 100644 --- 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 1b8522c2270a7..4fca73d544a38 100644 --- a/src/msg/async/ProtocolV2.cc +++ b/src/msg/async/ProtocolV2.cc @@ -309,17 +309,20 @@ struct SignedEncryptedFrame : public PayloadFrame { }; struct ClientIdentFrame - : public SignedEncryptedFrame { 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()); -- 2.39.5