Browse code

reload CRL only if file was modified

In order to prevent annoying delays upon client connection,
reload the CRL file only if it was modified since the last
reload operation.
If not, keep on using the already stored CRL.

This change will boost client connection time in instances
where the CRL file is quite large (dropping from several
seconds to few milliseconds).

Cc: Steffan Karger <steffan.karger@fox-it.com>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20161201104145.23821-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13345.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Antonio Quartulli authored on 2016/12/01 19:41:45
Showing 7 changed files
... ...
@@ -255,6 +255,11 @@ User-visible Changes
255 255
   restarts the ``dnscache`` service - this had unwanted side effects, and
256 256
   seems to be no longer necessary with currently supported Windows versions.
257 257
 
258
+- OpenVPN now reloads a CRL only if the modication time or file size has
259
+  changed, instead of for each new connection.  This reduces the connection
260
+  setup time, in particular when using large CRLs.
261
+
262
+
258 263
 Maintainer-visible changes
259 264
 --------------------------
260 265
 - OpenVPN no longer supports building with crypto support, but without TLS
... ...
@@ -512,6 +512,54 @@ tls_version_parse(const char *vstr, const char *extra)
512 512
     return TLS_VER_BAD;
513 513
 }
514 514
 
515
+/**
516
+ * Load (or possibly reload) the CRL file into the SSL context.
517
+ * No reload is performed under the following conditions:
518
+ * - the CRL file was passed inline
519
+ * - the CRL file was not modified since the last (re)load
520
+ *
521
+ * @param ssl_ctx       The TLS context to use when reloading the CRL
522
+ * @param crl_file      The file name to load the CRL from, or
523
+ *                      "[[INLINE]]" in the case of inline files.
524
+ * @param crl_inline    A string containing the CRL
525
+ */
526
+static void
527
+tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
528
+		   const char *crl_file_inline)
529
+{
530
+  /* if something goes wrong with stat(), we'll store 0 as mtime */
531
+  platform_stat_t crl_stat = {0};
532
+
533
+  /*
534
+   * an inline CRL can't change at runtime, therefore there is no need to
535
+   * reload it. It will be reloaded upon config change + SIGHUP.
536
+   * Use always '1' as dummy timestamp in this case: it will trigger the
537
+   * first load, but will prevent any future reload.
538
+   */
539
+  if (crl_file_inline)
540
+    {
541
+      crl_stat.st_mtime = 1;
542
+    }
543
+  else if (platform_stat(crl_file, &crl_stat) < 0)
544
+    {
545
+      msg(M_WARN, "WARNING: Failed to stat CRL file, not (re)loading CRL.");
546
+      return;
547
+    }
548
+
549
+  /*
550
+   * Store the CRL if this is the first time or if the file was changed since
551
+   * the last load.
552
+   * Note: Windows does not support tv_nsec.
553
+   */
554
+  if ((ssl_ctx->crl_last_size == crl_stat.st_size) &&
555
+      (ssl_ctx->crl_last_mtime.tv_sec == crl_stat.st_mtime))
556
+    return;
557
+
558
+  ssl_ctx->crl_last_mtime.tv_sec = crl_stat.st_mtime;
559
+  ssl_ctx->crl_last_size = crl_stat.st_size;
560
+  backend_tls_ctx_reload_crl(ssl_ctx, crl_file, crl_file_inline);
561
+}
562
+
515 563
 /*
516 564
  * Initialize SSL context.
517 565
  * All files are in PEM format.
... ...
@@ -2581,7 +2629,10 @@ tls_process (struct tls_multi *multi,
2581 2581
 	  ks->state = S_START;
2582 2582
 	  state_change = true;
2583 2583
 
2584
-	  /* Reload the CRL before TLS negotiation */
2584
+	  /*
2585
+	   * Attempt CRL reload before TLS negotiation. Won't be performed if
2586
+	   * the file was not modified since the last reload
2587
+	   */
2585 2588
 	  if (session->opt->crl_file &&
2586 2589
 	      !(session->opt->ssl_flags & SSLF_CRL_VERIFY_DIR))
2587 2590
 	    {
... ...
@@ -353,7 +353,7 @@ void key_state_ssl_free(struct key_state_ssl *ks_ssl);
353 353
  *                      "[[INLINE]]" in the case of inline files.
354 354
  * @param crl_inline    A string containing the CRL
355 355
  */
356
-void tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx,
356
+void backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx,
357 357
     const char *crl_file, const char *crl_inline);
358 358
 
359 359
 /**
... ...
@@ -771,7 +771,7 @@ static void tls_version_to_major_minor(int tls_ver, int *major, int *minor) {
771 771
 }
772 772
 
773 773
 void
774
-tls_ctx_reload_crl(struct tls_root_ctx *ctx, const char *crl_file,
774
+backend_tls_ctx_reload_crl(struct tls_root_ctx *ctx, const char *crl_file,
775 775
     const char *crl_inline)
776 776
 {
777 777
   ASSERT (crl_file);
... ...
@@ -74,6 +74,8 @@ struct tls_root_ctx {
74 74
     mbedtls_x509_crt *ca_chain;		/**< CA chain for remote verification */
75 75
     mbedtls_pk_context *priv_key;	/**< Local private key */
76 76
     mbedtls_x509_crl *crl;              /**< Certificate Revocation List */
77
+    struct timespec crl_last_mtime;     /**< CRL last modification time */
78
+    off_t crl_last_size;		/**< size of last loaded CRL */
77 79
 #if defined(ENABLE_PKCS11)
78 80
     mbedtls_pkcs11_context *priv_key_pkcs11;	/**< PKCS11 private key */
79 81
 #endif
... ...
@@ -772,7 +772,7 @@ end:
772 772
 }
773 773
 
774 774
 void
775
-tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
775
+backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
776 776
         const char *crl_inline)
777 777
 {
778 778
   X509_CRL *crl = NULL;
... ...
@@ -49,6 +49,8 @@
49 49
  */
50 50
 struct tls_root_ctx {
51 51
     SSL_CTX *ctx;
52
+    struct timespec crl_last_mtime;
53
+    off_t crl_last_size;
52 54
 };
53 55
 
54 56
 struct key_state_ssl {