Use SHA256 for the internal digest, instead of MD5

Our internal options digest uses MD5 hashes to store the state, instead of
storing the full options string.  There's nothing wrong with that, but it
would still be better to use SHA256 because:
 * That makes it easier to make OpenVPN "FIPS-compliant" (forbids MD5)
 * We don't have to explain anymore that MD5 is fine too

The slightly less bytes for the digest (16 instead of 32) and operations
per connection setup are not worth sticking to MD5.

Note that might SHA256 not be available in de crypto lib, OpenVPN will
refuse to start and shout "Message hash algorithm 'SHA256' not found".

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1485101081-9784-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13926.html
Signed-off-by: David Sommerseth <davids@openvpn.net>

diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 42a46d93e564b366131038f34a312c7788626b84..afd6fe51cddfcf51e215369c76f58b51a685f9af 100644 (file)
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -131,9 +131,9 @@
 #include "packet_id.h"
 #include "mtu.h"
 
-/** Wrapper struct to pass around MD5 digests */
-struct md5_digest {
-    uint8_t digest[MD5_DIGEST_LENGTH];
+/** Wrapper struct to pass around SHA256 digests */
+struct sha256_digest {
+    uint8_t digest[SHA256_DIGEST_LENGTH];
 };
 
 /*

diff --git a/src/openvpn/crypto_mbedtls.h b/src/openvpn/crypto_mbedtls.h
index 525b256a6febf1a3104a1f2c2de45341159c0a27..da2db166aa3e64c2688e11a565654b0de6ceffce 100644 (file)
--- a/src/openvpn/crypto_mbedtls.h
+++ b/src/openvpn/crypto_mbedtls.h
@@ -73,6 +73,7 @@ typedef mbedtls_md_context_t hmac_ctx_t;
 #define MD4_DIGEST_LENGTH       16
 #define MD5_DIGEST_LENGTH       16
 #define SHA_DIGEST_LENGTH       20
+#define SHA256_DIGEST_LENGTH    32
 #define DES_KEY_LENGTH 8
 
 /**

diff --git a/src/openvpn/crypto_openssl.h b/src/openvpn/crypto_openssl.h
index 56ec6e1c67f0e2697114d8ba1eed2d80f31d79c0..f8ddbc8665206e0f3b0fcc063e3fe7ab0b259dcf 100644 (file)
--- a/src/openvpn/crypto_openssl.h
+++ b/src/openvpn/crypto_openssl.h
@@ -33,6 +33,7 @@
 #include <openssl/evp.h>
 #include <openssl/hmac.h>
 #include <openssl/md5.h>
+#include <openssl/sha.h>
 
 /** Generic cipher key type %context. */
 typedef EVP_CIPHER cipher_kt_t;

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index f2e75c87db20d6df4ffe585732114ed31d7fb4d8..756bf366f90549c0942c8cc99ab4446647657c76 100644 (file)
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1919,12 +1919,12 @@ tun_abort()
  * equal, or either one is all-zeroes.
  */
 static bool
-options_hash_changed_or_zero(const struct md5_digest *a,
-                             const struct md5_digest *b)
+options_hash_changed_or_zero(const struct sha256_digest *a,
+                             const struct sha256_digest *b)
 {
-    const struct md5_digest zero = {{0}};
-    return memcmp(a, b, sizeof(struct md5_digest))
-           || !memcmp(a, &zero, sizeof(struct md5_digest));
+    const struct sha256_digest zero = {{0}};
+    return memcmp(a, b, sizeof(struct sha256_digest))
+           || !memcmp(a, &zero, sizeof(struct sha256_digest));
 }
 #endif /* P2MP */
 

diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index 37edec4f725c7b5b408c0bf7f4f8d7d4027adfc1..893296edc539800db6463b5861e6f7c83cbac115 100644 (file)
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -202,7 +202,7 @@ struct context_1
 #endif
 
     /* if client mode, hash of option strings we pulled from server */
-    struct md5_digest pulled_options_digest_save;
+    struct sha256_digest pulled_options_digest_save;
     /**< Hash of option strings received from the
      *   remote OpenVPN server.  Only used in
      *   client-mode. */
@@ -471,9 +471,9 @@ struct context_2
     bool did_pre_pull_restore;
 
     /* hash of pulled options, so we can compare when options change */
-    bool pulled_options_md5_init_done;
+    bool pulled_options_digest_init_done;
     md_ctx_t pulled_options_state;
-    struct md5_digest pulled_options_digest;
+    struct sha256_digest pulled_options_digest;
 
     struct event_timeout scheduled_exit;
     int scheduled_exit_signal;

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index c9c04a630c99ce6e55d6d7d45d2b81b8e5cb50a9..8c3104ed7690cdf24463c1d610cecf955d744cf4 100644 (file)
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -720,10 +720,10 @@ process_incoming_push_msg(struct context *c,
         if (ch == ',')
         {
             struct buffer buf_orig = buf;
-            if (!c->c2.pulled_options_md5_init_done)
+            if (!c->c2.pulled_options_digest_init_done)
             {
-                md_ctx_init(&c->c2.pulled_options_state, md_kt_get("MD5"));
-                c->c2.pulled_options_md5_init_done = true;
+                md_ctx_init(&c->c2.pulled_options_state, md_kt_get("SHA256"));
+                c->c2.pulled_options_digest_init_done = true;
             }
             if (!c->c2.did_pre_pull_restore)
             {
@@ -744,7 +744,7 @@ process_incoming_push_msg(struct context *c,
                     case 1:
                         md_ctx_final(&c->c2.pulled_options_state, c->c2.pulled_options_digest.digest);
                         md_ctx_cleanup(&c->c2.pulled_options_state);
-                        c->c2.pulled_options_md5_init_done = false;
+                        c->c2.pulled_options_digest_init_done = false;
                         ret = PUSH_MSG_REPLY;
                         break;