OK, I added config support to disable NTP authentication and used unconditional frees.
On Thu, Oct 11, 2018 at 10:40 AM Brandon Enochs <enochs.bran...@gmail.com> wrote: > diff --git a/networking/ntpd.c b/networking/ntpd.c > index 1ebdc34..62fc52c 100644 > --- a/networking/ntpd.c > +++ b/networking/ntpd.c > @@ -62,6 +62,24 @@ > //config: help > //config: Make ntpd look in /etc/ntp.conf for peers. Only "server > address" > //config: is supported. > +//config:config FEATURE_NTP_AUTH > +//config: bool "Make ntpd support authentication" > +//config: default n > +//config: depends on NTPD > +//config: help > +//config: Make ntpd support authentication" > +//config:config FEATURE_NTP_AUTH_MD5 > +//config: bool "Make ntpd support MD5 authentication" > +//config: default n > +//config: depends on FEATURE_NTP_AUTH > +//config: help > +//config: Make ntpd support MD5 authentication" > +//config:config FEATURE_NTP_AUTH_SHA1 > +//config: bool "Make ntpd support SHA1 authentication" > +//config: default n > +//config: depends on FEATURE_NTP_AUTH > +//config: help > +//config: Make ntpd support SHA1 authentication" > > //applet:IF_NTPD(APPLET(ntpd, BB_DIR_USR_SBIN, BB_SUID_DROP)) > > @@ -78,6 +96,8 @@ > > //usage: "\n -w Do not set time (only query peers), implies -n" > //usage: "\n -S PROG Run PROG after stepping time, stratum > change, and every 11 mins" > //usage: "\n -p PEER Obtain time from PEER (may be repeated)" > +//usage: "\n -K key number for preceding PEER (may be repeated)" > +//usage: "\n -k key file (see man 5 ntp.keys)" > //usage: IF_FEATURE_NTPD_CONF( > //usage: "\n If -p is not given, 'server HOST' lines" > //usage: "\n from /etc/ntp.conf are used" > @@ -227,15 +247,24 @@ > > #define FLL (MAXPOLL + 1) > /* Parameter averaging constant */ > #define AVG 4 > - > +#define MAX_KEY_NUMBER 65535 > +#define KEYID_SIZE sizeof(uint32_t) > > enum { > NTP_VERSION = 4, > NTP_MAXSTRATUM = 15, > > +#if ENABLE_FEATURE_NTP_AUTH > > + NTP_MD5_DIGESTSIZE = 16, > + NTP_SHA1_DIGESTSIZE = 20, > + NTP_MSGSIZE_NOAUTH = 48, > + NTP_MSGSIZE_MD5_AUTH = NTP_MSGSIZE_NOAUTH + NTP_MD5_DIGESTSIZE + > KEYID_SIZE, > + NTP_MSGSIZE_SHA1_AUTH = NTP_MSGSIZE_NOAUTH + NTP_SHA1_DIGESTSIZE + > KEYID_SIZE, > +#else > NTP_DIGESTSIZE = 16, > NTP_MSGSIZE_NOAUTH = 48, > > NTP_MSGSIZE = (NTP_MSGSIZE_NOAUTH + 4 + NTP_DIGESTSIZE), > +#endif > > > /* Status Masks */ > MODE_MASK = (7 << 0), > @@ -288,7 +317,11 @@ typedef struct { > > l_fixedpt_t m_rectime; > l_fixedpt_t m_xmttime; > uint32_t m_keyid; > +#if ENABLE_FEATURE_NTP_AUTH > + uint8_t m_digest[NTP_SHA1_DIGESTSIZE]; > +#else > uint8_t m_digest[NTP_DIGESTSIZE]; > +#endif > } msg_t; > > typedef struct { > @@ -297,6 +330,26 @@ typedef struct { > double d_dispersion; > } datapoint_t; > > +#if ENABLE_FEATURE_NTP_AUTH > +enum hash_type { > + HASH_NONE, > +#if ENABLE_FEATURE_NTP_AUTH_MD5 > + HASH_MD5, > +#endif > +#if ENABLE_FEATURE_NTP_AUTH_SHA1 > + HASH_SHA1 > +#endif > > +}; > + > +typedef struct { > + unsigned id; > + enum hash_type type; > + char *key; > + size_t key_length; > + size_t msg_size; > +} key_entry_t; > +#endif > > + > typedef struct { > len_and_sockaddr *p_lsa; > char *p_dotted; > @@ -326,6 +379,9 @@ typedef struct { > > /* last sent packet: */ > msg_t p_xmt_msg; > char p_hostname[1]; > +#if ENABLE_FEATURE_NTP_AUTH > + key_entry_t *key_entry; > +#endif > } peer_t; > > > @@ -337,13 +393,25 @@ enum { > > OPT_q = (1 << 1), > OPT_N = (1 << 2), > OPT_x = (1 << 3), > +#if ENABLE_FEATURE_NTP_AUTH > > + OPT_k = (1 << 4), > + OPT_K = (1 << 5), > +#endif > > /* Insert new options above this line. */ > /* Non-compat options: */ > +#if ENABLE_FEATURE_NTP_AUTH > > + OPT_w = (1 << 6), > + OPT_p = (1 << 7), > + OPT_S = (1 << 8), > + OPT_l = (1 << 9) * ENABLE_FEATURE_NTPD_SERVER, > + OPT_I = (1 << 10) * ENABLE_FEATURE_NTPD_SERVER, > +#else > OPT_w = (1 << 4), > OPT_p = (1 << 5), > OPT_S = (1 << 6), > > OPT_l = (1 << 7) * ENABLE_FEATURE_NTPD_SERVER, > OPT_I = (1 << 8) * ENABLE_FEATURE_NTPD_SERVER, > +#endif > > /* We hijack some bits for other purposes */ > OPT_qq = (1 << 31), > }; > @@ -816,8 +884,21 @@ resolve_peer_hostname(peer_t *p) > return lsa; > } > > +#if ENABLE_FEATURE_NTP_AUTH > +static void > > +free_key_entry(void *data) { > + key_entry_t *key_entry = (key_entry_t *) data; > + > + free(key_entry->key); > + free(key_entry); > +} > + > +static void > +add_peers(const char *s, key_entry_t *key_entry) > +#else > static void > add_peers(const char *s) > +#endif > { > llist_t *item; > peer_t *p; > @@ -840,12 +921,18 @@ add_peers(const char *s) > > bb_error_msg("duplicate peer %s (%s)", s, p->p_dotted); > free(p->p_lsa); > free(p->p_dotted); > +#if ENABLE_FEATURE_NTP_AUTH > + free_key_entry(p->key_entry); > +#endif > free(p); > return; > } > } > } > > +#if ENABLE_FEATURE_NTP_AUTH > > + p->key_entry = key_entry; > +#endif > llist_add_to(&G.ntp_peers, p); > G.peer_cnt++; > } > @@ -870,6 +957,80 @@ do_sendto(int fd, > return 0; > } > > +#if ENABLE_FEATURE_NTP_AUTH > +static void > > +hash(peer_t *p) { > + size_t hash_size = sizeof(msg_t) - sizeof(p->p_xmt_msg.m_keyid) - > sizeof(p->p_xmt_msg.m_digest); > + > + p->p_xmt_msg.m_keyid = htonl(p->key_entry->id); > + switch (p->key_entry->type) { > +#if ENABLE_FEATURE_NTP_AUTH_MD5 > > + case HASH_MD5: { > + md5_ctx_t ctx; > + > + md5_begin(&ctx); > + md5_hash(&ctx, p->key_entry->key, p->key_entry->key_length); > + md5_hash(&ctx, &p->p_xmt_msg, hash_size); > + md5_end(&ctx, &p->p_xmt_msg.m_digest); > + } > + break; > +#endif > +#if ENABLE_FEATURE_NTP_AUTH_SHA1 > > + case HASH_SHA1: { > + sha1_ctx_t ctx; > + > + sha1_begin(&ctx); > + sha1_hash(&ctx, p->key_entry->key, p->key_entry->key_length); > + sha1_hash(&ctx, &p->p_xmt_msg, hash_size); > + sha1_end(&ctx, &p->p_xmt_msg.m_digest); > + } > + break; > +#endif > + case HASH_NONE: > + bb_perror_msg_and_die("ntpd is in an invalid state."); > > + break; > + } > +} > + > +static int compare_hashes(peer_t *p, const msg_t *msg) { > + size_t hash_size = sizeof(msg_t) - sizeof(msg->m_keyid) - > sizeof(msg->m_digest); > + key_entry_t *key_entry = p->key_entry; > + int result = 1; > + > + switch (p->key_entry->type) { > +#if ENABLE_FEATURE_NTP_AUTH_MD5 > > + case HASH_MD5: { > + md5_ctx_t ctx; > + char digest[NTP_MD5_DIGESTSIZE]; > + > + md5_begin(&ctx); > + md5_hash(&ctx, key_entry->key, key_entry->key_length); > + md5_hash(&ctx, msg, hash_size); > + md5_end(&ctx, &digest); > + result = memcmp(digest, msg->m_digest, NTP_MD5_DIGESTSIZE); > + } > + break; > +#endif > +#if ENABLE_FEATURE_NTP_AUTH_SHA1 > > + case HASH_SHA1: { > + sha1_ctx_t ctx; > + char digest[NTP_SHA1_DIGESTSIZE]; > + > + sha1_begin(&ctx); > + sha1_hash(&ctx, key_entry->key, key_entry->key_length); > + sha1_hash(&ctx, msg, hash_size); > + sha1_end(&ctx, &digest); > + result = memcmp(digest, msg->m_digest, NTP_SHA1_DIGESTSIZE); > + } > + break; > +#endif > + case HASH_NONE: > + bb_perror_msg_and_die("ntpd is in an invalid state."); > > + } > + return result; > +} > +#endif > + > static void > send_query_to_peer(peer_t *p) > { > @@ -946,9 +1107,18 @@ send_query_to_peer(peer_t *p) > > */ > p->reachable_bits <<= 1; > > +#if ENABLE_FEATURE_NTP_AUTH > > + if (p->key_entry != NULL) { > + hash(p); > + } > + if (do_sendto(p->p_fd, /*from:*/ NULL, /*to:*/ &p->p_lsa->u.sa, > /*addrlen:*/ p->p_lsa->len, > + &p->p_xmt_msg, p->key_entry == NULL ? NTP_MSGSIZE_NOAUTH : > p->key_entry->msg_size) == -1 > + ) { > +#else > > if (do_sendto(p->p_fd, /*from:*/ NULL, /*to:*/ &p->p_lsa->u.sa, > /*addrlen:*/ p->p_lsa->len, > &p->p_xmt_msg, NTP_MSGSIZE_NOAUTH) == -1 > ) { > +#endif > > close(p->p_fd); > p->p_fd = -1; > /* > @@ -1924,10 +2094,21 @@ recv_and_process_peer_pkt(peer_t *p) > > bb_perror_msg_and_die("recv(%s) error", p->p_dotted); > } > > +#if ENABLE_FEATURE_NTP_AUTH > > + if (size != NTP_MSGSIZE_NOAUTH && size != NTP_MSGSIZE_MD5_AUTH && > size != NTP_MSGSIZE_SHA1_AUTH) { > + bb_error_msg("malformed packet received from %s", p->p_dotted); > + return; > + } > > + if (p->key_entry != NULL && compare_hashes(p, &msg) != 0) { > + bb_error_msg("invalid cryptographic hash received from %s", > p->p_dotted); > + return; > + } > +#else > > if (size != NTP_MSGSIZE_NOAUTH && size != NTP_MSGSIZE) { > bb_error_msg("malformed packet received from %s", p->p_dotted); > return; > } > +#endif > > > if (msg.m_orgtime.int_partl != p->p_xmt_msg.m_xmttime.int_partl > || msg.m_orgtime.fractionl != p->p_xmt_msg.m_xmttime.fractionl > @@ -2135,7 +2316,11 @@ recv_and_process_client_pkt(void /*int fd*/) > > from = xzalloc(to->len); > > size = recv_from_to(G_listen_fd, &msg, sizeof(msg), MSG_DONTWAIT, > from, &to->u.sa, to->len); > +#if ENABLE_FEATURE_NTP_AUTH > > + if (size != NTP_MSGSIZE_NOAUTH && size != NTP_MSGSIZE_SHA1_AUTH && > size != NTP_MSGSIZE_MD5_AUTH) { > +#else > > if (size != NTP_MSGSIZE_NOAUTH && size != NTP_MSGSIZE) { > +#endif > > char *addr; > if (size < 0) { > if (errno == EAGAIN) > @@ -2278,6 +2463,28 @@ recv_and_process_client_pkt(void /*int fd*/) > * with the -g and -q options. See the tinker command for other > options. > > * Note: The kernel time discipline is disabled with this option. > */ > +#if ENABLE_FEATURE_NTP_AUTH > > +static key_entry_t * > +find_key_entry(llist_t *key_entries, unsigned id) { > + key_entry_t *result = NULL; > + llist_t *iterator = key_entries; > + key_entry_t *temporary; > + > + while(iterator != NULL && result == NULL) { > + temporary = (key_entry_t *) iterator->data; > + if(temporary->id == id) { > + result = xzalloc(sizeof(key_entry_t)); > + result->id = temporary->id; > + result->type = temporary->type; > + result->key = xstrdup(temporary->key); > + result->key_length = temporary->key_length; > + result->msg_size = temporary->msg_size; > + } > + iterator = iterator->link; > + } > + return result; > +} > +#endif > > > /* By doing init in a separate function we decrease stack usage > * in main loop. > @@ -2286,6 +2493,11 @@ static NOINLINE void ntp_init(char **argv) > { > unsigned opts; > llist_t *peers; > +#if ENABLE_FEATURE_NTP_AUTH > > + llist_t *key_ids; > + llist_t *key_entries = NULL; > + char *key_file_path = NULL; > +#endif > > srand(getpid()); > > @@ -2304,6 +2516,9 @@ static NOINLINE void ntp_init(char **argv) > > peers = NULL; > opts = getopt32(argv, "^" > "nqNx" /* compat */ > +#if ENABLE_FEATURE_NTP_AUTH > + "K:*k:" > +#endif > > "wp:*S:"IF_FEATURE_NTPD_SERVER("l") /* NOT compat */ > IF_FEATURE_NTPD_SERVER("I:") /* compat */ > "d" /* compat */ > @@ -2311,6 +2526,9 @@ static NOINLINE void ntp_init(char **argv) > > "\0" > "dd:wn" /* -d: counter; -p: list; -w implies -n */ > IF_FEATURE_NTPD_SERVER(":Il") /* -I implies -l */ > +#if ENABLE_FEATURE_NTP_AUTH > + , &key_ids, &key_file_path > +#endif > > , &peers, &G.script_name, > #if ENABLE_FEATURE_NTPD_SERVER > &G.if_name, > @@ -2341,9 +2559,79 @@ static NOINLINE void ntp_init(char **argv) > logmode = LOGMODE_NONE; > } > > +#if ENABLE_FEATURE_NTP_AUTH > > + if (key_ids && !(opts & OPT_k)) { > + bb_error_msg_and_die("A key file (-k) is required to use a key > (-K) option"); > + } > + > + if (opts & OPT_k) { > + char *tokens[4]; > + parser_t *key_file_parser; > + int token_count; > + > + key_file_parser = config_open(key_file_path); > + while ((token_count = config_read(key_file_parser, tokens, 4, 3, > "# \t", PARSE_NORMAL | PARSE_MIN_DIE)) == 3) { > + enum hash_type hash_type = HASH_NONE; > > + size_t key_length; > + key_entry_t *key_entry = (key_entry_t *) > xzalloc(sizeof(key_entry_t)); > + > +#if ENABLE_FEATURE_NTP_AUTH_MD5 > > + if (strcasecmp(tokens[1], "MD5") == 0) { > + hash_type = HASH_MD5; > + key_entry->msg_size = NTP_MSGSIZE_MD5_AUTH; > + } > +#endif > +#if ENABLE_FEATURE_NTP_AUTH_SHA1 > + if (strcasecmp(tokens[1], "SHA1") == 0) { > > + hash_type = HASH_SHA1; > + key_entry->msg_size = NTP_MSGSIZE_SHA1_AUTH; > + } > +#endif > + if (hash_type == HASH_NONE) { > > + bb_error_msg_and_die("busybox only supports MD5 and SHA1 > NTP keys"); > + } > + key_entry->type = hash_type; > + key_entry->id = xatou_range(tokens[0], 1, MAX_KEY_NUMBER); > + key_length = strlen(tokens[2]); > + if (key_length <= 20) { > + key_entry->key = xstrdup(tokens[2]); > + key_entry->key_length = key_length; > + } else if ((key_length & 1) == 0) { > + char buffer[64]; > + char *key; > + size_t byte_count = key_length / 2; > + > + memset(buffer, 0, sizeof(buffer)); > + key = hex2bin(buffer, tokens[2], byte_count); > + if (key == NULL) { > + bb_error_msg_and_die("key #%d is malformed", > key_entry->id); > + } > + key_entry->key = xstrdup(buffer); > + key_entry->key_length = byte_count; > + } else { > + bb_error_msg_and_die("key #%d is malformed", > key_entry->id); > + } > + llist_add_to(&key_entries, key_entry); > + } > + config_close(key_file_parser); > + } > + > +#endif > > if (peers) { > - while (peers) > + while (peers) { > +#if ENABLE_FEATURE_NTP_AUTH > > + key_entry_t *key_entry = NULL; > + > + if (key_ids) { > + int key_id = xatou_range(llist_pop(&key_ids), 1, > MAX_KEY_NUMBER); > + > + key_entry = find_key_entry(key_entries, key_id); > + } > + add_peers(llist_pop(&peers), key_entry); > +#else > add_peers(llist_pop(&peers)); > +#endif > + } > } > #if ENABLE_FEATURE_NTPD_CONF > else { > @@ -2353,7 +2641,11 @@ static NOINLINE void ntp_init(char **argv) > > parser = config_open("/etc/ntp.conf"); > while (config_read(parser, token, 3, 1, "# \t", PARSE_NORMAL)) { > if (strcmp(token[0], "server") == 0 && token[1]) { > +#if ENABLE_FEATURE_NTP_AUTH > > + add_peers(token[1], NULL); > +#else > add_peers(token[1]); > +#endif > > continue; > } > bb_error_msg("skipping %s:%u: unimplemented command '%s'", > @@ -2394,6 +2686,9 @@ static NOINLINE void ntp_init(char **argv) > > | (1 << SIGCHLD) > , SIG_IGN > ); > +#if ENABLE_FEATURE_NTP_AUTH > + llist_free(key_entries, free_key_entry); > +#endif > > } > > int ntpd_main(int argc UNUSED_PARAM, char **argv) MAIN_EXTERNALLY_VISIBLE; > > On Sat, Oct 6, 2018 at 11:59 AM Bernhard Reutner-Fischer < > rep.dot....@gmail.com> wrote: > >> On 6 October 2018 11:54:00 CEST, Xabier Oneca -- xOneca < >> xon...@gmail.com> wrote: >> >Hi Brandon, >> > >> >> It's 4k of bloat. Do you want a configuration option? >> > >> >Thanks Brandon. I think 4k is a lot of bloat for a rarely used >> >function to be enabled by default. >> >> Agree. >> Also please free() unconditionally, it's perfectly fine to free(NULL). >> >> I would add options to turn on separately each of MD5 and SHA1 >> (resp.256). Also I'd throw in RFC5906 to the discussion, FWIW ;) >> >
_______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox