Refactor tls_ctx_use_external_private_key()

OpenSSL's tls_ctx_load_cert_file() had a parameter in which a copy of the
context's certificate chain was stored on return, used by
tls_ctx_use_external_private_key() only and free()d immediately thereafter.

PolarSSL also supported this output parameter, but returned a pointer to
the
context's certificate chain (rather than to a copy of the certificate, as
OpenSSL does) - which meant that we would have to #ifdef the free().

PolarSSL cannot make a copy of a certificate chain, and OpenSSL cannot
store a
pointer to (instead of a copy of) the cert.

So remove the output parameter from tls_ctx_load_cert_file() and
incorporate
the needed functionality directly into tls_ctx_use_external_private_key()
(which is straightforward for both OpenSSL and PolarSSL, as long as you
don't
try to support both at once.)

Signed-off-by: Joachim Schipper <joachim.schipper@fox-it.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1379587649-25506-2-git-send-email-steffan.karger@fox-it.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/7888
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 4203fc5c22b9a86db4617aee22bca71810fbe82b..bd19d75421ed9190b9b4d4743605e7312b02a933 100644 (file)
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -509,12 +509,8 @@ init_ssl (const struct options *options, struct tls_root_ctx *new_ctx)
 #ifdef MANAGMENT_EXTERNAL_KEY
   else if ((options->management_flags & MF_EXTERNAL_KEY) && options->cert_file)
     {
-      openvpn_x509_cert_t *my_cert = NULL;
-      tls_ctx_load_cert_file(new_ctx, options->cert_file, options->cert_file_inline,
-         &my_cert);
-      tls_ctx_use_external_private_key(new_ctx, my_cert);
-
-      tls_ctx_free_cert_file(my_cert);
+      tls_ctx_use_external_private_key(new_ctx, options->cert_file,
+         options->cert_file_inline);
     }
 #endif
   else
@@ -522,7 +518,7 @@ init_ssl (const struct options *options, struct tls_root_ctx *new_ctx)
       /* Load Certificate */
       if (options->cert_file)
        {
-          tls_ctx_load_cert_file(new_ctx, options->cert_file, options->cert_file_inline, NULL);
+          tls_ctx_load_cert_file(new_ctx, options->cert_file, options->cert_file_inline);
        }
 
       /* Load Private Key */

diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index 4d2958c7167c6e29bd2b57dc715aad1e5268b214..07cb9abce4b519c2df215e5a4779e818d826cae3 100644 (file)
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -215,27 +215,13 @@ void tls_ctx_load_cryptoapi(struct tls_root_ctx *ctx, const char *cryptoapi_cert
  * Load certificate file into the given TLS context. If the given certificate
  * file contains a certificate chain, load the whole chain.
  *
- * If the x509 parameter is not NULL, the certificate will be returned in it.
- *
  * @param ctx                  TLS context to use
  * @param cert_file            The file name to load the certificate from, or
  *                             "[[INLINE]]" in the case of inline files.
  * @param cert_file_inline     A string containing the certificate
- * @param x509                 An optional certificate, if x509 is NULL,
- *                             do nothing, if x509 is not NULL, *x509 will be
- *                             allocated and filled with the loaded certificate.
- *                             *x509 must be NULL.
  */
 void tls_ctx_load_cert_file (struct tls_root_ctx *ctx, const char *cert_file,
-    const char *cert_file_inline, openvpn_x509_cert_t **x509
-    );
-
-/**
- * Free the given certificate
- *
- * @param x509                 certificate to free
- */
-void tls_ctx_free_cert_file (openvpn_x509_cert_t *x509);
+    const char *cert_file_inline);
 
 /**
  * Load private key file into the given TLS context.
@@ -255,17 +241,19 @@ int tls_ctx_load_priv_file (struct tls_root_ctx *ctx, const char *priv_key_file,
 #ifdef MANAGMENT_EXTERNAL_KEY
 
 /**
- * Tell the management interface to load the external private key matching
- * the given certificate.
+ * Tell the management interface to load the given certificate and the external
+ * private key matching the given certificate.
  *
  * @param ctx                  TLS context to use
- * @param cert                 The certificate file to load the private key for
+ * @param cert_file            The file name to load the certificate from, or
  *                             "[[INLINE]]" in the case of inline files.
+ * @param cert_file_inline     A string containing the certificate
  *
  * @return                     1 if an error occurred, 0 if parsing was
  *                             successful.
  */
-int tls_ctx_use_external_private_key (struct tls_root_ctx *ctx, openvpn_x509_cert_t *cert);
+int tls_ctx_use_external_private_key (struct tls_root_ctx *ctx,
+    const char *cert_file, const char *cert_file_inline);
 #endif
 
 

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index e3926914a8e32c76b9db559fd21090c1c19848ed..d6e194af13dc9393df454f594bf06b0ef18d72fe 100644 (file)
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -473,9 +473,10 @@ tls_ctx_add_extra_certs (struct tls_root_ctx *ctx, BIO *bio)
     }
 }
 
-void
-tls_ctx_load_cert_file (struct tls_root_ctx *ctx, const char *cert_file,
-    const char *cert_file_inline, X509 **x509
+/* Like tls_ctx_load_cert, but returns a copy of the certificate in **X509 */
+static void
+tls_ctx_load_cert_file_and_copy (struct tls_root_ctx *ctx,
+    const char *cert_file, const char *cert_file_inline, X509 **x509
     )
 {
   BIO *in = NULL;
@@ -529,6 +530,13 @@ end:
     X509_free (x);
 }
 
+void
+tls_ctx_load_cert_file (struct tls_root_ctx *ctx, const char *cert_file,
+    const char *cert_file_inline)
+{
+  tls_ctx_load_cert_file_ext(ctx, cert_file, cert_file_inline, NULL);
+}
+
 void
 tls_ctx_free_cert_file (X509 *x509)
 {
@@ -665,15 +673,19 @@ rsa_priv_enc(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, i
 }
 
 int
-tls_ctx_use_external_private_key (struct tls_root_ctx *ctx, X509 *cert)
+tls_ctx_use_external_private_key (struct tls_root_ctx *ctx,
+    const char *cert_file, const char *cert_file_inline)
 {
   RSA *rsa = NULL;
   RSA *pub_rsa;
   RSA_METHOD *rsa_meth;
+  X509 *cert = NULL;
 
   ASSERT (NULL != ctx);
   ASSERT (NULL != cert);
 
+  tls_ctx_load_cert_file_ext(ctx, cert_file, cert_file_inline, &cert);
+
   /* allocate custom RSA method object */
   ALLOC_OBJ_CLEAR (rsa_meth, RSA_METHOD);
   rsa_meth->name = "OpenVPN external private key RSA Method";
@@ -708,10 +720,13 @@ tls_ctx_use_external_private_key (struct tls_root_ctx *ctx, X509 *cert)
   if (!SSL_CTX_use_RSAPrivateKey(ctx->ctx, rsa))
     goto err;
 
+  X509_free(cert);
   RSA_free(rsa); /* doesn't necessarily free, just decrements refcount */
   return 1;
 
  err:
+  if (cert)
+    X509_free(cert);
   if (rsa)
     RSA_free(rsa);
   else

diff --git a/src/openvpn/ssl_polarssl.c b/src/openvpn/ssl_polarssl.c
index fb7322542278ada518229cf4db8f943ba99e58ef..1e2c4d59e20156bec3aac4f541c3d9c727fe529c 100644 (file)
--- a/src/openvpn/ssl_polarssl.c
+++ b/src/openvpn/ssl_polarssl.c
@@ -237,13 +237,10 @@ tls_ctx_load_cryptoapi(struct tls_root_ctx *ctx, const char *cryptoapi_cert)
 
 void
 tls_ctx_load_cert_file (struct tls_root_ctx *ctx, const char *cert_file,
-    const char *cert_file_inline,
-    openvpn_x509_cert_t **x509
+    const char *cert_file_inline
     )
 {
   ASSERT(NULL != ctx);
-  if (NULL != x509)
-    ASSERT(NULL == *x509);
 
   if (!strcmp (cert_file, INLINE_FILE_TAG) && cert_file_inline)
     {
@@ -256,16 +253,6 @@ tls_ctx_load_cert_file (struct tls_root_ctx *ctx, const char *cert_file,
       if (0 != x509parse_crtfile(ctx->crt_chain, cert_file))
        msg (M_FATAL, "Cannot load certificate file %s", cert_file);
     }
-  if (x509)
-    {
-      *x509 = ctx->crt_chain;
-    }
-}
-
-void
-tls_ctx_free_cert_file (openvpn_x509_cert_t *x509)
-{
-  x509_free(x509);
 }
 
 int
@@ -322,8 +309,9 @@ tls_ctx_load_priv_file (struct tls_root_ctx *ctx, const char *priv_key_file,
 
 #ifdef MANAGMENT_EXTERNAL_KEY
 
-int
-tls_ctx_use_external_private_key (struct tls_root_ctx *ctx, openvpn_x509_cert_t *cert)
+tls_ctx_use_external_private_key (struct tls_root_ctx *ctx,
+    const char *cert_file, const char *cert_file_inline
+    )
 {
   msg(M_FATAL, "Use of management external keys not yet supported for PolarSSL.");
   return false;