netdev - Re: [PATCH][RFC] tcp: fix ambiguity in the `before' relation

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]

Message-Id: <E1H2GGi-0008E5-00@gondolin.me.apana.org.au>
Date:	Thu, 04 Jan 2007 11:15:40 +1100
From:	Herbert Xu <herbert@...dor.apana.org.au>
To:	gerrit@....abdn.ac.uk (Gerrit Renker)
Cc:	herbert@...dor.apana.org.au, davem@...emloft.net,
	netdev@...r.kernel.org
Subject: Re: [PATCH][RFC] tcp: fix ambiguity in the `before' relation

Gerrit Renker <gerrit@....abdn.ac.uk> wrote:
>
> |  Prior to the patch, we have values x and y such that both
> |  before(x, y) and before(y, x) are true.  Now for those same
> |  values both before(x, y) and before(y, x) are false.
> |
> |  It's still as ambiguous as ever.  Surely to resolve the
> |  ambiguity we want to make before(x, y) = !before(y, x), no?
>
> Please let me restate:
> Ambiguity here means that for those numbers x,y such that  (x + 2^31) % 2^32) = y
> before(x, y) = 1 and before(y, x) = 1. With the previous implementation, one could 
> not tell the difference here: and there are 2^32 such cases where this occurs.
> 
> With the implementation now, the output of before(x,y) is reliable: it returns true
> if (and only if) x is indeed `before' y.

Sorry but I don't think you've answered my question.

Let y = (x + 2^31) % 2^32, how is making

	before(x, y) == before(y, x) == 0

any better than

	before(x, y) == before(y, x) == 1

For an unambiguous before, we must have before(x, y) != before(y, x)
if x != y.

For a more concrete example, look at the code in tcp_ack:

        /* If the ack is newer than sent or older than previous acks
         * then we can probably ignore it.
         */
        if (after(ack, tp->snd_nxt))
                goto uninteresting_ack;

        if (before(ack, prior_snd_una))
                goto old_ack;

Here we have two checks that weed out cases that we do not wish to
process.  When all data have been acknowledged, we have

	snd_nxt == snd_una

At this point, we only want the value of ack == snd_nxt == snd_una
to pass this check.  With your change, the value snd_nxt + 2^31 can
also pass this check, which may have security implications.

Granted I have not looked at other checks in the TCP path that may
prevent this code from being invoked in that case.  However, this
does illustrate the need to audit every single before/after check
if they were ambiguous.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
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