Subject: Re: TCP MD5 and socket accept

From: Stephen Hemminger <[email-masked]>
Date: Thu, 26 Jun 2008 14:33:58 -0700

On Thu, 26 Jun 2008 07:46:59 -0700
"Adam Langley" <[email-masked]> wrote:

> On Wed, Jun 25, 2008 at 10:56 PM, Stephen Hemminger
> <[email-masked]> wrote:
> > What will break if tcp_create_openreq_child was fixed to copy md5_info if
> > present?
> >
> > This all comes about because right now using Quagga a Linux to Linux
> > works with TCP MD5. But a Linux to Cisco connection fails if using
> > TCP MD5.
> 
> I'll have a look at this later today but, as you say, Linux to Linux
> works, and getting the key wrong certainly breaks it (and without
> setsockopt on the child, I believe). So some MD5 information is
> getting copied from listening sockets to children.
> 
> Also note the MD5 on Linux is pretty badly broken in the face of
> packet loss at the moment. I have patches floating around to fix it,
> but not in any trees yet.
> 
> 
> AGL
> 

The problem is that md5 calculation assumes that the data buffer is
linear! It doesn't handle any kind of scatter-gather in skb!

int tcp_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key,
		      int bplen,
		      struct tcphdr *th, unsigned int tcplen,
		      struct tcp_md5sig_pool *hp)
{
...
	/* 3. The TCP segment data (if any) */
	data_len = tcplen - (th->doff << 2);
	if (data_len > 0) {
		u8 *data = (u8 *)th + (th->doff << 2);
		sg_set_buf(&sg[block++], data, data_len);
		nbytes += data_len;
	}

This is wrong, it needs to handle fragmented skb's.  I'll work out a patch
but it means passing skb to calc_md5_hash or just turn off using scatter/gather
on MD5 connections.