Bug 65383

The same thing happens in the function check_tcp_syn_cookie(). It is naturally good that they match even if they are both wrong.

Not a bug, the count is being placed there on purpose so that
cookies expire properly.  Any kind of even theoretical exploit
would require the attacker to guess the value of jiffies on
the remote system.  Furthermore this 4 byte value is only
4 bytes out of the (80 * 2) bytes of total randomness fed into
the algorithm.

Hello Dave,
     My reading of Dan Bernstein's posts to the newsgroup in which syn cookies was designed (http://cr.yp.to/syncookies/archive) indicates that the second secret is 
important in making a syn cookie more difficult to forge. In the algorithm as it stands the second secret is always zero and therefore not a secret. It may indeed be 
secure enough as it stands as you suggest, however it doesn't match the intention of Dan Bernstein nor the author of the code as stated in the comments.
     It is so clearly an error in coding that I'm surprised you haven't accepted it. You can even tell how it happened: code was copied from the calculation of the first 
hash which has only four inputs to the hash and not corrected for the five elements in the calculation of the second hash. In any case I have never met a coder who 
would assign a value and then deliberately overwrite it before using it.
     I don't question the need for the count in the cookie but in this code the count overwrites the second secret which Dan Bernstein believes is needed.
     If you insist that this is the Red Hat Linux version of syn cookies then you could consider it a documentation bug and change the comment to agree with the actual 
algorithm. I would encourage you to check the Bernstein site before deciding.
     Thanks for giving this further consideration.
Jerry Gordon

I don't understand what you're talking about.  The comments explicitly
say that "For Linux I implement the 1 minute counter by looking at the
jiffies clock." The code acts as intended by the author.

Instead of us talking in circles, why don't you attach a patch
you think would fix whatever problem it is you think this code
has so we can talk about specifics.

Hello Dave,
     I have a response from Andi Kleen that explains the issue and I am partly wrong.
     Just so you'll know what was bothering me I'll show the code:
	memcpy(tmp+3, syncookie_secret[1], sizeof(syncookie_secret[1]));
	tmp[0]=saddr;
	tmp[1]=daddr;
	tmp[2]=(sport << 16) + dport;
	tmp[3] = count;	/* minute counter */
	HASH_TRANSFORM(tmp+16, tmp);
     My issue was that *(tmp+3) is the same as tmp[3] and therefore tmp[3] = count; overwrites the memcpy and my solution would be to use
                memcpy(tmp+4, syncookie_secret[1], sizeof(syncookie_secret[1]));.
      However as Andi Kleen points out only the first 32 bits of the secret is overwritten. The secret is much longer than 32 bits and that is the point I missed. Here is his 
response to me:
<quoting Andi Kleen>
Only the first word of the secret is overwritten. The input data 
of the SHA always consists of 64 bytes. syncookie_secret[1] is 14 long
words long. Of these the first long word is overwritten for the second case.

Arguably the memcpy could copy to tmp + 4.
I don't think it makes much difference security wise although 2^48bits is a bit
on the low side for a secret.
<end quote>
     So I am only partly right and the issue is no where near as severe as I originally thought.
     I will bow out of the discussion unless you have further questions.
Jerry Gordon

It seems that we are in agreement, so I am closing this.