OpenSSL: don't use direct access to the internal of HMAC_CTX

OpenSSL 1.1 does not allow us to directly access the internal of
any data type, including HMAC_CTX. We have to use the defined
functions to do so.

Compatibility with OpenSSL 1.0 is kept by defining the corresponding
functions when they are not found in the library.

Signed-off-by: Emmanuel Deloget <logout@free.fr>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20170612134330.20971-8-logout@free.fr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14797.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/configure.ac b/configure.ac
index 07fc33920681df5e9828149063e4320c29b81850..56ce5f82eaa354846d33f4b75a8dc66eef4560aa 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -921,6 +921,10 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
                [ \
                        EVP_CIPHER_CTX_new \
                        EVP_CIPHER_CTX_free \
+                       HMAC_CTX_new \
+                       HMAC_CTX_free \
+                       HMAC_CTX_reset \
+                       HMAC_CTX_init \
                        EVP_MD_CTX_new \
                        EVP_MD_CTX_free \
                        EVP_MD_CTX_reset \

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index f7c4d60aa050bf0f79e90ef80d9f7e6296c3399c..191fee8e0964f8be66d35957dfbb6aaec54351a3 100644 (file)
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -853,7 +853,7 @@ init_key_ctx(struct key_ctx *ctx, struct key *key,
     }
     if (kt->digest && kt->hmac_length > 0)
     {
-        ALLOC_OBJ(ctx->hmac, hmac_ctx_t);
+        ctx->hmac = hmac_ctx_new();
         hmac_ctx_init(ctx->hmac, key->hmac, kt->hmac_length, kt->digest);
 
         msg(D_HANDSHAKE,
@@ -884,7 +884,7 @@ free_key_ctx(struct key_ctx *ctx)
     if (ctx->hmac)
     {
         hmac_ctx_cleanup(ctx->hmac);
-        free(ctx->hmac);
+        hmac_ctx_free(ctx->hmac);
         ctx->hmac = NULL;
     }
     ctx->implicit_iv_len = 0;

diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index 9679ee9b4f903474e64f94b8cd409a1952cf2ca8..b7f519b5bacf865509aec3a3dd450ea8e35b00b5 100644 (file)
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -582,6 +582,20 @@ void md_ctx_final(md_ctx_t *ctx, uint8_t *dst);
  *
  */
 
+/*
+ * Create a new HMAC context
+ *
+ * @return              A new HMAC context
+ */
+hmac_ctx_t *hmac_ctx_new(void);
+
+/*
+ * Free an existing HMAC context
+ *
+ * @param  ctx           HMAC context to free
+ */
+void hmac_ctx_free(hmac_ctx_t *ctx);
+
 /*
  * Initialises the given HMAC context, using the given digest
  * and key.

diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index 5f16a1f0ff030f6eedf36c3c507234abd8b0ef6f..24bc3158f21a720e9f9152673105e81f96988dbc 100644 (file)
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -840,6 +840,21 @@ md_ctx_final(mbedtls_md_context_t *ctx, uint8_t *dst)
 /*
  * TODO: re-enable dmsg for crypto debug
  */
+
+mbedtls_md_context_t *
+hmac_ctx_new(void)
+{
+    mbedtls_md_context_t *ctx;
+    ALLOC_OBJ(ctx, mbedtls_md_context_t);
+    return ctx;
+}
+
+void
+hmac_ctx_free(mbedtls_md_context_t *ctx)
+{
+    free(ctx);
+}
+
 void
 hmac_ctx_init(mbedtls_md_context_t *ctx, const uint8_t *key, int key_len,
               const mbedtls_md_info_t *kt)

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index f4470fc0a59dce93c90577a17cda89734eea030f..a55e65c10be2e64f7b88fe2ce9d6def07c4e80e1 100644 (file)
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -910,6 +910,19 @@ md_ctx_final(EVP_MD_CTX *ctx, uint8_t *dst)
  *
  */
 
+HMAC_CTX *
+hmac_ctx_new(void)
+{
+    HMAC_CTX *ctx = HMAC_CTX_new();
+    check_malloc_return(ctx);
+    return ctx;
+}
+
+void
+hmac_ctx_free(HMAC_CTX *ctx)
+{
+    HMAC_CTX_free(ctx);
+}
 
 void
 hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, int key_len,
@@ -917,8 +930,6 @@ hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, int key_len,
 {
     ASSERT(NULL != kt && NULL != ctx);
 
-    CLEAR(*ctx);
-
     HMAC_CTX_init(ctx);
     HMAC_Init_ex(ctx, key, key_len, kt, NULL);
 
@@ -929,7 +940,7 @@ hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, int key_len,
 void
 hmac_ctx_cleanup(HMAC_CTX *ctx)
 {
-    HMAC_CTX_cleanup(ctx);
+    HMAC_CTX_reset(ctx);
 }
 
 int

diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c
index 69b7d426fd7fb9eece3f731da80d90414427a454..16d60d2cdd078dddb50dc3717bc57bdcc4581ba0 100644 (file)
--- a/src/openvpn/ntlm.c
+++ b/src/openvpn/ntlm.c
@@ -85,13 +85,13 @@ static void
 gen_hmac_md5(const char *data, int data_len, const char *key, int key_len,char *result)
 {
     const md_kt_t *md5_kt = md_kt_get("MD5");
-    hmac_ctx_t hmac_ctx;
-    CLEAR(hmac_ctx);
+    hmac_ctx_t *hmac_ctx = hmac_ctx_new();
 
-    hmac_ctx_init(&hmac_ctx, key, key_len, md5_kt);
-    hmac_ctx_update(&hmac_ctx, (const unsigned char *)data, data_len);
-    hmac_ctx_final(&hmac_ctx, (unsigned char *)result);
-    hmac_ctx_cleanup(&hmac_ctx);
+    hmac_ctx_init(hmac_ctx, key, key_len, md5_kt);
+    hmac_ctx_update(hmac_ctx, (const unsigned char *)data, data_len);
+    hmac_ctx_final(hmac_ctx, (unsigned char *)result);
+    hmac_ctx_cleanup(hmac_ctx);
+    hmac_ctx_free(hmac_ctx);
 }
 
 static void

diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index c9e2692bba2b0b2ceffdb741c9e5398bfba103c7..c765f0bb9f5ba6388cad52b824ffe47ee85b9c3f 100644 (file)
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -116,6 +116,71 @@ EVP_CIPHER_CTX_new(void)
 }
 #endif
 
+#if !defined(HAVE_HMAC_CTX_RESET)
+/**
+ * Reset a HMAC context
+ *
+ * @param ctx                 The HMAC context
+ * @return                    1 on success, 0 on error
+ */
+static inline int
+HMAC_CTX_reset(HMAC_CTX *ctx)
+{
+    HMAC_CTX_cleanup(ctx);
+    return 1;
+}
+#endif
+
+#if !defined(HAVE_HMAC_CTX_INIT)
+/**
+ * Init a HMAC context
+ *
+ * @param ctx                 The HMAC context
+ *
+ * Contrary to many functions in this file, HMAC_CTX_init() is not
+ * an OpenSSL 1.1 function: it comes from previous versions and was
+ * removed in v1.1. As a consequence, there is no distincting in
+ * v1.1 between a cleanup, and init and a reset. Yet, previous OpenSSL
+ * version need this distinction.
+ *
+ * In order to respect previous OpenSSL versions, we implement init
+ * as reset for OpenSSL 1.1+.
+ */
+static inline void
+HMAC_CTX_init(HMAC_CTX *ctx)
+{
+    HMAC_CTX_reset(ctx);
+}
+#endif
+
+#if !defined(HAVE_HMAC_CTX_FREE)
+/**
+ * Free an existing HMAC context
+ *
+ * @param ctx                 The HMAC context
+ */
+static inline void
+HMAC_CTX_free(HMAC_CTX *c)
+{
+       free(c);
+}
+#endif
+
+#if !defined(HAVE_HMAC_CTX_NEW)
+/**
+ * Allocate a new HMAC context object
+ *
+ * @return                    A zero'ed HMAC context object
+ */
+static inline HMAC_CTX *
+HMAC_CTX_new(void)
+{
+    HMAC_CTX *ctx = NULL;
+    ALLOC_OBJ_CLEAR(ctx, HMAC_CTX);
+    return ctx;
+}
+#endif
+
 #if !defined(HAVE_SSL_CTX_GET_DEFAULT_PASSWD_CB_USERDATA)
 /**
  * Fetch the default password callback user data from the SSL context

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index cc7b183072c81dd7428bc581fcf0ea706bb39bb2..c658f3778dddda021b0a3516039825e17496733d 100644 (file)
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1606,8 +1606,8 @@ tls1_P_hash(const md_kt_t *md_kt,
 {
     struct gc_arena gc = gc_new();
     int chunk;
-    hmac_ctx_t ctx;
-    hmac_ctx_t ctx_tmp;
+    hmac_ctx_t *ctx;
+    hmac_ctx_t *ctx_tmp;
     uint8_t A1[MAX_HMAC_KEY_LENGTH];
     unsigned int A1_len;
 
@@ -1616,8 +1616,8 @@ tls1_P_hash(const md_kt_t *md_kt,
     const uint8_t *out_orig = out;
 #endif
 
-    CLEAR(ctx);
-    CLEAR(ctx_tmp);
+    ctx = hmac_ctx_new();
+    ctx_tmp = hmac_ctx_new();
 
     dmsg(D_SHOW_KEY_SOURCE, "tls1_P_hash sec: %s", format_hex(sec, sec_len, 0, &gc));
     dmsg(D_SHOW_KEY_SOURCE, "tls1_P_hash seed: %s", format_hex(seed, seed_len, 0, &gc));
@@ -1625,36 +1625,38 @@ tls1_P_hash(const md_kt_t *md_kt,
     chunk = md_kt_size(md_kt);
     A1_len = md_kt_size(md_kt);
 
-    hmac_ctx_init(&ctx, sec, sec_len, md_kt);
-    hmac_ctx_init(&ctx_tmp, sec, sec_len, md_kt);
+    hmac_ctx_init(ctx, sec, sec_len, md_kt);
+    hmac_ctx_init(ctx_tmp, sec, sec_len, md_kt);
 
-    hmac_ctx_update(&ctx,seed,seed_len);
-    hmac_ctx_final(&ctx, A1);
+    hmac_ctx_update(ctx,seed,seed_len);
+    hmac_ctx_final(ctx, A1);
 
     for (;; )
     {
-        hmac_ctx_reset(&ctx);
-        hmac_ctx_reset(&ctx_tmp);
-        hmac_ctx_update(&ctx,A1,A1_len);
-        hmac_ctx_update(&ctx_tmp,A1,A1_len);
-        hmac_ctx_update(&ctx,seed,seed_len);
+        hmac_ctx_reset(ctx);
+        hmac_ctx_reset(ctx_tmp);
+        hmac_ctx_update(ctx,A1,A1_len);
+        hmac_ctx_update(ctx_tmp,A1,A1_len);
+        hmac_ctx_update(ctx,seed,seed_len);
 
         if (olen > chunk)
         {
-            hmac_ctx_final(&ctx, out);
+            hmac_ctx_final(ctx, out);
             out += chunk;
             olen -= chunk;
-            hmac_ctx_final(&ctx_tmp, A1); /* calc the next A1 value */
+            hmac_ctx_final(ctx_tmp, A1); /* calc the next A1 value */
         }
         else    /* last one */
         {
-            hmac_ctx_final(&ctx, A1);
+            hmac_ctx_final(ctx, A1);
             memcpy(out,A1,olen);
             break;
         }
     }
-    hmac_ctx_cleanup(&ctx);
-    hmac_ctx_cleanup(&ctx_tmp);
+    hmac_ctx_cleanup(ctx);
+    hmac_ctx_free(ctx);
+    hmac_ctx_cleanup(ctx_tmp);
+    hmac_ctx_free(ctx_tmp);
     secure_memzero(A1, sizeof(A1));
 
     dmsg(D_SHOW_KEY_SOURCE, "tls1_P_hash out: %s", format_hex(out_orig, olen_orig, 0, &gc));