Upgrade OpenVPN 2.3 to PolarSSL 1.3

PATCH In review Steffan Karger 2 participants 4 messages Dec 19, 2015

Share & tools

Steffan Karger

author

2015-12-19 11:39 UTC #0 lore raw

PATCH 1/2 Upgrade OpenVPN 2.3 to PolarSSL 1.3

PolarSSL 1.2 is going end-of-support by 31-12-2015, so we have to move on. Newer versions of polarssl/mbedtls are already released (2.0-2.2), but as previously agreed upon, we will just move release/2.3 to polar 1.3, where master has been for a while now.

This commit removes support for PolarSSL 1.2. The mimimum required version of PolarSSL is now 1.3.8.

This commit is a combination of a number of commits related to upgrading or fixing polarssl 1.3 support from the master branch, adjusted to apply to the release/2.3 branch:
03df3a99 Upgrade to PolarSSL 1.3
cc1cee74 Update openvpn-plugin.h for PolarSSL 1.3.
4b9eaa1e Fix regression with password protected private keys (polarssl) d0f26fb5 polarssl: disable 1/n-1 record splitting
444a93ea polarssl: fix --client-cert-not-required
9571010a polarssl: also allocate PKCS#11 certificate object on demand 67a67e39 polarssl: don't use deprecated functions anymore 9d3b7cec polarssl: require >= 1.3.8

This commit was tested using:
* Regular private key file
* Password-protected private key file
* PKCS#11
* --management-external-key
* CRL file (with and w/o revoked cert)
* With and w/o tls-auth
* RSA and ECDSA key/certs

Signed-off-by Steffan Karger <[email protected]...>

---
 README.polarssl                   |   2 +-
 configure.ac                      |   4 +-
 include/openvpn-plugin.h          |   4 +-
 src/openvpn/crypto_polarssl.c     |  13 +-
 src/openvpn/pkcs11_polarssl.c     |  26 ++--
 src/openvpn/ssl_polarssl.c        | 299 ++++++++++++++++++++------------------
 src/openvpn/ssl_polarssl.h        |   7 +-
 src/openvpn/ssl_verify.c          |   2 +-
 src/openvpn/ssl_verify_backend.h  |   2 +-
 src/openvpn/ssl_verify_openssl.c  |   2 +-
 src/openvpn/ssl_verify_polarssl.c | 116 +++++----------
 src/openvpn/ssl_verify_polarssl.h |   6 +-
 12 files changed, 234 insertions(+), 249 deletions(-)

README.polarssl +1 -1

index 06f30bf..d34d44a 100644
@@ -7,7 +7,7 @@ To Build and Install,
 	make
 	make install
 
-This version depends on PolarSSL 1.2 (and requires at least 1.2.10).
+This version depends on PolarSSL 1.3 (and requires at least 1.3.8).
 
 *************************************************************************

configure.ac +2 -2

index a2f458d..7644c55 100644
@@ -835,13 +835,13 @@ if test "${with_crypto_library}" = "polarssl" ; then
 #include <polarssl/version.h>
 			]],
 			[[
-#if POLARSSL_VERSION_NUMBER < 0x01020A00 || POLARSSL_VERSION_NUMBER >= 0x01030000
+#if POLARSSL_VERSION_NUMBER < 0x01030800 || POLARSSL_VERSION_NUMBER >= 0x01040000
 #error invalid version
 #endif
 			]]
 		)],
 		[AC_MSG_RESULT([ok])],
-		[AC_MSG_ERROR([PolarSSL 1.2.x required and must be 1.2.10 or later])]
+		[AC_MSG_ERROR([PolarSSL 1.3.x required and must be 1.3.8 or later])]
 	)
 
 	polarssl_with_pkcs11="no"

include/openvpn-plugin.h +2 -2

index 03da92a..5f2d407 100644
@@ -29,10 +29,10 @@
 
 #ifdef ENABLE_SSL
 #ifdef ENABLE_CRYPTO_POLARSSL
-#include <polarssl/x509.h>
+#include <polarssl/x509_crt.h>
 #ifndef __OPENVPN_X509_CERT_T_DECLARED
 #define __OPENVPN_X509_CERT_T_DECLARED
-typedef x509_cert openvpn_x509_cert_t;
+typedef x509_crt openvpn_x509_cert_t;
 #endif
 #else
 #include <openssl/x509.h>

src/openvpn/crypto_polarssl.c +9 -4

index af79029..24712ed 100644
@@ -457,7 +457,7 @@ cipher_ctx_init (cipher_context_t *ctx, uint8_t *key, int key_len,
 
 void cipher_ctx_cleanup (cipher_context_t *ctx)
 {
-  cipher_free_ctx(ctx);
+  cipher_free(ctx);
 }
 
 int cipher_ctx_iv_length (const cipher_context_t *ctx)

@@ -487,7 +487,12 @@ cipher_ctx_get_cipher_kt (const cipher_ctx_t *ctx)
 
 int cipher_ctx_reset (cipher_context_t *ctx, uint8_t *iv_buf)
 {
-  return 0 == cipher_reset(ctx, iv_buf);
+  int retval = cipher_reset(ctx);
+
+  if (0 == retval)
+    retval = cipher_set_iv(ctx, iv_buf, ctx->cipher_info->iv_size);
+
+  return 0 == retval;
 }
 
 int cipher_ctx_update (cipher_context_t *ctx, uint8_t *dst, int *dst_len,

@@ -614,7 +619,7 @@ void
 md_ctx_final (md_context_t *ctx, uint8_t *dst)
 {
   ASSERT(0 == md_finish(ctx, dst));
-  ASSERT(0 == md_free_ctx(ctx));
+  md_free(ctx);
 }

@@ -645,7 +650,7 @@ hmac_ctx_init (md_context_t *ctx, const uint8_t *key, int key_len, const md_info
 void
 hmac_ctx_cleanup(md_context_t *ctx)
 {
-  ASSERT(0 == md_free_ctx(ctx));
+  md_free(ctx);
 }
 
 int

src/openvpn/pkcs11_polarssl.c +14 ~~-12~~

index 03b2bab..a58beef 100644
@@ -40,6 +40,7 @@
 #include "errlevel.h"
 #include "pkcs11_backend.h"
 #include <polarssl/pkcs11.h>
+#include <polarssl/x509.h>
 
 int
 pkcs11_init_tls_session(pkcs11h_certificate_t certificate,

@@ -49,20 +50,21 @@ pkcs11_init_tls_session(pkcs11h_certificate_t certificate,
 
   ASSERT (NULL != ssl_ctx);
 
+  ALLOC_OBJ_CLEAR (ssl_ctx->crt_chain, x509_crt);
   if (pkcs11_x509_cert_init(ssl_ctx->crt_chain, certificate)) {
       msg (M_FATAL, "PKCS#11: Cannot retrieve PolarSSL certificate object");
       goto cleanup;
   }
 
-  ssl_ctx->priv_key_pkcs11 = malloc(sizeof(pkcs11_context));
-
-  if (ssl_ctx->priv_key_pkcs11 == NULL) {
-      msg (M_FATAL, "PKCS#11: Cannot allocate PolarSSL private key object");
+  ALLOC_OBJ_CLEAR (ssl_ctx->priv_key_pkcs11, pkcs11_context);
+  if (pkcs11_priv_key_init(ssl_ctx->priv_key_pkcs11, certificate)) {
+      msg (M_FATAL, "PKCS#11: Cannot initialize PolarSSL private key object");
       goto cleanup;
   }
 
-  if (pkcs11_priv_key_init(ssl_ctx->priv_key_pkcs11, certificate)) {
-      msg (M_FATAL, "PKCS#11: Cannot initialize PolarSSL private key object");
+  ALLOC_OBJ_CLEAR (ssl_ctx->priv_key, pk_context);
+  if (0 != pk_init_ctx_rsa_alt(ssl_ctx->priv_key, ssl_ctx->priv_key_pkcs11,
+	ssl_pkcs11_decrypt, ssl_pkcs11_sign, ssl_pkcs11_key_len)) {
       goto cleanup;
   }

@@ -78,14 +80,14 @@ pkcs11_certificate_dn (pkcs11h_certificate_t cert, struct gc_arena *gc)
   char *ret = NULL;
   char dn[1024] = {0};
 
-  x509_cert polar_cert = {0};
+  x509_crt polar_cert = {0};
 
   if (pkcs11_x509_cert_init(&polar_cert, cert)) {
       msg (M_FATAL, "PKCS#11: Cannot retrieve PolarSSL certificate object");
       goto cleanup;
   }
 
-  if (-1 == x509parse_dn_gets (dn, sizeof(dn), &polar_cert.subject)) {
+  if (-1 == x509_dn_gets (dn, sizeof(dn), &polar_cert.subject)) {
       msg (M_FATAL, "PKCS#11: PolarSSL cannot parse subject");
       goto cleanup;
   }

@@ -93,7 +95,7 @@ pkcs11_certificate_dn (pkcs11h_certificate_t cert, struct gc_arena *gc)
   ret = string_alloc(dn, gc);
 
 cleanup:
-  x509_free(&polar_cert);
+  x509_crt_free(&polar_cert);
 
   return ret;
 }

@@ -104,14 +106,14 @@ pkcs11_certificate_serial (pkcs11h_certificate_t cert, char *serial,
 {
   int ret = 1;
 
-  x509_cert polar_cert = {0};
+  x509_crt polar_cert = {0};
 
   if (pkcs11_x509_cert_init(&polar_cert, cert)) {
       msg (M_FATAL, "PKCS#11: Cannot retrieve PolarSSL certificate object");
       goto cleanup;
   }
 
-  if (-1 == x509parse_serial_gets (serial, serial_len, &polar_cert.serial)) {
+  if (-1 == x509_serial_gets (serial, serial_len, &polar_cert.serial)) {
       msg (M_FATAL, "PKCS#11: PolarSSL cannot parse serial");
       goto cleanup;
   }

@@ -119,7 +121,7 @@ pkcs11_certificate_serial (pkcs11h_certificate_t cert, char *serial,
   ret = 0;
 
 cleanup:
-  x509_free(&polar_cert);
+  x509_crt_free(&polar_cert);
 
   return ret;
 }

src/openvpn/ssl_polarssl.c +159 ~~-140~~

471 lines not on this page · raw message

src/openvpn/ssl_polarssl.h +4 -3

index fc9aa78..b80a509 100644
@@ -33,6 +33,7 @@
 #include "syshead.h"
 
 #include <polarssl/ssl.h>
+#include <polarssl/x509_crt.h>
 
 #if defined(ENABLE_PKCS11)
 #include <polarssl/pkcs11.h>

@@ -64,9 +65,9 @@ struct tls_root_ctx {
     int endpoint; 		/**< Whether or not this is a server or a client */
 
     dhm_context *dhm_ctx;	/**< Diffie-Helmann-Merkle context */
-    x509_cert *crt_chain;	/**< Local Certificate chain */
-    x509_cert *ca_chain;	/**< CA chain for remote verification */
-    rsa_context *priv_key;	/**< Local private key */
+    x509_crt *crt_chain;	/**< Local Certificate chain */
+    x509_crt *ca_chain;		/**< CA chain for remote verification */
+    pk_context *priv_key;	/**< Local private key */
 #if defined(ENABLE_PKCS11)
     pkcs11_context *priv_key_pkcs11;	/**< PKCS11 private key */
 #endif

src/openvpn/ssl_verify.c +1 -1

index 9693b81..ca69b41 100644
@@ -619,7 +619,7 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep
   string_replace_leading (subject, '-', '_');
 
   /* extract the username (default is CN) */
-  if (SUCCESS != x509_get_username (common_name, sizeof(common_name),
+  if (SUCCESS != backend_x509_get_username (common_name, sizeof(common_name),
       opt->x509_username_field, cert))
     {
       if (!cert_depth)

src/openvpn/ssl_verify_backend.h +1 -1

index 6f118c9..01e453e 100644
@@ -109,7 +109,7 @@ unsigned char *x509_get_sha1_hash (openvpn_x509_cert_t *cert, struct gc_arena *g
  *
  * @return 		\c FAILURE, \c or SUCCESS
  */
-result_t x509_get_username (char *common_name, int cn_len,
+result_t backend_x509_get_username (char *common_name, int cn_len,
     char * x509_username_field, openvpn_x509_cert_t *peer_cert);
 
 /*

src/openvpn/ssl_verify_openssl.c +1 -1

index 309e832..4750f02 100644
@@ -200,7 +200,7 @@ extract_x509_field_ssl (X509_NAME *x509, const char *field_name, char *out,
 }
 
 result_t
-x509_get_username (char *common_name, int cn_len,
+backend_x509_get_username (char *common_name, int cn_len,
     char * x509_username_field, X509 *peer_cert)
 {
 #ifdef ENABLE_X509ALTUSERNAME

src/openvpn/ssl_verify_polarssl.c +37 ~~-79~~

232 lines not on this page · raw message

src/openvpn/ssl_verify_polarssl.h +3 -3

index b259081..b5157ed 100644
@@ -33,11 +33,11 @@
 #include "syshead.h"
 #include "misc.h"
 #include "manage.h"
-#include <polarssl/x509.h>
+#include <polarssl/x509_crt.h>
 
 #ifndef __OPENVPN_X509_CERT_T_DECLARED
 #define __OPENVPN_X509_CERT_T_DECLARED
-typedef x509_cert openvpn_x509_cert_t;
+typedef x509_crt openvpn_x509_cert_t;
 #endif
 
 /** @name Function for authenticating a new connection from a remote OpenVPN peer

@@ -72,7 +72,7 @@ typedef x509_cert openvpn_x509_cert_t;
  *
  * @return The return value is 0 unless a fatal error occurred.
  */
-int verify_callback (void *session_obj, x509_cert *cert, int cert_depth,
+int verify_callback (void *session_obj, x509_crt *cert, int cert_depth,
     int *flags);
 
 /** @} name Function for authenticating a new connection from a remote OpenVPN peer */

Signature · 1 line

2.5.0

Steffan Karger

author

2015-12-19 11:39 UTC #1 lore raw

PATCH 2/2 Warn user if their certificate has expired

Previously, client certificate expiry warnings would only visible in the server log, and server certificate expiry warnings in the client log. Both after a (failed) connection attempt. This patch adds a warning to log when a users own certificate has expired (or is not yet valid) to ease problem diagnosis / error reporting.

Note that this is just a warning, since on some systems (notably embedded devices) there might be no correct time available.

The SSL_CTX_get0_certificate() function is available in OpenSSL 1.0.2+ only. Older versions seem to not have a useful alternative, and the certificate reference we need is hidden in an opaque struct. The remaining option would then be to add extra workaround code for the select group of people that do use an up-to-date openvpn, but do not update their openssl. I don't think that's worth it. So just disable the code for older openssl versions.

(This is a combination of commits 091edd8e and 644f2cdd from the master branch, adjusted to apply to the release/2.3 branch cleanly)

Signed-off-by Steffan Karger <[email protected]...>

---
 src/openvpn/ssl.c          |  3 +++
 src/openvpn/ssl_backend.h  |  9 +++++++++
 src/openvpn/ssl_openssl.c  | 29 +++++++++++++++++++++++++++++
 src/openvpn/ssl_polarssl.c | 14 ++++++++++++++
 4 files changed, 55 insertions(+)

src/openvpn/ssl.c +3 -0

index 202a03f..f728ffb 100644
@@ -556,6 +556,9 @@ init_ssl (const struct options *options, struct tls_root_ctx *new_ctx)
       tls_ctx_load_extra_certs(new_ctx, options->extra_certs_file, options->extra_certs_file_inline);
     }
 
+  /* Check certificate notBefore and notAfter */
+  tls_ctx_check_cert_time(new_ctx);
+
   /* Allowable ciphers */
   if (options->cipher_list)
     {

src/openvpn/ssl_backend.h +9 -0

index 6d47bd0..4b35e51 100644
@@ -176,6 +176,15 @@ void tls_ctx_set_options (struct tls_root_ctx *ctx, unsigned int ssl_flags);
 void tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers);
 
 /**
+ * Check our certificate notBefore and notAfter fields, and warn if the cert is
+ * either not yet valid or has expired.  Note that this is a non-fatal error,
+ * since we compare against the system time, which might be incorrect.
+ *
+ * @param ctx		TLS context to get our certificate from.
+ */
+void tls_ctx_check_cert_time (const struct tls_root_ctx *ctx);
+
+/**
  * Load Diffie Hellman Parameters, and load them into the library-specific
  * TLS context.
  *

src/openvpn/ssl_openssl.c +29 -0

index be33caa..13cce5e 100644
@@ -335,6 +335,35 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 }
 
 void
+tls_ctx_check_cert_time (const struct tls_root_ctx *ctx)
+{
+#if OPENSSL_VERSION_NUMBER >= 0x10002000L
+  int ret;
+  const X509 *cert = SSL_CTX_get0_certificate(ctx->ctx);
+
+  ret = X509_cmp_time (X509_get_notBefore (cert), NULL);
+  if (ret == 0)
+    {
+      msg (D_TLS_DEBUG_MED, "Failed to read certificate notBefore field.");
+    }
+  if (ret > 0)
+    {
+      msg (M_WARN, "WARNING: Your certificate is not yet valid!");
+    }
+
+  ret = X509_cmp_time (X509_get_notAfter (cert), NULL);
+  if (ret == 0)
+    {
+      msg (D_TLS_DEBUG_MED, "Failed to read certificate notAfter field.");
+    }
+  if (ret < 0)
+    {
+      msg (M_WARN, "WARNING: Your certificate has expired!");
+    }
+#endif
+}
+
+void
 tls_ctx_load_dh_params (struct tls_root_ctx *ctx, const char *dh_file,
     const char *dh_file_inline
     )

src/openvpn/ssl_polarssl.c +14 -0

index 8d9d219..def397d 100644
@@ -205,6 +205,20 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 }
 
 void
+tls_ctx_check_cert_time (const struct tls_root_ctx *ctx)
+{
+  if (x509_time_future (&ctx->crt_chain->valid_from))
+    {
+      msg (M_WARN, "WARNING: Your certificate is not yet valid!");
+    }
+
+  if (x509_time_expired (&ctx->crt_chain->valid_to))
+    {
+      msg (M_WARN, "WARNING: Your certificate has expired!");
+    }
+}
+
+void
 tls_ctx_load_dh_params (struct tls_root_ctx *ctx, const char *dh_file,
     const char *dh_inline
     )

Signature · 1 line

2.5.0

Gert Doering

2015-12-19 13:28 UTC #2 lore raw

[Openvpn-devel] [PATCH applied] Re: Warn user if their certificate has expired

ACK (as this is basically just the combined and already-ACKed patches from master, 644f2cdd13f4 + 091edd8e29968, which we did not cherrypick because the PolarSSL 1.3 upgrade needed to be done first).

Your patch has been applied to the release/2.3 branch.

commit dfd940bb008feb2fe981ceab3ed66fdb3cf21bdb

Author: Steffan Karger
Date:   Sat Dec 19 12:39:29 2015 +0100

     Warn user if their certificate has expired

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/10855
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

2015-12-19 13:24 UTC #3 lore raw

In reply to Steffan Karger

ACK, based on

- most of the changes are fairly mechanical (cert->crt, rsa->pk, and so on)
- some of the larger "changes" are mostly whitespace changes, and the actual change makes sense (using features of 1.3 not available before)
- I remember (and ACKed) half of the other changes from master
- not touching stuff outside PolarSSL
- and works for my tests built with 1.3(.9) :-)

Your patch has been applied to the release/2.3 branch.

I have added the change to Changes.rst, and committed that separately.

commit a40b2304fe967053debb4249704a344223707c96 (release/2.3)

Author: Steffan Karger
Date:   Sat Dec 19 12:39:28 2015 +0100

     Upgrade OpenVPN 2.3 to PolarSSL 1.3

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/10856
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering