From: Brian Inglis Date: Thu, 23 Feb 2017 05:53:09 +0000 (+0000) Subject: increase struct key member type size from 10 to 16 to allow AES128CMAC and future... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=89fbc1546c0548a7a2ece574f9183a96a210646e;p=thirdparty%2Fntp.git increase struct key member type size from 10 to 16 to allow AES128CMAC and future longer types up to 15 chars, refactor ntpq digest list CMAC insertion into function, fix packetProcessing tests, add debug output to check CMAC working bk: 58ae78c5VOs33PiAYA5cC-xIMkHupA --- diff --git a/libntp/ssl_init.c b/libntp/ssl_init.c index 90dd73080..71f2eda04 100644 --- a/libntp/ssl_init.c +++ b/libntp/ssl_init.c @@ -124,6 +124,11 @@ keytype_from_text( if (!key_type && !strncmp(CMAC, upcased, strlen(CMAC) + 1)) { key_type = NID_cmac; + + if (debug) { + fprintf(stderr, "%s:%d:%s():%s:key\n", + __FILE__, __LINE__, __func__, CMAC); + } } #else key_type = 0; @@ -146,6 +151,11 @@ keytype_from_text( if (!md || digest_len <= 0) { if (key_type == NID_cmac) { digest_len = CMAC_LENGTH; + + if (debug) { + fprintf(stderr, "%s:%d:%s():%s:len\n", + __FILE__, __LINE__, __func__, CMAC); + } } else { fprintf(stderr, "key type %s is not supported by OpenSSL\n", @@ -197,6 +207,11 @@ keytype_name( if (NID_cmac == nid) { name = CMAC; + + if (debug) { + fprintf(stderr, "%s:%d:%s():%s:nid\n", + __FILE__, __LINE__, __func__, CMAC); + } } else if (NULL == name) { name = unknown_type; diff --git a/ntpq/ntpq.c b/ntpq/ntpq.c index 8ae3834cf..86178fb5b 100644 --- a/ntpq/ntpq.c +++ b/ntpq/ntpq.c @@ -2,10 +2,11 @@ * ntpq - query an NTP server using mode 6 commands */ #include -#include #include #include #include +#include +#include #include #include #ifdef HAVE_UNISTD_H @@ -235,6 +236,7 @@ static void list_md_fn(const EVP_MD *m, const char *from, const char *to, void *arg ); # endif #endif +static char *insert_cmac(char *list); static char *list_digest_names(void); /* @@ -452,13 +454,6 @@ main( } #endif -#ifdef OPENSSL -# ifdef HAVE_EVP_MD_DO_ALL_SORTED -# define K_PER_LINE 8 -# define K_NL_PFX_STR "\n " -# define K_DELIM_STR ", " -# endif -#endif #ifndef BUILD_AS_LIB int @@ -469,13 +464,6 @@ ntpqmain( { u_int ihost; size_t icmd; -#ifdef OPENSSL -# ifdef HAVE_EVP_MD_DO_ALL_SORTED - int nl; - int append; - size_t len; -# endif -#endif #ifdef SYS_VXWORKS @@ -501,69 +489,19 @@ ntpqmain( char *msg; list = list_digest_names(); - for (icmd = 0; icmd < sizeof(builtins)/sizeof(builtins[0]); icmd++) { - if (strcmp("keytype", builtins[icmd].keyword) == 0) + + for (icmd = 0; icmd < sizeof(builtins)/sizeof(*builtins); icmd++) { + if (strcmp("keytype", builtins[icmd].keyword) == 0) { break; + } } /* CID: 1295478 */ /* This should only "trip" if "keytype" is removed from builtins */ - INSIST(icmd < sizeof(builtins)/sizeof(builtins[0])); + INSIST(icmd < sizeof(builtins)/sizeof(*builtins)); #ifdef OPENSSL builtins[icmd].desc[0] = "digest-name"; - -# ifdef HAVE_EVP_MD_DO_ALL_SORTED - /* Append CMAC to SSL digests */ - - /* If list empty, we need to append CMAC and new line */ - append = nl = (!list || !*list); - - if (append) { - len = strlen(K_NL_PFX_STR) + strlen(CMAC); - list = (char *)erealloc(list, len + 1); - list[0] = '\0'; - } else { - /* Check if CMAC already in list */ - const char *cmac_sn; - char *cmac_p; - - cmac_sn = OBJ_nid2sn(NID_cmac); - cmac_p = strstr(list, cmac_sn); - - /* CMAC in list if found followed by null or "," */ - if (cmac_p) { - cmac_p += strlen(cmac_sn); - } - - append = !(cmac_p && (!*cmac_p || ',' == *cmac_p)); - - if (append) { - char *last_nl; - - len = strlen(list) + strlen(CMAC); - /* Check if new entry will fit on last line */ - last_nl = strrchr(list, '\n'); - - if (!last_nl) { - last_nl = list; - } - - /* Do we need a new line? */ - nl = (len - (last_nl - list) + strlen(K_DELIM_STR) > 72); - len += (nl) ? strlen(K_NL_PFX_STR) : strlen(K_DELIM_STR); - list = (char *)erealloc(list, len + 1); - } - } - - /* Check if we need to append an entry */ - if (append) { - sprintf(list + strlen(list), "%s%s", - ((nl) ? K_NL_PFX_STR : K_DELIM_STR), - CMAC); - } -# endif - my_easprintf(&msg, "set key type to use for authenticated requests, one of:%s", list); @@ -3639,6 +3577,10 @@ ntpq_custom_opt_handler( #ifdef OPENSSL # ifdef HAVE_EVP_MD_DO_ALL_SORTED +# define K_PER_LINE 8 +# define K_NL_PFX_STR "\n " +# define K_DELIM_STR ", " + struct hstate { char *list; const char **seen; @@ -3646,71 +3588,185 @@ struct hstate { }; -static void list_md_fn(const EVP_MD *m, const char *from, const char *to, void *arg ) +static void +list_md_fn(const EVP_MD *m, const char *from, const char *to, void *arg) { size_t len, n; const char *name, *cp, **seen; struct hstate *hstate = arg; - EVP_MD_CTX *ctx; - u_int digest_len; - u_char digest[EVP_MAX_MD_SIZE]; - if (!m) + /* m is MD obj, from is name or alias, to is base name for alias */ + if (!m || !from || to) { return; /* Ignore aliases */ + } + + /* Discard MACs that NTP won't accept. */ + /* Keep this consistent with keytype_from_text() in ssl_init.c. */ + if (EVP_MD_size(m) > (MAX_MAC_LEN - sizeof(keyid_t))) { + return; + } name = EVP_MD_name(m); /* Lowercase names aren't accepted by keytype_from_text in ssl_init.c */ - for( cp = name; *cp; cp++ ) { - if( islower((unsigned char)*cp) ) + for (cp = name; *cp; cp++) { + if (islower((unsigned char)*cp)) { return; + } } + len = (cp - name) + 1; /* There are duplicates. Discard if name has been seen. */ - for (seen = hstate->seen; *seen; seen++) - if (!strcmp(*seen, name)) + for (seen = hstate->seen; *seen; seen++) { + if (!strcmp(*seen, name)) { return; + } + } + n = (seen - hstate->seen) + 2; hstate->seen = erealloc(hstate->seen, n * sizeof(*seen)); hstate->seen[n-2] = name; hstate->seen[n-1] = NULL; - /* Discard MACs that NTP won't accept. - * Keep this consistent with keytype_from_text() in ssl_init.c. - */ - - ctx = EVP_MD_CTX_new(); - EVP_DigestInit(ctx, EVP_get_digestbyname(name)); - EVP_DigestFinal(ctx, digest, &digest_len); - EVP_MD_CTX_free(ctx); - if (digest_len > (MAX_MAC_LEN - sizeof(keyid_t))) - return; - - if (hstate->list != NULL) + if (hstate->list != NULL) { len += strlen(hstate->list); - len += (hstate->idx >= K_PER_LINE)? strlen(K_NL_PFX_STR): strlen(K_DELIM_STR); + } + + len += (hstate->idx >= K_PER_LINE) + ? strlen(K_NL_PFX_STR) + : strlen(K_DELIM_STR); if (hstate->list == NULL) { hstate->list = (char *)emalloc(len); hstate->list[0] = '\0'; - } else + } else { hstate->list = (char *)erealloc(hstate->list, len); + } sprintf(hstate->list + strlen(hstate->list), "%s%s", - ((hstate->idx >= K_PER_LINE)? K_NL_PFX_STR : K_DELIM_STR), + ((hstate->idx >= K_PER_LINE) ? K_NL_PFX_STR : K_DELIM_STR), name); - if (hstate->idx >= K_PER_LINE) + + if (hstate->idx >= K_PER_LINE) { hstate->idx = 1; - else + } else { hstate->idx++; + } +} + + +/* Insert CMAC into SSL digests list */ +static char * +insert_cmac(char *list) +{ + int insert; + size_t len; + + + /* If list empty, we need to insert CMAC on new line */ + insert = (!list || !*list); + + if (insert) { + len = strlen(K_NL_PFX_STR) + strlen(CMAC); + list = (char *)erealloc(list, len + 1); + sprintf(list, "%s%s", K_NL_PFX_STR, CMAC); + } else { /* List not empty */ + /* Check if CMAC already in list - future proofing */ + const char *cmac_sn; + char *cmac_p; + + cmac_sn = OBJ_nid2sn(NID_cmac); + cmac_p = list; + insert = cmac_sn != NULL && *cmac_sn != '\0'; + + /* CMAC in list if found, followed by nul char or ',' */ + while (insert && NULL != (cmac_p = strstr(cmac_p, cmac_sn))) { + cmac_p += strlen(cmac_sn); + /* Still need to insert if not nul and not ',' */ + insert = *cmac_p && ',' != *cmac_p; + } + + /* Find proper insertion point */ + if (insert) { + char *last_nl; + char *point; + char *delim; + int found; + + /* Default to start if list empty */ + found = 0; + delim = list; + len = strlen(list); + + /* While new lines */ + while (delim < list + len && *delim && + !strncmp(K_NL_PFX_STR, delim, strlen(K_NL_PFX_STR))) { + point = delim + strlen(K_NL_PFX_STR); + + /* While digest names on line */ + while (point < list + len && *point) { + /* Another digest after on same or next line? */ + delim = strstr( point, K_DELIM_STR); + last_nl = strstr( point, K_NL_PFX_STR); + + /* No - end of list */ + if (!delim && !last_nl) { + delim = list + len; + } else + /* New line and no delim or before delim? */ + if (last_nl && (!delim || last_nl < delim)) { + delim = last_nl; + } + + /* Found insertion point where CMAC before entry? */ + if (strncmp(CMAC, point, delim - point) < 0) { + found = 1; + break; + } + + if (delim < list + len && *delim && + !strncmp(K_DELIM_STR, delim, strlen(K_DELIM_STR))) { + point += strlen(K_DELIM_STR); + } else { + break; + } + } /* While digest names on line */ + } /* While new lines */ + + /* If found in list */ + if (found) { + /* insert cmac and delim */ + /* Space for list could move - save offset */ + ptrdiff_t p_offset = point - list; + len += strlen(CMAC) + strlen(K_DELIM_STR); + list = (char *)erealloc(list, len + 1); + point = list + p_offset; + /* move to handle src/dest overlap */ + memmove(point + strlen(CMAC) + strlen(K_DELIM_STR), + point, strlen(point) + 1); + strncpy(point, CMAC, strlen(CMAC)); + strncpy(point + strlen(CMAC), K_DELIM_STR, strlen(K_DELIM_STR)); + } else { /* End of list */ + /* append delim and cmac */ + len += strlen(K_DELIM_STR) + strlen(CMAC); + list = (char *)erealloc(list, len + 1); + strcpy(list + strlen(list), K_DELIM_STR); + strcpy(list + strlen(list), CMAC); + } + } /* insert */ + } /* List not empty */ + + return list; } # endif #endif -static char *list_digest_names(void) + +static char * +list_digest_names(void) { char *list = NULL; @@ -3718,12 +3774,16 @@ static char *list_digest_names(void) # ifdef HAVE_EVP_MD_DO_ALL_SORTED struct hstate hstate = { NULL, NULL, K_PER_LINE+1 }; - hstate.seen = (const char **) emalloc_zero(1*sizeof( const char * )); // replaces -> calloc(1, sizeof( const char * )); + /* replace calloc(1, sizeof(const char *)) */ + hstate.seen = (const char **)emalloc_zero(sizeof(const char *)); INIT_SSL(); EVP_MD_do_all_sorted(list_md_fn, &hstate); list = hstate.list; free(hstate.seen); + + list = insert_cmac(list); /* Insert CMAC into SSL digests list */ + # else list = (char *)emalloc(sizeof("md5, others (upgrade to OpenSSL-1.0 for full list)")); strcpy(list, "md5, others (upgrade to OpenSSL-1.0 for full list)"); diff --git a/sntp/crypto.c b/sntp/crypto.c index ecafb40f5..a0275ff47 100644 --- a/sntp/crypto.c +++ b/sntp/crypto.c @@ -37,6 +37,12 @@ make_mac( if (key_type == NID_cmac) { CMAC_CTX * ctx; + + if (debug) { + fprintf(stderr, "%s:%d:%s():%s:nid\n", + __FILE__, __LINE__, __func__, CMAC); + } + if (!(ctx = CMAC_CTX_new())) { fprintf(stderr, "make_mac: CMAC %s CTX new failed.\n", CMAC); msyslog(LOG_ERR, "make_mac: CMAC %s CTX new failed.", CMAC); @@ -214,7 +220,9 @@ auth_init( if (octothorpe) *octothorpe = '\0'; act = emalloc(sizeof(*act)); - scan_cnt = sscanf(kbuf, "%d %9s %128s", &act->key_id, act->type, keystring); + /* keep width 15 = sizeof struct key.type - 1 synced */ + scan_cnt = sscanf(kbuf, "%d %15s %128s", + &act->key_id, act->type, keystring); if (scan_cnt == 3) { int len = strlen(keystring); if (len <= 20) { diff --git a/sntp/crypto.h b/sntp/crypto.h index 19cdbc45e..e6a42ac49 100644 --- a/sntp/crypto.h +++ b/sntp/crypto.h @@ -20,7 +20,7 @@ struct key { struct key * next; int key_id; int key_len; - char type[10]; + char type[16]; char key_seq[64]; }; diff --git a/sntp/tests/packetProcessing.c b/sntp/tests/packetProcessing.c index ef03cb1f6..07882a526 100644 --- a/sntp/tests/packetProcessing.c +++ b/sntp/tests/packetProcessing.c @@ -5,7 +5,8 @@ #include "ntp_stdlib.h" #include "unity.h" -#define CMAC "AES128CMAC" +#define CMAC "AES128CMAC" +#define CMAC_LENGTH 16 const char * Version = "stub unit test Version string"; @@ -79,7 +80,7 @@ PrepareAuthenticationTest( key_ptr->next = NULL; key_ptr->key_id = key_id; key_ptr->key_len = key_len; - memcpy(key_ptr->type, "MD5", 3); + memcpy(key_ptr->type, type, strlen(type) + 1); TEST_ASSERT_TRUE(key_len < sizeof(key_ptr->key_seq)); @@ -463,13 +464,13 @@ test_CorrectAuthenticatedPacketSHA1(void) void test_CorrectAuthenticatedPacketCMAC(void) { - PrepareAuthenticationTest(30, 15, CMAC, "abcdefghijklmno"); + PrepareAuthenticationTest(30, CMAC_LENGTH, CMAC, "abcdefghijklmnop"); TEST_ASSERT_TRUE(ENABLED_OPT(AUTHENTICATION)); int pkt_len = LEN_PKT_NOMAC; /* Prepare the packet. */ - testpkt.p.exten[0] = htonl(20); + testpkt.p.exten[0] = htonl(30); int mac_len = make_mac(&testpkt.p, pkt_len, MAX_MAC_LEN, key_ptr, &testpkt.p.exten[1]);