Add support for NTP authentication

Brandon Enochs enochs.brandon at gmail.com
Thu Oct 11 16:40:00 UTC 2018


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 <enochs.brandon at gmail.com>
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 <
> rep.dot.nop at gmail.com> wrote:
>
>> On 11 October 2018 16:41:13 CEST, Brandon Enochs <
>> enochs.brandon at gmail.com> 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,
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20181011/41ad688f/attachment-0001.html>


More information about the busybox mailing list