netdev - Re: TCP MD5 and socket accept

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 26 Jun 2008 14:33:58 -0700
From:	Stephen Hemminger <shemminger@...tta.com>
To:	"Adam Langley" <agl@...erialviolet.org>
Cc:	"David Miller" <davem@...emloft.net>,
	"吉藤英明" <yoshfuji@...ux-ipv6.org>,
	netdev@...r.kernel.org
Subject: Re: TCP MD5 and socket accept

On Thu, 26 Jun 2008 07:46:59 -0700
"Adam Langley" <agl@...erialviolet.org> wrote:

> On Wed, Jun 25, 2008 at 10:56 PM, Stephen Hemminger
> <shemminger@...tta.com> 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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html