[Openvpn-devel,v3,06/18] A helper function to import private key for management-external-key

Message ID 20211214165928.30676-7-selva.nair@gmail.com
State Accepted
Headers show
Series External key provider for use with OpenSSL 3 | expand

Commit Message

Selva Nair Dec. 14, 2021, 5:59 a.m. UTC
From: Selva Nair <selva.nair@gmail.com>

- Leverage keymgmt_import through EVP_PKEY_new_fromdata() to
  import "management-external-key"

- When required, use this to set SSL_CTX_use_PrivateKey

The sign_op is not implemented yet. This will error out while
signing with --management-external-key. The next commit
fixes that.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpn/Makefile.am   |   1 +
 src/openvpn/ssl_openssl.c |  11 ++++
 src/openvpn/xkey_common.h |  11 ++++
 src/openvpn/xkey_helper.c | 106 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 129 insertions(+)
 create mode 100644 src/openvpn/xkey_helper.c

Comments

Arne Schwabe Jan. 19, 2022, 11:53 p.m. UTC | #1
Am 14.12.21 um 17:59 schrieb selva.nair@gmail.com:
> From: Selva Nair <selva.nair@gmail.com>
> 
> - Leverage keymgmt_import through EVP_PKEY_new_fromdata() to
>    import "management-external-key"
> 
> - When required, use this to set SSL_CTX_use_PrivateKey
> 
> The sign_op is not implemented yet. This will error out while
> signing with --management-external-key. The next commit
> fixes that.

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering Jan. 20, 2022, 3:43 a.m. UTC | #2
Compile tested with 3.0.1 and glanced over the code.  Not actually
tested (no management-external-key here) but I know that Arne is using
*this* in his Android app, so it got a good beating :-)

There might be a memory leak lurking here:

+#ifdef HAVE_XKEY_PROVIDER
+    EVP_PKEY *privkey = xkey_load_management_key(tls_libctx, pkey);
+    if (!privkey
+        || !SSL_CTX_use_PrivateKey(ctx->ctx, privkey))
+    {
+        goto cleanup;
+    }
+    EVP_PKEY_free(privkey);
+#else

if I read this right, the actual signing operation is happening
in SSL_CTX_use_PrivateKey() - so, if the key can be loaded fine
(privkey != NULL) but the actual signing fails, we "goto cleanup",
and never EVP_PKEY_free() it.  But I might be misunderstanding this.

Fixed one typo in a comment ("avaialble") on the fly.  Hope that 
won't come back as a "context not matching" conflict later on.


Your patch has been applied to the master branch.

commit c279986bf4814aad72f9358d8509aa35f54ff662
Author: Selva Nair
Date:   Tue Dec 14 11:59:16 2021 -0500

     A helper function to import private key for management-external-key

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20211214165928.30676-7-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23443.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index 432efe73..0331298b 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -129,6 +129,7 @@  openvpn_SOURCES = \
 	tun.c tun.h \
 	vlan.c vlan.h \
 	xkey_provider.c xkey_common.h \
+	xkey_helper.c \
 	win32.h win32.c \
 	win32-util.h win32-util.c \
 	cryptoapi.h cryptoapi.c
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index bdaa7a2b..23c74f55 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1486,6 +1486,15 @@  tls_ctx_use_management_external_key(struct tls_root_ctx *ctx)
     EVP_PKEY *pkey = X509_get0_pubkey(cert);
     ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
 
+#ifdef HAVE_XKEY_PROVIDER
+    EVP_PKEY *privkey = xkey_load_management_key(tls_libctx, pkey);
+    if (!privkey
+        || !SSL_CTX_use_PrivateKey(ctx->ctx, privkey))
+    {
+        goto cleanup;
+    }
+    EVP_PKEY_free(privkey);
+#else
     if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA)
     {
         if (!tls_ctx_use_external_rsa_key(ctx, pkey))
@@ -1514,6 +1523,8 @@  tls_ctx_use_management_external_key(struct tls_root_ctx *ctx)
     }
 #endif /* OPENSSL_VERSION_NUMBER > 1.1.0 dev && !defined(OPENSSL_NO_EC) */
 
+#endif /* HAVE_XKEY_PROVIDER */
+
     ret = 0;
 cleanup:
     if (ret)
diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h
index f46bacd2..5bda5e30 100644
--- a/src/openvpn/xkey_common.h
+++ b/src/openvpn/xkey_common.h
@@ -82,6 +82,17 @@  typedef int (XKEY_EXTERNAL_SIGN_fn)(void *handle, unsigned char *sig, size_t *si
  */
 typedef void (XKEY_PRIVKEY_FREE_fn)(void *handle);
 
+/**
+ * Generate an encapsulated EVP_PKEY for management-external-key
+ *
+ * @param libctx library context in which xkey provider has been loaded
+ * @param pubkey corresponding pubkey in the default provider's context
+ *
+ * @returns a new EVP_PKEY in the provider's keymgmt context.
+ * The pubkey is up-refd if retained -- the caller can free it after return
+ */
+EVP_PKEY *xkey_load_management_key(OSSL_LIB_CTX *libctx, EVP_PKEY *pubkey);
+
 #endif /* HAVE_XKEY_PROVIDER */
 
 #endif /* XKEY_COMMON_H_ */
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
new file mode 100644
index 00000000..51cfb12b
--- /dev/null
+++ b/src/openvpn/xkey_helper.c
@@ -0,0 +1,106 @@ 
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ *             over a single TCP/UDP port, with support for SSL/TLS-based
+ *             session authentication and key exchange,
+ *             packet encryption, packet authentication, and
+ *             packet compression.
+ *
+ *  Copyright (C) 2021 Selva Nair <selva.nair@gmail.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by the
+ *  Free Software Foundation, either version 2 of the License,
+ *  or (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#elif defined(_MSC_VER)
+#include "config-msvc.h"
+#endif
+
+#include "syshead.h"
+#include "error.h"
+#include "buffer.h"
+#include "xkey_common.h"
+
+#ifdef HAVE_XKEY_PROVIDER
+
+#include <openssl/provider.h>
+#include <openssl/params.h>
+#include <openssl/core_dispatch.h>
+#include <openssl/core_object.h>
+#include <openssl/core_names.h>
+#include <openssl/store.h>
+#include <openssl/evp.h>
+#include <openssl/err.h>
+
+static const char *const props = XKEY_PROV_PROPS;
+
+XKEY_EXTERNAL_SIGN_fn xkey_management_sign;
+
+/**
+ * Load external key for signing via management interface.
+ * The public key must be passed in by the caller as we may not
+ * be able to get it from the management.
+ * Returns an EVP_PKEY object attached to xkey provider.
+ * Caller must free it when no longer needed.
+ */
+EVP_PKEY *
+xkey_load_management_key(OSSL_LIB_CTX *libctx, EVP_PKEY *pubkey)
+{
+    EVP_PKEY *pkey = NULL;
+    ASSERT(pubkey);
+
+    /* Management interface doesnt require any handle to be
+     * stored in the key. We use a dummy pointer as we do need a
+     * non-NULL value to indicate private key is avaialble.
+     */
+    void *dummy = & "dummy";
+
+    const char *origin = "management";
+    XKEY_EXTERNAL_SIGN_fn *sign_op = xkey_management_sign;
+
+    /* UTF8 string pointers in here are only read from, so cast is safe */
+    OSSL_PARAM params[] = {
+        {"xkey-origin", OSSL_PARAM_UTF8_STRING, (char *) origin, 0, 0},
+        {"pubkey", OSSL_PARAM_OCTET_STRING, &pubkey, sizeof(pubkey), 0},
+        {"handle", OSSL_PARAM_OCTET_PTR, &dummy, sizeof(dummy), 0},
+        {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(sign_op), 0},
+        {NULL, 0, NULL, 0, 0}};
+
+    /* Do not use EVP_PKEY_new_from_pkey as that will take keymgmt from pubkey */
+    EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_from_name(libctx, EVP_PKEY_get0_type_name(pubkey), props);
+    if (!ctx
+        || EVP_PKEY_fromdata_init(ctx) != 1
+        || EVP_PKEY_fromdata(ctx, &pkey, EVP_PKEY_KEYPAIR, params) != 1)
+    {
+        msg(M_NONFATAL, "Error loading key into ovpn.xkey provider");
+    }
+    if (ctx)
+    {
+        EVP_PKEY_CTX_free(ctx);
+    }
+
+    return pkey;
+}
+
+/* not yet implemented */
+int
+xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
+                     const unsigned char *tbs, size_t tbslen, XKEY_SIGALG alg)
+{
+    msg(M_FATAL, "FATAL ERROR: A sign callback for this key is not implemented.");
+    return 0;
+}
+
+#endif /* HAVE_XKEY_PROVIDER */