Browse code

cleanup: remove md5 helper functions

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>

Steffan Karger authored on 2015/05/04 00:07:11
Showing 6 changed files
... ...
@@ -1335,62 +1335,4 @@ get_random()
1335 1335
   return l;
1336 1336
 }
1337 1337
 
1338
-/*
1339
- * md5 functions
1340
- */
1341
-
1342
-const char *
1343
-md5sum (uint8_t *buf, int len, int n_print_chars, struct gc_arena *gc)
1344
-{
1345
-  uint8_t digest[MD5_DIGEST_LENGTH];
1346
-  const md_kt_t *md5_kt = md_kt_get("MD5");
1347
-
1348
-  md_full(md5_kt, buf, len, digest);
1349
-
1350
-  return format_hex (digest, MD5_DIGEST_LENGTH, n_print_chars, gc);
1351
-}
1352
-
1353
-void
1354
-md5_state_init (struct md5_state *s)
1355
-{
1356
-  const md_kt_t *md5_kt = md_kt_get("MD5");
1357
-
1358
-  md_ctx_init(&s->ctx, md5_kt);
1359
-}
1360
-
1361
-void
1362
-md5_state_update (struct md5_state *s, void *data, size_t len)
1363
-{
1364
-  md_ctx_update(&s->ctx, data, len);
1365
-}
1366
-
1367
-void
1368
-md5_state_final (struct md5_state *s, struct md5_digest *out)
1369
-{
1370
-  md_ctx_final(&s->ctx, out->digest);
1371
-  md_ctx_cleanup(&s->ctx);
1372
-}
1373
-
1374
-void
1375
-md5_digest_clear (struct md5_digest *digest)
1376
-{
1377
-  CLEAR (*digest);
1378
-}
1379
-
1380
-bool
1381
-md5_digest_defined (const struct md5_digest *digest)
1382
-{
1383
-  int i;
1384
-  for (i = 0; i < MD5_DIGEST_LENGTH; ++i)
1385
-    if (digest->digest[i])
1386
-      return true;
1387
-  return false;
1388
-}
1389
-
1390
-bool
1391
-md5_digest_equal (const struct md5_digest *d1, const struct md5_digest *d2)
1392
-{
1393
-  return memcmp(d1->digest, d2->digest, MD5_DIGEST_LENGTH) == 0;
1394
-}
1395
-
1396 1338
 #endif /* ENABLE_CRYPTO */
... ...
@@ -421,26 +421,6 @@ void get_tls_handshake_key (const struct key_type *key_type,
421 421
 			    const unsigned int flags);
422 422
 
423 423
 /*
424
- * md5 functions
425
- */
426
-
427
-struct md5_state {
428
-  md_ctx_t ctx;
429
-};
430
-
431
-struct md5_digest {
432
-  uint8_t digest [MD5_DIGEST_LENGTH];
433
-};
434
-
435
-const char *md5sum(uint8_t *buf, int len, int n_print_chars, struct gc_arena *gc);
436
-void md5_state_init (struct md5_state *s);
437
-void md5_state_update (struct md5_state *s, void *data, size_t len);
438
-void md5_state_final (struct md5_state *s, struct md5_digest *out);
439
-void md5_digest_clear (struct md5_digest *digest);
440
-bool md5_digest_defined (const struct md5_digest *digest);
441
-bool md5_digest_equal (const struct md5_digest *d1, const struct md5_digest *d2);
442
-
443
-/*
444 424
  * Inline functions
445 425
  */
446 426
 
... ...
@@ -105,7 +105,6 @@
105 105
 #define D_X509_ATTR          LOGLEV(4, 59, 0)        /* show x509-track attributes on connection */
106 106
 #define D_INIT_MEDIUM        LOGLEV(4, 60, 0)        /* show medium frequency init messages */
107 107
 #define D_MTU_INFO           LOGLEV(4, 61, 0)        /* show terse MTU info */
108
-#define D_SHOW_OCC_HASH      LOGLEV(4, 62, 0)        /* show MD5 hash of option compatibility string */
109 108
 #define D_PID_DEBUG_LOW      LOGLEV(4, 63, 0)        /* show low-freq packet-id debugging info */
110 109
 #define D_PID_DEBUG_MEDIUM   LOGLEV(4, 64, 0)        /* show medium-freq packet-id debugging info */
111 110
 
... ...
@@ -1330,21 +1330,6 @@ do_route (const struct options *options,
1330 1330
 }
1331 1331
 
1332 1332
 /*
1333
- * Save current pulled options string in the c1 context store, so we can
1334
- * compare against it after possible future restarts.
1335
- */
1336
-#if P2MP
1337
-static void
1338
-save_pulled_options_digest (struct context *c, const struct md5_digest *newdigest)
1339
-{
1340
-  if (newdigest)
1341
-    c->c1.pulled_options_digest_save = *newdigest;
1342
-  else
1343
-    md5_digest_clear (&c->c1.pulled_options_digest_save);
1344
-}
1345
-#endif
1346
-
1347
-/*
1348 1333
  * initialize tun/tap device object
1349 1334
  */
1350 1335
 static void
... ...
@@ -1522,7 +1507,7 @@ do_close_tun_simple (struct context *c)
1522 1522
   c->c1.tuntap = NULL;
1523 1523
   c->c1.tuntap_owned = false;
1524 1524
 #if P2MP
1525
-  save_pulled_options_digest (c, NULL); /* delete C1-saved pulled_options_digest */
1525
+  CLEAR (c->c1.pulled_options_digest_save);
1526 1526
 #endif
1527 1527
 }
1528 1528
 
... ...
@@ -1634,6 +1619,20 @@ tun_abort()
1634 1634
  * Handle delayed tun/tap interface bringup due to --up-delay or --pull
1635 1635
  */
1636 1636
 
1637
+#if P2MP
1638
+/**
1639
+ * Helper for do_up().  Take two option hashes and return true if they are not
1640
+ * equal, or either one is all-zeroes.
1641
+ */
1642
+static bool
1643
+options_hash_changed_or_zero(const uint8_t (*a)[MD5_DIGEST_LENGTH],
1644
+    const uint8_t (*b)[MD5_DIGEST_LENGTH])
1645
+{
1646
+  const uint8_t zero[MD5_DIGEST_LENGTH] = {0};
1647
+  return memcmp (*a, *b, MD5_DIGEST_LENGTH) || memcmp (*a, zero, MD5_DIGEST_LENGTH);
1648
+}
1649
+#endif /* P2MP */
1650
+
1637 1651
 void
1638 1652
 do_up (struct context *c, bool pulled_options, unsigned int option_types_found)
1639 1653
 {
... ...
@@ -1658,8 +1657,8 @@ do_up (struct context *c, bool pulled_options, unsigned int option_types_found)
1658 1658
 	  if (!c->c2.did_open_tun
1659 1659
 	      && PULL_DEFINED (&c->options)
1660 1660
 	      && c->c1.tuntap
1661
-	      && (!md5_digest_defined (&c->c1.pulled_options_digest_save) || !md5_digest_defined (&c->c2.pulled_options_digest)
1662
-		  || !md5_digest_equal (&c->c1.pulled_options_digest_save, &c->c2.pulled_options_digest)))
1661
+	      && options_hash_changed_or_zero (&c->c1.pulled_options_digest_save,
1662
+		  &c->c2.pulled_options_digest))
1663 1663
 	    {
1664 1664
 	      /* if so, close tun, delete routes, then reinitialize tun and add routes */
1665 1665
 	      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)
1674 1674
       if (c->c2.did_open_tun)
1675 1675
 	{
1676 1676
 #if P2MP
1677
-	  save_pulled_options_digest (c, &c->c2.pulled_options_digest);
1677
+	  memcpy(c->c1.pulled_options_digest_save, c->c2.pulled_options_digest,
1678
+	      sizeof(c->c1.pulled_options_digest_save));
1678 1679
 #endif
1679 1680
 
1680 1681
 	  /* if --route-delay was specified, start timer */
... ...
@@ -2732,20 +2732,14 @@ do_compute_occ_strings (struct context *c)
2732 2732
   c->c2.options_string_remote =
2733 2733
     options_string (&c->options, &c->c2.frame, c->c1.tuntap, true, &gc);
2734 2734
 
2735
-  msg (D_SHOW_OCC, "Local Options String: '%s'", c->c2.options_string_local);
2736
-  msg (D_SHOW_OCC, "Expected Remote Options String: '%s'",
2737
-       c->c2.options_string_remote);
2735
+  msg (D_SHOW_OCC, "Local Options String (VER=%s): '%s'",
2736
+      options_string_version (c->c2.options_string_local, &gc),
2737
+      c->c2.options_string_local);
2738
+  msg (D_SHOW_OCC, "Expected Remote Options String (VER=%s): '%s'",
2739
+      options_string_version (c->c2.options_string_remote, &gc),
2740
+      c->c2.options_string_remote);
2738 2741
 
2739 2742
 #ifdef ENABLE_CRYPTO
2740
-  msg (D_SHOW_OCC_HASH, "Local Options hash (VER=%s): '%s'",
2741
-       options_string_version (c->c2.options_string_local, &gc),
2742
-       md5sum ((uint8_t*)c->c2.options_string_local,
2743
-	       strlen (c->c2.options_string_local), 9, &gc));
2744
-  msg (D_SHOW_OCC_HASH, "Expected Remote Options hash (VER=%s): '%s'",
2745
-       options_string_version (c->c2.options_string_remote, &gc),
2746
-       md5sum ((uint8_t*)c->c2.options_string_remote,
2747
-	       strlen (c->c2.options_string_remote), 9, &gc));
2748
-
2749 2743
   if (c->c2.tls_multi)
2750 2744
     tls_multi_init_set_options (c->c2.tls_multi,
2751 2745
 				c->c2.options_string_local,
... ...
@@ -202,7 +202,7 @@ struct context_1
202 202
 #endif
203 203
 
204 204
   /* if client mode, hash of option strings we pulled from server */
205
-  struct md5_digest pulled_options_digest_save;
205
+  uint8_t pulled_options_digest_save[MD5_DIGEST_LENGTH];
206 206
                                 /**< Hash of option strings received from the
207 207
                                  *   remote OpenVPN server.  Only used in
208 208
                                  *   client-mode. */
... ...
@@ -467,8 +467,8 @@ struct context_2
467 467
 
468 468
   /* hash of pulled options, so we can compare when options change */
469 469
   bool pulled_options_md5_init_done;
470
-  struct md5_state pulled_options_state;
471
-  struct md5_digest pulled_options_digest;
470
+  md_ctx_t pulled_options_state;
471
+  uint8_t pulled_options_digest[MD5_DIGEST_LENGTH];
472 472
 
473 473
   struct event_timeout server_poll_interval;
474 474
 
... ...
@@ -465,7 +465,7 @@ process_incoming_push_msg (struct context *c,
465 465
 	  struct buffer buf_orig = buf;
466 466
 	  if (!c->c2.pulled_options_md5_init_done)
467 467
 	    {
468
-	      md5_state_init (&c->c2.pulled_options_state);
468
+	      md_ctx_init(&c->c2.pulled_options_state, md_kt_get("MD5"));
469 469
 	      c->c2.pulled_options_md5_init_done = true;
470 470
 	    }
471 471
 	  if (!c->c2.did_pre_pull_restore)
... ...
@@ -482,13 +482,14 @@ process_incoming_push_msg (struct context *c,
482 482
 	      {
483 483
 	      case 0:
484 484
 	      case 1:
485
-		md5_state_update (&c->c2.pulled_options_state, BPTR(&buf_orig), BLEN(&buf_orig));
486
-		md5_state_final (&c->c2.pulled_options_state, &c->c2.pulled_options_digest);
485
+		md_ctx_update (&c->c2.pulled_options_state, BPTR(&buf_orig), BLEN(&buf_orig));
486
+		md_ctx_final (&c->c2.pulled_options_state, c->c2.pulled_options_digest);
487
+		md_ctx_cleanup (&c->c2.pulled_options_state);
487 488
 	        c->c2.pulled_options_md5_init_done = false;
488 489
 		ret = PUSH_MSG_REPLY;
489 490
 		break;
490 491
 	      case 2:
491
-		md5_state_update (&c->c2.pulled_options_state, BPTR(&buf_orig), BLEN(&buf_orig));
492
+		md_ctx_update (&c->c2.pulled_options_state, BPTR(&buf_orig), BLEN(&buf_orig));
492 493
 		ret = PUSH_MSG_CONTINUATION;
493 494
 		break;
494 495
 	      }