[Xorp-hackers] First cut app-level tcp-md5 patch

Bruce M Simpson bms@spc.org
Thu, 5 Aug 2004 15:23:51 -0700


--xHFwDpU9dbj6ez1V
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Aug 05, 2004 at 03:05:26PM -0700, Bruce M Simpson wrote:
> It's just become apparent that the property needs to reside in BGPPeerData
> anyway, for reasons related to socket semantics.  I'm going to post an
> updated patch here very shortly.

Here is an updated patch for tcp-md5 support.

Both SocketClient::connect_socket() and SocketServer::listen() are the
places where comm_set_tcpmd5() actually need to be called, for the
correct semantics to take place.

The TCP_MD5SIG socket option shouldn't be set in the middle of a TCP
session; I should probably add anti-foot-shooting code to FreeBSD to make
sure it can't be set for a socket in anything other than LISTEN or IDLE state.

However, in BGPMain::create_peer(), which instantiates SocketClient, the
SocketClient is instantiated before the BGPPeer.

So, push md5 socket option switch down to SocketClient as a bool property,
and add it to the constructor signature appropriately as an optional argument.

Amend the BGPMain::set_peer_md5_password() method accordingly with a
const_cast to obtain the BGPPeerData instance, and reference through that.

Move the accessors/mutators for _md5_password into BGPPeerData instead.
The XRL interface remains the same.

--

There is a potential problem here. The code here isn't guaranteed to
support tcp-md5 passive open properly with itojun's new iteration of
the tcp-md5 code.

There is no way of telling non-tcpmd5 peers apart on the listening socket
until such time as they actually connect - and by that time it's too late;
so until security policies a la PF_KEY SPD can be used, only active opens
will have the correct semantics.

--xHFwDpU9dbj6ez1V
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="md5-cut2.diff"

Index: bgp/bgp.cc
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/bgp/bgp.cc,v
retrieving revision 1.37
diff -u -p -r1.37 bgp.cc
--- bgp/bgp.cc	10 Jun 2004 22:40:28 -0000	1.37
+++ bgp/bgp.cc	5 Aug 2004 22:14:20 -0000
@@ -387,7 +387,9 @@ BGPMain::create_peer(BGPPeerData *pd)
 	return false;
     }
 
-    SocketClient *sock = new SocketClient(pd->iptuple(), eventloop());
+    bool md5sig = ("" == pd->get_md5_password() ? false : true);
+
+    SocketClient *sock = new SocketClient(pd->iptuple(), eventloop(), md5sig);
 
     BGPPeer *p = new BGPPeer(&_local_data, pd, sock, this);
     //    sock->set_eventloop(eventloop());
@@ -471,6 +473,24 @@ BGPMain::set_peer_state(const Iptuple& i
 }
 
 bool
+BGPMain::set_peer_md5_password(const Iptuple& iptuple, const string& password)
+{
+    BGPPeer *peer = find_peer(iptuple);
+
+    if (peer == NULL) {
+	XLOG_WARNING("Could not find peer: %s", iptuple.str().c_str());
+	return false;
+    }
+
+    // The md5-password property has to belong to BGPPeerData, because
+    // it is instantiated before BGPPeer and before SocketClient.
+    BGPPeerData* pd = const_cast<BGPPeerData*>(peer->peerdata());
+    pd->set_md5_password(password);
+
+    return true;
+}
+
+bool
 BGPMain::activate(const Iptuple& iptuple)
 {
     BGPPeer *peer = find_peer(iptuple);
Index: bgp/bgp.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/bgp/bgp.hh,v
retrieving revision 1.27
diff -u -p -r1.27 bgp.hh
--- bgp/bgp.hh	10 Jun 2004 22:40:28 -0000	1.27
+++ bgp/bgp.hh	5 Aug 2004 22:14:20 -0000
@@ -125,6 +125,16 @@ public:
     bool set_peer_state(const Iptuple& iptuple, bool state);
 
     /**
+     * Set peer TCP-MD5 password.
+     *
+     * @param iptuple iptuple.
+     * @param password The password to use for TCP-MD5 authentication; if this is the empty string, then authentication will be disabled.
+     *
+     * @return true on success.
+     */
+    bool set_peer_md5_password(const Iptuple& iptuple, const string& password);
+
+    /**
      * Activate peer.
      *
      * Enable the peering based on the peer state.
Index: bgp/peer_data.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/bgp/peer_data.hh,v
retrieving revision 1.12
diff -u -p -r1.12 peer_data.hh
--- bgp/peer_data.hh	10 Jun 2004 22:40:32 -0000	1.12
+++ bgp/peer_data.hh	5 Aug 2004 22:14:20 -0000
@@ -199,6 +199,9 @@ public:
 	return _next_hop_rewrite;
     }
 
+    void set_md5_password(const string &password) { _md5_password = password; }
+    const string& get_md5_password() const { return _md5_password; }
+
     /**
      * Dump the state of the peer data (debugging).
      */
@@ -272,6 +275,11 @@ private:
     ** temporary hack store the re-write value here.
     */
     IPv4 _next_hop_rewrite;
+
+    /**
+     * The password for TCP-MD5 authentication.
+     */
+    string _md5_password;
 };
 
 #endif // __BGP_PEER_DATA_HH__
Index: bgp/socket.cc
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/bgp/socket.cc,v
retrieving revision 1.15
diff -u -p -r1.15 socket.cc
--- bgp/socket.cc	10 Jun 2004 22:40:36 -0000	1.15
+++ bgp/socket.cc	5 Aug 2004 22:14:20 -0000
@@ -27,6 +27,7 @@
 #include "libxorp/xlog.h"
 #include "libxorp/exceptions.hh"
 #include "libxorp/callback.hh"
+#include "libcomm/comm_api.h"
 
 #include "socket.hh"
 #include "packet.hh"
@@ -111,8 +112,9 @@ Socket::init_sockaddr(struct sockaddr_in
 
 /* **************** BGPSocketClient - PUBLIC METHODS *********************** */
 
-SocketClient::SocketClient(const Iptuple& iptuple, EventLoop& e)
-    : Socket(iptuple, e)
+SocketClient::SocketClient(const Iptuple& iptuple, EventLoop& e,
+    bool md5sig = false)
+    : Socket(iptuple, e), _md5sig(md5sig)
 {
     debug_msg("SocketClient constructor called\n");
     _async_writer = 0;
@@ -168,6 +170,10 @@ SocketClient::connect(ConnectCallback cb
     XLOG_ASSERT(UNCONNECTED == get_sock());
 
     create_socket();
+
+    if (_md5sig)
+	comm_set_tcpmd5(get_sock(), _md5sig);
+
     return connect_socket(get_sock(), get_remote_addr(), get_remote_port(),
 			  get_local_addr(), cb);
 }
Index: bgp/socket.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/bgp/socket.hh,v
retrieving revision 1.7
diff -u -p -r1.7 socket.hh
--- bgp/socket.hh	10 Jun 2004 22:40:36 -0000	1.7
+++ bgp/socket.hh	5 Aug 2004 22:14:20 -0000
@@ -93,7 +93,7 @@ public:
     /**
      * @param iptuple specification of the connection endpoints.
      */
-    SocketClient(const Iptuple& iptuple, EventLoop& e);
+    SocketClient(const Iptuple& iptuple, EventLoop& e, bool md5sig = false);
     ~SocketClient();
 
     /**
@@ -246,6 +246,7 @@ private:
 
     bool _disconnecting;
     bool _connecting;
+    bool _md5sig;
 
     uint8_t _read_buf[MAXPACKETSIZE]; // Maximum allowed BGP message
 };
Index: bgp/xrl_target.cc
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/bgp/xrl_target.cc,v
retrieving revision 1.26
diff -u -p -r1.26 xrl_target.cc
--- bgp/xrl_target.cc	10 Jun 2004 22:40:39 -0000	1.26
+++ bgp/xrl_target.cc	5 Aug 2004 22:14:20 -0000
@@ -286,6 +286,28 @@ XrlBgpTarget::bgp_0_2_set_peer_state(
 }
 
 XrlCmdError 
+XrlBgpTarget::bgp_0_2_set_peer_md5_password(
+	// Input values, 
+	const string&	local_ip, 
+	const uint32_t&	local_port, 
+	const string&	peer_ip, 
+	const uint32_t&	peer_port, 
+	const string& password)
+{
+    debug_msg("local ip %s local port %d peer ip %s peer port %d password %s\n",
+	  local_ip.c_str(), local_port, peer_ip.c_str(), peer_port,
+	  password.c_str());
+
+    Iptuple iptuple(local_ip.c_str(), local_port, peer_ip.c_str(), peer_port);
+
+    if(!_bgp.set_peer_md5_password(iptuple, password))
+	return XrlCmdError::COMMAND_FAILED();
+
+    return XrlCmdError::OKAY();
+}
+
+
+XrlCmdError 
 XrlBgpTarget::bgp_0_2_activate(
 	// Input values, 
 	const string&	local_ip, 
Index: bgp/xrl_target.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/bgp/xrl_target.hh,v
retrieving revision 1.22
diff -u -p -r1.22 xrl_target.hh
--- bgp/xrl_target.hh	10 Jun 2004 22:40:39 -0000	1.22
+++ bgp/xrl_target.hh	5 Aug 2004 22:14:20 -0000
@@ -106,6 +106,14 @@ public:
 	const uint32_t&	peer_port,
 	const bool&	state);
 
+    XrlCmdError bgp_0_2_set_peer_md5_password(
+	// Input values,
+	const string&	local_ip,
+	const uint32_t&	local_port,
+	const string&	peer_ip,
+	const uint32_t&	peer_port,
+	const string&	password);
+
     XrlCmdError bgp_0_2_activate(
 	// Input values,
 	const string&	local_ip,
Index: etc/templates/bgp.tp
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/etc/templates/bgp.tp,v
retrieving revision 1.32
diff -u -p -r1.32 bgp.tp
--- etc/templates/bgp.tp	9 Jun 2004 22:40:49 -0000	1.32
+++ etc/templates/bgp.tp	5 Aug 2004 22:14:25 -0000
@@ -14,6 +14,7 @@ protocols {
 	    next-hop: ipv4;
 	    holdtime: u32 = 120;
 	    enabled: bool = true;
+	    md5-password: txt = "";
 	}
 
 	/*
@@ -87,6 +88,10 @@ protocols {
 		%set:;
 	    }
 
+	    md5-password {
+		%set:;
+	    }
+
 	    as {
 	        %allow-range: $(@) "0" "65535";
 		%set:;
@@ -210,6 +215,10 @@ protocols {
 		%help: short "Enable this peering.";
 	    }
 
+	    md5-password {
+		%help: short "Enable and set the password for TCP-MD5 authentication with this peer.";
+	    }
+
 	    set-parameter {
 	    }
 
Index: xrl/interfaces/bgp.xif
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/interfaces/bgp.xif,v
retrieving revision 1.18
diff -u -p -r1.18 bgp.xif
--- xrl/interfaces/bgp.xif	30 May 2004 23:17:33 -0000	1.18
+++ xrl/interfaces/bgp.xif	5 Aug 2004 22:14:28 -0000
@@ -131,6 +131,23 @@ interface bgp/0.2 {
 		& toggle:bool
 
 	/**
+         * Set the peer md5 password.
+	 *
+	 * @param local IP address.
+	 * @param local server port.
+	 * @param peer IP address.
+	 * @param peer port.
+         * @param password the password to use for TCP-MD5 authentication.
+	 */
+	set_peer_md5_password \
+		? \
+		local_ip:txt \
+	        & local_port:u32 \
+		& peer_ip:txt \
+		& peer_port:u32 \
+		& password:txt
+
+	/**
          * Enable or disable the peering based on the peer state.
 	 *
 	 * @param local IP address.
Index: xrl/interfaces/bgp_xif.cc
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/interfaces/bgp_xif.cc,v
retrieving revision 1.26
diff -u -p -r1.26 bgp_xif.cc
--- xrl/interfaces/bgp_xif.cc	10 Jun 2004 22:41:58 -0000	1.26
+++ xrl/interfaces/bgp_xif.cc	5 Aug 2004 22:14:28 -0000
@@ -422,6 +422,46 @@ XrlBgpV0p2Client::unmarshall_set_peer_st
 }
 
 bool
+XrlBgpV0p2Client::send_set_peer_md5_password(
+	const char*	the_tgt,
+	const string&	local_ip,
+	const uint32_t&	local_port,
+	const string&	peer_ip,
+	const uint32_t&	peer_port,
+	const string&	password,
+	const SetPeerMd5PasswordCB&	cb
+)
+{
+    Xrl x(the_tgt, "bgp/0.2/set_peer_md5_password");
+    x.args().add("local_ip", local_ip);
+    x.args().add("local_port", local_port);
+    x.args().add("peer_ip", peer_ip);
+    x.args().add("peer_port", peer_port);
+    x.args().add("password", password);
+    return _sender->send(x, callback(this, &XrlBgpV0p2Client::unmarshall_set_peer_md5_password, cb));
+}
+
+
+/* Unmarshall set_peer_md5_password */
+void
+XrlBgpV0p2Client::unmarshall_set_peer_md5_password(
+	const XrlError&	e,
+	XrlArgs*	a,
+	SetPeerMd5PasswordCB		cb
+)
+{
+    if (e != XrlError::OKAY()) {
+	cb->dispatch(e);
+	return;
+    } else if (a && a->size() != 0) {
+	XLOG_ERROR("Wrong number of arguments (%u != %u)", (uint32_t)a->size(), 0);
+	cb->dispatch(XrlError::BAD_ARGS());
+	return;
+    }
+    cb->dispatch(e);
+}
+
+bool
 XrlBgpV0p2Client::send_activate(
 	const char*	the_tgt,
 	const string&	local_ip,
Index: xrl/interfaces/bgp_xif.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/interfaces/bgp_xif.hh,v
retrieving revision 1.26
diff -u -p -r1.26 bgp_xif.hh
--- xrl/interfaces/bgp_xif.hh	10 Jun 2004 22:41:58 -0000	1.26
+++ xrl/interfaces/bgp_xif.hh	5 Aug 2004 22:14:28 -0000
@@ -213,6 +213,26 @@ public:
 	const SetPeerStateCB&	cb
     );
 
+    typedef XorpCallback1<void, const XrlError&>::RefPtr SetPeerMd5PasswordCB;
+    /**
+     *  Send Xrl intended to:
+     *
+     *  Set the peer md5 password.
+     *
+     *  @param tgt_name Xrl Target name
+     *
+     *  @param password the password to use for TCP-MD5 authentication.
+     */
+    bool send_set_peer_md5_password(
+	const char*	target_name,
+	const string&	local_ip,
+	const uint32_t&	local_port,
+	const string&	peer_ip,
+	const uint32_t&	peer_port,
+	const string&	password,
+	const SetPeerMd5PasswordCB&	cb
+    );
+
     typedef XorpCallback1<void, const XrlError&>::RefPtr ActivateCB;
     /**
      *  Send Xrl intended to:
@@ -626,6 +646,12 @@ private:
 	SetPeerStateCB		cb
     );
 
+    void unmarshall_set_peer_md5_password(
+	const XrlError&	e,
+	XrlArgs*	a,
+	SetPeerMd5PasswordCB		cb
+    );
+
     void unmarshall_activate(
 	const XrlError&	e,
 	XrlArgs*	a,
Index: xrl/targets/bgp.xrls
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/targets/bgp.xrls,v
retrieving revision 1.28
diff -u -p -r1.28 bgp.xrls
--- xrl/targets/bgp.xrls	3 Aug 2004 05:21:28 -0000	1.28
+++ xrl/targets/bgp.xrls	5 Aug 2004 22:14:28 -0000
@@ -99,6 +99,13 @@ finder://bgp/bgp/0.2/disable_peer?local_
 finder://bgp/bgp/0.2/set_peer_state?local_ip:txt&local_port:u32&peer_ip:txt&peer_port:u32&toggle:bool
 
 /**
+ *  Set the peer md5 password.
+ *
+ *  @param password the password to use for TCP-MD5 authentication.
+ */
+finder://bgp/bgp/0.2/set_peer_md5_password?local_ip:txt&local_port:u32&peer_ip:txt&peer_port:u32&password:txt
+
+/**
  *  Enable or disable the peering based on the peer state.
  */
 finder://bgp/bgp/0.2/activate?local_ip:txt&local_port:u32&peer_ip:txt&peer_port:u32
Index: xrl/targets/bgp_base.cc
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/targets/bgp_base.cc,v
retrieving revision 1.29
diff -u -p -r1.29 bgp_base.cc
--- xrl/targets/bgp_base.cc	10 Jun 2004 22:42:08 -0000	1.29
+++ xrl/targets/bgp_base.cc	5 Aug 2004 22:14:28 -0000
@@ -518,6 +518,35 @@ XrlBgpTargetBase::handle_bgp_0_2_set_pee
 }
 
 const XrlCmdError
+XrlBgpTargetBase::handle_bgp_0_2_set_peer_md5_password(const XrlArgs& xa_inputs, XrlArgs* /* pxa_outputs */)
+{
+    if (xa_inputs.size() != 5) {
+	XLOG_ERROR("Wrong number of arguments (%u != %u) handling %s",
+            5, (uint32_t)xa_inputs.size(), "bgp/0.2/set_peer_md5_password");
+	return XrlCmdError::BAD_ARGS();
+    }
+
+    /* Return value declarations */
+    try {
+	XrlCmdError e = bgp_0_2_set_peer_md5_password(
+	    xa_inputs.get_string("local_ip"),
+	    xa_inputs.get_uint32("local_port"),
+	    xa_inputs.get_string("peer_ip"),
+	    xa_inputs.get_uint32("peer_port"),
+	    xa_inputs.get_string("password"));
+	if (e != XrlCmdError::OKAY()) {
+	    XLOG_WARNING("Handling method for %s failed: %s",
+            		 "bgp/0.2/set_peer_md5_password", e.str().c_str());
+	    return e;
+        }
+    } catch (const XrlArgs::XrlAtomNotFound& e) {
+	XLOG_ERROR("Argument not found");
+	return XrlCmdError::BAD_ARGS();
+    }
+    return XrlCmdError::OKAY();
+}
+
+const XrlCmdError
 XrlBgpTargetBase::handle_bgp_0_2_activate(const XrlArgs& xa_inputs, XrlArgs* /* pxa_outputs */)
 {
     if (xa_inputs.size() != 4) {
@@ -1626,6 +1655,10 @@ XrlBgpTargetBase::add_handlers()
 	    callback(this, &XrlBgpTargetBase::handle_bgp_0_2_set_peer_state)) == false) {
 	    XLOG_ERROR("Failed to xrl handler finder://%s/%s", "bgp", "bgp/0.2/set_peer_state");
 	}
+	if (_cmds->add_handler("bgp/0.2/set_peer_md5_password",
+	    callback(this, &XrlBgpTargetBase::handle_bgp_0_2_set_peer_md5_password)) == false) {
+	    XLOG_ERROR("Failed to xrl handler finder://%s/%s", "bgp", "bgp/0.2/set_peer_md5_password");
+	}
 	if (_cmds->add_handler("bgp/0.2/activate",
 	    callback(this, &XrlBgpTargetBase::handle_bgp_0_2_activate)) == false) {
 	    XLOG_ERROR("Failed to xrl handler finder://%s/%s", "bgp", "bgp/0.2/activate");
@@ -1755,6 +1788,7 @@ XrlBgpTargetBase::remove_handlers()
 	_cmds->remove_handler("bgp/0.2/enable_peer");
 	_cmds->remove_handler("bgp/0.2/disable_peer");
 	_cmds->remove_handler("bgp/0.2/set_peer_state");
+	_cmds->remove_handler("bgp/0.2/set_peer_md5_password");
 	_cmds->remove_handler("bgp/0.2/activate");
 	_cmds->remove_handler("bgp/0.2/set_parameter");
 	_cmds->remove_handler("bgp/0.2/next_hop_rewrite_filter");
Index: xrl/targets/bgp_base.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/targets/bgp_base.hh,v
retrieving revision 1.34
diff -u -p -r1.34 bgp_base.hh
--- xrl/targets/bgp_base.hh	3 Aug 2004 05:21:28 -0000	1.34
+++ xrl/targets/bgp_base.hh	5 Aug 2004 22:14:28 -0000
@@ -235,6 +235,21 @@ protected:
     /**
      *  Pure-virtual function that needs to be implemented to:
      *
+     *  Set the peer md5 password.
+     *
+     *  @param password the password to use for TCP-MD5 authentication.
+     */
+    virtual XrlCmdError bgp_0_2_set_peer_md5_password(
+	// Input values,
+	const string&	local_ip,
+	const uint32_t&	local_port,
+	const string&	peer_ip,
+	const uint32_t&	peer_port,
+	const string&	password) = 0;
+
+    /**
+     *  Pure-virtual function that needs to be implemented to:
+     *
      *  Enable or disable the peering based on the peer state.
      */
     virtual XrlCmdError bgp_0_2_activate(
@@ -671,6 +686,8 @@ private:
 
     const XrlCmdError handle_bgp_0_2_set_peer_state(const XrlArgs& in, XrlArgs* out);
 
+    const XrlCmdError handle_bgp_0_2_set_peer_md5_password(const XrlArgs& in, XrlArgs* out);
+
     const XrlCmdError handle_bgp_0_2_activate(const XrlArgs& in, XrlArgs* out);
 
     const XrlCmdError handle_bgp_0_2_set_parameter(const XrlArgs& in, XrlArgs* out);

--xHFwDpU9dbj6ez1V--