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>

diff --git a/Changes.rst b/Changes.rst
index abd725862532c37fa5dec7534add93cded7a752a..0594731d514eab5fa51ce0191f4e08c493b58bc8 100644 (file)
--- a/Changes.rst
+++ b/Changes.rst
@@ -255,6 +255,11 @@ User-visible Changes
   restarts the ``dnscache`` service - this had unwanted side effects, and
   seems to be no longer necessary with currently supported Windows versions.
 
+- OpenVPN now reloads a CRL only if the modication time or file size has
+  changed, instead of for each new connection.  This reduces the connection
+  setup time, in particular when using large CRLs.
+
+
 Maintainer-visible changes
 --------------------------
 - OpenVPN no longer supports building with crypto support, but without TLS

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 14d13312982ba7391dc0c8fb08c72108e61c7b76..34d163f622a0ea40a6def982fd08bd54d3ede058 100644 (file)
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -512,6 +512,54 @@ tls_version_parse(const char *vstr, const char *extra)
     return TLS_VER_BAD;
 }
 
+/**
+ * Load (or possibly reload) the CRL file into the SSL context.
+ * No reload is performed under the following conditions:
+ * - the CRL file was passed inline
+ * - the CRL file was not modified since the last (re)load
+ *
+ * @param ssl_ctx       The TLS context to use when reloading the CRL
+ * @param crl_file      The file name to load the CRL from, or
+ *                      "[[INLINE]]" in the case of inline files.
+ * @param crl_inline    A string containing the CRL
+ */
+static void
+tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
+                  const char *crl_file_inline)
+{
+  /* if something goes wrong with stat(), we'll store 0 as mtime */
+  platform_stat_t crl_stat = {0};
+
+  /*
+   * an inline CRL can't change at runtime, therefore there is no need to
+   * reload it. It will be reloaded upon config change + SIGHUP.
+   * Use always '1' as dummy timestamp in this case: it will trigger the
+   * first load, but will prevent any future reload.
+   */
+  if (crl_file_inline)
+    {
+      crl_stat.st_mtime = 1;
+    }
+  else if (platform_stat(crl_file, &crl_stat) < 0)
+    {
+      msg(M_WARN, "WARNING: Failed to stat CRL file, not (re)loading CRL.");
+      return;
+    }
+
+  /*
+   * Store the CRL if this is the first time or if the file was changed since
+   * the last load.
+   * Note: Windows does not support tv_nsec.
+   */
+  if ((ssl_ctx->crl_last_size == crl_stat.st_size) &&
+      (ssl_ctx->crl_last_mtime.tv_sec == crl_stat.st_mtime))
+    return;
+
+  ssl_ctx->crl_last_mtime.tv_sec = crl_stat.st_mtime;
+  ssl_ctx->crl_last_size = crl_stat.st_size;
+  backend_tls_ctx_reload_crl(ssl_ctx, crl_file, crl_file_inline);
+}
+
 /*
  * Initialize SSL context.
  * All files are in PEM format.
@@ -2581,7 +2629,10 @@ tls_process (struct tls_multi *multi,
          ks->state = S_START;
          state_change = true;
 
-         /* Reload the CRL before TLS negotiation */
+         /*
+          * Attempt CRL reload before TLS negotiation. Won't be performed if
+          * the file was not modified since the last reload
+          */
          if (session->opt->crl_file &&
              !(session->opt->ssl_flags & SSLF_CRL_VERIFY_DIR))
            {

diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index 0777c61cec11aad7f4bea2d69821ff9ceb714a22..3fbd2b4fbef16d7d8ddd1f42ff94962fdeba0882 100644 (file)
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -353,7 +353,7 @@ void key_state_ssl_free(struct key_state_ssl *ks_ssl);
  *                      "[[INLINE]]" in the case of inline files.
  * @param crl_inline    A string containing the CRL
  */
-void tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx,
+void backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx,
     const char *crl_file, const char *crl_inline);
 
 /**

diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 7fa35a70f30e8a6a9384d2ec4ec3da64560d35a4..11ee65b77c6f957c9737b652176e80c8e2f95b45 100644 (file)
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -771,7 +771,7 @@ static void tls_version_to_major_minor(int tls_ver, int *major, int *minor) {
 }
 
 void
-tls_ctx_reload_crl(struct tls_root_ctx *ctx, const char *crl_file,
+backend_tls_ctx_reload_crl(struct tls_root_ctx *ctx, const char *crl_file,
     const char *crl_inline)
 {
   ASSERT (crl_file);

diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h
index 3edeedc4889b293c86a1ba6928f6ff19d869a27d..a4a7f05c7b106dce94fc702d528cc1ae03f1e2df 100644 (file)
--- a/src/openvpn/ssl_mbedtls.h
+++ b/src/openvpn/ssl_mbedtls.h
@@ -74,6 +74,8 @@ struct tls_root_ctx {
     mbedtls_x509_crt *ca_chain;                /**< CA chain for remote verification */
     mbedtls_pk_context *priv_key;      /**< Local private key */
     mbedtls_x509_crl *crl;              /**< Certificate Revocation List */
+    struct timespec crl_last_mtime;     /**< CRL last modification time */
+    off_t crl_last_size;               /**< size of last loaded CRL */
 #if defined(ENABLE_PKCS11)
     mbedtls_pkcs11_context *priv_key_pkcs11;   /**< PKCS11 private key */
 #endif

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 51669fcf37668481bb7cd4eb304398cb31f5b598..4f472ffc9f94f7f848b657f5d2a8fabec0fc549a 100644 (file)
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -772,7 +772,7 @@ end:
 }
 
 void
-tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
+backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
         const char *crl_inline)
 {
   X509_CRL *crl = NULL;

diff --git a/src/openvpn/ssl_openssl.h b/src/openvpn/ssl_openssl.h
index 97dc7422c94367ccb10a8c6e56db10f04ecf6444..115ac43eef79f0b1c34078bd747a2d1437603796 100644 (file)
--- a/src/openvpn/ssl_openssl.h
+++ b/src/openvpn/ssl_openssl.h
@@ -49,6 +49,8 @@
  */
 struct tls_root_ctx {
     SSL_CTX *ctx;
+    struct timespec crl_last_mtime;
+    off_t crl_last_size;
 };
 
 struct key_state_ssl {