]> git.ipfire.org Git - thirdparty/openvpn.git/commitdiff
cleanup: remove md5 helper functions
authorSteffan Karger <steffan@karger.me>
Sun, 3 May 2015 15:07:11 +0000 (17:07 +0200)
committerGert Doering <gert@greenie.muc.de>
Sat, 23 May 2015 12:17:59 +0000 (14:17 +0200)
The MD5 wrapper functions were used in just a few places, which imho is
not worth the extra code.  Instead of using these wrappers, just use
the generic md_ctx_*() functions directly.

The md5sum() function was only used for logging information that was not
useful to a user; first the full options string would be printed, and
later just the hash.  That hash is less informative than the full
string, so why print it at all?

Finally, also removed save_pulled_options_digest().  The two times it
was called, it executed either one of the possible branches in the
function, where one of these needed a comment to explain what passing
NULL as newdigest is supposed to do...

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1430665631-4022-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/9642
Signed-off-by: Gert Doering <gert@greenie.muc.de>
src/openvpn/crypto.c
src/openvpn/crypto.h
src/openvpn/errlevel.h
src/openvpn/init.c
src/openvpn/openvpn.h
src/openvpn/push.c

index c1b9df317127f0023684851cf7959db13896e8f0..588d9f05436d92e29075d63cdf61d6e02b16b652 100644 (file)
@@ -1335,62 +1335,4 @@ get_random()
   return l;
 }
 
-/*
- * md5 functions
- */
-
-const char *
-md5sum (uint8_t *buf, int len, int n_print_chars, struct gc_arena *gc)
-{
-  uint8_t digest[MD5_DIGEST_LENGTH];
-  const md_kt_t *md5_kt = md_kt_get("MD5");
-
-  md_full(md5_kt, buf, len, digest);
-
-  return format_hex (digest, MD5_DIGEST_LENGTH, n_print_chars, gc);
-}
-
-void
-md5_state_init (struct md5_state *s)
-{
-  const md_kt_t *md5_kt = md_kt_get("MD5");
-
-  md_ctx_init(&s->ctx, md5_kt);
-}
-
-void
-md5_state_update (struct md5_state *s, void *data, size_t len)
-{
-  md_ctx_update(&s->ctx, data, len);
-}
-
-void
-md5_state_final (struct md5_state *s, struct md5_digest *out)
-{
-  md_ctx_final(&s->ctx, out->digest);
-  md_ctx_cleanup(&s->ctx);
-}
-
-void
-md5_digest_clear (struct md5_digest *digest)
-{
-  CLEAR (*digest);
-}
-
-bool
-md5_digest_defined (const struct md5_digest *digest)
-{
-  int i;
-  for (i = 0; i < MD5_DIGEST_LENGTH; ++i)
-    if (digest->digest[i])
-      return true;
-  return false;
-}
-
-bool
-md5_digest_equal (const struct md5_digest *d1, const struct md5_digest *d2)
-{
-  return memcmp(d1->digest, d2->digest, MD5_DIGEST_LENGTH) == 0;
-}
-
 #endif /* ENABLE_CRYPTO */
index 82158f9c1609bacd8f4ab4fa7a4e2d5b90cf3e1d..504896dee13be7b34852296d9d832171e134c47d 100644 (file)
@@ -420,26 +420,6 @@ void get_tls_handshake_key (const struct key_type *key_type,
                            const int key_direction,
                            const unsigned int flags);
 
-/*
- * md5 functions
- */
-
-struct md5_state {
-  md_ctx_t ctx;
-};
-
-struct md5_digest {
-  uint8_t digest [MD5_DIGEST_LENGTH];
-};
-
-const char *md5sum(uint8_t *buf, int len, int n_print_chars, struct gc_arena *gc);
-void md5_state_init (struct md5_state *s);
-void md5_state_update (struct md5_state *s, void *data, size_t len);
-void md5_state_final (struct md5_state *s, struct md5_digest *out);
-void md5_digest_clear (struct md5_digest *digest);
-bool md5_digest_defined (const struct md5_digest *digest);
-bool md5_digest_equal (const struct md5_digest *d1, const struct md5_digest *d2);
-
 /*
  * Inline functions
  */
index 3ee4ebc36240d108f19defd524da4c9c6726a20e..da600ab8a1884793636ec1064bb0cec36f3c66da 100644 (file)
 #define D_X509_ATTR          LOGLEV(4, 59, 0)        /* show x509-track attributes on connection */
 #define D_INIT_MEDIUM        LOGLEV(4, 60, 0)        /* show medium frequency init messages */
 #define D_MTU_INFO           LOGLEV(4, 61, 0)        /* show terse MTU info */
-#define D_SHOW_OCC_HASH      LOGLEV(4, 62, 0)        /* show MD5 hash of option compatibility string */
 #define D_PID_DEBUG_LOW      LOGLEV(4, 63, 0)        /* show low-freq packet-id debugging info */
 #define D_PID_DEBUG_MEDIUM   LOGLEV(4, 64, 0)        /* show medium-freq packet-id debugging info */
 
index 769ab9b514bf7c25981a63a57da89ded8d4a04cf..3434ce07d9a08893cd896d28c4a0c7869e13e30b 100644 (file)
@@ -1329,21 +1329,6 @@ do_route (const struct options *options,
 #endif
 }
 
-/*
- * Save current pulled options string in the c1 context store, so we can
- * compare against it after possible future restarts.
- */
-#if P2MP
-static void
-save_pulled_options_digest (struct context *c, const struct md5_digest *newdigest)
-{
-  if (newdigest)
-    c->c1.pulled_options_digest_save = *newdigest;
-  else
-    md5_digest_clear (&c->c1.pulled_options_digest_save);
-}
-#endif
-
 /*
  * initialize tun/tap device object
  */
@@ -1522,7 +1507,7 @@ do_close_tun_simple (struct context *c)
   c->c1.tuntap = NULL;
   c->c1.tuntap_owned = false;
 #if P2MP
-  save_pulled_options_digest (c, NULL); /* delete C1-saved pulled_options_digest */
+  CLEAR (c->c1.pulled_options_digest_save);
 #endif
 }
 
@@ -1634,6 +1619,20 @@ tun_abort()
  * Handle delayed tun/tap interface bringup due to --up-delay or --pull
  */
 
+#if P2MP
+/**
+ * Helper for do_up().  Take two option hashes and return true if they are not
+ * equal, or either one is all-zeroes.
+ */
+static bool
+options_hash_changed_or_zero(const uint8_t (*a)[MD5_DIGEST_LENGTH],
+    const uint8_t (*b)[MD5_DIGEST_LENGTH])
+{
+  const uint8_t zero[MD5_DIGEST_LENGTH] = {0};
+  return memcmp (*a, *b, MD5_DIGEST_LENGTH) || memcmp (*a, zero, MD5_DIGEST_LENGTH);
+}
+#endif /* P2MP */
+
 void
 do_up (struct context *c, bool pulled_options, unsigned int option_types_found)
 {
@@ -1658,8 +1657,8 @@ do_up (struct context *c, bool pulled_options, unsigned int option_types_found)
          if (!c->c2.did_open_tun
              && PULL_DEFINED (&c->options)
              && c->c1.tuntap
-             && (!md5_digest_defined (&c->c1.pulled_options_digest_save) || !md5_digest_defined (&c->c2.pulled_options_digest)
-                 || !md5_digest_equal (&c->c1.pulled_options_digest_save, &c->c2.pulled_options_digest)))
+             && options_hash_changed_or_zero (&c->c1.pulled_options_digest_save,
+                 &c->c2.pulled_options_digest))
            {
              /* if so, close tun, delete routes, then reinitialize tun and add routes */
              msg (M_INFO, "NOTE: Pulled options changed on restart, will need to close and reopen TUN/TAP device.");
@@ -1674,7 +1673,8 @@ do_up (struct context *c, bool pulled_options, unsigned int option_types_found)
       if (c->c2.did_open_tun)
        {
 #if P2MP
-         save_pulled_options_digest (c, &c->c2.pulled_options_digest);
+         memcpy(c->c1.pulled_options_digest_save, c->c2.pulled_options_digest,
+             sizeof(c->c1.pulled_options_digest_save));
 #endif
 
          /* if --route-delay was specified, start timer */
@@ -2732,20 +2732,14 @@ do_compute_occ_strings (struct context *c)
   c->c2.options_string_remote =
     options_string (&c->options, &c->c2.frame, c->c1.tuntap, true, &gc);
 
-  msg (D_SHOW_OCC, "Local Options String: '%s'", c->c2.options_string_local);
-  msg (D_SHOW_OCC, "Expected Remote Options String: '%s'",
-       c->c2.options_string_remote);
+  msg (D_SHOW_OCC, "Local Options String (VER=%s): '%s'",
+      options_string_version (c->c2.options_string_local, &gc),
+      c->c2.options_string_local);
+  msg (D_SHOW_OCC, "Expected Remote Options String (VER=%s): '%s'",
+      options_string_version (c->c2.options_string_remote, &gc),
+      c->c2.options_string_remote);
 
 #ifdef ENABLE_CRYPTO
-  msg (D_SHOW_OCC_HASH, "Local Options hash (VER=%s): '%s'",
-       options_string_version (c->c2.options_string_local, &gc),
-       md5sum ((uint8_t*)c->c2.options_string_local,
-              strlen (c->c2.options_string_local), 9, &gc));
-  msg (D_SHOW_OCC_HASH, "Expected Remote Options hash (VER=%s): '%s'",
-       options_string_version (c->c2.options_string_remote, &gc),
-       md5sum ((uint8_t*)c->c2.options_string_remote,
-              strlen (c->c2.options_string_remote), 9, &gc));
-
   if (c->c2.tls_multi)
     tls_multi_init_set_options (c->c2.tls_multi,
                                c->c2.options_string_local,
index fb532a2f64bf48a7ccf18f4c799c36771a0dacd5..9ab50b80b5ea7dcc814d0bd4d91cefc0bc066a89 100644 (file)
@@ -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;
+  uint8_t pulled_options_digest_save[MD5_DIGEST_LENGTH];
                                 /**< Hash of option strings received from the
                                  *   remote OpenVPN server.  Only used in
                                  *   client-mode. */
@@ -467,8 +467,8 @@ struct context_2
 
   /* hash of pulled options, so we can compare when options change */
   bool pulled_options_md5_init_done;
-  struct md5_state pulled_options_state;
-  struct md5_digest pulled_options_digest;
+  md_ctx_t pulled_options_state;
+  uint8_t pulled_options_digest[MD5_DIGEST_LENGTH];
 
   struct event_timeout server_poll_interval;
 
index 932df5cf23cca10cf572e11b0634c655596d3d14..c99a097d1888c790881c728f1af5300e34f1dfcf 100644 (file)
@@ -465,7 +465,7 @@ process_incoming_push_msg (struct context *c,
          struct buffer buf_orig = buf;
          if (!c->c2.pulled_options_md5_init_done)
            {
-             md5_state_init (&c->c2.pulled_options_state);
+             md_ctx_init(&c->c2.pulled_options_state, md_kt_get("MD5"));
              c->c2.pulled_options_md5_init_done = true;
            }
          if (!c->c2.did_pre_pull_restore)
@@ -482,13 +482,14 @@ process_incoming_push_msg (struct context *c,
              {
              case 0:
              case 1:
-               md5_state_update (&c->c2.pulled_options_state, BPTR(&buf_orig), BLEN(&buf_orig));
-               md5_state_final (&c->c2.pulled_options_state, &c->c2.pulled_options_digest);
+               md_ctx_update (&c->c2.pulled_options_state, BPTR(&buf_orig), BLEN(&buf_orig));
+               md_ctx_final (&c->c2.pulled_options_state, c->c2.pulled_options_digest);
+               md_ctx_cleanup (&c->c2.pulled_options_state);
                c->c2.pulled_options_md5_init_done = false;
                ret = PUSH_MSG_REPLY;
                break;
              case 2:
-               md5_state_update (&c->c2.pulled_options_state, BPTR(&buf_orig), BLEN(&buf_orig));
+               md_ctx_update (&c->c2.pulled_options_state, BPTR(&buf_orig), BLEN(&buf_orig));
                ret = PUSH_MSG_CONTINUATION;
                break;
              }