oxidecomputer / oxidecomputer/maghemite

MD5 Auth may be broken for passive connections

Open
#659 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bgp Bug mgd rust testing
Dominant language
Rust
Stars
94
Forks
6
Avg merge
1d 8h
Merged PRs (30d)
12

Description

The way that MD5 works in the current architecture (on Illumos) is that we start a new thread when a BgpConnectionTcp is created that is responsible for programming TCP MD5 Security Associations fresh in the kernel. For outbound connections, the thread is started in connect() before the outbound connection is initiated (pre handshake). For inbound connections, this happens in Listener<BgpConnectionTcp>.accept(). The TcpListener is unaware of the MD5 config for the peer + forks new sockets via accept() when the kernel passes the connection to the listener -- this means that inbound connections do not have the MD5 SA state setup until after the handshake completes.

So there is a difference there across inbound and outbound connections.

When doing some manual testing for #658, I found that doing a config update for the ipv4 session to the arista (mgadm bgp config neighbor update ...) to add in --passive-connection + resetting the session (mgadm bgp clear neighbor <ip> hard) resulted in the session not coming back up.

When looking at a tcpdump taken in the ceos container, I see that periodically the arista will send us a TCP SYN with the MD5 option in it, and we send back a TCP SYN/ACK without the MD5 option:

bash-4.2# tcpdump -ennni any ip and tcp port 179
tcpdump: data link type LINUX_SLL2
tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on any, link-type LINUX_SLL2 (Linux cooked v2), snapshot length 262144 bytes
16:58:10.853097 eth1  Out ifindex 9 02:42:ac:12:00:02 ethertype IPv4 (0x0800), length 92: 169.254.10.2.38351 > 169.254.10.1.179: Flags [S], seq 3018371376, win 64240, options [nop,nop
,md5 shared secret not supplied with -M, can't check - d462a068d90042e74ed40a862ad38c79,mss 1460,nop,nop,sackOK,nop,wscale 7], length 0
16:58:10.853454 eth1  In  ifindex 9 02:08:20:c2:66:80 ethertype IPv4 (0x0800), length 66: 169.254.10.1.179 > 169.254.10.2.38351: Flags [.], ack 3018371377, win 64240, length 0
16:58:17.480722 eth1  In  ifindex 9 02:08:20:c2:66:80 ethertype IPv4 (0x0800), length 72: 169.254.10.1.179 > 169.254.10.2.38351: Flags [S.], seq 4250939444, ack 3018371377, win 64240,
 options [mss 1460,nop,nop,sackOK,nop,wscale 1], length 0
^C
3 packets captured
3 packets received by filter
0 packets dropped by kernel

Meanwhile show log inside ceos doesn't show any logs for this neighbor (and there are no bgp debugs available in ceos).
I know ceos runs atop linux and uses the linux kernel tcp implementation, which I believe means userspace won't be privy to any TCP establishment issues at the protocol level (just at the socket layer) so I'm not surprised by the lack of logs.

After updating the peer again to remove --md5-auth-key "foo" the peer immediately established.

This suggests to me that the linux kernel implementation of TCP MD5 may expect the option to be present in the three-way handshake. If that's the case, then we probably need to evaluate maintaining TCP MD5 SAs globally so Illumos will know to use MD5 prior to the TCP handshake completing... which assumes Illumos will include the MD5 option in the handshake if the SA is present ahead of time, and that's a big assumption that will also need to be tested.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at BgpConnectionTcp creation, connect(), and Listener.accept(), then reproduce the passive-connection reset with MD5 enabled and inspect the tcpdump handshake on Illumos and Linux. Done means determining whether inbound SYN/SYN-ACK traffic can include TCP MD5 and defining a tested approach that lets the passive BGP session establish.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.