[Bug 3447] AES-128-CMAC (fixes)

 - refactoring the MAC code, too

bk: 5a2a2dccad7AQidLrxu_DTdW1Qo7qg

diff --git a/ChangeLog b/ChangeLog
index 7fae02f687727607e813cad8ea2e6813d7cf041b..4aca479ad435ac2da502f91a493da6da9f67017b 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,7 @@
 ---
 
 * [Bug 3447] AES-128-CMAC (fixes) <perlinger@ntp.org>
+  - refactoring the MAC code, too
 * [Bug 3441] Validate the assumption that AF_UNSPEC is 0.  stenn@ntp.org
 * [Bug 3439] When running multiple commands / hosts in ntpq... <perlinger@ntp.org>
  - applied patch by ggarvey

diff --git a/include/ntp_stdlib.h b/include/ntp_stdlib.h
index a4e857425ff1dd9297a1c00f113a60c9318f0042..889c3b25ef4214f61fa35347d75fee9580f2edb6 100644 (file)
--- a/include/ntp_stdlib.h
+++ b/include/ntp_stdlib.h
@@ -97,8 +97,8 @@ extern        void    auth_prealloc_symkeys(int);
 extern int     ymd2yd          (int, int, int);
 
 /* a_md5encrypt.c */
-extern int     MD5authdecrypt  (int, const u_char *, u_int32 *, size_t, size_t);
-extern size_t  MD5authencrypt  (int, const u_char *, u_int32 *, size_t);
+extern int     MD5authdecrypt  (int, const u_char *, size_t, u_int32 *, size_t, size_t);
+extern size_t  MD5authencrypt  (int, const u_char *, size_t, u_int32 *, size_t);
 extern void    MD5auth_setkey  (keyid_t, int, const u_char *, size_t, KeyAccT *c);
 extern u_int32 addr2refid      (sockaddr_u *);
 

diff --git a/libntp/a_md5encrypt.c b/libntp/a_md5encrypt.c
index ed776f2e4c3e560d5b8d8962022a2d59ec92841e..064a79a70ac3750f840c986916d2999629b38243 100644 (file)
--- a/libntp/a_md5encrypt.c
+++ b/libntp/a_md5encrypt.c
@@ -18,115 +18,187 @@
 # define  AES_128_KEY_SIZE     16
 #endif
 
-/*
- * MD5authencrypt - generate message digest
- *
- * Returns length of MAC including key ID and digest.
- */
-size_t
-MD5authencrypt(
-       int             type,   /* hash algorithm */
-       const u_char *  key,    /* key pointer */
-       u_int32 *       pkt,    /* packet pointer */
-       size_t          length  /* packet length */
-       )
+typedef struct {
+       const void *    buf;
+       size_t          len;
+} robuffT;
+
+typedef struct {
+       void *          buf;
+       size_t          len;
+} rwbuffT;
+
+#ifdef OPENSSL
+static size_t
+cmac_ctx_size(
+       CMAC_CTX *      ctx)
 {
-       u_char  digest[EVP_MAX_MD_SIZE];
-       u_int   len = 0;
+       size_t mlen = 0;
 
+       if (ctx) {
+               EVP_CIPHER_CTX *        cctx;
+               if (NULL != (cctx = CMAC_CTX_get0_cipher_ctx (ctx)))
+                       mlen = EVP_CIPHER_CTX_block_size(cctx);
+       }
+       return mlen;
+}
+#endif
+
+
+static size_t
+make_mac(
+       const rwbuffT * digest,
+       int             ktype,
+       const robuffT * key,
+       const robuffT * msg)
+{
        /*
         * Compute digest of key concatenated with packet. Note: the
         * key type and digest type have been verified when the key
-        * was creaded.
+        * was created.
         */
-       INIT_SSL();
+       size_t  retlen = 0;
+       
 #ifdef OPENSSL
+       
+       INIT_SSL();
+
        /* Check if CMAC key type specific code required */
-       if (type == NID_cmac) {
-               CMAC_CTX *      ctx;
-               size_t          slen = 0;
+       if (ktype == NID_cmac) {
+               CMAC_CTX *      ctx = CMAC_CTX_new();
                
-               if (debug) {
-                       fprintf(stderr, "%s:%d:%s():%s:nid\n",
-                               __FILE__, __LINE__, __func__, CMAC);
+               if ( ! ctx) {
+                       msyslog(LOG_ERR, "MAC encrypt: CMAC %s CTX new failed.", CMAC);
+                       goto cmac_fail;
                }
-               
-               if (!(ctx = CMAC_CTX_new())) {
-                       fprintf(stderr,  "MAC encrypt: CMAC %s CTX new failed.\n", CMAC);
-                       msyslog(LOG_ERR, "MAC encrypt: CMAC %s CTX new failed.",   CMAC);
+               if (!CMAC_Init(ctx, key->buf, key->len,
+                              EVP_aes_128_cbc(), NULL)) {
+                       msyslog(LOG_ERR, "MAC encrypt: CMAC %s Init failed.",    CMAC);
+                       goto cmac_fail;
                }
-               else if (!CMAC_Init(ctx, key, AES_128_KEY_SIZE,
-                                   EVP_aes_128_cbc(), NULL)) {
-                       fprintf(stderr,  "MAC encrypt: CMAC %s Init failed.\n",    CMAC);
-                       msyslog(LOG_ERR, "MAC encrypt: CMAC %s Init failed.",      CMAC);
+               if (cmac_ctx_size(ctx) > digest->len) {
+                       msyslog(LOG_ERR, "MAC encrypt: CMAC %s buf too small.",  CMAC);
+                       goto cmac_fail;
                }
-               else if (!CMAC_Update(ctx, pkt, length)) {
-                       fprintf(stderr,  "MAC encrypt: CMAC %s Update failed.\n",  CMAC);
-                       msyslog(LOG_ERR, "MAC encrypt: CMAC %s Update failed.",    CMAC);
+               if (!CMAC_Update(ctx, msg->buf, msg->len)) {
+                       msyslog(LOG_ERR, "MAC encrypt: CMAC %s Update failed.",  CMAC);
+                       goto cmac_fail;
                }
-               else if (!CMAC_Final(ctx, digest, &slen)) {
-                       fprintf(stderr,  "MAC encrypt: CMAC %s Final failed.\n",   CMAC);
-                       msyslog(LOG_ERR, "MAC encrypt: CMAC %s Final failed.",     CMAC);
-                       slen = 0;
+               if (!CMAC_Final(ctx, digest->buf, &retlen)) {
+                       msyslog(LOG_ERR, "MAC encrypt: CMAC %s Final failed.",   CMAC);
+                       retlen = 0;
                }
-               len = (u_int)slen;
-               
-               CMAC_CTX_cleanup(ctx);
-       } else  /* generic MAC handling */
-#endif
-       {
-               EVP_MD_CTX *    ctx;
+         cmac_fail:
+               if (ctx)
+                       CMAC_CTX_cleanup(ctx);
+       }
+       else {  /* generic MAC handling */
+               EVP_MD_CTX *    ctx   = EVP_MD_CTX_new();
+               uint            uilen = 0;
                
-               if (!(ctx = EVP_MD_CTX_new())) {
-                       fprintf(stderr,  "MAC encrypt: MAC %s Digest CTX new failed.\n",
-                               OBJ_nid2sn(type));
+               if ( ! ctx) {
                        msyslog(LOG_ERR, "MAC encrypt: MAC %s Digest CTX new failed.",
-                               OBJ_nid2sn(type));
+                               OBJ_nid2sn(ktype));
+                       goto mac_fail;
                }
-#ifdef OPENSSL /* OpenSSL 1 supports return codes 0 fail, 1 okay */
-               else if (!EVP_DigestInit(ctx, EVP_get_digestbynid(type))) {
-                       fprintf(stderr,  "MAC encrypt: MAC %s Digest Init failed.\n",
-                               OBJ_nid2sn(type));
+               
+           #ifdef EVP_MD_CTX_FLAG_NON_FIPS_ALLOW
+               /* make sure MD5 is allowd */
+               EVP_MD_CTX_set_flags(ctx, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW);
+           #endif
+               
+               if (!EVP_DigestInit(ctx, EVP_get_digestbynid(ktype))) {
                        msyslog(LOG_ERR, "MAC encrypt: MAC %s Digest Init failed.",
-                               OBJ_nid2sn(type));
+                               OBJ_nid2sn(ktype));
+                       goto mac_fail;
+               }
+               if (EVP_MD_CTX_size(ctx) > digest->len) {
+                       msyslog(LOG_ERR, "MAC encrypt: MAC %s buf too small.",
+                               OBJ_nid2sn(ktype));
+                       goto mac_fail;
                }
-               else if (!EVP_DigestUpdate(ctx, key, (u_int)cache_secretsize)) {
-                       fprintf(stderr,  "MAC encrypt: MAC %s Digest Update key failed.\n",
-                               OBJ_nid2sn(type));
+               if (!EVP_DigestUpdate(ctx, key->buf, (u_int)key->len)) {
                        msyslog(LOG_ERR, "MAC encrypt: MAC %s Digest Update key failed.",
-                               OBJ_nid2sn(type));
+                               OBJ_nid2sn(ktype));
+                       goto mac_fail;
                }
-               else if (!EVP_DigestUpdate(ctx, (u_char *)pkt, (u_int)length)) {
-                       fprintf(stderr,  "MAC encrypt: MAC %s Digest Update data failed.\n",
-                               OBJ_nid2sn(type));
+               if (!EVP_DigestUpdate(ctx, msg->buf, (u_int)msg->len)) {
                        msyslog(LOG_ERR, "MAC encrypt: MAC %s Digest Update data failed.",
-                               OBJ_nid2sn(type));
+                               OBJ_nid2sn(ktype));
+                       goto mac_fail;
                }
-               else if (!EVP_DigestFinal(ctx, digest, &len)) {
-                       fprintf(stderr,  "MAC encrypt: MAC %s Digest Final failed.\n",
-                               OBJ_nid2sn(type));
+               if (!EVP_DigestFinal(ctx, digest->buf, &uilen)) {
                        msyslog(LOG_ERR, "MAC encrypt: MAC %s Digest Final failed.",
-                               OBJ_nid2sn(type));
-                       len = 0;
+                               OBJ_nid2sn(ktype));
+                       uilen = 0;
                }
-#else /* !OPENSSL */
-               if (!(ctx && EVP_DigestInit(ctx, EVP_get_digestbynid(type)))) {
-                       msyslog(LOG_ERR,
-                               "MAC encrypt: digest init failed");
+         mac_fail:
+               retlen = (size_t)uilen;
+               
+               if (ctx)
                        EVP_MD_CTX_free(ctx);
-                       return (0);
+       }
+
+#else /* !OPENSSL follows */
+       
+       if (ktype == NID_md5)
+       {
+               EVP_MD_CTX *    ctx   = EVP_MD_CTX_new();
+               uint            uilen = 0;
+
+               if (digest->len < 16) {
+                       msyslog(LOG_ERR, "%s", "MAC encrypt: MAC md5 buf too small.");
                }
-               EVP_DigestUpdate(ctx, key, cache_secretsize);
-               EVP_DigestUpdate(ctx, (u_char *)pkt, length);
-               EVP_DigestFinal(ctx, digest, &len);
-#endif
-               EVP_MD_CTX_free(ctx);
+               else if ( ! ctx) {
+                       msyslog(LOG_ERR, "%s", "MAC encrypt: MAC md5 Digest CTX new failed.");
+               }
+               else {
+                       EVP_DigestInit(ctx, EVP_get_digestbynid(ktype));
+                       EVP_DigestUpdate(ctx, key->buf, key->len);
+                       EVP_DigestUpdate(ctx, msg->buf, msg->len);
+                       EVP_DigestFinal(ctx, digest->buf, &uilen);
+               }
+               if (ctx)
+                       EVP_MD_CTX_free(ctx);
+               retlen = (size_t)uilen;
        }
+       else
+       {
+               msyslog(LOG_ERR, "MAC encrypt: invalid key type %d"  , ktype);
+       }
+       
+#endif /* !OPENSSL */
+
+       return retlen;
+}
+
+
+/*
+ * MD5authencrypt - generate message digest
+ *
+ * Returns length of MAC including key ID and digest.
+ */
+size_t
+MD5authencrypt(
+       int             type,   /* hash algorithm */
+       const u_char *  key,    /* key pointer */
+       size_t          klen,   /* key length */
+       u_int32 *       pkt,    /* packet pointer */
+       size_t          length  /* packet length */
+       )
+{
+       u_char  digest[EVP_MAX_MD_SIZE];
+       rwbuffT digb = { digest, sizeof(digest) };
+       robuffT keyb = { key, klen };
+       robuffT msgb = { pkt, length }; 
+       size_t  dlen = 0;
+
+       dlen = make_mac(&digb, type, &keyb, &msgb);
        /* If the MAC is longer than the MAX then truncate it. */
-       if (len > MAX_MDG_LEN)
-               len = MAX_MDG_LEN;
-       memmove((u_char *)pkt + length + KEY_MAC_LEN, digest, len);
-       return (len + KEY_MAC_LEN);
+       if (dlen > MAX_MDG_LEN)
+               dlen = MAX_MDG_LEN;
+       memcpy((u_char *)pkt + length + KEY_MAC_LEN, digest, dlen);
+       return (dlen + KEY_MAC_LEN);
 }
 
 
@@ -139,112 +211,30 @@ int
 MD5authdecrypt(
        int             type,   /* hash algorithm */
        const u_char *  key,    /* key pointer */
+       size_t          klen,   /* key length */
        u_int32 *       pkt,    /* packet pointer */
        size_t          length, /* packet length */
        size_t          size    /* MAC size */
        )
 {
        u_char  digest[EVP_MAX_MD_SIZE];
-       u_int   len = 0;
-
-       /*
-        * Compute digest of key concatenated with packet. Note: the
-        * key type and digest type have been verified when the key
-        * was created.
-        */
-       INIT_SSL();
-#ifdef OPENSSL
-       /* Check if CMAC key type specific code required */
-       if (type == NID_cmac) {
-               CMAC_CTX *      ctx;
-               size_t          slen = 0;
+       rwbuffT digb = { digest, sizeof(digest) };
+       robuffT keyb = { key, klen };
+       robuffT msgb = { pkt, length }; 
+       size_t  dlen = 0;
 
-               if (debug) {
-                       fprintf(stderr, "%s:%d:%s():%s:nid\n",
-                               __FILE__, __LINE__, __func__, CMAC);
-               }
-               
-               if (!(ctx = CMAC_CTX_new())) {
-                       fprintf(stderr,  "MAC decrypt: CMAC %s CTX new failed.\n", CMAC);
-                       msyslog(LOG_ERR, "MAC decrypt: CMAC %s CTX new failed.",   CMAC);
-               }
-               else if (!CMAC_Init(ctx, key, AES_128_KEY_SIZE,
-                                   EVP_aes_128_cbc(), NULL)) {
-                       fprintf(stderr,  "MAC decrypt: CMAC %s Init failed.\n",    CMAC);
-                       msyslog(LOG_ERR, "MAC decrypt: CMAC %s Init failed.",      CMAC);
-               }
-               else if (!CMAC_Update(ctx, pkt, length)) {
-                       fprintf(stderr,  "MAC decrypt: CMAC %s Update failed.\n",  CMAC);
-                       msyslog(LOG_ERR, "MAC decrypt: CMAC %s Update failed.",    CMAC);
-               }
-               else if (!CMAC_Final(ctx, digest, &slen)) {
-                       fprintf(stderr,  "MAC decrypt: CMAC %s Final failed.\n",   CMAC);
-                       msyslog(LOG_ERR, "MAC decrypt: CMAC %s Final failed.",     CMAC);
-                       slen = 0;
-               }
-               len = (u_int)slen;
-               
-               CMAC_CTX_cleanup(ctx);
-       } else  /* generic MAC handling */
-#endif
-       {
-               EVP_MD_CTX *    ctx;
-               
-               if (!(ctx = EVP_MD_CTX_new())) {
-                       fprintf(stderr,  "MAC decrypt: MAC %s Digest CTX new failed.\n",
-                               OBJ_nid2sn(type));
-                       msyslog(LOG_ERR, "MAC decrypt: MAC %s Digest CTX new failed.",
-                               OBJ_nid2sn(type));
-               }
-#ifdef OPENSSL /* OpenSSL 1 supports return codes 0 fail, 1 okay */
-               else if (!EVP_DigestInit(ctx, EVP_get_digestbynid(type))) {
-                       fprintf(stderr,  "MAC decrypt: MAC %s Digest Init failed.\n",
-                               OBJ_nid2sn(type));
-                       msyslog(LOG_ERR, "MAC decrypt: MAC %s Digest Init failed.",
-                               OBJ_nid2sn(type));
-               }
-               else if (!EVP_DigestUpdate(ctx, key, (u_int)cache_secretsize)) {
-                       fprintf(stderr,  "MAC decrypt: MAC %s Digest Update key failed.\n",
-                               OBJ_nid2sn(type));
-                       msyslog(LOG_ERR, "MAC decrypt: MAC %s Digest Update key failed.",
-                               OBJ_nid2sn(type));
-               }
-               else if (!EVP_DigestUpdate(ctx, (u_char *)pkt, (u_int)length)) {
-                       fprintf(stderr,  "MAC decrypt: MAC %s Digest Update data failed.\n",
-                               OBJ_nid2sn(type));
-                       msyslog(LOG_ERR, "MAC decrypt: MAC %s Digest Update data failed.",
-                               OBJ_nid2sn(type));
-               }
-               else if (!EVP_DigestFinal(ctx, digest, &len)) {
-                       fprintf(stderr,  "MAC decrypt: MAC %s Digest Final failed.\n",
-                               OBJ_nid2sn(type));
-                       msyslog(LOG_ERR, "MAC decrypt: MAC %s Digest Final failed.",
-                               OBJ_nid2sn(type));
-                       len = 0;
-               }
-#else /* !OPENSSL */
-               if (!(ctx && EVP_DigestInit(ctx, EVP_get_digestbynid(type)))) {
-                       msyslog(LOG_ERR,
-                               "MAC decrypt: digest init failed");
-                       EVP_MD_CTX_free(ctx);
-                       return (0);
-               }
-               EVP_DigestUpdate(ctx, key, cache_secretsize);
-               EVP_DigestUpdate(ctx, (u_char *)pkt, (u_int)length);
-               EVP_DigestFinal(ctx, digest, &len);
-#endif
-               EVP_MD_CTX_free(ctx);
-       }
+       dlen = make_mac(&digb, type, &keyb, &msgb);
+       
        /* If the MAC is longer than the MAX then truncate it. */
-       if (len > MAX_MDG_LEN)
-               len = MAX_MDG_LEN;
-       if (size != (size_t)len + KEY_MAC_LEN) {
+       if (dlen > MAX_MDG_LEN)
+               dlen = MAX_MDG_LEN;
+       if (size != (size_t)dlen + KEY_MAC_LEN) {
                msyslog(LOG_ERR,
                    "MAC decrypt: MAC length error");
                return (0);
        }
        return !isc_tsmemcmp(digest,
-                (u_char *)pkt + length + KEY_MAC_LEN, len);
+                (u_char *)pkt + length + KEY_MAC_LEN, dlen);
 }
 
 /*
@@ -267,7 +257,6 @@ addr2refid(sockaddr_u *addr)
        INIT_SSL();
 
        ctx = EVP_MD_CTX_new();
-       EVP_MD_CTX_init(ctx);
 #ifdef EVP_MD_CTX_FLAG_NON_FIPS_ALLOW
        /* MD5 is not used as a crypto hash here. */
        EVP_MD_CTX_set_flags(ctx, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW);

diff --git a/libntp/authkeys.c b/libntp/authkeys.c
index b2ff410cd476817c08e3c166926f82b72415f7a1..d7af9bcf739d62a32a8706bad8cc569064b8c1e0 100644 (file)
--- a/libntp/authkeys.c
+++ b/libntp/authkeys.c
@@ -795,7 +795,9 @@ authencrypt(
                return 0;
        }
 
-       return MD5authencrypt(cache_type, cache_secret, pkt, length);
+       return MD5authencrypt(cache_type,
+                             cache_secret, cache_secretsize,
+                             pkt, length);
 }
 
 
@@ -822,6 +824,7 @@ authdecrypt(
                return FALSE;
        }
 
-       return MD5authdecrypt(cache_type, cache_secret, pkt, length,
-                             size);
+       return MD5authdecrypt(cache_type,
+                             cache_secret, cache_secretsize,
+                             pkt, length, size);
 }

diff --git a/libntp/libssl_compat.c b/libntp/libssl_compat.c
index afe4d07a4b0a1c3d4c3c78f48e98595a7e07f092..5527682b34f9e45b60f07e9c0264508302a41eeb 100644 (file)
--- a/libntp/libssl_compat.c
+++ b/libntp/libssl_compat.c
@@ -74,7 +74,10 @@ sslshimBN_GENCB_free(
 EVP_MD_CTX*
 sslshim_EVP_MD_CTX_new(void)
 {
-       return calloc(1, sizeof(EVP_MD_CTX));
+       EVP_MD_CTX *    ctx;
+       if (NULL != (ctx = calloc(1, sizeof(EVP_MD_CTX))))
+               EVP_MD_CTX_init(ctx);
+       return ctx;
 }
 
 void

diff --git a/tests/libntp/a_md5encrypt.c b/tests/libntp/a_md5encrypt.c
index a87aa796de72e03d5b3b17d848603adf5a11ca1c..844be16fa6f8c05a315d0c3a2d821d7f75194114 100644 (file)
--- a/tests/libntp/a_md5encrypt.c
+++ b/tests/libntp/a_md5encrypt.c
@@ -52,11 +52,9 @@ test_Encrypt(void) {
        packetPtr = emalloc_zero(totalLength * sizeof(*packetPtr));
        memcpy(packetPtr, packet, packetLength);
 
-       cache_secretsize = keyLength;
+       length = MD5authencrypt(keytype, key, keyLength, packetPtr, packetLength);
 
-       length = MD5authencrypt(keytype, key, packetPtr, packetLength);
-
-       TEST_ASSERT_TRUE(MD5authdecrypt(keytype, key, packetPtr, packetLength, length));
+       TEST_ASSERT_TRUE(MD5authdecrypt(keytype, key, keyLength, packetPtr, packetLength, length));
 
        TEST_ASSERT_EQUAL(20, length);
        TEST_ASSERT_EQUAL_MEMORY(expectedPacket.u8, packetPtr, totalLength);
@@ -66,14 +64,12 @@ test_Encrypt(void) {
 
 void
 test_DecryptValid(void) {
-       cache_secretsize = keyLength;
-       TEST_ASSERT_TRUE(MD5authdecrypt(keytype, key, expectedPacket.u32, packetLength, 20));
+       TEST_ASSERT_TRUE(MD5authdecrypt(keytype, key, keyLength, expectedPacket.u32, packetLength, 20));
 }
 
 void
 test_DecryptInvalid(void) {
-       cache_secretsize = keyLength;
-       TEST_ASSERT_FALSE(MD5authdecrypt(keytype, key, invalidPacket.u32, packetLength, 20));
+       TEST_ASSERT_FALSE(MD5authdecrypt(keytype, key, keyLength, invalidPacket.u32, packetLength, 20));
 }
 
 void