Bug 65383 - the second secret in calculating a syncookie is overwritten before it is used
the second secret in calculating a syncookie is overwritten before it is used
Product: Red Hat Linux
Classification: Retired
Component: kernel (Show other bugs)
i386 Linux
medium Severity medium
: ---
: ---
Assigned To: Arjan van de Ven
Brian Brock
: Security
Depends On:
  Show dependency treegraph
Reported: 2002-05-22 17:50 EDT by Jerry Gordon
Modified: 2007-04-18 12:42 EDT (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2002-05-23 10:08:46 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Jerry Gordon 2002-05-22 17:50:57 EDT
Description of Problem: This is a bug I found in the source code. I have not tried to break syncookies with this knowledge though I presume it makes syncookies 
significantly easier to break. The problem is in the function secure_tcp_syn_cookie() in file random.c. In line 2178 the second secret is assigned to *(tmp+3). Then in 
line 2182 count is assigned to tmp[3] overwriting the secret. Then in the next line tmp is hashed without benefit of secret. Please let Andi Kleen know about this if he is 
still maintaining this code. Thanks very much.

Version-Release number of selected component (if applicable):

How Reproducible:

Steps to Reproduce:

Actual Results:

Expected Results:

Additional Information:
Comment 1 Jerry Gordon 2002-05-22 18:20:11 EDT
The same thing happens in the function check_tcp_syn_cookie(). It is naturally good that they match even if they are both wrong.
Comment 2 David Miller 2002-05-23 08:29:34 EDT
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.
Comment 3 Jerry Gordon 2002-05-23 09:22:49 EDT
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
Comment 4 David Miller 2002-05-23 09:44:29 EDT
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.
Comment 5 Jerry Gordon 2002-05-23 10:08:40 EDT
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[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
Comment 6 David Miller 2002-05-23 10:35:24 EDT
It seems that we are in agreement, so I am closing this.

Note You need to log in before you can comment on or make changes to this bug.