NTP users are strongly urged to take immediate action to ensure that their NTP daemons are not susceptible to being used in distributed denial-of-service (DDoS) attacks. Please also take this opportunity to defeat denial-of-service attacks by implementing Ingress and Egress filtering through BCP38.

ntp-4.2.8p18 was released on 25 May 2024 and addresses 40 bugs and provides 40 improvements.

Please see the NTP 4.2.8p18 Changelog for details.

Bug 2666 - non-cryptographic random number generator with weak seed
Summary: non-cryptographic random number generator with weak seed
Status: RESOLVED FIXED
Alias: None
Product: ntp
Classification: Unclassified
Component: ntpd (show other bugs)
Version: 4.2.6
Hardware: N/A All
: P2 critical
Assignee: Harlan Stenn
URL:
Depends on:
Blocks: 2655
  Show dependency tree
 
Reported: 2014-11-03 00:19 UTC by Harlan Stenn
Modified: 2023-01-20 07:36 UTC (History)
6 users (show)

See Also:
stenn: blocking4.2.6+


Attachments
Patch to use arc4random() (667 bytes, patch)
2014-11-26 11:03 UTC, Harlan Stenn
stenn: patchReview?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Harlan Stenn 2014-11-03 00:19:41 UTC
+++ This bug was initially created as a clone of Bug #2655 +++

2) non-cryptographic random number generator with weak seed
 * util/ntp-keygen.c:724 <gen_md5> (weak symmetric keys)
 fix: use OS provided random numbers for crypto
Comment 1 Harlan Stenn 2014-11-03 01:01:13 UTC
ntp_random.c was added during a time when a *lot* of OSes had significantly worse random number generators.

It looks like the "worst" use of ntp_random.c is when we need more than 31 bits of data and we use the entire result from ntp_random() for this.

While we do need to use a better seed with ntp_srandom(), it looks like the current code is OK if we use the low-order 8 or 16 bits.

I'm currently planning to use the arc4random() routines if available, and completely fill the seed buffer.
Comment 2 Danny Mayer 2014-11-03 05:06:56 UTC
(In reply to comment #1)
> I'm currently planning to use the arc4random() routines if available, and
> completely fill the seed buffer.

There is a portable version of arc4random() from OpenBSD which looks like it is free for any reuse. You can find it here: https://github.com/libevent/libevent/blob/master/arc4random.c but of course someone needs to review it to make sure it's not considered weak and can be reused in this case.
Comment 3 Harlan Stenn 2014-11-03 06:20:59 UTC
Thanks, Danny!
Comment 4 Harlan Stenn 2014-11-05 05:51:39 UTC
This bug has already been fixed in ntp-dev.
Comment 5 Harlan Stenn 2014-11-23 00:07:32 UTC
Specifically, the seed was made significantly better in 4.2.7p230, on 1 Nov 2011.
Comment 6 Stephen Röttger 2014-11-25 14:09:05 UTC
The state used for generating MD5 keys is still 32 bit, which is way to short for cryptographic keys.
Comment 7 Harlan Stenn 2014-11-26 11:03:41 UTC
Created attachment 1163 [details]
Patch to use arc4random()

Stephen, what do you think of my proposed patch?
Comment 8 Stephen Röttger 2014-11-28 09:10:49 UTC
I'm not familiar with arc4random, is it available on all supported platforms?
I read (sorry, can't find the link atm) that freebsd switched to a chacha implementation since RC4 has weaknesses but that the Linux implementation (in some library) didn't follow.
Comment 9 Harlan Stenn 2014-11-28 10:23:25 UTC
arc4random() should be available, and if it's not I'll build a copy from the copy Danny mentions in comment#2.

If you have recommendations for something better I'm happy to give that a shot.

When I dug in to this it was my understanding that arc4andom() was much better suited for use where cryptographic-quality random numbers were needed.

For *ix boxes I could do something with /dev/urandom if that would be better, and somebody could write something for Windows that used CryptGenRandom (or similar, http://msdn.microsoft.com/en-us/library/windows/desktop/aa379942(v=vs.85).aspx).  I'm not sure what we'd do about other OSes.

Plan B would be to use OpenSSL's RAND_bytes(), which means we'd need to require OpenSSL.  This might be reasonable for cases where we will be linked with OpenSSL.
Comment 10 Danny Mayer 2014-12-01 16:17:19 UTC
I implemented the Windows CryptGenRandom code in BIND9 in the lib/isc/win32/entropy.c file. It shouldn't be too hard to do the same thing here.
Comment 11 Harlan Stenn 2014-12-04 09:34:08 UTC
Stephen,

My current thought is that the ntp codebase use wrappers for accessing random number stuff, and we use:

- OpenSSL's RAND_*() routines if they are available, otherwise
- arc4random(3), otherwise
- our local implementation of arc4random(3), noting https://lists.freebsd.org/pipermail/freebsd-bugs/2013-October/054018.html

There are still potential problems with some of the above choices.

How far should we really go here, knowing that this is likely going to be a never-ending quest?

What do you recommend?
Comment 12 Stephen Röttger 2014-12-04 10:18:15 UTC
what would the local implementation of arc4random look like?
Comment 13 Harlan Stenn 2014-12-12 10:49:56 UTC
(In reply to comment #12)
> what would the local implementation of arc4random look like?

sntp/libevent/arc4random.c

and I haven't recently looked to see if there is a newer version of that file available.
Comment 14 Harlan Stenn 2014-12-14 16:15:06 UTC
I've implemented comment #11 and am testing it now.
Comment 15 Harlan Stenn 2014-12-20 05:59:39 UTC
Fixed in 4.2.8.