Subject: Re: [secdir] Secdir review of draft-ietf-tsvwg-port-randomization-06.txt

From: Fernando Gont <[email-masked]>
Date: Tue, 02 Mar 2010 04:31:13 -0300

Hello, Charlie,

Thanks so much for your feedback! Comments in-line....

> Detailed security-relevant comments:
> 
> p13 para 2: says random must return integers in the range 0 - (2^n-1)
> where n>15. Larger values of n (e.g. n=31) are preferred, since
> otherwise distribution when reduced mod num_ephemeral will not be
> uniform.

Would requiring random() to return 32-bit random values address this
comment?

> p16 para 2: suggests use of MD5, which is fine from the point of view
> of documenting current practice, and in fact is cryptographically
> just fine in this context. 
> Nevertheless, crypto weenies the world
> over will pounce on you if it looks like you're suggesting that
> people use it. It's probably better to just suggest "some
> cryptographic hash function", or if you have to suggest something
> specific, perhaps SHA-256. 

My take on this is that as we're just obfuscating port numbers, MD5 is
good enough. But please let me know if you argue that we should
s/MD5/SHA-256/.....

> The appendix at the end should specify the
> hash algorithm used by existing implementations.

You mean Appendix A? If so, we could probably just indicate that Linux
uses MD5 for the hash-based port randomization algorithm.

> Your sample code in many cases, but in particular on page 17 figure 5
> ignores the possibility of integer overflow when incrementing things
> (e.g. table[index]++). The results of modular arithmetic when one of
> the operands is negative is language dependent, and in ANSI C is not
> defined, so this code could corrupt memory (unless the values being
> incremented are unsigned).

All operands are assumed to be unsigned. Please let me know if I should
clarify this.

> p20 para 2: suggests use of a 32 bit key, which has exploitable
> security problems even though MD5 is fine. 64 bits is a bare minimum,
> and since bits are cheap someone should really use 128.

Ok, I'll s/32/128/

> p24 para 3: Since the local offset function is a cryptographic hash
> reduced to a small number of bits, there will be identical offsets
> for different inputs and they are generally harmless. If they
> occurred at greater frequency than would be expected by chance, the
> port-offset mechanism proposed in this document would have a reduced
> effect.

Ok. I'll tweak the text.

> p24 para 4: Similarly, seeding the random number generator with a
> good  source of randomness will make an implementation more secure,
> but "must" is a bit strong.

Not sure what you mean. This para just refers to the quality of random
numbers. i.e., that if your going to use only random numbers for the
secret key, they should be "good enough" (with "good enough" being RFC4086)

> Detailed non-security relevant comments:
> 
> The document is titled "Transport Protocol Port Randomization
> Recommendations", but it doesn't really make recommendations. It
> enumerates current practice and calls out the pros and cons, but it
> doesn't really recommend what an implementer should do. That's
> probably OK, but it seemed a bit odd.

How about "Transport Protocol Port Randomization", instead?

> p6 last para section 1: Given that this document never uses SHOULD,
> MUST, ... outside of this paragraph, does it really need this
> paragraph and the reference to RFC 2119?

It does. See Section 3.2 and Section 3.3.

> p7 end of section 2.1 says ports in the range 49152-65535 are meant
> for the selection of ephemeral ports p12 1st para says:  "ephemeral
> port selection algorithms should use the whole range 1024-49151". Is
> this saying you should use port numbers not intended for this use? Do
> you mean should use the whole range 1024-65535?

The later: the text should say "...port selection algorithms should use
the whole range 1024-49151" (I screwed up the text in my last rev....
will fix this).

> 
> Typos:
> 
> p12 line 2: "is not affected is not affected" -> "is not affected" 

Fixed.

> p13 para 2 line 2: "interger" -> "integer" p16 para 3: "chosen as
> random as possible" -> "chosen to be as random as possible" p20 para
> 3: "reasonable" -> "reasonably"

Thanks!

Kind regards,
-- 
Fernando Gont
e-mail: [email-masked] || [email-masked]
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1

---

From: Lars Eggert <[email-masked]>
Date: Tue, 2 Mar 2010 06:28:11 -0800

Hi,

when you make some of the text changes that Charlie suggested, please make them in a way that doesn't break what the document says about [Allman]. (Mark Allman ran some tests with some of the algorithms in this document, and if we change their description retroactively, it may affect what we can say about his effort.)

>> probably OK, but it seemed a bit odd.

this came up during the WG discussion. If I remember correctly, the document is the way it is because the WG felt that multiple schemes work well, and because this is not something that requires interoperability, there was no strong need to pick a "winner".

Lars

---

From: Fernando Gont <[email-masked]>
Date: Thu, 04 Mar 2010 01:09:16 -0300

Hi, Lars,

> when you make some of the text changes that Charlie suggested, please
> make them in a way that doesn't break what the document says about
> [Allman]. (Mark Allman ran some tests with some of the algorithms in
> this document, and if we change their description retroactively, it
> may affect what we can say about his effort.)

I don't think the resulting changes would affect the results in
[Allman]. For instance, I'd bet the random() function he used provided
32-bit pseudo-random numbers (rathern than 16-bit numbers, as previously
suggested in the I-D).

>>> enumerates current practice and calls out the pros and cons, but
>>> it doesn't really recommend what an implementer should do. That's
> this came up during the WG discussion. If I remember correctly, the
> document is the way it is because the WG felt that multiple schemes
> work well, and because this is not something that requires
> interoperability, there was no strong need to pick a "winner".

I didn't apply any change in response to this. (Just for the record, *I*
would have been happier with the document recommending a particular
algorithm... but I do see the point of why the wg didn't think so).

Thanks!

Kind regards,

---

From: Lars Eggert <[email-masked]>
Date: Thu, 4 Mar 2010 10:50:41 +0100

Hi,

> 32-bit pseudo-random numbers (rathern than 16-bit numbers, as previously

please check with Mark on this. I believe you are right, but it's better to be sure than to bet.

> algorithm... but I do see the point of why the wg didn't think so).

I would have been happier, too, but I think the argument was that we didn't want to needlessly say that some existing implementations don't comply to the recommendations in the RFC when it doesn't matter because their algorithm is good enough.

Lars

---

From: Charlie Kaufman <[email-masked]>
Date: Thu, 4 Mar 2010 18:50:05 +0000

> Would requiring random() to return 32-bit random values address this
In most (all?) C implementations, rand() returns int, and so would return values between 0 and 2^15-1, 2^31-1, or 2^63-1. I would change the wording to something like:

Note: "random()" is a function that returns a pseudo-random non-negative
   integer number in the range 0 to at least 65535 (it is preferable if it returns values in a range larger
   than 65535, as is the case with the "random()" C-language function).

> > of documenting current practice, and in fact is cryptographically just
> > fine in this context.
> > Nevertheless, crypto weenies the world over will pounce on you if it
> > looks like you're suggesting that people use it. It's probably better
> > to just suggest "some cryptographic hash function", or if you have to
> > suggest something specific, perhaps SHA-256.
> 
> My take on this is that as we're just obfuscating port numbers, MD5 is good
> enough. But please let me know if you argue that we should s/MD5/SHA-
> 256/.....
> 
I agree. In this context, MD5 is more than good enough. But I have lost enough battles with crypto weenies when trying to use MD5 in a context where cryptographic strength is not an issue that I'd reluctantly recommend that you say SHA-256 instead in this paragraph. In the paragraph that follows, where it is explaining that using a weak cryptographic function is OK, the reference to MD5 is the best one to use. In both cases, the algorithms are mentioned as examples, which is also the right thing to do.
> 
> 
> > The appendix at the end should specify the hash algorithm used by
> > existing implementations.
> 
> You mean Appendix A? If so, we could probably just indicate that Linux uses
> MD5 for the hash-based port randomization algorithm.
> 

Yes.

> All operands are assumed to be unsigned. Please let me know if I should

That would technically be OK. The current text says it's an array of short integers, which if someone implemented as:

short int table[TABLE_LENGTH];

would do bad things on implementations where short is 16 bits signed.

If this code is copied from a real implementation, it's probably better to just add the declaration. Otherwise, it would be better to avoid the overflows altogether with code like:

     /* Initialization at system boot time */
     num_ephemeral = max_ephemeral - min_ephemeral + 1;
     for(i = 0; i < TABLE_LENGTH; i++)
         table[i] = random() % TABLE_LENGTH;

     /* Ephemeral port selection function */
     offset = F(local_IP, remote_IP, remote_port, secret_key1) % num_ephemeral;
     index = G(local_IP, remote_IP, remote_port, secret_key2) % TABLE_LENGTH;
     count = num_ephemeral;

     do {
         port = min_ephemeral + (offset + table[index]) % num_ephemeral;
         table[index] = (table[index]+1) % num_ephemeral;

         if(resulting five-tuple is unique)
             return port;

        count--;

     } while (count > 0);

     return ERROR;

By adding the mod functions to F() and G() in the code, you avoid the need to say that F and G are hash functions that return integers in a particular range.

The other place where there is a problem with overflow is in figure 4, where after the line:

          next_ephemeral++;

I would add:

          if (next_ephemeral == num_ephemeral)
               next_ephemeral = 0;        

While in theory these changes could invalidate the citing of [Allman], none have an affect bigger than seeding the random number generate with different values, so I would claim that to the extent his results are useful they apply to the slightly modified algorithms that avoid integer overflows.

> > and since bits are cheap someone should really use 128.

That works (except in reality, the wording change is a little bigger than that).
> > reduced to a small number of bits, there will be identical offsets for
> > different inputs and they are generally harmless. If they occurred at
> > greater frequency than would be expected by chance, the port-offset
> > mechanism proposed in this document would have a reduced effect.
> 
> Ok. I'll tweak the text.
> 
> 
> 
> > p24 para 4: Similarly, seeding the random number generator with a good
> > source of randomness will make an implementation more secure, but
> > "must" is a bit strong.
> 
> numbers. i.e., that if your going to use only random numbers for the secret
> key, they should be "good enough" (with "good enough" being RFC4086)
> 

I guess I would change the word "must" to "should". If an implementation does not follow this recommendation, it will probably be just fine.

I hesitate to mention that the standard C language implementation of rand() does not satisfy this requirement, but getting random values from rand() (following some variation of randomize() or srand(time)) is a lot better than nothing and arguably adequate for this application. I would hate to guide people away from using rand() because it might discourage them from doing this at all. On the other hand, I would never want to guide anyone towards rand() because (at least the last time I looked at it) it was a horrible random number generator not even suitable for statistical randomness unless used very carefully.
> How about "Transport Protocol Port Randomization", instead?

Works for me, but I suspect someone had a good reason for naming it what they did.
> It does. See Section 3.2 and Section 3.3.

My mistake. I searched the document, but somehow missed the two MUSTs.
> The later: the text should say "...port selection algorithms should use the
> whole range 1024-49151" (I screwed up the text in my last rev....
> will fix this).
> 
> 
> >
> > Typos:
> >
> e-mail: [email-masked] || [email-masked] PGP Fingerprint: 7809 84F5
> 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1
> 
> 
> 
>