Which free_keys function are you talking about? On Thu, Oct 11, 2018 at 12:40 PM Brandon Enochs <[email protected]> wrote:
> OK, I extracted a function to perform the hashing. Here are the > bloat-o-meter results: > function old new delta > .rodata 166664 166934 +270 > > ------------------------------------------------------------------------------ > (add/remove: 0/0 grow/shrink: 1/0 up/down: 270/0) Total: 270 > bytes > > > > On Thu, Oct 11, 2018 at 12:38 PM Brandon Enochs <[email protected]> > wrote: > >> diff --git a/networking/ntpd.c b/networking/ntpd.c >> index 1ebdc34..ff9ac5d 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(key_entry_t *key_entry, const msg_t *msg, uint8_t *output) >> +{ >> >> + size_t hash_size = sizeof(msg_t) - sizeof(msg->m_keyid) - >> sizeof(msg->m_digest); >> + >> + switch (key_entry->type) { >> >> +#if ENABLE_FEATURE_NTP_AUTH_MD5 >> + case HASH_MD5: { >> + md5_ctx_t ctx; >> + >> + md5_begin(&ctx); >> + md5_hash(&ctx, key_entry->key, key_entry->key_length); >> + md5_hash(&ctx, msg, hash_size); >> + md5_end(&ctx, output); >> >> + } >> + break; >> +#endif >> +#if ENABLE_FEATURE_NTP_AUTH_SHA1 >> + case HASH_SHA1: { >> + sha1_ctx_t ctx; >> + >> + sha1_begin(&ctx); >> + sha1_hash(&ctx, key_entry->key, key_entry->key_length); >> + sha1_hash(&ctx, msg, hash_size); >> + sha1_end(&ctx, output); >> >> + } >> + break; >> +#endif >> + case HASH_NONE: >> + bb_perror_msg_and_die("ntpd is in an invalid state."); >> + break; >> + } >> +} >> + >> +static void >> +hash_peer(peer_t *p) >> >> +{ >> + p->p_xmt_msg.m_keyid = htonl(p->key_entry->id); >> + hash(p->key_entry, &p->p_xmt_msg, p->p_xmt_msg.m_digest); >> >> +} >> + >> +static int compare_hashes(peer_t *p, const msg_t *msg) >> +{ >> >> + int result = 1; >> + >> + switch (p->key_entry->type) { >> +#if ENABLE_FEATURE_NTP_AUTH_MD5 >> + case HASH_MD5: { >> + uint8_t digest[NTP_MD5_DIGESTSIZE]; >> + >> + hash(p->key_entry, msg, digest); >> >> + result = memcmp(digest, msg->m_digest, NTP_MD5_DIGESTSIZE); >> + } >> + break; >> +#endif >> +#if ENABLE_FEATURE_NTP_AUTH_SHA1 >> + case HASH_SHA1: { >> + uint8_t digest[NTP_SHA1_DIGESTSIZE]; >> + >> + hash(p->key_entry, msg, digest); >> >> + result = memcmp(digest, msg->m_digest, NTP_SHA1_DIGESTSIZE); >> + } >> + break; >> +#endif >> + case HASH_NONE: >> + default: >> >> + 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_peer(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 Thu, Oct 11, 2018 at 11:22 AM Bernhard Reutner-Fischer < >> [email protected]> wrote: >> >>> On 11 October 2018 16:41:13 CEST, Brandon Enochs < >>> [email protected]> wrote: >>> >OK, I added config support to disable NTP authentication and used >>> >unconditional frees. >>> >>> hash() and compare_hash() look redundant. Can you use the former to >>> implement the latter? >>> >>> free_keys could maybe use llist_free() (sp?) if you'd change the keys to >>> be list elts? >>> >>> And please always provide bloat-o-meter output: >>> make baseline (with pristine master) >>> ./scripts/bloat-o-meter.py old yours, see make bloat-check (IIRC). >>> >>> thanks, >>> >>
_______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
