[Git][NTPsec/ntpsec][master] random number cleanup
Hal Murray
gitlab at mg.gitlab.com
Wed Feb 19 11:08:44 UTC 2020
Hal Murray pushed to branch master at NTPsec / ntpsec
Commits:
59e3ed56 by Hal Murray at 2020-02-18T22:55:09-08:00
random number cleanup
remove ntp_random() - call random(3) directly.
remove ntp_random64() - no longer used
add ntp_RAND_bytes() to check return code
and avoid check/msyslog/crash at each usage of RAND_bytes()
- - - - -
12 changed files:
- attic/sht.c
- devel/hacking.adoc
- include/ntp.h
- libntp/ntp_random.c
- libntp/ssl_init.c
- ntpd/ntp_control.c
- ntpd/ntp_peer.c
- ntpd/ntp_proto.c
- ntpd/nts.c
- ntpd/nts_cookie.c
- ntpd/nts_extens.c
- tests/libntp/ntp_random.c
Changes:
=====================================
attic/sht.c
=====================================
@@ -145,7 +145,7 @@ again:
case 'w': {
/* To show some life action, we read the system
- * clock and use a bit of fuzz from 'ntp_random()' to get a
+ * clock and use a bit of fuzz from 'random()' to get a
* bit of wobbling into the values (so we can observe a
* certain jitter!)
*/
@@ -163,11 +163,11 @@ again:
else
{
time(&rcv_sec);
- rcv_frc = (unsigned int)ntp_random() % 1000000000U;
+ rcv_frc = (unsigned int)random() % 1000000000U;
}
/* add a wobble of ~3.5msec to the clock time */
clk_sec = rcv_sec;
- clk_frc = rcv_frc + (unsigned int)(ntp_random()%7094713 - 3547356);
+ clk_frc = rcv_frc + (unsigned int)(random()%7094713 - 3547356);
/* normalise result -- the SHM driver is picky! */
while ((int)clk_frc < 0) {
clk_frc += 1000000000;
=====================================
devel/hacking.adoc
=====================================
@@ -155,6 +155,37 @@ want that to happen again. Thus, if you must refer to struct timeval due to
an external interface, move the data to/from a struct timespec as
close to that call site as possible.
+=== Random numbers
+
+There are 2 types of random numbers - pseudo random and
+cryptographically secure. For cryptographically secure needs,
+we use RAND_bytes() and RAND_priv_bytes() from OpenSSL.
+The actual use is funneled through ntp_RAND_bytes() to
+check the return code and crash if it doesn't work.
+RAND_bytes() can fail if the system doesn't collect enough
+entropy but we have never seen that happen yet.
+
+For general purpose pseudo randomness, we use random(3). Note that
+it only returns 31 bits. It is much faster than RAND_bytes()
+
+RAND_MAX on FreeBSD is 0xfffffffd.
+
+Python also has 2 forms of randomness. random is older. secure
+was added in Python 3.6. We only use it in ntpkeygen.
+Random isn't cryptographically secure. secure is.
+
+=== Other Packages
+
+We depend on the host distro to provide OpenSSL.
+NTS-KE (key exchange) uses TLS-1.2 or 1.3.
+Shared key authentication uses their crypto package.
+We also use their RAND_bytes().
+
+Note that OpenSSL 1.0.1 is no longer supported. 2020-Feb-17
+
+We also use waf, libaes_siv, and unity but we have local copies
+rather than depending the host distro to provide them.
+
=== Coding style and indentation
Dr. Dave Mills liked this code indented formatted in a consistent way.
=====================================
include/ntp.h
=====================================
@@ -16,8 +16,10 @@
#include "ntp_net.h"
#include "nts.h"
-extern int32_t ntp_random (void);
-extern uint64_t ntp_random64 (void);
+/* common place to check/crash on unlikely error return */
+void ntp_RAND_bytes(unsigned char *buf, int num);
+void ntp_RAND_priv_bytes(unsigned char *buf, int num);
+
/*
* Calendar arithmetic - contributed by G. Healton
=====================================
libntp/ntp_random.c
=====================================
@@ -6,6 +6,7 @@
#include <stdint.h>
+#include <openssl/opensslv.h>
#include <openssl/rand.h>
#include "config.h"
@@ -13,32 +14,29 @@
/* NB: RAND_bytes comes from OpenSSL
* Starting in version 1.1.1, it reseeds itself occasionally.
- * That needs access to /dev/urandom which may be blocked by chroot jails.
+ * That may need access to /dev/urandom which may be blocked by chroot jails.
+ * getrandom(2) is used when available. It was added to Linux kernel 3.17
+ * so this won't be a problem on newer Linux systems.
*/
-int32_t
-ntp_random(void)
-{
+void ntp_RAND_bytes(unsigned char *buf, int num) {
int err;
- uint32_t rnd = 0;
- err = RAND_bytes((unsigned char *)&rnd, sizeof(rnd));
+ err = RAND_bytes(buf, num);
if (1 != err) {
- msyslog(LOG_ERR, "ERR: ntp_random - RAND_bytes failed");
+ msyslog(LOG_ERR, "ERR: RAND_bytes failed");
exit(1);
}
- return rnd;
}
-uint64_t
-ntp_random64(void)
-{
+void ntp_RAND_priv_bytes(unsigned char *buf, int num) {
int err;
- uint64_t rnd = 0;
- err = RAND_bytes((unsigned char *)&rnd, sizeof(rnd));
+#if (OPENSSL_VERSION_NUMBER > 0x1010100fL)
+ err = RAND_priv_bytes(buf, num);
+#else
+ err = RAND_bytes(buf, num);
+#endif
if (1 != err) {
- msyslog(LOG_ERR, "ERR: ntp_random64 - RAND_bytes failed");
+ msyslog(LOG_ERR, "ERR: RAND_priv_bytes failed");
exit(1);
}
- return rnd;
}
-
=====================================
libntp/ssl_init.c
=====================================
@@ -7,6 +7,7 @@
#include "config.h"
#include "ntp_stdlib.h"
+#include "ntp.h"
#include <stdbool.h>
#include <openssl/ssl.h>
@@ -30,9 +31,11 @@ CMAC_CTX *cmac_ctx;
void
ssl_init(void)
{
+ unsigned char dummy;
+
if (ssl_init_done) {
return;
-}
+ }
#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
SSL_library_init();
@@ -41,6 +44,10 @@ ssl_init(void)
atexit(&atexit_ssl_cleanup);
#endif
+ /* More initialization help for seccomp */
+ /* RAND_poll in OpenSSL on Raspbian needs get{u,g,eu,eg}id() */
+ ntp_RAND_bytes(&dummy, 1);
+
digest_ctx = EVP_MD_CTX_new();
cmac_ctx = CMAC_CTX_new();
ssl_init_done = true;
=====================================
ntpd/ntp_control.c
=====================================
@@ -3091,8 +3091,8 @@ static uint32_t derive_nonce(
uint32_t ts_f
)
{
- static uint32_t salt[4];
- static unsigned long last_salt_update;
+ static uint8_t salt[16];
+ static unsigned long last_salt_update = 0;
union d_tag {
uint8_t digest[EVP_MAX_MD_SIZE];
uint32_t extract;
@@ -3100,12 +3100,9 @@ static uint32_t derive_nonce(
EVP_MD_CTX *ctx;
unsigned int len;
- while (!salt[0] || current_time - last_salt_update >= SECSPERHR) {
- salt[0] = (uint32_t)ntp_random();
- salt[1] = (uint32_t)ntp_random();
- salt[2] = (uint32_t)ntp_random();
- salt[3] = (uint32_t)ntp_random();
- last_salt_update = current_time;
+ while (!last_salt_update || current_time - last_salt_update >= SECSPERHR) {
+ ntp_RAND_bytes(&salt[0], sizeof(salt));
+ last_salt_update = current_time;
}
ctx = EVP_MD_CTX_create();
@@ -3198,7 +3195,7 @@ send_random_tag_value(
int noise;
char buf[32];
- noise = ntp_random();
+ noise = random();
buf[0] = 'a' + noise % 26;
noise >>= 5;
buf[1] = 'a' + noise % 26;
@@ -3239,7 +3236,7 @@ send_mru_entry(
remaining = COUNTOF(sent);
ZERO(sent);
- noise = (uint32_t)ntp_random();
+ noise = (uint32_t)random();
while (remaining > 0) {
#ifdef USE_RANDOMIZE_RESPONSES
which = (noise & 7) % COUNTOF(sent);
@@ -3718,7 +3715,7 @@ send_ifstats_entry(
noisebits = 0;
while (remaining > 0) {
if (noisebits < 4) {
- noise = (uint32_t)ntp_random();
+ noise = (uint32_t)random();
noisebits = 31;
}
#ifdef USE_RANDOMIZE_RESPONSES
@@ -3895,7 +3892,7 @@ send_restrict_entry(
noisebits = 0;
while (remaining > 0) {
if (noisebits < 2) {
- noise = (uint32_t)ntp_random();
+ noise = (uint32_t)random();
noisebits = 31;
}
#ifdef USE_RANDOMIZE_RESPONSES
=====================================
ntpd/ntp_peer.c
=====================================
@@ -98,7 +98,7 @@ init_peer(void)
* Initialize our first association ID
*/
do
- current_association_ID = ntp_random() & ASSOCID_MAX;
+ current_association_ID = random() & ASSOCID_MAX;
while (!current_association_ID);
initial_association_ID = current_association_ID;
}
=====================================
ntpd/ntp_proto.c
=====================================
@@ -19,6 +19,7 @@
#endif
#include <unistd.h>
+
/*
* Byte order conversion
*/
@@ -477,8 +478,7 @@ static bool check_early_restrictions(
{
return (restrict_mask & RES_IGNORE) ||
((restrict_mask & RES_FLAKE) &&
-/* (double)ntp_random() / 0x7fffffff < .1) || */
- (double)random() / 0x7fffffff < .1) ||
+ (double)random() / RAND_MAX < .1) ||
(restrict_mask & (is_control_packet(rbufp) ? RES_NOQUERY : RES_DONTSERVE)) ||
rbufp->recv_length < 1 ||
((restrict_mask & RES_VERSION) &&
@@ -1071,7 +1071,7 @@ clock_update(
}
#endif /* HAVE_LIBSCF_H */
msyslog(LOG_ERR, "CLOCK: Panic: offset too big: %.3f",
- clkstate.sys_offset);
+ clkstate.sys_offset);
exit (1);
/* not reached */
@@ -1220,7 +1220,8 @@ poll_update(
next = 1U << hpoll;
else
#endif /* REFCLOCK */
-/* next = ((0x1000UL | (ntp_random() & 0x0ff)) << */
+ /* add a bit of randomess to next polling time
+ * to disperse traffic */
next = ((0x1000UL | (random() & 0x0ff)) <<
hpoll) >> 12;
next += peer->outdate;
@@ -2128,7 +2129,7 @@ peer_xmit(
sendlen = LEN_PKT_NOMAC;
if (NTP_VERSION == peer->cfg.version) {
/* Hide most of info for privacy
- * RFC in progress - draft-ietf-ntp-data-minimization, 2018-Jul-07
+ * RFC in progress - draft-ietf-ntp-data-minimization, 2020-Feb-16
*/
xpkt.li_vn_mode = PKT_LI_VN_MODE(
LEAP_NOWARNING, peer->cfg.version, MODE_CLIENT);
@@ -2141,7 +2142,8 @@ peer_xmit(
xpkt.reftime = htonl_fp(0);
xpkt.org = htonl_fp(0);
xpkt.rec = htonl_fp(0);
- peer->org_rand = ntp_random64();
+ ntp_RAND_bytes((unsigned char *)&peer->org_rand,
+ sizeof(peer->org_rand));
get_systime(&peer->org_ts); /* as late as possible */
} else {
xpkt.li_vn_mode = PKT_LI_VN_MODE(
@@ -2163,9 +2165,9 @@ peer_xmit(
xpkt.xmt = htonl_fp(peer->org_rand); /* out in xmt, back in org */
/* 3 way branch to add authentication:
- * 1) NTS
- * 2) Shared KEY
- * 3) none
+ * 1) NTS
+ * 2) Shared KEY
+ * 3) none
*/
if (FLAG_NTS & peer->cfg.flags) {
if (0 < peer->nts_state.count)
@@ -2174,7 +2176,7 @@ peer_xmit(
restart_nts_ke(peer); /* out of cookies */
return;
}
- } else if (0 != peer->cfg.peerkey) {
+ } else if (0 != peer->cfg.peerkey) {
auth_info *auth = authlookup(peer->cfg.peerkey, true);
if (NULL == auth) {
report_event(PEVNT_AUTH, peer, "no key");
=====================================
ntpd/nts.c
=====================================
@@ -46,6 +46,8 @@ void nts_log_version(void);
/*****************************************************/
+/* More SSL initialization in ssl_init() from libntp/ssl_init.c */
+
void nts_init(void) {
bool ok = true;
nts_log_version();
=====================================
ntpd/nts_cookie.c
=====================================
@@ -28,7 +28,6 @@
#include <pthread.h>
#include <unistd.h>
-#include <openssl/rand.h>
#include <aes_siv.h>
#include "ntpd.h"
@@ -243,19 +242,10 @@ bool nts_read_cookie_keys(void) {
* after a one-time copy of the cookie file from NTP server to KE server.
*/
void nts_make_cookie_key(void) {
- int err;
memcpy(&K2, &K, sizeof(K2)); /* Push current cookie to old */
I2 = I;
-#if (OPENSSL_VERSION_NUMBER > 0x1010100fL)
- err = RAND_priv_bytes(K, sizeof(K));
-#else
- err = RAND_bytes(K, sizeof(K));
-#endif
- err += RAND_bytes((uint8_t *)&I, sizeof(I));
- if (2 != err) {
- msyslog(LOG_ERR, "ERR: nts_make_cookie_key - RAND_bytes failed");
- exit(1);
- }
+ ntp_RAND_priv_bytes(K, sizeof(K));
+ ntp_RAND_bytes((uint8_t *)&I, sizeof(I));
return;
}
@@ -299,7 +289,7 @@ int nts_make_cookie(uint8_t *cookie,
uint8_t *c2s, uint8_t *s2c, int keylen) {
uint8_t plaintext[NTS_MAX_COOKIELEN];
uint8_t *nonce;
- int err, used, plainlength;
+ int used, plainlength;
bool ok;
uint8_t * finger;
uint32_t temp; /* keep 4 byte alignment */
@@ -333,11 +323,7 @@ int nts_make_cookie(uint8_t *cookie,
finger += sizeof(I);
nonce = finger;
- err = RAND_bytes(finger, NONCE_LENGTH);
- if (1 != err) {
- msyslog(LOG_ERR, "ERR: nts_make_cookie - Error from RAND_bytes");
- exit(1);
- }
+ ntp_RAND_bytes(finger, NONCE_LENGTH);
finger += NONCE_LENGTH;
used = finger-cookie;
=====================================
ntpd/nts_extens.c
=====================================
@@ -18,7 +18,6 @@
#include <stdint.h>
#include <string.h>
-#include <openssl/rand.h>
#include <aes_siv.h>
#include "ntp_stdlib.h"
@@ -63,7 +62,7 @@ bool extens_init(void) {
int extens_client_send(struct peer *peer, struct pkt *xpkt) {
struct BufCtl_t buf;
- int err, used, adlength, idx;
+ int used, adlength, idx;
size_t left;
uint8_t *nonce, *packet;
bool ok;
@@ -73,11 +72,7 @@ int extens_client_send(struct peer *peer, struct pkt *xpkt) {
buf.left = MAX_EXT_LEN;
/* UID */
- err = RAND_bytes(peer->nts_state.UID, NTS_UID_LENGTH);
- if (1 != err) {
- msyslog(LOG_ERR, "ERR: extens_client_send - RAND_bytes failed");
- exit(1);
- }
+ ntp_RAND_bytes(peer->nts_state.UID, NTS_UID_LENGTH);
ex_append_record_bytes(&buf, Unique_Identifier,
peer->nts_state.UID, NTS_UID_LENGTH);
@@ -103,11 +98,7 @@ int extens_client_send(struct peer *peer, struct pkt *xpkt) {
append_uint16(&buf, NONCE_LENGTH);
append_uint16(&buf, CMAC_LENGTH);
nonce = buf.next;
- err = RAND_bytes(nonce, NONCE_LENGTH);
- if (1 != err) {
- msyslog(LOG_ERR, "ERR: extens_client_send - RAND_bytes failed");
- exit(1);
- }
+ ntp_RAND_bytes(nonce, NONCE_LENGTH);
buf.next += NONCE_LENGTH;
buf.left -= NONCE_LENGTH;
left = buf.left;
@@ -267,7 +258,7 @@ int extens_server_send(struct ntspacket_t *ntspacket, struct pkt *xpkt) {
uint8_t *nonce, *packet;
uint8_t *plaintext, *ciphertext;;
uint8_t cookie[NTS_MAX_COOKIELEN];
- int err, cookielen, plainleng, aeadlen;
+ int cookielen, plainleng, aeadlen;
bool ok;
/* get first cookie now so we have length */
@@ -294,11 +285,7 @@ int extens_server_send(struct ntspacket_t *ntspacket, struct pkt *xpkt) {
append_uint16(&buf, plainleng+CMAC_LENGTH);
nonce = buf.next;
- err = RAND_bytes(nonce, NONCE_LENGTH);
- if (1 != err) {
- msyslog(LOG_ERR, "ERR: extens_client_send - RAND_bytes failed");
- exit(1);
- }
+ ntp_RAND_bytes(nonce, NONCE_LENGTH);
buf.next += NONCE_LENGTH;
buf.left -= NONCE_LENGTH;
=====================================
tests/libntp/ntp_random.c
=====================================
@@ -10,10 +10,12 @@ TEST_SETUP(random) {}
TEST_TEAR_DOWN(random) {}
-
+/* leftover from testing ntp_random()
+ * random(3) is 31 bits.
+ */
TEST(random, random32) {
uint32_t ones = 0;
- uint32_t zeros = ~0;
+ uint32_t zeros = RAND_MAX;
/* This is just a crude sanity check.
* It could fail when working correctly,
@@ -22,18 +24,29 @@ TEST(random, random32) {
* You can test this code by making the loop count smaller.
*/
for (int i=0; i<99; i++) {
- uint32_t sample = ntp_random();
+ uint32_t sample = random();
ones |= sample;
zeros &= sample;
}
- TEST_ASSERT_EQUAL_INT32(~0, ones);
+ /* RAND_MAX on FreeBSD is 0x7ffffffd */
+ TEST_ASSERT_EQUAL_INT32(0x7fffffff, ones);
TEST_ASSERT_EQUAL_INT32(0, zeros);
}
-TEST(random, random64) {
- uint64_t ones = 0;
- uint64_t zeros = ~0;
+TEST(random, random_bytes) {
+#define BYTES 100
+ unsigned char zeros[BYTES]; /* collected zeros */
+ unsigned char ones[BYTES]; /* collected ones */
+ unsigned char clear[BYTES]; /* expected all zeros */
+ unsigned char full[BYTES]; /* expected all ones */
+
+ for (int j=0; j<BYTES; j++) {
+ zeros[j] = ~0;
+ ones[j] = 0;
+ clear[j] = 0;
+ full[j] = ~0;
+ }
/* This is just a crude sanity check.
* It could fail when working correctly,
@@ -42,16 +55,19 @@ TEST(random, random64) {
* You can test this code by making the loop count smaller.
*/
for (int i=0; i<99; i++) {
- uint64_t sample = ntp_random64();
- ones |= sample;
- zeros &= sample;
+ unsigned char sample[BYTES];
+ ntp_RAND_bytes(&sample[0], BYTES);
+ for (int j=0; j<BYTES; j++) {
+ zeros[j] &= ~sample[j];
+ ones[j] |= sample[j];
+ }
}
- TEST_ASSERT_EQUAL_INT64(~0, ones);
- TEST_ASSERT_EQUAL_INT64(0, zeros);
+ TEST_ASSERT_EQUAL_MEMORY(full, ones, BYTES);
+ TEST_ASSERT_EQUAL_MEMORY(clear, zeros, BYTES);
}
TEST_GROUP_RUNNER(random) {
RUN_TEST_CASE(random, random32);
- RUN_TEST_CASE(random, random64);
+ RUN_TEST_CASE(random, random_bytes);
}
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/commit/59e3ed5615c442ff98e317984e4054d96b627be9
--
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/commit/59e3ed5615c442ff98e317984e4054d96b627be9
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20200219/f6e4a1ff/attachment-0001.htm>
More information about the vc
mailing list