* [Openvpn-devel] [PATCH v3 01/18] A built-in provider for using external key with OpenSSL 3.0
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 10:14 ` Arne Schwabe
2022-01-20 13:49 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 02/18] Implement KEYMGMT in the xkey provider selva.nair
` (16 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
Hooking into callbacks in RSA_METHOD and EVP_PKEY_METHOD
structures is deprecated in OpenSSL 3.0. For signing with
external keys that are not exportable (tokens, stores, etc.)
requires a custom provider interface so that key operations
are done under its context.
A single provider is enough for handling all external keys
we support -- management-external-key, cryptoapicert(CNG) and
pkcs11-helper. The series of patches starting with this implement
such a provider.
This patch implements only the provider_init function so
that it can be loaded, but has no capabilities. The required
interfaces are added in following commits.
v2 changes:
- Require OpenSSL 3.0.1 or newer: 3.0.0 is "buggy" as it
does not preferentially fetch operations from the keymgmt
of the key. This causes either an unsuccessful attempt at
exporting unexportable keys or an onerous requirement that
the external key's KEYMGMT should support a whole lot
of unrelated functionalities including key generation and
key exchange.
Fixed by PR #16725 in OpenSSL.
- Use a child libctx for internal use in the provider
v3 changes:
- Move OpenSSL version check for 3.0.1+ from configure to
xkey_common.h
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/Makefile.am | 1 +
src/openvpn/xkey_common.h | 45 ++++++++++
src/openvpn/xkey_provider.c | 169 ++++++++++++++++++++++++++++++++++++
3 files changed, 215 insertions(+)
create mode 100644 src/openvpn/xkey_common.h
create mode 100644 src/openvpn/xkey_provider.c
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index 5883c291..432efe73 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -128,6 +128,7 @@ openvpn_SOURCES = \
tls_crypt.c tls_crypt.h \
tun.c tun.h \
vlan.c vlan.h \
+ xkey_provider.c xkey_common.h \
win32.h win32.c \
win32-util.h win32-util.c \
cryptoapi.h cryptoapi.c
diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h
new file mode 100644
index 00000000..a3bc3f2a
--- /dev/null
+++ b/src/openvpn/xkey_common.h
@@ -0,0 +1,45 @@
+/*
+ * 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@...277...>
+ *
+ * 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.
+ */
+
+#ifndef XKEY_COMMON_H_
+#define XKEY_COMMON_H_
+
+#include <openssl/opensslv.h>
+#if OPENSSL_VERSION_NUMBER >= 0x30000010L && !defined(DISABLE_XKEY_PROVIDER)
+#define HAVE_XKEY_PROVIDER 1
+
+#include <openssl/provider.h>
+#include <openssl/core_dispatch.h>
+
+/**
+ * Initialization function for OpenVPN external key provider for OpenSSL
+ * Follows the function signature of OSSL_PROVIDER init()
+ */
+OSSL_provider_init_fn xkey_provider_init;
+
+#define XKEY_PROV_PROPS "provider=ovpn.xkey"
+
+#endif /* HAVE_XKEY_PROVIDER */
+
+#endif /* XKEY_COMMON_H_ */
diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c
new file mode 100644
index 00000000..d47faf0a
--- /dev/null
+++ b/src/openvpn/xkey_provider.c
@@ -0,0 +1,169 @@
+/*
+ * 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@...277...>
+ *
+ * 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>
+
+/* A descriptive name */
+static const char *provname = "OpenVPN External Key Provider";
+
+typedef struct
+{
+ OSSL_LIB_CTX *libctx; /**< a child libctx for our own use */
+} XKEY_PROVIDER_CTX;
+
+/* helper to print debug messages */
+#define xkey_dmsg(f, ...) \
+ do { \
+ dmsg(f|M_NOLF, "xkey_provider: In %s: ", __func__); \
+ dmsg(f|M_NOPREFIX, __VA_ARGS__); \
+ } while(0)
+
+/* main provider interface */
+
+/* provider callbacks we implement */
+static OSSL_FUNC_provider_query_operation_fn query_operation;
+static OSSL_FUNC_provider_gettable_params_fn gettable_params;
+static OSSL_FUNC_provider_get_params_fn get_params;
+static OSSL_FUNC_provider_teardown_fn teardown;
+
+static const OSSL_ALGORITHM *
+query_operation(void *provctx, int op, int *no_store)
+{
+ xkey_dmsg(D_LOW, "op = %d", op);
+
+ *no_store = 0;
+
+ switch (op)
+ {
+ case OSSL_OP_SIGNATURE:
+ return NULL;
+
+ case OSSL_OP_KEYMGMT:
+ return NULL;
+
+ default:
+ xkey_dmsg(D_LOW, "op not supported");
+ break;
+ }
+ return NULL;
+}
+
+static const OSSL_PARAM *
+gettable_params(void *provctx)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ static const OSSL_PARAM param_types[] = {
+ OSSL_PARAM_DEFN(OSSL_PROV_PARAM_NAME, OSSL_PARAM_UTF8_PTR, NULL, 0),
+ OSSL_PARAM_END
+ };
+
+ return param_types;
+}
+static int
+get_params(void *provctx, OSSL_PARAM params[])
+{
+ OSSL_PARAM *p;
+
+ xkey_dmsg(D_LOW, "entry");
+
+ p = OSSL_PARAM_locate(params, OSSL_PROV_PARAM_NAME);
+ if (p)
+ {
+ return (OSSL_PARAM_set_utf8_ptr(p, provname) != 0);
+ }
+
+ return 0;
+}
+
+static void
+teardown(void *provctx)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ XKEY_PROVIDER_CTX *prov = provctx;
+ if (prov && prov->libctx)
+ {
+ OSSL_LIB_CTX_free(prov->libctx);
+ }
+ OPENSSL_free(prov);
+}
+
+static const OSSL_DISPATCH dispatch_table[] = {
+ {OSSL_FUNC_PROVIDER_GETTABLE_PARAMS, (void (*)(void)) gettable_params},
+ {OSSL_FUNC_PROVIDER_GET_PARAMS, (void (*)(void)) get_params},
+ {OSSL_FUNC_PROVIDER_QUERY_OPERATION, (void (*)(void)) query_operation},
+ {OSSL_FUNC_PROVIDER_TEARDOWN, (void (*)(void)) teardown},
+ {0, NULL}
+};
+
+int
+xkey_provider_init(const OSSL_CORE_HANDLE *handle, const OSSL_DISPATCH *in,
+ const OSSL_DISPATCH **out, void **provctx)
+{
+ XKEY_PROVIDER_CTX *prov;
+
+ xkey_dmsg(D_LOW, "entry");
+
+ prov = OPENSSL_zalloc(sizeof(*prov));
+ if (!prov)
+ {
+ msg(M_NONFATAL, "xkey_provider_init: out of memory");
+ return 0;
+ }
+
+ /* Make a child libctx for our use and set default prop query
+ * on it to ensure calls we delegate won't loop back to us.
+ */
+ prov->libctx = OSSL_LIB_CTX_new_child(handle, in);
+
+ EVP_set_default_properties(prov->libctx, "provider!=ovpn.xkey");
+
+ *out = dispatch_table;
+ *provctx = prov;
+
+ return 1;
+}
+
+#endif /* HAVE_XKEY_PROVIDER */
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* [Openvpn-devel] [PATCH v3 02/18] Implement KEYMGMT in the xkey provider
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 01/18] A built-in provider for using external key with OpenSSL 3.0 selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 10:15 ` Arne Schwabe
2022-01-20 13:57 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 03/18] Implement SIGNATURE operations in " selva.nair
` (15 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
A minimal set of functions for keymgmt are implemented.
No support for external key import as yet, only native
keys. Support for native keys is required as keys may
get imported into us for some operations as well as
for comparison with unexportable external keys that we hold.
Implementation of signature callbacks is in the next commit.
v2 changes: This was commit 3/9 in v1
v3 changes: When OpenSSL native key is imported instead of duplicating
the whole key, use only the public components for public key.
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/xkey_provider.c | 375 +++++++++++++++++++++++++++++++++++-
1 file changed, 374 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c
index d47faf0a..a083ec2d 100644
--- a/src/openvpn/xkey_provider.c
+++ b/src/openvpn/xkey_provider.c
@@ -44,6 +44,9 @@
#include <openssl/evp.h>
#include <openssl/err.h>
+/* propq set all on all ops we implement */
+static const char *const props = XKEY_PROV_PROPS;
+
/* A descriptive name */
static const char *provname = "OpenVPN External Key Provider";
@@ -59,6 +62,376 @@ typedef struct
dmsg(f|M_NOPREFIX, __VA_ARGS__); \
} while(0)
+typedef enum
+{
+ ORIGIN_UNDEFINED = 0,
+ OPENSSL_NATIVE, /* native key imported in */
+ EXTERNAL_KEY
+} XKEY_ORIGIN;
+
+/**
+ * XKEY_KEYDATA: Our keydata encapsulation:
+ *
+ * We keep an opaque handle provided by the backend for the loaded
+ * key. It's passed back to the backend for any operation on private
+ * keys --- in practice, sign() op only.
+ *
+ * We also keep the public key in the form of a native OpenSSL EVP_PKEY.
+ * This allows us to do all public ops by calling ops in the default provider.
+ */
+typedef struct
+{
+ /* opaque handle dependent on KEY_ORIGIN -- could be NULL */
+ void *handle;
+ /* associated public key as an openvpn native key */
+ EVP_PKEY *pubkey;
+ /* origin of key -- native or external */
+ XKEY_ORIGIN origin;
+ XKEY_PROVIDER_CTX *prov;
+ int refcount; /* reference count */
+} XKEY_KEYDATA;
+
+#define KEYTYPE(key) ((key)->pubkey ? EVP_PKEY_get_id((key)->pubkey) : 0)
+#define KEYSIZE(key) ((key)->pubkey ? EVP_PKEY_get_size((key)->pubkey) : 0)
+
+/* keymgmt provider */
+
+/* keymgmt callbacks we implement */
+static OSSL_FUNC_keymgmt_new_fn keymgmt_new;
+static OSSL_FUNC_keymgmt_free_fn keymgmt_free;
+static OSSL_FUNC_keymgmt_load_fn keymgmt_load;
+static OSSL_FUNC_keymgmt_has_fn keymgmt_has;
+static OSSL_FUNC_keymgmt_match_fn keymgmt_match;
+static OSSL_FUNC_keymgmt_import_fn rsa_keymgmt_import;
+static OSSL_FUNC_keymgmt_import_fn ec_keymgmt_import;
+static OSSL_FUNC_keymgmt_import_types_fn keymgmt_import_types;
+static OSSL_FUNC_keymgmt_get_params_fn keymgmt_get_params;
+static OSSL_FUNC_keymgmt_gettable_params_fn keymgmt_gettable_params;
+static OSSL_FUNC_keymgmt_set_params_fn keymgmt_set_params;
+static OSSL_FUNC_keymgmt_query_operation_name_fn rsa_keymgmt_name;
+static OSSL_FUNC_keymgmt_query_operation_name_fn ec_keymgmt_name;
+
+static XKEY_KEYDATA *
+keydata_new()
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ XKEY_KEYDATA *key = OPENSSL_zalloc(sizeof(*key));
+ if (!key)
+ {
+ msg(M_NONFATAL, "xkey_keydata_new: out of memory");
+ }
+
+ return key;
+}
+
+static void
+keydata_free(XKEY_KEYDATA *key)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ if (!key || key->refcount-- > 0) /* free when refcount goes to zero */
+ {
+ return;
+ }
+ if (key->pubkey)
+ {
+ EVP_PKEY_free(key->pubkey);
+ }
+ OPENSSL_free(key);
+}
+
+static void *
+keymgmt_new(void *provctx)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ XKEY_KEYDATA *key = keydata_new();
+ if (key)
+ {
+ key->prov = provctx;
+ }
+
+ return key;
+}
+
+static void *
+keymgmt_load(const void *reference, size_t reference_sz)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ return NULL;
+}
+
+/**
+ * Key import function
+ * When key operations like sign/verify are done in our context
+ * the key gets imported into us. We will also use import to
+ * load an external key into the provider.
+ *
+ * For native keys we get called with standard OpenSSL params
+ * appropriate for the key. We just use it to create a native
+ * EVP_PKEY from params and assign to keydata->handle.
+ *
+ * Import of external keys -- to be implemented
+ */
+static int
+keymgmt_import(void *keydata, int selection, const OSSL_PARAM params[], const char *name)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ XKEY_KEYDATA *key = keydata;
+ ASSERT(key);
+
+ /* Our private key is immutable -- we import only if keydata is empty */
+ if (key->handle || key->pubkey)
+ {
+ msg(M_WARN, "Error: keymgmt_import: keydata not empty -- our keys are immutable");
+ return 0;
+ }
+
+ /* create a native public key and assign it to key->pubkey */
+ EVP_PKEY *pkey = NULL;
+ int selection_pub = selection & ~OSSL_KEYMGMT_SELECT_PRIVATE_KEY;
+
+ EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_from_name(key->prov->libctx, name, NULL);
+ if (!ctx
+ || (EVP_PKEY_fromdata_init(ctx) != 1)
+ || (EVP_PKEY_fromdata(ctx, &pkey, selection_pub, (OSSL_PARAM*) params) !=1))
+ {
+ msg(M_WARN, "Error: keymgmt_import failed for key type <%s>", name);
+ if (pkey)
+ {
+ EVP_PKEY_free(pkey);
+ }
+ if (ctx)
+ {
+ EVP_PKEY_CTX_free(ctx);
+ }
+ return 0;
+ }
+
+ key->pubkey = pkey;
+ key->origin = OPENSSL_NATIVE;
+ if (selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY)
+ {
+ /* create private key */
+ pkey = NULL;
+ if (EVP_PKEY_fromdata(ctx, &pkey, selection, (OSSL_PARAM*) params) == 1)
+ {
+ key->handle = pkey;
+ key->free = (XKEY_PRIVKEY_FREE_fn *) EVP_PKEY_free;
+ }
+ }
+ EVP_PKEY_CTX_free(ctx);
+
+ xkey_dmsg(D_LOW, "imported native %s key", EVP_PKEY_get0_type_name(pkey));
+ return 1;
+}
+
+static int
+rsa_keymgmt_import(void *keydata, int selection, const OSSL_PARAM params[])
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ return keymgmt_import(keydata, selection, params, "RSA");
+}
+
+static int
+ec_keymgmt_import(void *keydata, int selection, const OSSL_PARAM params[])
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ return keymgmt_import(keydata, selection, params, "EC");
+}
+
+/* This function has to exist for key import to work
+ * though we do not support import of individual params
+ * like n or e. We simply return an empty list here for
+ * both rsa and ec, which works.
+ */
+static const OSSL_PARAM *
+keymgmt_import_types(int selection)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ static const OSSL_PARAM key_types[] = { OSSL_PARAM_END };
+
+ if (selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY)
+ {
+ return key_types;
+ }
+ return NULL;
+}
+
+static void
+keymgmt_free(void *keydata)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ keydata_free(keydata);
+}
+
+static int
+keymgmt_has(const void *keydata, int selection)
+{
+ xkey_dmsg(D_LOW, "selection = %d", selection);
+
+ const XKEY_KEYDATA *key = keydata;
+ int ok = (key != NULL);
+
+ if (selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY)
+ {
+ ok = ok && key->pubkey;
+ }
+ if (selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY)
+ {
+ ok = ok && key->handle;
+ }
+
+ return ok;
+}
+
+static int
+keymgmt_match(const void *keydata1, const void *keydata2, int selection)
+{
+ const XKEY_KEYDATA *key1 = keydata1;
+ const XKEY_KEYDATA *key2 = keydata2;
+
+ xkey_dmsg(D_LOW, "entry");
+
+ int ret = key1 && key2 && key1->pubkey && key2->pubkey;
+
+ /* our keys always have pubkey -- we only match them */
+
+ if (selection & OSSL_KEYMGMT_SELECT_KEYPAIR)
+ {
+ ret = ret && EVP_PKEY_eq(key1->pubkey, key2->pubkey);
+ xkey_dmsg(D_LOW, "checking key pair match: res = %d", ret);
+ }
+
+ if (selection & OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS)
+ {
+ ret = ret && EVP_PKEY_parameters_eq(key1->pubkey, key2->pubkey);
+ xkey_dmsg(D_LOW, "checking parameter match: res = %d", ret);
+ }
+
+ return ret;
+}
+
+/* A minimal set of key params that we can return */
+static const OSSL_PARAM *
+keymgmt_gettable_params(void *provctx)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ static OSSL_PARAM gettable[] = {
+ OSSL_PARAM_int(OSSL_PKEY_PARAM_BITS, NULL),
+ OSSL_PARAM_int(OSSL_PKEY_PARAM_SECURITY_BITS, NULL),
+ OSSL_PARAM_int(OSSL_PKEY_PARAM_MAX_SIZE, NULL),
+ OSSL_PARAM_END
+ };
+ return gettable;
+}
+
+static int
+keymgmt_get_params(void *keydata, OSSL_PARAM *params)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ XKEY_KEYDATA *key = keydata;
+ if (!key || !key->pubkey)
+ {
+ return 0;
+ }
+
+ return EVP_PKEY_get_params(key->pubkey, params);
+}
+
+/**
+ * If the key is an encapsulated native key, we just call
+ * EVP_PKEY_set_params in the default context. Only those params
+ * supported by the default provider would work in this case.
+ */
+static int
+keymgmt_set_params(void *keydata, const OSSL_PARAM *params)
+{
+ XKEY_KEYDATA *key = keydata;
+ ASSERT(key);
+
+ xkey_dmsg(D_LOW, "entry");
+
+ if (key->origin != OPENSSL_NATIVE)
+ {
+ return 0; /* to be implemented */
+ }
+ else if (key->handle == NULL) /* once handle is set our key is immutable */
+ {
+ /* pubkey is always native -- just delegate */
+ return EVP_PKEY_set_params(key->pubkey, (OSSL_PARAM *)params);
+ }
+ else
+ {
+ msg(M_WARN, "xkey keymgmt_set_params: key is immutable");
+ }
+ return 1;
+}
+
+static const char *
+rsa_keymgmt_name(int id)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ return "RSA";
+}
+
+static const char *
+ec_keymgmt_name(int id)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ return "EC";
+}
+
+static const OSSL_DISPATCH rsa_keymgmt_functions[] = {
+ {OSSL_FUNC_KEYMGMT_NEW, (void (*)(void)) keymgmt_new},
+ {OSSL_FUNC_KEYMGMT_FREE, (void (*)(void)) keymgmt_free},
+ {OSSL_FUNC_KEYMGMT_LOAD, (void (*)(void)) keymgmt_load},
+ {OSSL_FUNC_KEYMGMT_HAS, (void (*)(void)) keymgmt_has},
+ {OSSL_FUNC_KEYMGMT_MATCH, (void (*)(void)) keymgmt_match},
+ {OSSL_FUNC_KEYMGMT_IMPORT, (void (*)(void)) rsa_keymgmt_import},
+ {OSSL_FUNC_KEYMGMT_IMPORT_TYPES, (void (*)(void)) keymgmt_import_types},
+ {OSSL_FUNC_KEYMGMT_GETTABLE_PARAMS, (void (*) (void)) keymgmt_gettable_params},
+ {OSSL_FUNC_KEYMGMT_GET_PARAMS, (void (*) (void)) keymgmt_get_params},
+ {OSSL_FUNC_KEYMGMT_SET_PARAMS, (void (*) (void)) keymgmt_set_params},
+ {OSSL_FUNC_KEYMGMT_SETTABLE_PARAMS, (void (*) (void)) keymgmt_gettable_params}, /* same as gettable */
+ {OSSL_FUNC_KEYMGMT_QUERY_OPERATION_NAME, (void (*)(void)) rsa_keymgmt_name},
+ {0, NULL }
+};
+
+static const OSSL_DISPATCH ec_keymgmt_functions[] = {
+ {OSSL_FUNC_KEYMGMT_NEW, (void (*)(void)) keymgmt_new},
+ {OSSL_FUNC_KEYMGMT_FREE, (void (*)(void)) keymgmt_free},
+ {OSSL_FUNC_KEYMGMT_LOAD, (void (*)(void)) keymgmt_load},
+ {OSSL_FUNC_KEYMGMT_HAS, (void (*)(void)) keymgmt_has},
+ {OSSL_FUNC_KEYMGMT_MATCH, (void (*)(void)) keymgmt_match},
+ {OSSL_FUNC_KEYMGMT_IMPORT, (void (*)(void)) ec_keymgmt_import},
+ {OSSL_FUNC_KEYMGMT_IMPORT_TYPES, (void (*)(void)) keymgmt_import_types},
+ {OSSL_FUNC_KEYMGMT_GETTABLE_PARAMS, (void (*) (void)) keymgmt_gettable_params},
+ {OSSL_FUNC_KEYMGMT_GET_PARAMS, (void (*) (void)) keymgmt_get_params},
+ {OSSL_FUNC_KEYMGMT_SET_PARAMS, (void (*) (void)) keymgmt_set_params},
+ {OSSL_FUNC_KEYMGMT_SETTABLE_PARAMS, (void (*) (void)) keymgmt_gettable_params}, /* same as gettable */
+ {OSSL_FUNC_KEYMGMT_QUERY_OPERATION_NAME, (void (*)(void)) ec_keymgmt_name},
+ {0, NULL }
+};
+
+const OSSL_ALGORITHM keymgmts[] = {
+ {"RSA:rsaEncryption", props, rsa_keymgmt_functions, "OpenVPN xkey RSA Key Manager"},
+ {"RSA-PSS:RSASSA-PSS", props, rsa_keymgmt_functions, "OpenVPN xkey RSA-PSS Key Manager"},
+ {"EC:id-ecPublicKey", props, ec_keymgmt_functions, "OpenVPN xkey EC Key Manager"},
+ {NULL, NULL, NULL, NULL}
+};
+
/* main provider interface */
/* provider callbacks we implement */
@@ -80,7 +453,7 @@ query_operation(void *provctx, int op, int *no_store)
return NULL;
case OSSL_OP_KEYMGMT:
- return NULL;
+ return keymgmts;
default:
xkey_dmsg(D_LOW, "op not supported");
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 02/18] Implement KEYMGMT in the xkey provider
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 02/18] Implement KEYMGMT in the xkey provider selva.nair
@ 2022-01-20 10:15 ` Arne Schwabe
2022-01-20 13:57 ` [Openvpn-devel] [PATCH applied] " Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 10:15 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> A minimal set of functions for keymgmt are implemented.
> No support for external key import as yet, only native
> keys. Support for native keys is required as keys may
> get imported into us for some operations as well as
> for comparison with unexportable external keys that we hold.
>
> Implementation of signature callbacks is in the next commit.
>
> v2 changes: This was commit 3/9 in v1
> v3 changes: When OpenSSL native key is imported instead of duplicating
> the whole key, use only the public components for public key.
>
Acked-By: Arne Schwabe <arne@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH applied] Re: Implement KEYMGMT in the xkey provider
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 02/18] Implement KEYMGMT in the xkey provider selva.nair
2022-01-20 10:15 ` Arne Schwabe
@ 2022-01-20 13:57 ` Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 13:57 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
Only compile-tested on Linux / OpenSSL 3.0.1 (and briefly glanced over
the code to see what happens). It breaks...
xkey_provider.c:223:16: error: 'XKEY_KEYDATA' has no member named 'free'
223 | key->free = (XKEY_PRIVKEY_FREE_fn *) EVP_PKEY_free;
| ^~
xkey_provider.c:223:26: error: 'XKEY_PRIVKEY_FREE_fn' undeclared (first use in this function)
223 | key->free = (XKEY_PRIVKEY_FREE_fn *) EVP_PKEY_free;
| ^~~~~~~~~~~~~~~~~~~~
*but* this is fixed in 03/18 - and as the whole patchset is basically
a no-op before all is in (it will break compilation with 3.0.1, so
might annoy someone doing bisect in the future, hitting just that commit
by chance...) I'll still merge & push.
Your patch has been applied to the master branch.
commit 44509116da50b53a5588318fb27b2e4a3da4ab6c
Author: Selva Nair
Date: Tue Dec 14 11:59:12 2021 -0500
Implement KEYMGMT in the xkey provider
Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20211214165928.30676-3-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23438.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 03/18] Implement SIGNATURE operations in xkey provider
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 01/18] A built-in provider for using external key with OpenSSL 3.0 selva.nair
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 02/18] Implement KEYMGMT in the xkey provider selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 10:16 ` Arne Schwabe
2022-01-20 14:08 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 04/18] Implement import of custom external keys selva.nair
` (14 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
- Basic frame work for announcing support for signature
operations
- DigestSign and Sign functions for native keys are also
implemented. Though strictly not needed, these functions
for native keys sets up the framework for signature operations.
They also help loading an exportable key from a file through
the provider for testing.
Subsequent commits will add support for signing with
external keys.
v2 changes:
- Remove verify operations which are no longer
required with proposed changes in OpenSSL 3.0.1 that we target.
- Undigested message is passed to the backend sign operation when
possible. This would allow more flexibility as some backends
prefer to do the hash operation internally.
This was 4/9 in v1
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/xkey_common.h | 43 +++
src/openvpn/xkey_provider.c | 530 +++++++++++++++++++++++++++++++++++-
2 files changed, 566 insertions(+), 7 deletions(-)
diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h
index a3bc3f2a..db58d077 100644
--- a/src/openvpn/xkey_common.h
+++ b/src/openvpn/xkey_common.h
@@ -40,6 +40,49 @@ OSSL_provider_init_fn xkey_provider_init;
#define XKEY_PROV_PROPS "provider=ovpn.xkey"
+/**
+ * Stuct to encapsulate signature algorithm parameters to pass
+ * to sign operation.
+ */
+typedef struct {
+ const char *padmode; /**< "pkcs1", "pss" or "none" */
+ const char *mdname; /**< "SHA256" or "SHA2-256" etc. */
+ const char *saltlen; /**< "digest", "auto" or "max" */
+ const char *keytype; /**< "EC" or "RSA" */
+ const char *op; /**< "Sign" or "DigestSign" */
+} XKEY_SIGALG;
+
+/**
+ * Callback for sign operation -- must be implemented for each backend and
+ * is used in xkey_signature_sign(), or set when loading the key.
+ * (custom key loading not yet implemented).
+ *
+ * @param handle opaque key handle provided by the backend -- could be null
+ * or unused for management interface.
+ * @param sig On return caller should fill this with the signature
+ * @param siglen On entry *siglen has max size of sig and on return must be
+ * set to the actual size of the signature
+ * @param tbs buffer to sign
+ * @param tbslen size of data in tbs buffer
+ * @sigalg contains the signature algorithm parameters
+ *
+ * @returns 1 on success, 0 on error.
+ *
+ * The data in tbs is just the digest with no DigestInfo header added. This is
+ * unlike the deprecated RSA_sign callback which provides encoded digest.
+ * For RSA_PKCS1 signatures, the external signing function must encode the digest
+ * before signing. The digest algorithm used is passed in the sigalg structure.
+ */
+typedef int (XKEY_EXTERNAL_SIGN_fn)(void *handle, unsigned char *sig, size_t *siglen,
+ const unsigned char *tbs, size_t tbslen,
+ XKEY_SIGALG sigalg);
+/**
+ * Signature of private key free function callback used
+ * to free the opaque private key handle obtained from the
+ * backend. Not required for management-external-key.
+ */
+typedef void (XKEY_PRIVKEY_FREE_fn)(void *handle);
+
#endif /* HAVE_XKEY_PROVIDER */
#endif /* XKEY_COMMON_H_ */
diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c
index a083ec2d..09138ae8 100644
--- a/src/openvpn/xkey_provider.c
+++ b/src/openvpn/xkey_provider.c
@@ -81,18 +81,40 @@ typedef enum
*/
typedef struct
{
- /* opaque handle dependent on KEY_ORIGIN -- could be NULL */
+ /** opaque handle dependent on KEY_ORIGIN -- could be NULL */
void *handle;
- /* associated public key as an openvpn native key */
+ /** associated public key as an openvpn native key */
EVP_PKEY *pubkey;
- /* origin of key -- native or external */
+ /** origin of key -- native or external */
XKEY_ORIGIN origin;
+ /** sign function in backend to call */
+ XKEY_EXTERNAL_SIGN_fn *sign;
+ /** keydata handle free function of backend */
+ XKEY_PRIVKEY_FREE_fn *free;
XKEY_PROVIDER_CTX *prov;
- int refcount; /* reference count */
+ int refcount; /**< reference count */
} XKEY_KEYDATA;
-#define KEYTYPE(key) ((key)->pubkey ? EVP_PKEY_get_id((key)->pubkey) : 0)
-#define KEYSIZE(key) ((key)->pubkey ? EVP_PKEY_get_size((key)->pubkey) : 0)
+static int
+KEYTYPE(const XKEY_KEYDATA *key)
+{
+ return key->pubkey ? EVP_PKEY_get_id(key->pubkey) : 0;
+}
+
+static int
+KEYSIZE(const XKEY_KEYDATA *key)
+{
+ return key->pubkey ? EVP_PKEY_get_size(key->pubkey) : 0;
+}
+
+/**
+ * Helper sign function for native keys
+ * Implemented using OpenSSL calls.
+ */
+int
+xkey_native_sign(XKEY_KEYDATA *key, unsigned char *sig, size_t *siglen,
+ const unsigned char *tbs, size_t tbslen, XKEY_SIGALG sigalg);
+
/* keymgmt provider */
@@ -390,6 +412,19 @@ ec_keymgmt_name(int id)
{
xkey_dmsg(D_LOW, "entry");
+ if (id == OSSL_OP_SIGNATURE)
+ {
+ return "ECDSA";
+ }
+ /* though we do not implement keyexch we could be queried for
+ * keyexch mechanism supported by EC keys
+ */
+ else if (id == OSSL_OP_KEYEXCH)
+ {
+ return "ECDH";
+ }
+
+ msg(D_LOW, "xkey ec_keymgmt_name called with op_id != SIGNATURE or KEYEXCH id=%d", id);
return "EC";
}
@@ -432,6 +467,487 @@ const OSSL_ALGORITHM keymgmts[] = {
{NULL, NULL, NULL, NULL}
};
+
+/* signature provider */
+
+/* signature provider callbacks we provide */
+static OSSL_FUNC_signature_newctx_fn signature_newctx;
+static OSSL_FUNC_signature_freectx_fn signature_freectx;
+static OSSL_FUNC_signature_sign_init_fn signature_sign_init;
+static OSSL_FUNC_signature_sign_fn signature_sign;
+static OSSL_FUNC_signature_digest_verify_init_fn signature_digest_verify_init;
+static OSSL_FUNC_signature_digest_verify_fn signature_digest_verify;
+static OSSL_FUNC_signature_digest_sign_init_fn signature_digest_sign_init;
+static OSSL_FUNC_signature_digest_sign_fn signature_digest_sign;
+static OSSL_FUNC_signature_set_ctx_params_fn signature_set_ctx_params;
+static OSSL_FUNC_signature_settable_ctx_params_fn signature_settable_ctx_params;
+static OSSL_FUNC_signature_get_ctx_params_fn signature_get_ctx_params;
+static OSSL_FUNC_signature_gettable_ctx_params_fn signature_gettable_ctx_params;
+
+typedef struct
+{
+ XKEY_PROVIDER_CTX *prov;
+ XKEY_KEYDATA *keydata;
+ XKEY_SIGALG sigalg;
+} XKEY_SIGNATURE_CTX;
+
+static const XKEY_SIGALG default_sigalg = { .mdname="MD5-SHA1", .saltlen="digest",
+ .padmode="pkcs1", .keytype = "RSA"};
+
+const struct {
+ int nid;
+ const char *name;
+} digest_names[] = {{NID_md5_sha1, "MD5-SHA1"}, {NID_sha1, "SHA1"},
+ {NID_sha224, "SHA224",}, {NID_sha256, "SHA256"}, {NID_sha384, "SHA384"},
+ {NID_sha512, "SHA512"}, {0, NULL}};
+/* Use of NIDs as opposed to EVP_MD_fetch is okay here
+ * as these are only used for converting names passed in
+ * by OpenSSL to const strings.
+ */
+
+static struct {
+ int id;
+ const char *name;
+} padmode_names[] = {{RSA_PKCS1_PADDING, "pkcs1"},
+ {RSA_PKCS1_PSS_PADDING, "pss"},
+ {RSA_NO_PADDING, "none"},
+ {0, NULL}};
+
+static const char *saltlen_names[] = {"digest", "max", "auto", NULL};
+
+/* Return a string literal for digest name - normalizes
+ * alternate names like SHA2-256 to SHA256 etc.
+ */
+static const char *
+xkey_mdname(const char *name)
+{
+ int i = 0;
+
+ int nid = EVP_MD_get_type(EVP_get_digestbyname(name));
+
+ while (digest_names[i].name && nid != digest_names[i].nid)
+ {
+ i++;
+ }
+ return digest_names[i].name ? digest_names[i].name : "MD5-SHA1";
+}
+
+static void *
+signature_newctx(void *provctx, const char *propq)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ (void) propq; /* unused */
+
+ XKEY_SIGNATURE_CTX *sctx = OPENSSL_zalloc(sizeof(*sctx));
+ if (!sctx)
+ {
+ msg(M_NONFATAL, "xkey_signature_newctx: out of memory");
+ return NULL;
+ }
+
+ sctx->prov = provctx;
+ sctx->sigalg = default_sigalg;
+
+ return sctx;
+}
+
+static void
+signature_freectx(void *ctx)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ XKEY_SIGNATURE_CTX *sctx = ctx;
+
+ keydata_free(sctx->keydata);
+
+ OPENSSL_free(sctx);
+}
+
+static const OSSL_PARAM *
+signature_settable_ctx_params(void *ctx, void *provctx)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ static OSSL_PARAM settable[] = {
+ OSSL_PARAM_utf8_string(OSSL_SIGNATURE_PARAM_PAD_MODE, NULL, 0),
+ OSSL_PARAM_utf8_string(OSSL_SIGNATURE_PARAM_DIGEST, NULL, 0),
+ OSSL_PARAM_utf8_string(OSSL_SIGNATURE_PARAM_PSS_SALTLEN, NULL, 0),
+ OSSL_PARAM_END
+ };
+
+ return settable;
+}
+
+static int
+signature_set_ctx_params(void *ctx, const OSSL_PARAM params[])
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ XKEY_SIGNATURE_CTX *sctx = ctx;
+ const OSSL_PARAM *p;
+
+ if (params == NULL)
+ {
+ return 1; /* not an error */
+ }
+ p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_PAD_MODE);
+ if (p && p->data_type == OSSL_PARAM_UTF8_STRING)
+ {
+ sctx->sigalg.padmode = NULL;
+ for (int i = 0; padmode_names[i].id != 0; i++)
+ {
+ if (!strcmp(p->data, padmode_names[i].name))
+ {
+ sctx->sigalg.padmode = padmode_names[i].name;
+ break;
+ }
+ }
+ if (sctx->sigalg.padmode == NULL)
+ {
+ msg(M_WARN, "xkey signature_ctx: padmode <%s>, treating as <none>",
+ (char *)p->data);
+ sctx->sigalg.padmode = "none";
+ }
+ xkey_dmsg(D_LOW, "setting padmode as %s", sctx->sigalg.padmode);
+ }
+ else if (p && p->data_type == OSSL_PARAM_INTEGER)
+ {
+ sctx->sigalg.padmode = NULL;
+ int padmode = 0;
+ if (OSSL_PARAM_get_int(p, &padmode))
+ {
+ for (int i = 0; padmode_names[i].id != 0; i++)
+ {
+ if (padmode == padmode_names[i].id)
+ {
+ sctx->sigalg.padmode = padmode_names[i].name;
+ break;
+ }
+ }
+ }
+ if (padmode == 0 || sctx->sigalg.padmode == NULL)
+ {
+ msg(M_WARN, "xkey signature_ctx: padmode <%d>, treating as <none>", padmode);
+ sctx->sigalg.padmode = "none";
+ }
+ xkey_dmsg(D_LOW, "setting padmode <%s>", sctx->sigalg.padmode);
+ }
+ else if (p)
+ {
+ msg(M_WARN, "xkey_signature_params: unknown padmode ignored");
+ }
+
+ p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_DIGEST);
+ if (p && p->data_type == OSSL_PARAM_UTF8_STRING)
+ {
+ sctx->sigalg.mdname = xkey_mdname(p->data);
+ xkey_dmsg(D_LOW, "setting hashalg as %s", sctx->sigalg.mdname);
+ }
+ else if (p)
+ {
+ msg(M_WARN, "xkey_signature_params: unknown digest type ignored");
+ }
+
+ p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_PSS_SALTLEN);
+ if (p && p->data_type == OSSL_PARAM_UTF8_STRING)
+ {
+ sctx->sigalg.saltlen = NULL;
+ for (int i = 0; saltlen_names[i] != NULL; i++)
+ {
+ if (!strcmp(p->data, saltlen_names[i]))
+ {
+ sctx->sigalg.saltlen = saltlen_names[i];
+ break;
+ }
+ }
+ if (sctx->sigalg.saltlen == NULL)
+ {
+ msg(M_WARN, "xkey_signature_params: unknown saltlen <%s>",
+ (char *)p->data);
+ sctx->sigalg.saltlen = "digest"; /* most common */
+ }
+ xkey_dmsg(D_LOW, "setting saltlen to %s", sctx->sigalg.saltlen);
+ }
+ else if (p)
+ {
+ msg(M_WARN, "xkey_signature_params: unknown saltlen ignored");
+ }
+
+ return 1;
+}
+
+static const OSSL_PARAM *
+signature_gettable_ctx_params(void *ctx, void *provctx)
+{
+ xkey_dmsg(D_LOW,"entry");
+
+ static OSSL_PARAM gettable[] = { OSSL_PARAM_END }; /* Empty list */
+
+ return gettable;
+}
+
+static int
+signature_get_ctx_params(void *ctx, OSSL_PARAM params[])
+{
+ xkey_dmsg(D_LOW, "not implemented");
+ return 0;
+}
+
+static int
+signature_sign_init(void *ctx, void *provkey, const OSSL_PARAM params[])
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ XKEY_SIGNATURE_CTX *sctx = ctx;
+
+ if (sctx->keydata)
+ {
+ keydata_free(sctx->keydata);
+ }
+ sctx->keydata = provkey;
+ sctx->keydata->refcount++; /* we are keeping a copy */
+ sctx->sigalg.keytype = KEYTYPE(sctx->keydata) == EVP_PKEY_RSA ? "RSA" : "EC";
+
+ signature_set_ctx_params(sctx, params);
+
+ return 1;
+}
+
+/* Sign digest or message using sign function */
+static int
+xkey_sign_dispatch(XKEY_SIGNATURE_CTX *sctx, unsigned char *sig, size_t *siglen,
+ const unsigned char *tbs, size_t tbslen)
+{
+ XKEY_EXTERNAL_SIGN_fn *sign = sctx->keydata->sign;
+ int ret = 0;
+
+ if (sctx->keydata->origin == OPENSSL_NATIVE)
+ {
+ ret = xkey_native_sign(sctx->keydata, sig, siglen, tbs, tbslen, sctx->sigalg);
+ }
+ else if (sign)
+ {
+ ret = sign(sctx->keydata->handle, sig, siglen, tbs, tbslen, sctx->sigalg);
+ xkey_dmsg(D_LOW, "xkey_provider: external sign op returned ret = %d siglen = %d", ret, (int) *siglen);
+ }
+ else
+ {
+ msg(M_NONFATAL, "xkey_provider: Internal error: No sign callback for external key.");
+ }
+
+ return ret;
+}
+
+static int
+signature_sign(void *ctx, unsigned char *sig, size_t *siglen, size_t sigsize,
+ const unsigned char *tbs, size_t tbslen)
+{
+ xkey_dmsg(D_LOW, "entry with siglen = %zu\n", *siglen);
+
+ XKEY_SIGNATURE_CTX *sctx = ctx;
+ ASSERT(sctx);
+ ASSERT(sctx->keydata);
+
+ if (!sig)
+ {
+ *siglen = KEYSIZE(sctx->keydata);
+ return 1;
+ }
+
+ sctx->sigalg.op = "Sign";
+ return xkey_sign_dispatch(sctx, sig, siglen, tbs, tbslen);
+}
+
+static int
+signature_digest_verify_init(void *ctx, const char *mdname, void *provkey,
+ const OSSL_PARAM params[])
+{
+ xkey_dmsg(D_LOW, "mdname <%s>", mdname);
+
+ msg(M_WARN, "xkey_provider: DigestVerifyInit is not implemented");
+ return 0;
+}
+
+/* We do not expect to be called for DigestVerify() but still
+ * return an empty function for it in the sign dispatch array
+ * for debugging purposes.
+ */
+static int
+signature_digest_verify(void *ctx, const unsigned char *sig, size_t siglen,
+ const unsigned char *tbs, size_t tbslen)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ msg(M_WARN, "xkey_provider: DigestVerify is not implemented");
+ return 0;
+}
+
+static int
+signature_digest_sign_init(void *ctx, const char *mdname,
+ void *provkey, const OSSL_PARAM params[])
+{
+ xkey_dmsg(D_LOW, "mdname = <%s>", mdname);
+
+ XKEY_SIGNATURE_CTX *sctx = ctx;
+
+ ASSERT(sctx);
+ ASSERT(provkey);
+ ASSERT(sctx->prov);
+
+ if (sctx->keydata)
+ {
+ keydata_free(sctx->keydata);
+ }
+ sctx->keydata = provkey; /* used by digest_sign */
+ sctx->keydata->refcount++;
+ sctx->sigalg.keytype = KEYTYPE(sctx->keydata) == EVP_PKEY_RSA ? "RSA" : "EC";
+
+ signature_set_ctx_params(ctx, params);
+ if (mdname)
+ {
+ sctx->sigalg.mdname = xkey_mdname(mdname); /* get a string literal pointer */
+ }
+ else
+ {
+ msg(M_WARN, "xkey digest_sign_init: mdname is NULL.");
+ }
+ return 1;
+}
+
+static int
+signature_digest_sign(void *ctx, unsigned char *sig, size_t *siglen,
+ size_t sigsize, const unsigned char *tbs, size_t tbslen)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ XKEY_SIGNATURE_CTX *sctx = ctx;
+
+ ASSERT(sctx);
+ ASSERT(sctx->keydata);
+
+ if (!sig) /* set siglen and return */
+ {
+ *siglen = KEYSIZE(sctx->keydata);
+ return 1;
+ }
+
+ if (sctx->keydata->origin != OPENSSL_NATIVE)
+ {
+ /* pass the message itself to the backend */
+ sctx->sigalg.op = "DigestSign";
+ return xkey_sign_dispatch(ctx, sig, siglen, tbs, tbslen);
+ }
+
+ /* create digest and pass on to signature_sign() */
+
+ const char *mdname = sctx->sigalg.mdname;
+ EVP_MD *md = EVP_MD_fetch(sctx->prov->libctx, mdname, NULL);
+ if (!md)
+ {
+ msg(M_WARN, "WARN: xkey digest_sign_init: MD_fetch failed for <%s>", mdname);
+ return 0;
+ }
+
+ /* construct digest using OpenSSL */
+ unsigned char buf[EVP_MAX_MD_SIZE];
+ unsigned int sz;
+ if (EVP_Digest(tbs, tbslen, buf, &sz, md, NULL) != 1)
+ {
+ msg(M_WARN, "WARN: xkey digest_sign: EVP_Digest failed");
+ EVP_MD_free(md);
+ return 0;
+ }
+ EVP_MD_free(md);
+
+ return signature_sign(ctx, sig, siglen, sigsize, buf, sz);
+}
+
+/* Sign digest using native sign function -- will only work for native keys
+ */
+int
+xkey_native_sign(XKEY_KEYDATA *key, unsigned char *sig, size_t *siglen,
+ const unsigned char *tbs, size_t tbslen, XKEY_SIGALG sigalg)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ ASSERT(key);
+
+ EVP_PKEY *pkey = key->handle;
+ int ret = 0;
+
+ ASSERT(sig);
+
+ if (!pkey)
+ {
+ msg(M_NONFATAL, "Error: xkey provider: signature request with empty private key");
+ return 0;
+ }
+
+ const char *saltlen = sigalg.saltlen;
+ const char *mdname = sigalg.mdname;
+ const char *padmode = sigalg.padmode;
+
+ xkey_dmsg(D_LOW, "digest=<%s>, padmode=<%s>, saltlen=<%s>", mdname, padmode, saltlen);
+
+ int i = 0;
+ OSSL_PARAM params[6];
+ if (EVP_PKEY_get_id(pkey) == EVP_PKEY_RSA)
+ {
+ params[i++] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_DIGEST, (char *)mdname, 0);
+ params[i++] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_PAD_MODE, (char *)padmode, 0);
+ if (!strcmp(sigalg.padmode, "pss"))
+ {
+ params[i++] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_PSS_SALTLEN, (char *) saltlen, 0);
+ /* same digest for mgf1 */
+ params[i++] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_MGF1_DIGEST, (char *) mdname, 0);
+ }
+ }
+ params[i++] = OSSL_PARAM_construct_end();
+
+ EVP_PKEY_CTX *ectx = EVP_PKEY_CTX_new_from_pkey(key->prov->libctx, pkey, NULL);
+
+ if (!ectx)
+ {
+ msg(M_WARN, "WARN: xkey test_sign: call to EVP_PKEY_CTX_new...failed");
+ return 0;
+ }
+
+ if (EVP_PKEY_sign_init_ex(ectx, NULL) != 1)
+ {
+ msg(M_WARN, "WARN: xkey test_sign: call to EVP_PKEY_sign_init failed");
+ return 0;
+ }
+ EVP_PKEY_CTX_set_params(ectx, params);
+
+ ret = EVP_PKEY_sign(ectx, sig, siglen, tbs, tbslen);
+ EVP_PKEY_CTX_free(ectx);
+
+ return ret;
+}
+
+static const OSSL_DISPATCH signature_functions[] = {
+ {OSSL_FUNC_SIGNATURE_NEWCTX, (void (*)(void)) signature_newctx},
+ {OSSL_FUNC_SIGNATURE_FREECTX, (void (*)(void)) signature_freectx},
+ {OSSL_FUNC_SIGNATURE_SIGN_INIT, (void (*)(void)) signature_sign_init},
+ {OSSL_FUNC_SIGNATURE_SIGN, (void (*)(void)) signature_sign},
+ {OSSL_FUNC_SIGNATURE_DIGEST_VERIFY_INIT, (void (*)(void)) signature_digest_verify_init},
+ {OSSL_FUNC_SIGNATURE_DIGEST_VERIFY, (void (*)(void)) signature_digest_verify},
+ {OSSL_FUNC_SIGNATURE_DIGEST_SIGN_INIT, (void (*)(void)) signature_digest_sign_init},
+ {OSSL_FUNC_SIGNATURE_DIGEST_SIGN, (void (*)(void)) signature_digest_sign},
+ {OSSL_FUNC_SIGNATURE_SET_CTX_PARAMS, (void (*)(void)) signature_set_ctx_params},
+ {OSSL_FUNC_SIGNATURE_SETTABLE_CTX_PARAMS, (void (*)(void)) signature_settable_ctx_params},
+ {OSSL_FUNC_SIGNATURE_GET_CTX_PARAMS, (void (*)(void)) signature_get_ctx_params},
+ {OSSL_FUNC_SIGNATURE_GETTABLE_CTX_PARAMS, (void (*)(void)) signature_gettable_ctx_params},
+ {0, NULL }
+};
+
+const OSSL_ALGORITHM signatures[] = {
+ {"RSA:rsaEncryption", props, signature_functions, "OpenVPN xkey RSA Signature"},
+ {"ECDSA", props, signature_functions, "OpenVPN xkey ECDSA Signature"},
+ {NULL, NULL, NULL, NULL}
+};
+
/* main provider interface */
/* provider callbacks we implement */
@@ -450,7 +966,7 @@ query_operation(void *provctx, int op, int *no_store)
switch (op)
{
case OSSL_OP_SIGNATURE:
- return NULL;
+ return signatures;
case OSSL_OP_KEYMGMT:
return keymgmts;
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 03/18] Implement SIGNATURE operations in xkey provider
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 03/18] Implement SIGNATURE operations in " selva.nair
@ 2022-01-20 10:16 ` Arne Schwabe
2022-01-20 14:08 ` [Openvpn-devel] [PATCH applied] " Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 10:16 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> - Basic frame work for announcing support for signature
> operations
>
> - DigestSign and Sign functions for native keys are also
> implemented. Though strictly not needed, these functions
> for native keys sets up the framework for signature operations.
> They also help loading an exportable key from a file through
> the provider for testing.
>
> Subsequent commits will add support for signing with
> external keys.
>
> v2 changes:
> - Remove verify operations which are no longer
> required with proposed changes in OpenSSL 3.0.1 that we target.
>
> - Undigested message is passed to the backend sign operation when
> possible. This would allow more flexibility as some backends
> prefer to do the hash operation internally.
>
> This was 4/9 in v1
>
>
Acked-By: Arne Schwabe <arne@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH applied] Re: Implement SIGNATURE operations in xkey provider
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 03/18] Implement SIGNATURE operations in " selva.nair
2022-01-20 10:16 ` Arne Schwabe
@ 2022-01-20 14:08 ` Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 14:08 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
This fixes compilation again. Compile-tested, briefly glanced at the code.
Your patch has been applied to the master branch.
commit 25f9c47127190c487eb3b4b4a3f5553fb2d62b21
Author: Selva Nair
Date: Tue Dec 14 11:59:13 2021 -0500
Implement SIGNATURE operations in xkey provider
Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20211214165928.30676-4-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23437.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 04/18] Implement import of custom external keys
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (2 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 03/18] Implement SIGNATURE operations in " selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 10:18 ` Arne Schwabe
2022-01-20 14:18 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 05/18] Initialize the xkey provider and use it in SSL context selva.nair
` (13 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
Our key object retains info about the external
key as an opaque handle to the backend. We also
need the public key as an EVP_PKEY *.
For native keys we use OpenSSL API to import
data into the key. The 'handle' representing the
private key in that case is the OpenSSL EVP_PKEY
object itself.
For importing custom keys, we define custom
parameters describing the key using OSSL_PARAM
structure. We define 4 required and 1 optional
parameters for loading the key:
Required params of type OSSL_PARAM:
{.key="xkey-origin", .data_type = OSSL_PARAM_UTF8_STRING
.data = "foobar", .data_size = 0 }
Note: data_size = 0 refer to NUL terminated string in OpenSSL.
This parameter is only used to identify that the key as non-native
with an opaque handle. We really do not check the content of
the string. Should not be NULL.
{.key="handle", .data_type = OSSL_PARAM_OCTET_PTR,
.data = &handle, .data_size = sizeof(handle)}
{.key="pubkey", .data_type = OSSL_PARAM_OCTET_STRING,
.data = &pubkey, .data_size = sizeof(pubkey)}
{.key="sign_op", .data_type = OSSL_PARAM_OCTET_PTR,
.data = &sign_op_ptr, .data_size = sizeof(sign_op_ptr)}
Optional param:
{.key="free_op", .data_type = OSSL_PARAM_OCTET_PTR,
.data = &free_op_ptr, .data_size = sizeof(free_op_ptr)}
The 'handle' is opaque to us and is retained. The caller
should not free it. We will free it when no longer required
by calling 'free_op()', if provided. The 'handle' should
not be NULL as that indicates missing private key.
The 'pubkey' must be an 'EVP_PKEY *' variable, and is duplicated
by us. The caller may free it after return from import.
The 'sign_op' and 'free_op' function pointers should be of type
'XKEY_EXTERNAL_SIGN_fn' and 'XKEY_PRIVKEY_FREE_fn' defined
in xkey_common.h
For example, for management-external-key, we really do not
need any 'handle'. Pass anything that will live long and
won't dereference to NULL. We do not use it for any other
purpose. Pointer to a const string could be a choice.
In this case, free_op = NULL is the safest choice.
For a usage of keymgmt_import(), see the helper function
implemented using it to load the management key in the next commit.
v2 changes: "origin" --> "xkey-origin"
This was 5/9 in v1
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/xkey_provider.c | 135 +++++++++++++++++++++++++++++++++++-
1 file changed, 133 insertions(+), 2 deletions(-)
diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c
index 09138ae8..c2d560c5 100644
--- a/src/openvpn/xkey_provider.c
+++ b/src/openvpn/xkey_provider.c
@@ -78,6 +78,13 @@ typedef enum
*
* We also keep the public key in the form of a native OpenSSL EVP_PKEY.
* This allows us to do all public ops by calling ops in the default provider.
+ * Both these are references retained by us and freed when the key is
+ * destroyed. As the pubkey is native, we free it using EVP_PKEY_free().
+ * To free the handle we call the backend if a free function
+ * has been set for that key. It could be set when the key is
+ * created/imported.
+ * For native keys, there is no need to free the handle as its either
+ * NULL of the same as the pubkey which we always free.
*/
typedef struct
{
@@ -133,6 +140,9 @@ static OSSL_FUNC_keymgmt_set_params_fn keymgmt_set_params;
static OSSL_FUNC_keymgmt_query_operation_name_fn rsa_keymgmt_name;
static OSSL_FUNC_keymgmt_query_operation_name_fn ec_keymgmt_name;
+static int
+keymgmt_import_helper(XKEY_KEYDATA *key, const OSSL_PARAM params[]);
+
static XKEY_KEYDATA *
keydata_new()
{
@@ -156,6 +166,11 @@ keydata_free(XKEY_KEYDATA *key)
{
return;
}
+ if (key->free && key->handle)
+ {
+ key->free(key->handle);
+ key->handle = NULL;
+ }
if (key->pubkey)
{
EVP_PKEY_free(key->pubkey);
@@ -195,7 +210,27 @@ keymgmt_load(const void *reference, size_t reference_sz)
* appropriate for the key. We just use it to create a native
* EVP_PKEY from params and assign to keydata->handle.
*
- * Import of external keys -- to be implemented
+ * For non-native keys the params[] array should include a custom
+ * value with name "xkey-origin".
+ *
+ * Other required parameters in the params array are:
+ *
+ * pubkey - pointer to native public key as a OCTET_STRING
+ * the public key is duplicated on receipt
+ * handle - reference to opaque handle to private key -- if not required
+ * pass a dummy value that is not zero. type = OCTET_PTR
+ * The reference is retained -- caller must _not_ free it.
+ * sign_op - function pointer for sign operation. type = OCTET_PTR
+ * Must be a reference to XKEY_EXTERNAL_SIGN_fn
+ * xkey-origin - A custom string to indicate the external key origin. UTF8_STRING
+ * The value doesn't really matter, but must be present.
+ *
+ * Optional params
+ * free_op - Called as free(handle) when the key is deleted. If the
+ * handle should not be freed, do not include. type = OCTET_PTR
+ * Must be a reference to XKEY_PRIVKEY_FREE_fn
+ *
+ * See xkey_load_management_key for an example use.
*/
static int
keymgmt_import(void *keydata, int selection, const OSSL_PARAM params[], const char *name)
@@ -212,6 +247,17 @@ keymgmt_import(void *keydata, int selection, const OSSL_PARAM params[], const ch
return 0;
}
+ /* if params contain a custom origin, call our helper to import custom keys */
+ const OSSL_PARAM *p = OSSL_PARAM_locate_const(params, "xkey-origin");
+ if (p && p->data_type == OSSL_PARAM_UTF8_STRING)
+ {
+ key->origin = EXTERNAL_KEY;
+ xkey_dmsg(D_LOW, "importing external key");
+ return keymgmt_import_helper(key, params);
+ }
+
+ xkey_dmsg(D_LOW, "importing native key");
+
/* create a native public key and assign it to key->pubkey */
EVP_PKEY *pkey = NULL;
int selection_pub = selection & ~OSSL_KEYMGMT_SELECT_PRIVATE_KEY;
@@ -370,10 +416,95 @@ keymgmt_get_params(void *keydata, OSSL_PARAM *params)
return EVP_PKEY_get_params(key->pubkey, params);
}
+/* Helper used by keymgmt_import and keymgmt_set_params
+ * for our keys. Not to be used for OpenSSL native keys.
+ */
+static int
+keymgmt_import_helper(XKEY_KEYDATA *key, const OSSL_PARAM *params)
+{
+ xkey_dmsg(D_LOW, "entry");
+
+ const OSSL_PARAM *p;
+ EVP_PKEY *pkey = NULL;
+
+ ASSERT(key);
+ /* calling this with native keys is a coding error */
+ ASSERT(key->origin != OPENSSL_NATIVE);
+
+ if (params == NULL)
+ {
+ return 1; /* not an error */
+ }
+
+ /* our keys are immutable, we do not allow resetting parameters */
+ if (key->pubkey)
+ {
+ return 0;
+ }
+
+ /* only check params we understand and ignore the rest */
+
+ p = OSSL_PARAM_locate_const(params, "pubkey"); /*setting pubkey on our keydata */
+ if (p && p->data_type == OSSL_PARAM_OCTET_STRING
+ && p->data_size == sizeof(pkey))
+ {
+ pkey = *(EVP_PKEY **)p->data;
+ ASSERT(pkey);
+
+ int id = EVP_PKEY_get_id(pkey);
+ if (id != EVP_PKEY_RSA && id != EVP_PKEY_EC)
+ {
+ msg(M_WARN, "Error: xkey keymgmt_import: unknown key type (%d)", id);
+ return 0;
+ }
+
+ key->pubkey = EVP_PKEY_dup(pkey);
+ if (key->pubkey == NULL)
+ {
+ msg(M_NONFATAL, "Error: xkey keymgmt_import: duplicating pubkey failed.");
+ return 0;
+ }
+ }
+
+ p = OSSL_PARAM_locate_const(params, "handle"); /*setting privkey */
+ if (p && p->data_type == OSSL_PARAM_OCTET_PTR
+ && p->data_size == sizeof(key->handle))
+ {
+ key->handle = *(void **)p->data;
+ /* caller should keep the reference alive until we call free */
+ ASSERT(key->handle); /* fix your params array */
+ }
+
+ p = OSSL_PARAM_locate_const(params, "sign_op"); /*setting sign_op */
+ if (p && p->data_type == OSSL_PARAM_OCTET_PTR
+ && p->data_size == sizeof(key->sign))
+ {
+ key->sign = *(void **)p->data;
+ ASSERT(key->sign); /* fix your params array */
+ }
+
+ /* optional parameters */
+ p = OSSL_PARAM_locate_const(params, "free_op"); /*setting free_op */
+ if (p && p->data_type == OSSL_PARAM_OCTET_PTR
+ && p->data_size == sizeof(key->free))
+ {
+ key->free = *(void **)p->data;
+ }
+ xkey_dmsg(D_LOW, "imported external %s key", EVP_PKEY_get0_type_name(key->pubkey));
+
+ return 1;
+}
+
/**
+ * Set params on a key.
+ *
* If the key is an encapsulated native key, we just call
* EVP_PKEY_set_params in the default context. Only those params
* supported by the default provider would work in this case.
+ *
+ * We treat our key object as immutable, so this works only with an
+ * empty key. Supported params for external keys are the
+ * same as those listed in the description of keymgmt_import.
*/
static int
keymgmt_set_params(void *keydata, const OSSL_PARAM *params)
@@ -385,7 +516,7 @@ keymgmt_set_params(void *keydata, const OSSL_PARAM *params)
if (key->origin != OPENSSL_NATIVE)
{
- return 0; /* to be implemented */
+ return keymgmt_import_helper(key, params);
}
else if (key->handle == NULL) /* once handle is set our key is immutable */
{
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 04/18] Implement import of custom external keys
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 04/18] Implement import of custom external keys selva.nair
@ 2022-01-20 10:18 ` Arne Schwabe
2022-01-20 14:18 ` [Openvpn-devel] [PATCH applied] " Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 10:18 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> Our key object retains info about the external
> key as an opaque handle to the backend. We also
> need the public key as an EVP_PKEY *.
>
> For native keys we use OpenSSL API to import
> data into the key. The 'handle' representing the
> private key in that case is the OpenSSL EVP_PKEY
> object itself.
>
> For importing custom keys, we define custom
> parameters describing the key using OSSL_PARAM
> structure. We define 4 required and 1 optional
> parameters for loading the key:
>
> Required params of type OSSL_PARAM:
>
> {.key="xkey-origin", .data_type = OSSL_PARAM_UTF8_STRING
> .data = "foobar", .data_size = 0 }
>
> Note: data_size = 0 refer to NUL terminated string in OpenSSL.
> This parameter is only used to identify that the key as non-native
> with an opaque handle. We really do not check the content of
> the string. Should not be NULL.
>
> {.key="handle", .data_type = OSSL_PARAM_OCTET_PTR,
> .data = &handle, .data_size = sizeof(handle)}
>
> {.key="pubkey", .data_type = OSSL_PARAM_OCTET_STRING,
> .data = &pubkey, .data_size = sizeof(pubkey)}
>
> {.key="sign_op", .data_type = OSSL_PARAM_OCTET_PTR,
> .data = &sign_op_ptr, .data_size = sizeof(sign_op_ptr)}
>
> Optional param:
>
> {.key="free_op", .data_type = OSSL_PARAM_OCTET_PTR,
> .data = &free_op_ptr, .data_size = sizeof(free_op_ptr)}
>
> The 'handle' is opaque to us and is retained. The caller
> should not free it. We will free it when no longer required
> by calling 'free_op()', if provided. The 'handle' should
> not be NULL as that indicates missing private key.
>
> The 'pubkey' must be an 'EVP_PKEY *' variable, and is duplicated
> by us. The caller may free it after return from import.
>
> The 'sign_op' and 'free_op' function pointers should be of type
> 'XKEY_EXTERNAL_SIGN_fn' and 'XKEY_PRIVKEY_FREE_fn' defined
> in xkey_common.h
>
> For example, for management-external-key, we really do not
> need any 'handle'. Pass anything that will live long and
> won't dereference to NULL. We do not use it for any other
> purpose. Pointer to a const string could be a choice.
> In this case, free_op = NULL is the safest choice.
>
> For a usage of keymgmt_import(), see the helper function
> implemented using it to load the management key in the next commit.
>
> v2 changes: "origin" --> "xkey-origin"
> This was 5/9 in v1
Acked-By: Arne Schwabe <arne@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread* [Openvpn-devel] [PATCH applied] Re: Implement import of custom external keys
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 04/18] Implement import of custom external keys selva.nair
2022-01-20 10:18 ` Arne Schwabe
@ 2022-01-20 14:18 ` Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 14:18 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
Compile-tested only (and glanced over the code).
Your patch has been applied to the master branch.
commit ab3a8e5c28c433fd405f964d55bb754571191b9c
Author: Selva Nair
Date: Tue Dec 14 11:59:14 2021 -0500
Implement import of custom external keys
Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20211214165928.30676-5-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23439.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 05/18] Initialize the xkey provider and use it in SSL context
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (3 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 04/18] Implement import of custom external keys selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 10:42 ` Arne Schwabe
2022-01-20 14:32 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 06/18] A helper function to import private key for management-external-key selva.nair
` (12 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
- Add function to check when external key is in use
- Load xkey provider into a custom library context when required
- Use the custom libctx in SSL CTX when external key is in use
As no keys are yet loaded through the provider,
no functionality gets delegated to it as yet.
v2 changes: Provider loading is reworked to activate only when external
keys are in use
This was 2/9 in v1
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/openssl_compat.h | 8 ++++
src/openvpn/options.c | 16 +++++++
src/openvpn/options.h | 2 +
src/openvpn/ssl.c | 5 ++
src/openvpn/ssl.h | 6 +++
src/openvpn/ssl_mbedtls.c | 6 +++
src/openvpn/ssl_openssl.c | 93 +++++++++++++++++++++++++++++++++++-
src/openvpn/xkey_common.h | 1 -
8 files changed, 134 insertions(+), 3 deletions(-)
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index dcc210c7..5c9da9eb 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -760,6 +760,14 @@ int EVP_PKEY_get_group_name(EVP_PKEY *pkey, char *gname, size_t gname_sz,
#define EVP_CIPHER_get0_name EVP_CIPHER_name
#define EVP_CIPHER_CTX_get_mode EVP_CIPHER_CTX_mode
+/** Reduce SSL_CTX_new_ex() to SSL_CTX_new() for OpenSSL < 3 */
+#define SSL_CTX_new_ex(libctx, propq, method) \
+ SSL_CTX_new((method))
+
+/* Some safe typedefs to avoid too many ifdefs */
+typedef void OSSL_LIB_CTX;
+typedef void OSSL_PROVIDER;
+
/* Mimics the functions but only when the default context without
* options is chosen */
static inline const EVP_CIPHER *
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b840b767..fb427410 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5337,6 +5337,22 @@ show_compression_warning(struct compress_options *info)
}
#endif
+bool key_is_external(const struct options *options)
+{
+ bool ret = false;
+#ifdef ENABLE_MANAGEMENT
+ ret = ret || (options->management_flags & MF_EXTERNAL_KEY);
+#endif
+#ifdef ENABLE_PKCS11
+ ret = ret || (options->pkcs11_providers[0] != NULL);
+#endif
+#ifdef ENABLE_CRYPTOAPI
+ ret = ret || options->cryptoapi_cert;
+#endif
+
+ return ret;
+}
+
static void
add_option(struct options *options,
char *p[],
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index d4f41cd7..8dc06343 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -862,4 +862,6 @@ void options_string_import(struct options *options,
unsigned int *option_types_found,
struct env_set *es);
+bool key_is_external(const struct options *options);
+
#endif /* ifndef OPTIONS_H */
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 05096ee0..0c4e3234 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -603,6 +603,11 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx, bool in_ch
tls_clear_error();
+ if (key_is_external(options))
+ {
+ load_xkey_provider();
+ }
+
if (options->tls_server)
{
tls_ctx_server_new(new_ctx);
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index b14453fe..784ddd32 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -627,4 +627,10 @@ show_available_tls_ciphers(const char *cipher_list,
bool
tls_session_generate_data_channel_keys(struct tls_session *session);
+/**
+ * Load ovpn.xkey provider used for external key signing
+ */
+void
+load_xkey_provider(void);
+
#endif /* ifndef OPENVPN_SSL_H */
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 94605801..15cd8b16 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1550,4 +1550,10 @@ get_ssl_library_version(void)
return mbedtls_version;
}
+void
+load_xkey_provider(void)
+{
+ return; /* no external key provider in mbedTLS build */
+}
+
#endif /* defined(ENABLE_CRYPTO_MBEDTLS) */
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 724664bb..bdaa7a2b 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -45,6 +45,7 @@
#include "ssl_common.h"
#include "base64.h"
#include "openssl_compat.h"
+#include "xkey_common.h"
#ifdef ENABLE_CRYPTOAPI
#include "cryptoapi.h"
@@ -69,6 +70,10 @@
#include <openssl/applink.c>
#endif
+static OSSL_LIB_CTX *tls_libctx;
+
+static void unload_xkey_provider(void);
+
/*
* Allocate space in SSL objects in which to store a struct tls_session
* pointer back to parent.
@@ -113,7 +118,7 @@ tls_ctx_server_new(struct tls_root_ctx *ctx)
{
ASSERT(NULL != ctx);
- ctx->ctx = SSL_CTX_new(SSLv23_server_method());
+ ctx->ctx = SSL_CTX_new_ex(tls_libctx, NULL, SSLv23_server_method());
if (ctx->ctx == NULL)
{
@@ -131,7 +136,7 @@ tls_ctx_client_new(struct tls_root_ctx *ctx)
{
ASSERT(NULL != ctx);
- ctx->ctx = SSL_CTX_new(SSLv23_client_method());
+ ctx->ctx = SSL_CTX_new_ex(tls_libctx, NULL, SSLv23_client_method());
if (ctx->ctx == NULL)
{
@@ -150,6 +155,7 @@ tls_ctx_free(struct tls_root_ctx *ctx)
ASSERT(NULL != ctx);
SSL_CTX_free(ctx->ctx);
ctx->ctx = NULL;
+ unload_xkey_provider(); /* in case it is loaded */
}
bool
@@ -2284,4 +2290,87 @@ get_ssl_library_version(void)
return OpenSSL_version(OPENSSL_VERSION);
}
+
+/** Some helper routines for provider load/unload */
+#ifdef HAVE_XKEY_PROVIDER
+static int
+provider_load(OSSL_PROVIDER *prov, void *dest_libctx)
+{
+ const char *name = OSSL_PROVIDER_get0_name(prov);
+ OSSL_PROVIDER_load(dest_libctx, name);
+ return 1;
+}
+
+static int
+provider_unload(OSSL_PROVIDER *prov, void *unused)
+{
+ (void) unused;
+ OSSL_PROVIDER_unload(prov);
+ return 1;
+}
+#endif /* HAVE_XKEY_PROVIDER */
+
+/**
+ * Setup ovpn.xey provider for signing with external keys.
+ * It is loaded into a custom library context so as not to pollute
+ * the default context. Alternatively we could override any
+ * system-wide property query set on the default context. But we
+ * want to avoid that.
+ */
+void
+load_xkey_provider(void)
+{
+#ifdef HAVE_XKEY_PROVIDER
+
+ /* Make a new library context for use in TLS context */
+ if (!tls_libctx)
+ {
+ tls_libctx = OSSL_LIB_CTX_new();
+ check_malloc_return(tls_libctx);
+
+ /* Load all providers in default LIBCTX into this libctx.
+ * OpenSSL has a child libctx functionality to automate this,
+ * but currently that is usable only from within providers.
+ * So we do something close to it manually here.
+ */
+ OSSL_PROVIDER_do_all(NULL, provider_load, tls_libctx);
+ }
+
+ if (!OSSL_PROVIDER_available(tls_libctx, "ovpn.xkey"))
+ {
+ OSSL_PROVIDER_add_builtin(tls_libctx, "ovpn.xkey", xkey_provider_init);
+ if (!OSSL_PROVIDER_load(tls_libctx, "ovpn.xkey"))
+ {
+ msg(M_NONFATAL, "ERROR: failed loading external key provider: "
+ "Signing with external keys will not work.");
+ }
+ }
+
+ /* We only implement minimal functionality in ovpn.xkey, so we do not want
+ * methods in xkey to be picked unless absolutely required (i.e, when the key
+ * is external). Ensure this by setting a default propquery for the custom
+ * libctx that unprefers, but does not forbid, ovpn.xkey. See also man page
+ * of "property" in OpenSSL 3.0.
+ */
+ EVP_set_default_properties(tls_libctx, "?provider!=ovpn.xkey");
+
+#endif /* HAVE_XKEY_PROVIDER */
+}
+
+/**
+ * Undo steps in load_xkey_provider
+ */
+static void
+unload_xkey_provider(void)
+{
+#ifdef HAVE_XKEY_PROVIDER
+ if (tls_libctx)
+ {
+ OSSL_PROVIDER_do_all(tls_libctx, provider_unload, NULL);
+ OSSL_LIB_CTX_free(tls_libctx);
+ }
+#endif /* HAVE_XKEY_PROVIDER */
+ tls_libctx = NULL;
+}
+
#endif /* defined(ENABLE_CRYPTO_OPENSSL) */
diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h
index db58d077..f46bacd2 100644
--- a/src/openvpn/xkey_common.h
+++ b/src/openvpn/xkey_common.h
@@ -28,7 +28,6 @@
#include <openssl/opensslv.h>
#if OPENSSL_VERSION_NUMBER >= 0x30000010L && !defined(DISABLE_XKEY_PROVIDER)
#define HAVE_XKEY_PROVIDER 1
-
#include <openssl/provider.h>
#include <openssl/core_dispatch.h>
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* [Openvpn-devel] [PATCH v3 06/18] A helper function to import private key for management-external-key
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (4 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 05/18] Initialize the xkey provider and use it in SSL context selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 10:53 ` Arne Schwabe
2022-01-20 14:43 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 07/18] Enable signing via provider " selva.nair
` (11 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
- 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@...277...>
---
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
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@...277...>
+ *
+ * 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 */
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 06/18] A helper function to import private key for management-external-key
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 06/18] A helper function to import private key for management-external-key selva.nair
@ 2022-01-20 10:53 ` Arne Schwabe
2022-01-20 14:43 ` [Openvpn-devel] [PATCH applied] " Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 10:53 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> - 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@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH applied] Re: A helper function to import private key for management-external-key
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 06/18] A helper function to import private key for management-external-key selva.nair
2022-01-20 10:53 ` Arne Schwabe
@ 2022-01-20 14:43 ` Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 14:43 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
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@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20211214165928.30676-7-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23443.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 07/18] Enable signing via provider for management-external-key
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (5 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 06/18] A helper function to import private key for management-external-key selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 10:54 ` Arne Schwabe
2022-01-20 15:18 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 08/18] Add a function to encode digests with PKCS1 DigestInfo wrapper selva.nair
` (10 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
- Add a function to set as sign_op during key import. The
function passes the signature request to management interface,
and returns the result to the provider.
v2 changes: Method to do digest added to match the changes in
the provider signature callback.
TODO:
- Allow passing the undigested message to management interface
- Add pkcs1 DigestInfo header when required
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/ssl_openssl.c | 4 +-
src/openvpn/xkey_common.h | 7 ++-
src/openvpn/xkey_helper.c | 108 ++++++++++++++++++++++++++++++++++++--
3 files changed, 113 insertions(+), 6 deletions(-)
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 23c74f55..8f0281b1 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1169,7 +1169,7 @@ end:
}
-#ifdef ENABLE_MANAGEMENT
+#if defined(ENABLE_MANAGEMENT) && !defined(HAVE_XKEY_PROVIDER)
/* encrypt */
static int
@@ -1470,7 +1470,9 @@ err:
return 0;
}
#endif /* OPENSSL_VERSION_NUMBER > 1.1.0 dev && !defined(OPENSSL_NO_EC) */
+#endif /* ENABLE_MANAGEMENT && !HAVE_XKEY_PROVIDER */
+#ifdef ENABLE_MANAGEMENT
int
tls_ctx_use_management_external_key(struct tls_root_ctx *ctx)
{
diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h
index 5bda5e30..608afe99 100644
--- a/src/openvpn/xkey_common.h
+++ b/src/openvpn/xkey_common.h
@@ -67,10 +67,13 @@ typedef struct {
*
* @returns 1 on success, 0 on error.
*
- * The data in tbs is just the digest with no DigestInfo header added. This is
+ * If sigalg.op = "Sign", the data in tbs is the digest. If sigalg.op = "DigestSign"
+ * it is the message that the backend should hash wih appropriate hash algorithm before
+ * signing. In the former case no DigestInfo header is added to tbs. This is
* unlike the deprecated RSA_sign callback which provides encoded digest.
* For RSA_PKCS1 signatures, the external signing function must encode the digest
- * before signing. The digest algorithm used is passed in the sigalg structure.
+ * before signing. The digest algorithm used (or to be used) is passed in the sigalg
+ * structure.
*/
typedef int (XKEY_EXTERNAL_SIGN_fn)(void *handle, unsigned char *sig, size_t *siglen,
const unsigned char *tbs, size_t tbslen,
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index 51cfb12b..aac78a2c 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -32,6 +32,8 @@
#include "error.h"
#include "buffer.h"
#include "xkey_common.h"
+#include "manage.h"
+#include "base64.h"
#ifdef HAVE_XKEY_PROVIDER
@@ -48,6 +50,31 @@ static const char *const props = XKEY_PROV_PROPS;
XKEY_EXTERNAL_SIGN_fn xkey_management_sign;
+/** helper to compute digest */
+static int
+xkey_digest(const unsigned char *src, size_t srclen, unsigned char *buf,
+ size_t *buflen, const char *mdname)
+{
+ dmsg(D_LOW, "In xkey_digest");
+ EVP_MD *md = EVP_MD_fetch(NULL, mdname, NULL); /* from default context */
+ if (!md)
+ {
+ msg(M_WARN, "WARN: xkey_digest: MD_fetch failed for <%s>", mdname);
+ return 0;
+ }
+
+ unsigned int len = (unsigned int) *buflen;
+ if (EVP_Digest(src, srclen, buf, &len, md, NULL) != 1)
+ {
+ msg(M_WARN, "WARN: xkey_digest: EVP_Digest failed");
+ return 0;
+ }
+ EVP_MD_free(md);
+
+ *buflen = len;
+ return 1;
+}
+
/**
* Load external key for signing via management interface.
* The public key must be passed in by the caller as we may not
@@ -94,13 +121,88 @@ xkey_load_management_key(OSSL_LIB_CTX *libctx, EVP_PKEY *pubkey)
return pkey;
}
-/* not yet implemented */
+/**
+ * Signature callback for xkey_provider with management-external-key
+ *
+ * @param handle Unused -- may be null
+ * @param sig On successful return signature is in sig.
+ * @param siglen On entry *siglen has length of buffer sig,
+ * on successful return size of signature
+ * @param tbs hash or message to be signed
+ * @param tbslen len of data in dgst
+ * @param sigalg extra signature parameters
+ *
+ * @return signature length or -1 on error.
+ */
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;
+ (void) unused;
+ char alg_str[128];
+ unsigned char buf[EVP_MAX_MD_SIZE]; /* for computing digest if required */
+ size_t buflen = sizeof(buf);
+
+ if (!strcmp(alg.op, "DigestSign"))
+ {
+ dmsg(D_LOW, "xkey_management_sign: computing digest");
+ if (xkey_digest(tbs, tbslen, buf, &buflen, alg.mdname))
+ {
+ tbs = buf;
+ tbslen = buflen;
+ alg.op = "Sign";
+ }
+ else
+ {
+ return 0;
+ }
+ }
+
+ if (!strcmp(alg.keytype, "EC"))
+ {
+ strncpynt(alg_str, "ECDSA", sizeof(alg_str));
+ }
+ /* else assume RSA key */
+ else if (!strcmp(alg.padmode, "pkcs1"))
+ {
+ strncpynt(alg_str, "RSA_PKCS1_PADDING", sizeof(alg_str));
+ }
+ else if (!strcmp(alg.padmode, "none"))
+ {
+ strncpynt(alg_str, "RSA_NO_PADDING", sizeof(alg_str));
+ }
+ else if (!strcmp(alg.padmode, "pss"))
+ {
+ openvpn_snprintf(alg_str, sizeof(alg_str), "%s,hashalg=%s,saltlen=%s",
+ "RSA_PKCS1_PSS_PADDING", alg.mdname,alg.saltlen);
+ }
+ else {
+ msg(M_NONFATAL, "Unsupported RSA padding mode in signature request<%s>",
+ alg.padmode);
+ return 0;
+ }
+ dmsg(D_LOW, "xkey management_sign: requesting sig with algorithm <%s>", alg_str);
+
+ char *in_b64 = NULL;
+ char *out_b64 = NULL;
+ int len = -1;
+
+ int bencret = openvpn_base64_encode(tbs, (int) tbslen, &in_b64);
+
+ if (management && bencret > 0)
+ {
+ out_b64 = management_query_pk_sig(management, in_b64, alg_str);
+ }
+ if (out_b64)
+ {
+ len = openvpn_base64_decode(out_b64, sig, (int) *siglen);
+ }
+ free(in_b64);
+ free(out_b64);
+
+ *siglen = (len > 0) ? len : 0;
+
+ return (*siglen > 0);
}
#endif /* HAVE_XKEY_PROVIDER */
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 07/18] Enable signing via provider for management-external-key
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 07/18] Enable signing via provider " selva.nair
@ 2022-01-20 10:54 ` Arne Schwabe
2022-01-20 15:18 ` [Openvpn-devel] [PATCH applied] " Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 10:54 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> - Add a function to set as sign_op during key import. The
> function passes the signature request to management interface,
> and returns the result to the provider.
>
> v2 changes: Method to do digest added to match the changes in
> the provider signature callback.
> TODO:
> - Allow passing the undigested message to management interface
> - Add pkcs1 DigestInfo header when required
>
> Signed-off-by: Selva Nair <selva.nair@...277...>
>
Acked-By: Arne Schwabe <arne@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH applied] Re: Enable signing via provider for management-external-key
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 07/18] Enable signing via provider " selva.nair
2022-01-20 10:54 ` Arne Schwabe
@ 2022-01-20 15:18 ` Gert Doering
2022-01-20 16:32 ` Selva Nair
1 sibling, 1 reply; 60+ messages in thread
From: Gert Doering @ 2022-01-20 15:18 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
Compile and client tested on 1.1.1 and 3.0.1.
Glancing at the code related to management_external_key() does
not make me very happy... too many build time variants.
Maybe we should look into "external key is only supported with
OpenSSL 3.0.1+ builds" for 2.7 and get rid of all the #ifdef'ed
code there...
Your patch has been applied to the master branch.
commit 199df03bf57339661a853cb764ea41a0c8349b95
Author: Selva Nair
Date: Tue Dec 14 11:59:17 2021 -0500
Enable signing via provider for management-external-key
Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20211214165928.30676-8-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23428.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH applied] Re: Enable signing via provider for management-external-key
2022-01-20 15:18 ` [Openvpn-devel] [PATCH applied] " Gert Doering
@ 2022-01-20 16:32 ` Selva Nair
2022-01-20 17:24 ` Gert Doering
0 siblings, 1 reply; 60+ messages in thread
From: Selva Nair @ 2022-01-20 16:32 UTC (permalink / raw)
To: Gert Doering <gert@; +Cc: openvpn-devel
[-- Attachment #1: Type: text/plain, Size: 746 bytes --]
Hi,
On Thu, Jan 20, 2022 at 10:18 AM Gert Doering <gert@...1296...> wrote:
> Compile and client tested on 1.1.1 and 3.0.1.
>
> Glancing at the code related to management_external_key() does
> not make me very happy... too many build time variants.
"Happiness" is never a word that comes to mind while reading OpenVPN code :)
...
Even at our snail's pace, 2.7 may be out before we can break free of
OpenSSL 1, LibreSSL xyz etc. An option may be to require OpenSSL 3+ or
similar for external keys, or at least for management-external-key.
That feature is really used by only a few platforms (only Android for
now?). Although it's a nifty option that could potentially be leveraged to
remove pkcs11-helper, CNG etc out of OpenVPN core.
Selva
[-- Attachment #2: Type: text/html, Size: 1187 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [Openvpn-devel] [PATCH applied] Re: Enable signing via provider for management-external-key
2022-01-20 16:32 ` Selva Nair
@ 2022-01-20 17:24 ` Gert Doering
0 siblings, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 17:24 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: Gert Doering <gert@
[-- Attachment #1: Type: text/plain, Size: 1922 bytes --]
Hi,
On Thu, Jan 20, 2022 at 11:32:40AM -0500, Selva Nair wrote:
> On Thu, Jan 20, 2022 at 10:18 AM Gert Doering <gert@...1296...> wrote:
>
> > Compile and client tested on 1.1.1 and 3.0.1.
> >
> > Glancing at the code related to management_external_key() does
> > not make me very happy... too many build time variants.
>
>
> "Happiness" is never a word that comes to mind while reading OpenVPN code :)
> ...
Oh, some of the code paths are really nice these days :-) - but the
#ifdef maze regarding SSL libraries / crypto features is getting truly
annoying.
> Even at our snail's pace, 2.7 may be out before we can break free of
> OpenSSL 1, LibreSSL xyz etc. An option may be to require OpenSSL 3+ or
> similar for external keys, or at least for management-external-key.
>
> That feature is really used by only a few platforms (only Android for
> now?).
That was my idea - since only Windows and Android use the "xkey" code
paths today (as far as I understand), make 3.0.1 a hard requirement
for Windows and Android, and disable --management-external-key for
older SSL builds. Maybe this is a bit too drastic, but it would
reduce code paths to be maintained and tested quite a bit.
For Windows and Android, we bundle the SSL library to be used anyway,
so we do not need to care what the OS might bring along.
> Although it's a nifty option that could potentially be leveraged to
> remove pkcs11-helper, CNG etc out of OpenVPN core.
Whatever reduces #ifdef and library dependencies :-)
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany gert@...1296...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 630 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 08/18] Add a function to encode digests with PKCS1 DigestInfo wrapper
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (6 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 07/18] Enable signing via provider " selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 10:54 ` Arne Schwabe
2022-01-20 15:28 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 09/18] Allow management client to announce pss padding support selva.nair
` (9 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
The EVP_PKEY interface as well as provider passes the raw
digest to the sign() function. In case of RSA_PKCS1,
our management interface expects an encoded hash, which
has the DigestInfo header added as per PKCSv1.5 specs,
unless the hash algorithm is legacy MD5_SHA1.
Fix this by
- add a function to perform the pkcs1 encoding before passing the
data to sign to the management interface. The implementation
is not pretty, but should work.
(Unfortunately OpenSSL does not expose a function for this).
Note:
1. cryptoki interface used by pkcs11-helper also requires this
to be done before calling the Sign op. This will come handy there
too.
2. We have a similar function in ssl_mbedtls.c but its not prettier,
and require porting.
v2 changes: Use hard-coded headers for known hash algorithms instead
of assembling it from the ASN.1 objects.
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/xkey_common.h | 20 ++++++
src/openvpn/xkey_helper.c | 130 ++++++++++++++++++++++++++++++++++++++
2 files changed, 150 insertions(+)
diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h
index 608afe99..c04c9c5c 100644
--- a/src/openvpn/xkey_common.h
+++ b/src/openvpn/xkey_common.h
@@ -96,6 +96,26 @@ typedef void (XKEY_PRIVKEY_FREE_fn)(void *handle);
*/
EVP_PKEY *xkey_load_management_key(OSSL_LIB_CTX *libctx, EVP_PKEY *pubkey);
+/**
+ * Add PKCS1 DigestInfo to tbs and return the result in *enc.
+ *
+ * @param enc pointer to output buffer
+ * @param enc_len capacity in bytes of output buffer
+ * @param mdname name of the hash algorithm (SHA256, SHA1 etc.)
+ * @param tbs pointer to digest to be encoded
+ * @param tbslen length of data in bytes
+ *
+ * @return false on error, true on success
+ *
+ * On return enc_len is set to actual size of the result.
+ * enc is NULL or enc_len is not enough to store the result, it is set
+ * to the required size and false is returned.
+ *
+ */
+bool
+encode_pkcs1(unsigned char *enc, size_t *enc_len, const char *mdname,
+ const unsigned char *tbs, size_t tbslen);
+
#endif /* HAVE_XKEY_PROVIDER */
#endif /* XKEY_COMMON_H_ */
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index aac78a2c..b2546cec 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -143,6 +143,9 @@ xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
unsigned char buf[EVP_MAX_MD_SIZE]; /* for computing digest if required */
size_t buflen = sizeof(buf);
+ unsigned char enc[EVP_MAX_MD_SIZE + 32]; /* 32 bytes enough for digest inf structure */
+ size_t enc_len = sizeof(enc);
+
if (!strcmp(alg.op, "DigestSign"))
{
dmsg(D_LOW, "xkey_management_sign: computing digest");
@@ -165,6 +168,14 @@ xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
/* else assume RSA key */
else if (!strcmp(alg.padmode, "pkcs1"))
{
+ /* management interface expects a pkcs1 encoded digest -- add it */
+ if (!encode_pkcs1(enc, &enc_len, alg.mdname, tbs, tbslen))
+ {
+ return 0;
+ }
+ tbs = enc;
+ tbslen = enc_len;
+
strncpynt(alg_str, "RSA_PKCS1_PADDING", sizeof(alg_str));
}
else if (!strcmp(alg.padmode, "none"))
@@ -205,4 +216,123 @@ xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
return (*siglen > 0);
}
+/**
+ * Add PKCS1 DigestInfo to tbs and return the result in *enc.
+ *
+ * @param enc pointer to output buffer
+ * @param enc_len capacity in bytes of output buffer
+ * @param mdname name of the hash algorithm (SHA256, SHA1 etc.)
+ * @param tbs pointer to digest to be encoded
+ * @param tbslen length of data in bytes
+ *
+ * @return false on error, true on success
+ *
+ * On return enc_len is set to actual size of the result.
+ * enc is NULL or enc_len is not enough to store the result, it is set
+ * to the required size and false is returned.
+ */
+bool
+encode_pkcs1(unsigned char *enc, size_t *enc_len, const char *mdname,
+ const unsigned char *tbs, size_t tbslen)
+{
+ ASSERT(enc_len != NULL);
+ ASSERT(tbs != NULL);
+
+ /* Tabulate the digest info header for expected hash algorithms
+ * These were pre-computed using the DigestInfo definition:
+ * DigestInfo ::= SEQUENCE {
+ * digestAlgorithm DigestAlgorithmIdentifier,
+ * digest Digest }
+ * Also see the table in RFC 8017 section 9.2, Note 1.
+ */
+
+ const unsigned char sha1[] = {0x30, 0x21, 0x30, 0x09, 0x06, 0x05, 0x2b,
+ 0x0e, 0x03, 0x02, 0x1a, 0x05, 0x00, 0x04, 0x14};
+ const unsigned char sha256[] = {0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48,
+ 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x04, 0x20};
+ const unsigned char sha384[] = {0x30, 0x41, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48,
+ 0x01, 0x65, 0x03, 0x04, 0x02, 0x02, 0x05, 0x00, 0x04, 0x30};
+ const unsigned char sha512[] = {0x30, 0x51, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48,
+ 0x01, 0x65, 0x03, 0x04, 0x02, 0x03, 0x05, 0x00, 0x04, 0x40};
+ const unsigned char sha224[] = {0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48,
+ 0x01, 0x65, 0x03, 0x04, 0x02, 0x04, 0x05, 0x00, 0x04, 0x1c};
+ const unsigned char sha512_224[] = {0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48,
+ 0x01, 0x65, 0x03, 0x04, 0x02, 0x05, 0x05, 0x00, 0x04, 0x1c};
+ const unsigned char sha512_256[] = {0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48,
+ 0x01, 0x65, 0x03, 0x04, 0x02, 0x06, 0x05, 0x00, 0x04, 0x20};
+
+ typedef struct {
+ const int nid;
+ const unsigned char *header;
+ size_t sz;
+ } DIG_INFO;
+
+#define MAKE_DI(x) {NID_##x, x, sizeof(x)}
+
+ DIG_INFO dinfo[] = {MAKE_DI(sha1), MAKE_DI(sha256), MAKE_DI(sha384),
+ MAKE_DI(sha512), MAKE_DI(sha224), MAKE_DI(sha512_224),
+ MAKE_DI(sha512_256), {0,NULL,0}};
+
+ int out_len = 0;
+ int ret = 0;
+
+ int nid = OBJ_sn2nid(mdname);
+ if(nid == NID_undef)
+ {
+ /* try harder -- name variants like SHA2-256 doesn't work */
+ nid = EVP_MD_type(EVP_get_digestbyname(mdname));
+ if(nid == NID_undef)
+ {
+ msg(M_WARN, "Error: encode_pkcs11: invalid digest name <%s>", mdname);
+ goto done;
+ }
+ }
+
+ if (tbslen != EVP_MD_size(EVP_get_digestbyname(mdname)))
+ {
+ msg(M_WARN, "Error: encode_pkcs11: invalid input length <%d>", (int)tbslen);
+ goto done;
+ }
+
+ if (nid == NID_md5_sha1) /* no encoding needed -- just copy */
+ {
+ if (enc && (*enc_len >= tbslen))
+ {
+ memcpy(enc, tbs, tbslen);
+ ret = true;
+ }
+ out_len = tbslen;
+ goto done;
+ }
+
+ /* locate entry for nid in dinfo table */
+ DIG_INFO *di = dinfo;
+ while((di->nid != nid) && (di->nid != 0))
+ {
+ di++;
+ }
+ if (di->nid != nid) /* not found in our table */
+ {
+ msg(M_WARN, "Error: encode_pkcs11: unsupported hash algorithm <%s>", mdname);
+ goto done;
+ }
+
+ out_len = tbslen + di->sz;
+
+ if (enc && (out_len <= (int) *enc_len))
+ {
+ /* combine header and digest */
+ memcpy(enc, di->header, di->sz);
+ memcpy(enc + di->sz, tbs, tbslen);
+ dmsg(D_LOW, "encode_pkcs1: digest length = %d encoded length = %d",
+ (int) tbslen, (int) out_len);
+ ret = true;
+ }
+
+done:
+ *enc_len = out_len; /* assignment safe as out_len is > 0 at this point */
+
+ return ret;
+}
+
#endif /* HAVE_XKEY_PROVIDER */
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 08/18] Add a function to encode digests with PKCS1 DigestInfo wrapper
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 08/18] Add a function to encode digests with PKCS1 DigestInfo wrapper selva.nair
@ 2022-01-20 10:54 ` Arne Schwabe
2022-01-20 15:28 ` [Openvpn-devel] [PATCH applied] " Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 10:54 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> The EVP_PKEY interface as well as provider passes the raw
> digest to the sign() function. In case of RSA_PKCS1,
> our management interface expects an encoded hash, which
> has the DigestInfo header added as per PKCSv1.5 specs,
> unless the hash algorithm is legacy MD5_SHA1.
>
> Fix this by
> - add a function to perform the pkcs1 encoding before passing the
> data to sign to the management interface. The implementation
> is not pretty, but should work.
> (Unfortunately OpenSSL does not expose a function for this).
>
> Note:
> 1. cryptoki interface used by pkcs11-helper also requires this
> to be done before calling the Sign op. This will come handy there
> too.
> 2. We have a similar function in ssl_mbedtls.c but its not prettier,
> and require porting.
>
> v2 changes: Use hard-coded headers for known hash algorithms instead
> of assembling it from the ASN.1 objects.
Acked-By: Arne Schwabe <arne@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH applied] Re: Add a function to encode digests with PKCS1 DigestInfo wrapper
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 08/18] Add a function to encode digests with PKCS1 DigestInfo wrapper selva.nair
2022-01-20 10:54 ` Arne Schwabe
@ 2022-01-20 15:28 ` Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 15:28 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
Looked at the code, did client tests on 3.0.1, added a few spaces
in code like "if(nid == NID_undef)" :-)
As for the actual digest / encoding parts, no idea what that does,
but the code looks safe wrt memcpy(), length of things, etc.
Your patch has been applied to the master branch.
commit cf704eef472515e3d6469bd5377065a1378eb5c2
Author: Selva Nair
Date: Tue Dec 14 11:59:18 2021 -0500
Add a function to encode digests with PKCS1 DigestInfo wrapper
Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20211214165928.30676-9-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23433.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 09/18] Allow management client to announce pss padding support
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (7 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 08/18] Add a function to encode digests with PKCS1 DigestInfo wrapper selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 10:55 ` Arne Schwabe
2022-01-20 15:35 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 10/18] Respect algorithm support announced by management client selva.nair
` (8 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
The --management-external-key option can currently indicate support
for 'nopadding' or 'pkcs1' signatures in the client. Add 'pss' as an
option to announce that PSS signing requests are accepted.
To match, extend the algorithm string in PK_SIGN request to
include the following format:
- RSA_PKCS1_PSS_PADDING,hashlag=name,saltlen=[max|digest]
Here 'name' is the short common name of the hash algorithm.
E.g., SHA1, SHA256 etc.
Existing formats 'ECDSA' and 'RSA_PKCS1_PADDING' are unchanged.
v2 changes: Fix typos and other sloppiness in documentation and
commit message.
Signed-off-by: Selva Nair <selva.nair@...277...>
---
doc/man-sections/management-options.rst | 8 +++++++-
doc/management-notes.txt | 22 ++++++++++++++++++----
src/openvpn/manage.h | 1 +
src/openvpn/options.c | 11 ++++++++---
4 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/doc/man-sections/management-options.rst b/doc/man-sections/management-options.rst
index de0d47e7..b173a1ea 100644
--- a/doc/man-sections/management-options.rst
+++ b/doc/man-sections/management-options.rst
@@ -90,9 +90,15 @@ server and client mode operations.
management-external-key
management-external-key nopadding
management-external-key pkcs1
+ management-external-key pss
+
+ or any combination like:
+ ::
+
management-external-key nopadding pkcs1
+ management-external-key pkcs1 pss
- The optional parameters :code:`nopadding` and :code:`pkcs1` signal
+ The optional parameters :code:`nopadding` :code:`pkcs1` and :code:`pss` signal
support for different padding algorithms. See
:code:`doc/mangement-notes.txt` for a complete description of this
feature.
diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 84e3d04b..169a5efe 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -1019,10 +1019,24 @@ can be indicated in the signing request only if the client version is > 2"
The currently defined padding algorithms are:
- - RSA_PKCS1_PADDING - PKCS1 padding and RSA signature
- - RSA_NO_PADDING - No padding may be added for the signature
- - ECDSA - EC signature.
-
+ - RSA_PKCS1_PADDING - PKCS1 padding and RSA signature
+ - RSA_NO_PADDING - No padding may be added for the signature
+ - ECDSA - EC signature.
+ - RSA_PKCS1_PSS_PADDING,params - RSA signature with PSS padding
+
+ The params for PSS are specified as 'hashalg=name,saltlen=[max|digest]'.
+
+ The hashalg names are short common names such as SHA256, SHA224, etc.
+ PSS saltlen="digest" means use the same size as the hash to sign, while
+ "max" indicates maximum possible saltlen which is
+ '(nbits-1)/8 - hlen - 2'. Here 'nbits' is the number of bits in the
+ key modulus and 'hlen' the size in octets of the hash.
+ (See: RFC 8017 sec 8.1.1 and 9.1.1)
+
+ In the case of PKCS1_PADDING, when the hash algorithm is not legacy
+ MD5-SHA1, the hash is encoded with DigestInfo header before presenting
+ to the management interface. This is identical to CKM_RSA_PKCS in Cryptoki
+ as well as what RSA_private_encrypt() in OpenSSL expects.
COMMAND -- certificate (OpenVPN 2.4 or higher)
----------------------------------------------
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index 04dc98d1..5ed27c0c 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -339,6 +339,7 @@ struct management *management_init(void);
#define MF_QUERY_REMOTE (1<<13)
#define MF_QUERY_PROXY (1<<14)
#define MF_EXTERNAL_CERT (1<<15)
+#define MF_EXTERNAL_KEY_PSSPAD (1<<16)
bool management_open(struct management *man,
const char *addr,
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index fb427410..3ec9025b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -60,6 +60,7 @@
#include "forward.h"
#include "ssl_verify.h"
#include "platform.h"
+#include "xkey_common.h"
#include <ctype.h>
#include "memdbg.h"
@@ -2207,14 +2208,14 @@ options_postprocess_verify_ce(const struct options *options,
#endif /* ifdef ENABLE_MANAGEMENT */
-#if defined(ENABLE_MANAGEMENT)
+#if defined(ENABLE_MANAGEMENT) && !defined(HAVE_XKEY_PROVIDER)
if ((tls_version_max() >= TLS_VER_1_3)
&& (options->management_flags & MF_EXTERNAL_KEY)
&& !(options->management_flags & (MF_EXTERNAL_KEY_NOPADDING))
)
{
- msg(M_ERR, "management-external-key with OpenSSL 1.1.1 requires "
- "the nopadding argument/support");
+ msg(M_FATAL, "management-external-key with TLS 1.3 or later requires "
+ "nopadding argument/support");
}
#endif
/*
@@ -5571,6 +5572,10 @@ add_option(struct options *options,
{
options->management_flags |= MF_EXTERNAL_KEY_PKCS1PAD;
}
+ else if (streq(p[j], "pss"))
+ {
+ options->management_flags |= MF_EXTERNAL_KEY_PSSPAD;
+ }
else
{
msg(msglevel, "Unknown management-external-key flag: %s", p[j]);
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* [Openvpn-devel] [PATCH v3 10/18] Respect algorithm support announced by management client
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (8 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 09/18] Allow management client to announce pss padding support selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 10:55 ` Arne Schwabe
2022-01-20 15:42 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 11/18] Support sending DigestSign request to " selva.nair
` (7 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
Support for padding algorithms in management-client is indicated
in the optional argument to --management-external-key as "pkcs1",
"pss" etc. We currently use it only for an early exit based on heuristics
that a required algorithm may not be handled by the client. When
signature is requested we do not check whether the padding is indeed
supported by the client. This leads to situations like the client announcing
nopadding support but we request pss signature.
Here we add a check while requesting signature as well. If the padding
treat it as an error instead of submitting the request to the
management-interface regardless.
This change is made only when xkey provider is in use, though such a check
would be appropriate always.
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/xkey_helper.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index b2546cec..d63943d2 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -146,6 +146,8 @@ xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
unsigned char enc[EVP_MAX_MD_SIZE + 32]; /* 32 bytes enough for digest inf structure */
size_t enc_len = sizeof(enc);
+ unsigned int flags = management->settings.flags;
+
if (!strcmp(alg.op, "DigestSign"))
{
dmsg(D_LOW, "xkey_management_sign: computing digest");
@@ -166,7 +168,7 @@ xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
strncpynt(alg_str, "ECDSA", sizeof(alg_str));
}
/* else assume RSA key */
- else if (!strcmp(alg.padmode, "pkcs1"))
+ else if (!strcmp(alg.padmode, "pkcs1") && (flags & MF_EXTERNAL_KEY_PKCS1PAD))
{
/* management interface expects a pkcs1 encoded digest -- add it */
if (!encode_pkcs1(enc, &enc_len, alg.mdname, tbs, tbslen))
@@ -178,17 +180,17 @@ xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
strncpynt(alg_str, "RSA_PKCS1_PADDING", sizeof(alg_str));
}
- else if (!strcmp(alg.padmode, "none"))
+ else if (!strcmp(alg.padmode, "none") && (flags & MF_EXTERNAL_KEY_NOPADDING))
{
strncpynt(alg_str, "RSA_NO_PADDING", sizeof(alg_str));
}
- else if (!strcmp(alg.padmode, "pss"))
+ else if (!strcmp(alg.padmode, "pss") && (flags & MF_EXTERNAL_KEY_PSSPAD))
{
openvpn_snprintf(alg_str, sizeof(alg_str), "%s,hashalg=%s,saltlen=%s",
"RSA_PKCS1_PSS_PADDING", alg.mdname,alg.saltlen);
}
else {
- msg(M_NONFATAL, "Unsupported RSA padding mode in signature request<%s>",
+ msg(M_NONFATAL, "RSA padding mode unknown or not supported by management-client <%s>",
alg.padmode);
return 0;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 10/18] Respect algorithm support announced by management client
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 10/18] Respect algorithm support announced by management client selva.nair
@ 2022-01-20 10:55 ` Arne Schwabe
2022-01-20 15:42 ` [Openvpn-devel] [PATCH applied] " Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 10:55 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> Support for padding algorithms in management-client is indicated
> in the optional argument to --management-external-key as "pkcs1",
> "pss" etc. We currently use it only for an early exit based on heuristics
> that a required algorithm may not be handled by the client. When
> signature is requested we do not check whether the padding is indeed
> supported by the client. This leads to situations like the client announcing
> nopadding support but we request pss signature.
>
> Here we add a check while requesting signature as well. If the padding
> treat it as an error instead of submitting the request to the
> management-interface regardless.
>
> This change is made only when xkey provider is in use, though such a check
> would be appropriate always.
Acked-By: Arne Schwabe <arne@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH applied] Re: Respect algorithm support announced by management client
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 10/18] Respect algorithm support announced by management client selva.nair
2022-01-20 10:55 ` Arne Schwabe
@ 2022-01-20 15:42 ` Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 15:42 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
Glanced a bit at the code and compile-tested on 3.0.1 - looks
straightforward enough :-) (and yes to the comment about "such
a check would be appropriate always", but I'm leaning more to
"drop support for OpenSSL < 3.0.1 for external-key features" :-) ).
Your patch has been applied to the master branch.
commit 43de9f547d70cab2eb3e4478bf975e139ad966f7
Author: Selva Nair
Date: Tue Dec 14 11:59:20 2021 -0500
Respect algorithm support announced by management client
Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20211214165928.30676-11-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23441.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 11/18] Support sending DigestSign request to management client
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (9 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 10/18] Respect algorithm support announced by management client selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 11:11 ` Arne Schwabe
2022-01-20 16:26 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 12/18] Increase ERR_BUF_SIZE when management interface support is enabled selva.nair
` (6 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
To receive undigested message for signing, indicate support
for handling message digesting in the client using an argument
"digest" to --management-external-key.
For example, to announce pkcs1 padding and digesting support use:
--management-external-key pkcs1 pss digest
In PK_SIGN, the algorithm string will get data=message
in addition to other relevant options.
Note that it is not guaranteed that the client will be prompted
with undigested message. This is possible only when OpenSSL
calls our provider for DigestSign() as opposed to Sign(). In
practice, signature operation always appears to result in
a DigestSign() call through the provider interface.
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/manage.h | 1 +
src/openvpn/options.c | 4 +++
src/openvpn/xkey_helper.c | 52 ++++++++++++++++++++++++++++++---------
3 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index 5ed27c0c..9621f479 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -340,6 +340,7 @@ struct management *management_init(void);
#define MF_QUERY_PROXY (1<<14)
#define MF_EXTERNAL_CERT (1<<15)
#define MF_EXTERNAL_KEY_PSSPAD (1<<16)
+#define MF_EXTERNAL_KEY_DIGEST (1<<17)
bool management_open(struct management *man,
const char *addr,
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3ec9025b..a323367c 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5576,6 +5576,10 @@ add_option(struct options *options,
{
options->management_flags |= MF_EXTERNAL_KEY_PSSPAD;
}
+ else if (streq(p[j], "digest"))
+ {
+ options->management_flags |= MF_EXTERNAL_KEY_DIGEST;
+ }
else
{
msg(msglevel, "Unknown management-external-key flag: %s", p[j]);
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index d63943d2..d09ad635 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -138,17 +138,22 @@ int
xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
const unsigned char *tbs, size_t tbslen, XKEY_SIGALG alg)
{
+ dmsg(D_LOW, "In xkey_management_sign with keytype = %s, op = %s",
+ alg.keytype, alg.op);
+
(void) unused;
char alg_str[128];
unsigned char buf[EVP_MAX_MD_SIZE]; /* for computing digest if required */
size_t buflen = sizeof(buf);
- unsigned char enc[EVP_MAX_MD_SIZE + 32]; /* 32 bytes enough for digest inf structure */
+ unsigned char enc[EVP_MAX_MD_SIZE + 32]; /* 32 bytes enough for digest info structure */
size_t enc_len = sizeof(enc);
unsigned int flags = management->settings.flags;
+ bool is_message = !strcmp(alg.op, "DigestSign"); /* tbs is message, not digest */
- if (!strcmp(alg.op, "DigestSign"))
+ /* if management client cannot do digest -- we do it here */
+ if (!strcmp(alg.op, "DigestSign") && !(flags & MF_EXTERNAL_KEY_DIGEST))
{
dmsg(D_LOW, "xkey_management_sign: computing digest");
if (xkey_digest(tbs, tbslen, buf, &buflen, alg.mdname))
@@ -156,6 +161,7 @@ xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
tbs = buf;
tbslen = buflen;
alg.op = "Sign";
+ is_message = false;
}
else
{
@@ -165,22 +171,38 @@ xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
if (!strcmp(alg.keytype, "EC"))
{
- strncpynt(alg_str, "ECDSA", sizeof(alg_str));
+ if (!strcmp(alg.op, "Sign"))
+ {
+ strncpynt(alg_str, "ECDSA", sizeof(alg_str));
+ }
+ else
+ {
+ openvpn_snprintf(alg_str, sizeof(alg_str), "ECDSA,hashalg=%s", alg.mdname);
+ }
}
/* else assume RSA key */
else if (!strcmp(alg.padmode, "pkcs1") && (flags & MF_EXTERNAL_KEY_PKCS1PAD))
{
- /* management interface expects a pkcs1 encoded digest -- add it */
- if (!encode_pkcs1(enc, &enc_len, alg.mdname, tbs, tbslen))
+ /* For Sign, management interface expects a pkcs1 encoded digest -- add it */
+ if (!strcmp(alg.op, "Sign"))
{
- return 0;
+ if (!encode_pkcs1(enc, &enc_len, alg.mdname, tbs, tbslen))
+ {
+ return 0;
+ }
+ tbs = enc;
+ tbslen = enc_len;
+ strncpynt(alg_str, "RSA_PKCS1_PADDING", sizeof(alg_str));
+ }
+ /* For undigested message, add hashalg=digest parameter */
+ else
+ {
+ openvpn_snprintf(alg_str, sizeof(alg_str), "%s,hashalg=%s",
+ "RSA_PKCS1_PADDING", alg.mdname);
}
- tbs = enc;
- tbslen = enc_len;
-
- strncpynt(alg_str, "RSA_PKCS1_PADDING", sizeof(alg_str));
}
- else if (!strcmp(alg.padmode, "none") && (flags & MF_EXTERNAL_KEY_NOPADDING))
+ else if (!strcmp(alg.padmode, "none") && (flags & MF_EXTERNAL_KEY_NOPADDING)
+ &&!strcmp(alg.op, "Sign")) /* NO_PADDING requires digested data */
{
strncpynt(alg_str, "RSA_NO_PADDING", sizeof(alg_str));
}
@@ -190,10 +212,16 @@ xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
"RSA_PKCS1_PSS_PADDING", alg.mdname,alg.saltlen);
}
else {
- msg(M_NONFATAL, "RSA padding mode unknown or not supported by management-client <%s>",
+ msg(M_NONFATAL, "RSA padding mode not supported by management-client <%s>",
alg.padmode);
return 0;
}
+
+ if (is_message)
+ {
+ strncat(alg_str, ",data=message", sizeof(alg_str) - strlen(alg_str) - 1);
+ }
+
dmsg(D_LOW, "xkey management_sign: requesting sig with algorithm <%s>", alg_str);
char *in_b64 = NULL;
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 11/18] Support sending DigestSign request to management client
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 11/18] Support sending DigestSign request to " selva.nair
@ 2022-01-20 11:11 ` Arne Schwabe
2022-01-20 16:26 ` [Openvpn-devel] [PATCH applied] " Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 11:11 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> To receive undigested message for signing, indicate support
> for handling message digesting in the client using an argument
> "digest" to --management-external-key.
>
> For example, to announce pkcs1 padding and digesting support use:
>
> --management-external-key pkcs1 pss digest
>
> In PK_SIGN, the algorithm string will get data=message
> in addition to other relevant options.
>
> Note that it is not guaranteed that the client will be prompted
> with undigested message. This is possible only when OpenSSL
> calls our provider for DigestSign() as opposed to Sign(). In
> practice, signature operation always appears to result in
> a DigestSign() call through the provider interface.
>
Acked-By: Arne Schwabe <arne@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH applied] Re: Support sending DigestSign request to management client
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 11/18] Support sending DigestSign request to " selva.nair
2022-01-20 11:11 ` Arne Schwabe
@ 2022-01-20 16:26 ` Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 16:26 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
Compile-tested on 3.0.1 and stared at the code for a bit. The "global"
change is trivial enough, the xkey_helper changes look safe wrt memory
overflows etc, though I lack the greater understanding on how all
the wheels work together (so it's good that Arne tested and ACKed this).
Your patch has been applied to the master branch.
commit 0f6781fa3639d05ce1afb83a45c3bb12c6f97f1b
Author: Selva Nair
Date: Tue Dec 14 11:59:21 2021 -0500
Support sending DigestSign request to management client
Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20211214165928.30676-12-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23435.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 12/18] Increase ERR_BUF_SIZE when management interface support is enabled
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (10 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 11/18] Support sending DigestSign request to " selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 11:11 ` Arne Schwabe
2022-01-20 16:33 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 13/18] Add a generic key loading helper function for xkey provider selva.nair
` (5 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
Sending largish messages to the management interface errors due to
the limited size used for the "error" buffer in x_msg_va(). Although
all intermediate steps allocate required space for the data to
send, it gets truncated at the last step.
This really requires a smarter fix. As a quick relief, we just increase
the buffer size to 10240 when management support is compiled in. Should
be enough for PK_SIGN with undigested message.
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/error.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index 533354b3..66c1722e 100644
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -37,8 +37,8 @@
/* #define ABORT_ON_ERROR */
-#ifdef ENABLE_PKCS11
-#define ERR_BUF_SIZE 8192
+#if defined(ENABLE_PKCS11) || defined(ENABLE_MANAGEMENT)
+#define ERR_BUF_SIZE 10240
#else
#define ERR_BUF_SIZE 1280
#endif
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 12/18] Increase ERR_BUF_SIZE when management interface support is enabled
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 12/18] Increase ERR_BUF_SIZE when management interface support is enabled selva.nair
@ 2022-01-20 11:11 ` Arne Schwabe
2022-01-20 16:33 ` [Openvpn-devel] [PATCH applied] " Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 11:11 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> Sending largish messages to the management interface errors due to
> the limited size used for the "error" buffer in x_msg_va(). Although
> all intermediate steps allocate required space for the data to
> send, it gets truncated at the last step.
>
> This really requires a smarter fix. As a quick relief, we just increase
> the buffer size to 10240 when management support is compiled in. Should
> be enough for PK_SIGN with undigested message.
>
> Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-By: Arne Schwabe <arne@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH applied] Re: Increase ERR_BUF_SIZE when management interface support is enabled
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 12/18] Increase ERR_BUF_SIZE when management interface support is enabled selva.nair
2022-01-20 11:11 ` Arne Schwabe
@ 2022-01-20 16:33 ` Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 16:33 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
I seem to have seen a similar patch in Arne's series, and we didn't
like it there much either :-) - so yeah, smarter fix, eventually.
That said, this is "only" wasting another 2048 byte as the buffer is
already at 8k if PKCS11 is enabled, and it's not static but gc_malloc().
Pretty weird to use *ERR*_BUF_SIZE in manage.c and plugin.c, though :-)
(and maybe the plugin_vlog() code could be changed to be smarter about
the way it builds the msg_fmt string based on plugin name + format string)
Anyway, client tested with 1.1.1, just for good measure.
Your patch has been applied to the master branch.
commit eeb019acee57ef5b9485569ec4d3279a822c4eb0
Author: Selva Nair
Date: Tue Dec 14 11:59:22 2021 -0500
Increase ERR_BUF_SIZE when management interface support is enabled
Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20211214165928.30676-13-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23440.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 13/18] Add a generic key loading helper function for xkey provider
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (11 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 12/18] Increase ERR_BUF_SIZE when management interface support is enabled selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 11:11 ` Arne Schwabe
2022-01-20 16:58 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 14/18] pkcs11: Interface the xkey provider with pkcs11-helper selva.nair
` (4 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
- Load keys by specifying the opaque privtae key handle,
public key, sign-op and free-op required for loading keys
from Windows store and pkcs11.
- xkey_load_management_key is refactored to use the new function
- Also make xkey_digest non-static
Used in following commits to load CNG and pkcs11 keys
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/xkey_common.h | 35 +++++++++++++++++++++++++++++++++++
src/openvpn/xkey_helper.c | 37 +++++++++++++++++++++++++++++++------
2 files changed, 66 insertions(+), 6 deletions(-)
diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h
index c04c9c5c..e2ddc178 100644
--- a/src/openvpn/xkey_common.h
+++ b/src/openvpn/xkey_common.h
@@ -116,6 +116,41 @@ bool
encode_pkcs1(unsigned char *enc, size_t *enc_len, const char *mdname,
const unsigned char *tbs, size_t tbslen);
+/**
+ * Compute message digest
+ *
+ * @param src pointer to message to be hashed
+ * @param srclen length of data in bytes
+ * @param buf pointer to output buffer
+ * @param buflen *buflen = capacity in bytes of output buffer
+ * @param mdname name of the hash algorithm (SHA256, SHA1 etc.)
+ *
+ * @return false on error, true on success
+ *
+ * On successful return *buflen is set to the actual size of the result.
+ * TIP: EVP_MD_MAX_SIZE should be enough capacity of buf for al algorithms.
+ */
+int
+xkey_digest(const unsigned char *src, size_t srclen, unsigned char *buf,
+ size_t *buflen, const char *mdname);
+
+/**
+ * Load a generic external key with custom sign and free ops
+ *
+ * @param libctx library context in which xkey provider has been loaded
+ * @param handle an opaque handle to the backend -- passed to alll callbacks
+ * @param pubkey corresponding pubkey in the default provider's context
+ * @param sign_op private key signature operation to callback
+ * @param sign_op private key signature operation to callback
+ *
+ * @returns a new EVP_PKEY in the provider's keymgmt context.
+ * IMPORTANT: a reference to the handle is retained by the provider and
+ * relased by callng free_op. The caller should not free it.
+ */
+EVP_PKEY *
+xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey,
+ XKEY_EXTERNAL_SIGN_fn sign_op, XKEY_PRIVKEY_FREE_fn free_op);
+
#endif /* HAVE_XKEY_PROVIDER */
#endif /* XKEY_COMMON_H_ */
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index d09ad635..19de64ff 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -50,8 +50,18 @@ static const char *const props = XKEY_PROV_PROPS;
XKEY_EXTERNAL_SIGN_fn xkey_management_sign;
+static void
+print_openssl_errors()
+{
+ unsigned long e;
+ while ((e = ERR_get_error()))
+ {
+ msg(M_WARN, "OpenSSL error %lu: %s\n", e, ERR_error_string(e, NULL));
+ }
+}
+
/** helper to compute digest */
-static int
+int
xkey_digest(const unsigned char *src, size_t srclen, unsigned char *buf,
size_t *buflen, const char *mdname)
{
@@ -85,24 +95,38 @@ xkey_digest(const unsigned char *src, size_t srclen, unsigned char *buf,
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
+ /* Management interface doesn't 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;
+ return xkey_load_generic_key(libctx, dummy, pubkey, sign_op, NULL);
+}
+
+/**
+ * Load a generic key into the xkey provider.
+ * Returns an EVP_PKEY object attached to xkey provider.
+ * Caller must free it when no longer needed.
+ */
+EVP_PKEY *
+xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey,
+ XKEY_EXTERNAL_SIGN_fn sign_op, XKEY_PRIVKEY_FREE_fn free_op)
+{
+ EVP_PKEY *pkey = NULL;
+ const char *origin = "external";
+
/* 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},
+ {"handle", OSSL_PARAM_OCTET_PTR, &handle, sizeof(handle), 0},
{"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(sign_op), 0},
+ {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(free_op), 0},
{NULL, 0, NULL, 0, 0}};
/* Do not use EVP_PKEY_new_from_pkey as that will take keymgmt from pubkey */
@@ -111,7 +135,8 @@ xkey_load_management_key(OSSL_LIB_CTX *libctx, EVP_PKEY *pubkey)
|| 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");
+ print_openssl_errors();
+ msg(M_FATAL, "OpenSSL error: failed to load key into ovpn.xkey provider");
}
if (ctx)
{
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* [Openvpn-devel] [PATCH v3 14/18] pkcs11: Interface the xkey provider with pkcs11-helper
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (12 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 13/18] Add a generic key loading helper function for xkey provider selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 11:12 ` Arne Schwabe
2022-01-20 17:08 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 15/18] Enable signing using CNG through xkey provider selva.nair
` (3 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
- Load the 'private key' handle through the provider and set it in
SSL_CTX
- Add a sign op function to interface provider with pkcs11-helper.
Previously we used its "OpenSSL Session" which internally sets up
callbacks in RSA and EC key methods. Not useful for the provider
interface, so, we directly call the PKCS#11 sign operation
as done with mbedTLS.
- tls_libctx is made global for accessing from pkcs11_openssl.c
Supports ECDSA and RSA_PKCS1_PADDING signatures. PSS support
will be added when pkcs11-helper with our PR for specifying
CK_MECHANISM variable in sign operations is released.
(i.e., next release of pkcs11-helper).
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/pkcs11_openssl.c | 151 +++++++++++++++++++++++++++++++++++
src/openvpn/ssl_openssl.c | 2 +-
src/openvpn/xkey_common.h | 2 +
3 files changed, 154 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/pkcs11_openssl.c b/src/openvpn/pkcs11_openssl.c
index b29504b6..9cf46b2c 100644
--- a/src/openvpn/pkcs11_openssl.c
+++ b/src/openvpn/pkcs11_openssl.c
@@ -39,12 +39,163 @@
#include "errlevel.h"
#include "pkcs11_backend.h"
#include "ssl_verify.h"
+#include "xkey_common.h"
#include <pkcs11-helper-1.0/pkcs11h-openssl.h>
+#ifdef HAVE_XKEY_PROVIDER
+static XKEY_EXTERNAL_SIGN_fn xkey_pkcs11h_sign;
+
+/**
+ * Sign op called from xkey provider
+ *
+ * We support ECDSA, RSA_NO_PADDING, RSA_PKCS1_PADDING
+ */
+static int
+xkey_pkcs11h_sign(void *handle, unsigned char *sig,
+ size_t *siglen, const unsigned char *tbs, size_t tbslen, XKEY_SIGALG sigalg)
+{
+ pkcs11h_certificate_t cert = handle;
+ CK_MECHANISM mech = {CKM_RSA_PKCS, NULL, 0}; /* default value */
+
+ unsigned char buf[EVP_MAX_MD_SIZE];
+ size_t buflen;
+
+ if (!strcmp(sigalg.op, "DigestSign"))
+ {
+ dmsg(D_LOW, "xkey_pkcs11h_sign: computing digest");
+ if (xkey_digest(tbs, tbslen, buf, &buflen, sigalg.mdname))
+ {
+ tbs = buf;
+ tbslen = (size_t) buflen;
+ sigalg.op = "Sign";
+ }
+ else
+ {
+ return 0;
+ }
+ }
+
+ if (!strcmp(sigalg.keytype, "EC"))
+ {
+ mech.mechanism = CKM_ECDSA;
+ }
+ else if (!strcmp(sigalg.keytype, "RSA"))
+ {
+ if (!strcmp(sigalg.padmode,"none"))
+ {
+ mech.mechanism = CKM_RSA_X_509;
+ }
+ else if (!strcmp(sigalg.padmode, "pss"))
+ {
+ msg(M_NONFATAL, "PKCS#11: Error: PSS padding is not yet supported.");
+ return 0;
+ }
+ else if (!strcmp(sigalg.padmode, "pkcs1"))
+ {
+ /* CMA_RSA_PKCS needs pkcs1 encoded digest */
+
+ unsigned char enc[EVP_MAX_MD_SIZE + 32]; /* 32 bytes enough for DigestInfo header */
+ size_t enc_len = sizeof(enc);
+
+ if (!encode_pkcs1(enc, &enc_len, sigalg.mdname, tbs, tbslen))
+ {
+ return 0;
+ }
+ tbs = enc;
+ tbslen = enc_len;
+ }
+ else /* should not happen */
+ {
+ msg(M_WARN, "PKCS#11: Unknown padmode <%s>", sigalg.padmode);
+ }
+ }
+ else
+ {
+ ASSERT(0); /* coding error -- we couldnt have created any such key */
+ }
+
+ return CKR_OK == pkcs11h_certificate_signAny(cert, mech.mechanism,
+ tbs, tbslen, sig, siglen);
+}
+
+/* wrapper for handle free */
+static void
+xkey_handle_free(void *handle)
+{
+ pkcs11h_certificate_freeCertificate(handle);
+}
+
+
+/**
+ * Load certificate and public key from pkcs11h to SSL_CTX
+ * through xkey provider.
+ *
+ * @param certificate pkcs11h certificate object
+ * @param ctx OpenVPN root tls context
+ *
+ * @returns 1 on success, 0 on error to match
+ * other xkey_load_.. routines
+ */
+static int
+xkey_load_from_pkcs11h(pkcs11h_certificate_t certificate,
+ struct tls_root_ctx *const ctx)
+{
+ int ret = 0;
+
+ X509 *x509 = pkcs11h_openssl_getX509(certificate);
+ if (!x509)
+ {
+ msg(M_WARN, "PKCS#11: Unable get x509 certificate object");
+ return 0;
+ }
+
+ EVP_PKEY *pubkey = X509_get0_pubkey(x509);
+
+ XKEY_PRIVKEY_FREE_fn *free_op = xkey_handle_free; /* it calls pkcs11h_..._freeCertificate() */
+ XKEY_EXTERNAL_SIGN_fn *sign_op = xkey_pkcs11h_sign;
+
+ EVP_PKEY *pkey = xkey_load_generic_key(tls_libctx, certificate, pubkey, sign_op, free_op);
+ if (!pkey)
+ {
+ msg(M_WARN, "PKCS#11: Failed to load private key into xkey provider");
+ goto cleanup;
+ }
+ /* provider took ownership of the pkcs11h certificate object -- do not free below */
+ certificate = NULL;
+
+ if (!SSL_CTX_use_cert_and_key(ctx->ctx, x509, pkey, NULL, 0))
+ {
+ msg(M_WARN, "PKCS#11: Failed to set cert and private key for OpenSSL");
+ goto cleanup;
+ }
+ ret = 1;
+
+cleanup:
+ if (x509)
+ {
+ X509_free(x509);
+ }
+ if (pkey)
+ {
+ EVP_PKEY_free(pkey);
+ }
+ if (certificate)
+ {
+ pkcs11h_certificate_freeCertificate(certificate);
+ }
+ return ret;
+}
+#endif /* HAVE_XKEY_PROVIDER */
+
int
pkcs11_init_tls_session(pkcs11h_certificate_t certificate,
struct tls_root_ctx *const ssl_ctx)
{
+
+#ifdef HAVE_XKEY_PROVIDER
+ return (xkey_load_from_pkcs11h(certificate, ssl_ctx) == 0); /* inverts the return value */
+#endif
+
int ret = 1;
X509 *x509 = NULL;
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 8f0281b1..b48845eb 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -70,7 +70,7 @@
#include <openssl/applink.c>
#endif
-static OSSL_LIB_CTX *tls_libctx;
+OSSL_LIB_CTX *tls_libctx; /* Global */
static void unload_xkey_provider(void);
diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h
index e2ddc178..8eac4c7c 100644
--- a/src/openvpn/xkey_common.h
+++ b/src/openvpn/xkey_common.h
@@ -151,6 +151,8 @@ EVP_PKEY *
xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey,
XKEY_EXTERNAL_SIGN_fn sign_op, XKEY_PRIVKEY_FREE_fn free_op);
+extern OSSL_LIB_CTX *tls_libctx; /* Global */
+
#endif /* HAVE_XKEY_PROVIDER */
#endif /* XKEY_COMMON_H_ */
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 14/18] pkcs11: Interface the xkey provider with pkcs11-helper
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 14/18] pkcs11: Interface the xkey provider with pkcs11-helper selva.nair
@ 2022-01-20 11:12 ` Arne Schwabe
2022-01-20 17:08 ` [Openvpn-devel] [PATCH applied] " Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 11:12 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> - Load the 'private key' handle through the provider and set it in
> SSL_CTX
> - Add a sign op function to interface provider with pkcs11-helper.
> Previously we used its "OpenSSL Session" which internally sets up
> callbacks in RSA and EC key methods. Not useful for the provider
> interface, so, we directly call the PKCS#11 sign operation
> as done with mbedTLS.
> - tls_libctx is made global for accessing from pkcs11_openssl.c
>
> Supports ECDSA and RSA_PKCS1_PADDING signatures. PSS support
> will be added when pkcs11-helper with our PR for specifying
> CK_MECHANISM variable in sign operations is released.
> (i.e., next release of pkcs11-helper).
>
Acked-By: Arne Schwabe <arne@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH applied] Re: pkcs11: Interface the xkey provider with pkcs11-helper
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 14/18] pkcs11: Interface the xkey provider with pkcs11-helper selva.nair
2022-01-20 11:12 ` Arne Schwabe
@ 2022-01-20 17:08 ` Gert Doering
1 sibling, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 17:08 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
client tested with 3.0.1 (no pkcs#11 though), and stared at the code a bit.
This change looks like it really wants an "#else" and move the #endif
to the end of the function... (though the compiler does not warn)
pkcs11_init_tls_session(pkcs11h_certificate_t certificate,
struct tls_root_ctx *const ssl_ctx)
{
+
+#ifdef HAVE_XKEY_PROVIDER
+ return (xkey_load_from_pkcs11h(certificate, ssl_ctx) == 0); /* inverts the return value */
+#endif
+
int ret = 1;
(more stuff)
This prototype looks a bit surprising
+static XKEY_EXTERNAL_SIGN_fn xkey_pkcs11h_sign;
given that the function is defined just below? Is this to ensure
XKEY_EXTERNAL_SIGN_fn matches the actual function definition?
Your patch has been applied to the master branch.
commit 6121001ed82914f336da081bb8aefaeb055450cb
Author: Selva Nair
Date: Tue Dec 14 11:59:24 2021 -0500
pkcs11: Interface the xkey provider with pkcs11-helper
Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20211214165928.30676-15-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23442.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 15/18] Enable signing using CNG through xkey provider
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (13 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 14/18] pkcs11: Interface the xkey provider with pkcs11-helper selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 11:21 ` Arne Schwabe
2022-01-20 17:16 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 16/18] Add a unit test for external key provider selva.nair
` (2 subsequent siblings)
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
- Add xkey_cng_sign() as sign_op for the provider
and load the key using xkey_generic_load.
- Enable/Disable old code when provider is available or not.
- xkey_digest is made non-static for use in cryptoapi.c
One function cng_padding_type() is moved down to reduce number
of ifdef's.
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/cryptoapi.c | 241 +++++++++++++++++++++++++++++++++++-----
1 file changed, 211 insertions(+), 30 deletions(-)
diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index 7fe3c57c..08cb434f 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -52,7 +52,9 @@
#include "buffer.h"
#include "openssl_compat.h"
#include "win32.h"
+#include "xkey_common.h"
+#ifndef HAVE_XKEY_PROVIDER
/* index for storing external data in EC_KEY: < 0 means uninitialized */
static int ec_data_idx = -1;
@@ -61,44 +63,19 @@ static EVP_PKEY_METHOD *pmethod;
static int (*default_pkey_sign_init) (EVP_PKEY_CTX *ctx);
static int (*default_pkey_sign) (EVP_PKEY_CTX *ctx, unsigned char *sig,
size_t *siglen, const unsigned char *tbs, size_t tbslen);
+#else
+static XKEY_EXTERNAL_SIGN_fn xkey_cng_sign;
+#endif /* HAVE_XKEY_PROVIDER */
typedef struct _CAPI_DATA {
const CERT_CONTEXT *cert_context;
HCRYPTPROV_OR_NCRYPT_KEY_HANDLE crypt_prov;
+ EVP_PKEY *pubkey;
DWORD key_spec;
BOOL free_crypt_prov;
int ref_count;
} CAPI_DATA;
-/* Translate OpenSSL padding type to CNG padding type
- * Returns 0 for unknown/unsupported padding.
- */
-static DWORD
-cng_padding_type(int padding)
-{
- DWORD pad = 0;
-
- switch (padding)
- {
- case RSA_NO_PADDING:
- break;
-
- case RSA_PKCS1_PADDING:
- pad = BCRYPT_PAD_PKCS1;
- break;
-
- case RSA_PKCS1_PSS_PADDING:
- pad = BCRYPT_PAD_PSS;
- break;
-
- default:
- msg(M_WARN|M_INFO, "cryptoapicert: unknown OpenSSL padding type %d.",
- padding);
- }
-
- return pad;
-}
-
/*
* Translate OpenSSL hash OID to CNG algorithm name. Returns
* "UNKNOWN" for unsupported algorithms and NULL for MD5+SHA1
@@ -164,9 +141,42 @@ CAPI_DATA_free(CAPI_DATA *cd)
{
CertFreeCertificateContext(cd->cert_context);
}
+ EVP_PKEY_free(cd->pubkey); /* passing NULL is okay */
+
free(cd);
}
+#ifndef HAVE_XKEY_PROVIDER
+
+/* Translate OpenSSL padding type to CNG padding type
+ * Returns 0 for unknown/unsupported padding.
+ */
+static DWORD
+cng_padding_type(int padding)
+{
+ DWORD pad = 0;
+
+ switch (padding)
+ {
+ case RSA_NO_PADDING:
+ break;
+
+ case RSA_PKCS1_PADDING:
+ pad = BCRYPT_PAD_PKCS1;
+ break;
+
+ case RSA_PKCS1_PSS_PADDING:
+ pad = BCRYPT_PAD_PSS;
+ break;
+
+ default:
+ msg(M_WARN|M_INFO, "cryptoapicert: unknown OpenSSL padding type %d.",
+ padding);
+ }
+
+ return pad;
+}
+
/**
* Sign the hash in 'from' using NCryptSignHash(). This requires an NCRYPT
* key handle in cd->crypt_prov. On return the signature is in 'to'. Returns
@@ -255,6 +265,7 @@ ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp)
{
return 1;
}
+#endif /* HAVE_XKEY_PROVIDER */
/**
* Helper to convert ECDSA signature returned by NCryptSignHash
@@ -290,6 +301,8 @@ err:
return NULL;
}
+#ifndef HAVE_XKEY_PROVIDER
+
/** EC_KEY_METHOD callback sign_sig(): sign and return an ECDSA_SIG pointer. */
static ECDSA_SIG *
ecdsa_sign_sig(const unsigned char *dgst, int dgstlen,
@@ -421,6 +434,8 @@ err:
return 0;
}
+#endif /* !HAVE_XKEY_PROVIDER */
+
static const CERT_CONTEXT *
find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store)
{
@@ -521,6 +536,8 @@ out:
return rv;
}
+#ifndef HAVE_XKEY_PROVIDER
+
static const CAPI_DATA *
retrieve_capi_data(EVP_PKEY *pkey)
{
@@ -765,6 +782,158 @@ cleanup:
return ret;
}
+#else /* HAVE_XKEY_PROVIDER */
+
+/** Sign hash in tbs using EC key in cd and NCryptSignHash */
+static int
+xkey_cng_ec_sign(CAPI_DATA *cd, unsigned char *sig, size_t *siglen, const unsigned char *tbs,
+ size_t tbslen)
+{
+ BYTE buf[1024]; /* large enough for EC keys upto 1024 bits */
+ DWORD len = _countof(buf);
+
+ msg(D_LOW, "Signing using NCryptSignHash with EC key");
+
+ DWORD status = NCryptSignHash(cd->crypt_prov, NULL, (BYTE *)tbs, tbslen, buf, len, &len, 0);
+
+ if (status != ERROR_SUCCESS)
+ {
+ SetLastError(status);
+ msg(M_NONFATAL|M_ERRNO, "Error in cryptoapicert: ECDSA signature using CNG failed.");
+ return 0;
+ }
+
+ /* NCryptSignHash returns r|s -- convert to OpenSSL's ECDSA_SIG */
+ ECDSA_SIG *ecsig = ecdsa_bin2sig(buf, len);
+ if (!ecsig)
+ {
+ msg(M_NONFATAL, "Error in cryptopicert: Failed to convert ECDSA signature");
+ return 0;
+ }
+
+ /* convert internal signature structure 's' to DER encoded byte array in sig */
+ if (i2d_ECDSA_SIG(ecsig, NULL) > EVP_PKEY_size(cd->pubkey))
+ {
+ ECDSA_SIG_free(ecsig);
+ msg(M_NONFATAL, "Error in cryptoapicert: DER encoded ECDSA signature is too long");
+ return 0;
+ }
+
+ *siglen = i2d_ECDSA_SIG(ecsig, &sig);
+ ECDSA_SIG_free(ecsig);
+
+ return (*siglen > 0);
+}
+
+/** Sign hash in tbs using RSA key in cd and NCryptSignHash */
+static int
+xkey_cng_rsa_sign(CAPI_DATA *cd, unsigned char *sig, size_t *siglen, const unsigned char *tbs,
+ size_t tbslen, XKEY_SIGALG sigalg)
+{
+ dmsg(D_LOW, "In xkey_cng_rsa_sign");
+
+ ASSERT(cd);
+ ASSERT(sig);
+ ASSERT(tbs);
+
+ DWORD status = ERROR_SUCCESS;
+ DWORD len = 0;
+
+ const wchar_t *hashalg = cng_hash_algo(OBJ_sn2nid(sigalg.mdname));
+
+ if (hashalg && wcscmp(hashalg, L"UNKNOWN") == 0)
+ {
+ msg(M_NONFATAL, "Error in cryptoapicert: Unknown hash name <%s>", sigalg.mdname);
+ return 0;
+ }
+
+ if (!strcmp(sigalg.padmode, "pkcs1"))
+ {
+ msg(D_LOW, "Signing using NCryptSignHash with PKCS1 padding: hashalg <%s>", sigalg.mdname);
+
+ BCRYPT_PKCS1_PADDING_INFO padinfo = {hashalg};
+ status = NCryptSignHash(cd->crypt_prov, &padinfo, (BYTE *)tbs, (DWORD)tbslen,
+ sig, (DWORD)*siglen, &len, BCRYPT_PAD_PKCS1);
+ }
+ else if (!strcmp(sigalg.padmode, "pss"))
+ {
+ int saltlen = tbslen; /* digest size by default */
+ if (!strcmp(sigalg.saltlen, "max"))
+ {
+ saltlen = (EVP_PKEY_bits(cd->pubkey) - 1)/8 - tbslen - 2;
+ if (saltlen < 0)
+ {
+ msg(M_NONFATAL, "Error in cryptoapicert: invalid salt length (%d)", saltlen);
+ return 0;
+ }
+ }
+
+ msg(D_LOW, "Signing using NCryptSignHash with PSS padding: hashalg <%s>, saltlen <%d>",
+ sigalg.mdname, saltlen);
+
+ BCRYPT_PSS_PADDING_INFO padinfo = {hashalg, (DWORD) saltlen}; /* cast is safe as saltlen >= 0 */
+ status = NCryptSignHash(cd->crypt_prov, &padinfo, (BYTE *)tbs, (DWORD) tbslen,
+ sig, (DWORD)*siglen, &len, BCRYPT_PAD_PSS);
+ }
+ else
+ {
+ msg(M_NONFATAL, "Error in cryptoapicert: Unsupported padding mode <%s>", sigalg.padmode);
+ return 0;
+ }
+
+ if (status != ERROR_SUCCESS)
+ {
+ SetLastError(status);
+ msg(M_NONFATAL|M_ERRNO, "Error in cryptoapicert: RSA signature using CNG failed.");
+ return 0;
+ }
+
+ *siglen = len;
+ return (*siglen > 0);
+}
+
+/** Dispatch sign op to xkey_cng_<rsa/ec>_sign */
+static int
+xkey_cng_sign(void *handle, unsigned char *sig, size_t *siglen, const unsigned char *tbs,
+ size_t tbslen, XKEY_SIGALG sigalg)
+{
+ dmsg(D_LOW, "In xkey_cng_sign");
+
+ CAPI_DATA *cd = handle;
+ ASSERT(cd);
+ ASSERT(sig);
+ ASSERT(tbs);
+
+ unsigned char mdbuf[EVP_MAX_MD_SIZE];
+ size_t buflen = _countof(mdbuf);
+
+ /* compute digest if required */
+ if (!strcmp(sigalg.op, "DigestSign"))
+ {
+ if(!xkey_digest(tbs, tbslen, mdbuf, &buflen, sigalg.mdname))
+ {
+ return 0;
+ }
+ tbs = mdbuf;
+ tbslen = buflen;
+ }
+
+ if (!strcmp(sigalg.keytype, "EC"))
+ {
+ return xkey_cng_ec_sign(cd, sig, siglen, tbs, tbslen);
+ }
+ else if (!strcmp(sigalg.keytype, "RSA"))
+ {
+ return xkey_cng_rsa_sign(cd, sig, siglen, tbs, tbslen, sigalg);
+ }
+ else
+ {
+ return 0; /* Unknown keytype -- should not happen */
+ }
+}
+
+#endif /* HAVE_XKEY_PROVIDER */
+
int
SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
{
@@ -835,13 +1004,23 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
}
/* the public key */
- EVP_PKEY *pkey = X509_get0_pubkey(cert);
+ EVP_PKEY *pkey = X509_get_pubkey(cert);
+ cd->pubkey = pkey; /* will be freed with cd */
/* SSL_CTX_use_certificate() increased the reference count in 'cert', so
* we decrease it here with X509_free(), or it will never be cleaned up. */
X509_free(cert);
cert = NULL;
+#ifdef HAVE_XKEY_PROVIDER
+
+ EVP_PKEY *privkey = xkey_load_generic_key(tls_libctx, cd, pkey,
+ xkey_cng_sign, (XKEY_PRIVKEY_FREE_fn *) CAPI_DATA_free);
+ SSL_CTX_use_PrivateKey(ssl_ctx, privkey);
+ return 1; /* do not free cd -- its kept by xkey provider */
+
+#else
+
if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA)
{
if (!ssl_ctx_set_rsakey(ssl_ctx, cd, pkey))
@@ -865,6 +1044,8 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
CAPI_DATA_free(cd); /* this will do a ref_count-- */
return 1;
+#endif /* HAVE_XKEY_PROVIDER */
+
err:
CAPI_DATA_free(cd);
return 0;
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* [Openvpn-devel] [PATCH v3 16/18] Add a unit test for external key provider
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (14 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 15/18] Enable signing using CNG through xkey provider selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 11:22 ` Arne Schwabe
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature selva.nair
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 18/18] Add xkey_provider sources and includes to MSVC project selva.nair
17 siblings, 1 reply; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
Tests:
- Check SIGNATURE and KEYMGMT methods can be fetched
from the provider
- Load sample RSA and EC keys as management-external-key
and check that their sign callbacks are correctly exercised:
with and without digest support mocked in the client
capability flag.
Signed-off-by: Selva Nair <selva.nair@...277...>
---
configure.ac | 2 +
tests/unit_tests/openvpn/Makefile.am | 20 ++
tests/unit_tests/openvpn/test_provider.c | 305 +++++++++++++++++++++++
3 files changed, 327 insertions(+)
create mode 100644 tests/unit_tests/openvpn/test_provider.c
diff --git a/configure.ac b/configure.ac
index e0f9c332..c446f631 100644
--- a/configure.ac
+++ b/configure.ac
@@ -766,6 +766,8 @@ PKG_CHECK_MODULES(
[]
)
+AM_CONDITIONAL([HAVE_XKEY_PROVIDER], [false])
+
if test "${with_crypto_library}" = "openssl"; then
AC_ARG_VAR([OPENSSL_CFLAGS], [C compiler flags for OpenSSL])
AC_ARG_VAR([OPENSSL_LIBS], [linker flags for OpenSSL])
diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index 44b77cc5..96b670ae 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -11,6 +11,10 @@ if HAVE_LD_WRAP_SUPPORT
test_binaries += tls_crypt_testdriver
endif
+if HAVE_XKEY_PROVIDER
+test_binaries += provider_testdriver
+endif
+
TESTS = $(test_binaries)
check_PROGRAMS = $(test_binaries)
@@ -95,6 +99,22 @@ networking_testdriver_SOURCES = test_networking.c mock_msg.c \
$(openvpn_srcdir)/platform.c
endif
+if HAVE_XKEY_PROVIDER
+provider_testdriver_CFLAGS = @TEST_CFLAGS@ \
+ -I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) \
+ $(OPTIONAL_CRYPTO_CFLAGS)
+provider_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
+ $(OPTIONAL_CRYPTO_LIBS)
+
+provider_testdriver_SOURCES = test_provider.c mock_msg.c \
+ $(openvpn_srcdir)/xkey_helper.c \
+ $(openvpn_srcdir)/xkey_provider.c \
+ $(openvpn_srcdir)/buffer.c \
+ $(openvpn_srcdir)/base64.c \
+ mock_get_random.c \
+ $(openvpn_srcdir)/platform.c
+endif
+
auth_token_testdriver_CFLAGS = @TEST_CFLAGS@ \
-I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) \
$(OPTIONAL_CRYPTO_CFLAGS)
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
new file mode 100644
index 00000000..dcf39019
--- /dev/null
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -0,0 +1,305 @@
+/*
+ * OpenVPN -- An application to securely tunnel IP networks
+ * over a single 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@...277...>
+ *
+ * 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 <setjmp.h>
+#include <cmocka.h>
+#include <openssl/bio.h>
+#include <openssl/pem.h>
+#include <openssl/core_names.h>
+#include <openssl/evp.h>
+
+#include "manage.h"
+#include "xkey_common.h"
+
+struct management *management; /* global */
+static int mgmt_callback_called;
+
+#ifndef _countof
+#define _countof(x) sizeof((x))/sizeof(*(x))
+#endif
+
+static OSSL_PROVIDER *prov[2];
+
+/* public keys for testing -- RSA and EC */
+static const char * const pubkey1 = "-----BEGIN PUBLIC KEY-----\n"
+ "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA7GWP6RLCGlvmVioIqYI6\n"
+ "LUR4owA7sJ/nJxBAk+/xzD6gqgSigBsTqeb+gdZwkKjY1N4w2DUA0r5i8Eja/BWN\n"
+ "xMZtC5nxK4MACtMqIwvlzfk130NhFXKtlZj2cyFBXqDdRyeg1ZrUQagcHVcgcReP\n"
+ "9yiePgfO7NUOQk8edEeOR53SFCgnLBQQ9dGWtZN0hO/5BN6NSm/fd6vq0VjTRP5a\n"
+ "BAH/BnqX9/3jV0jh8N9AE59mI1rjVVQ9VDnuAPkS8dLfdC661/CNxt0YWByTIgt1\n"
+ "+qjW4LUvLbnU/rlPhuJ1SBZg+z/JtDBCKfs7syu5WYFqRvNFg7/91Rr/NwxvW/1h\n"
+ "8QIDAQAB\n"
+ "-----END PUBLIC KEY-----\n";
+
+static const char * const pubkey2 = "-----BEGIN PUBLIC KEY-----\n"
+ "MFYwEAYHKoZIzj0CAQYFK4EEAAoDQgAEO85iXW+HgnUkwlj1DohNVw0GsnGIh1gZ\n"
+ "u95ff1JiUaJIkYNIkZA+hwIPFVH5aJcSCv3SPIeDS2VUAESNKHZJBQ==\n"
+ "-----END PUBLIC KEY-----\n";
+
+static const char *pubkeys[] = {pubkey1, pubkey2};
+
+static const char *prov_name = "ovpn.xkey";
+
+static const char* test_msg = "Lorem ipsum dolor sit amet, consectetur "
+ "adipisici elit, sed eiusmod tempor incidunt "
+ "ut labore et dolore magna aliqua.";
+
+static const char* test_msg_b64 =
+ "TG9yZW0gaXBzdW0gZG9sb3Igc2l0IGFtZXQsIGNvbnNlY3RldHVyIGFkaXBpc2ljaS"
+ "BlbGl0LCBzZWQgZWl1c21vZCB0ZW1wb3IgaW5jaWR1bnQgdXQgbGFib3JlIGV0IGRv"
+ "bG9yZSBtYWduYSBhbGlxdWEu";
+
+/* Sha256 digest of test_msg excluding NUL terminator */
+static const uint8_t test_digest[] =
+ {0x77, 0x38, 0x65, 0x00, 0x1e, 0x96, 0x48, 0xc6, 0x57, 0x0b, 0xae,
+ 0xc0, 0xb7, 0x96, 0xf9, 0x66, 0x4d, 0x5f, 0xd0, 0xb7, 0xdb, 0xf3,
+ 0x3a, 0xbf, 0x02, 0xcc, 0x78, 0x61, 0x83, 0x20, 0x20, 0xee};
+
+static const char *test_digest_b64 = "dzhlAB6WSMZXC67At5b5Zk1f0Lfb8zq/Asx4YYMgIO4=";
+
+/* Dummy signature used only to check that the expected callback
+ * was successfully exercised. Keep this shorter than 64 bytes
+ * --- the smallest size of the actual signature with the above
+ * keys.
+ */
+const uint8_t good_sig[] =
+ {0xd8, 0xa7, 0xd9, 0x81, 0xd8, 0xaa, 0xd8, 0xad, 0x20, 0xd9, 0x8a, 0xd8,
+ 0xa7, 0x20, 0xd8, 0xb3, 0xd9, 0x85, 0xd8, 0xb3, 0xd9, 0x85, 0x0};
+
+const char *good_sig_b64 = "2KfZgdiq2K0g2YrYpyDYs9mF2LPZhQA=";
+
+static EVP_PKEY *
+load_pubkey(const char *pem)
+{
+ BIO *in = BIO_new_mem_buf(pem, -1);
+ assert_non_null(in);
+
+ EVP_PKEY *pkey = PEM_read_bio_PUBKEY(in, NULL, NULL, NULL);
+ assert_non_null(pkey);
+
+ BIO_free(in);
+ return pkey;
+}
+
+static void
+init_test()
+{
+ prov[0] = OSSL_PROVIDER_load(NULL,"default");
+ OSSL_PROVIDER_add_builtin(NULL, prov_name, xkey_provider_init);
+ prov[1] = OSSL_PROVIDER_load(NULL, prov_name);
+
+ /* set default propq matching what we use in ssl_openssl.c */
+ EVP_set_default_properties(NULL, "?provider!=ovpn.xkey");
+
+ management = test_calloc(sizeof(*management), 1);
+}
+
+static void
+uninit_test()
+{
+ for (size_t i = 0; i < _countof(prov); i++)
+ {
+ if (prov[i])
+ {
+ OSSL_PROVIDER_unload(prov[i]);
+ }
+ }
+ test_free(management);
+}
+
+/* Mock management callback for signature.
+ * We check that the received data to sign matches test_msg or
+ * test_digest and return a predefined string as signature so that
+ * the caller can validate all steps up to sending the data to
+ * the management client.
+ */
+char *
+management_query_pk_sig(struct management *man, const char *b64_data,
+ const char *algorithm)
+{
+ char *out = NULL;
+
+ /* indicate entry to the callback */
+ mgmt_callback_called = 1;
+
+ const char *expected_tbs = test_digest_b64;
+ if (strstr(algorithm, "data=message"))
+ {
+ expected_tbs = test_msg_b64;
+ }
+
+ assert_string_equal(b64_data, expected_tbs);
+
+ /* Return a predefined string as sig so that the caller
+ * can confirm that this callback was exercised.
+ */
+ out = strdup(good_sig_b64);
+ assert_non_null(out);
+
+ return out;
+}
+
+/* Check signature and keymgmt methods can be fetched from the provider */
+static void
+xkey_provider_test_fetch(void **state)
+{
+ assert_true(OSSL_PROVIDER_available(NULL, prov_name));
+
+ const char *algs[] = {"RSA", "ECDSA"};
+
+ for (size_t i = 0; i < _countof(algs); i++)
+ {
+ EVP_SIGNATURE *sig = EVP_SIGNATURE_fetch(NULL, algs[i], "provider=ovpn.xkey");
+ assert_non_null(sig);
+ assert_string_equal(OSSL_PROVIDER_get0_name(EVP_SIGNATURE_get0_provider(sig)), prov_name);
+
+ EVP_SIGNATURE_free(sig);
+ }
+
+ const char *names[] = {"RSA", "EC"};
+
+ for (size_t i = 0; i < _countof(names); i++)
+ {
+ EVP_KEYMGMT *km = EVP_KEYMGMT_fetch(NULL, names[i], "provider=ovpn.xkey");
+ assert_non_null(km);
+ assert_string_equal(OSSL_PROVIDER_get0_name(EVP_KEYMGMT_get0_provider(km)), prov_name);
+
+ EVP_KEYMGMT_free(km);
+ }
+}
+
+/* sign a test message using pkey -- caller must free the returned sig */
+static uint8_t *
+digest_sign(EVP_PKEY *pkey)
+{
+ uint8_t *sig = NULL;
+ size_t siglen = 0;
+
+ OSSL_PARAM params[6] = {OSSL_PARAM_END};
+
+ const char *mdname = "SHA256";
+ const char *padmode = "pss";
+ const char *saltlen = "digest";
+
+ if (EVP_PKEY_get_id(pkey) == EVP_PKEY_RSA)
+ {
+ params[0] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_DIGEST, (char *)mdname, 0);
+ params[1] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_PAD_MODE, (char *)padmode, 0);
+ params[2] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_PSS_SALTLEN, (char *)saltlen, 0);
+ /* same digest for mgf1 */
+ params[3] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_MGF1_DIGEST, (char *)saltlen, 0);
+ params[4] = OSSL_PARAM_construct_end();
+ }
+
+ EVP_PKEY_CTX *pctx = NULL;
+ EVP_MD_CTX *mctx = EVP_MD_CTX_new();
+
+ if (!mctx
+ || EVP_DigestSignInit_ex(mctx, &pctx, mdname, NULL, NULL, pkey, params) <= 0)
+ {
+ fail_msg("Failed to initialize EVP_DigestSignInit_ex()");
+ goto done;
+ }
+
+
+ /* sign with sig = NULL to get required siglen */
+ assert_int_equal(EVP_DigestSign(mctx, sig, &siglen, (uint8_t*)test_msg, strlen(test_msg)), 1);
+ assert_true(siglen > 0);
+
+ if ((sig = test_calloc(1, siglen)) == NULL)
+ {
+ fail_msg("Out of memory");
+ }
+ assert_int_equal(EVP_DigestSign(mctx, sig, &siglen, (uint8_t*)test_msg, strlen(test_msg)), 1);
+
+done:
+ if (mctx)
+ {
+ EVP_MD_CTX_free(mctx); /* pctx is internally allocated and freed by mctx */
+ }
+ return sig;
+}
+
+/* Check loading of management external key and have sign callback exercised
+ * for RSA and EC keys with and without digest support in management client.
+ * Sha256 digest used for both cases with pss padding for RSA.
+ */
+static void
+xkey_provider_test_mgmt_sign_cb(void **state)
+{
+ EVP_PKEY *pubkey;
+ for (size_t i = 0; i < _countof(pubkeys); i++)
+ {
+ pubkey = load_pubkey(pubkeys[i]);
+ assert_true(pubkey != NULL);
+ EVP_PKEY *privkey = xkey_load_management_key(NULL, pubkey);
+ assert_true(privkey != NULL);
+
+ management->settings.flags = MF_EXTERNAL_KEY|MF_EXTERNAL_KEY_PSSPAD;
+
+ /* first without digest support in management client */
+again:
+ mgmt_callback_called = 0;
+ uint8_t *sig = digest_sign(privkey);
+ assert_non_null(sig);
+
+ /* check callback for signature got exercised */
+ assert_int_equal(mgmt_callback_called, 1);
+ assert_memory_equal(sig, good_sig, sizeof(good_sig));
+ test_free(sig);
+
+ if (!(management->settings.flags & MF_EXTERNAL_KEY_DIGEST))
+ {
+ management->settings.flags |= MF_EXTERNAL_KEY_DIGEST;
+ goto again; /* this time with digest support announced */
+ }
+
+ EVP_PKEY_free(pubkey);
+ EVP_PKEY_free(privkey);
+ }
+}
+
+int
+main(void)
+{
+ init_test();
+
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(xkey_provider_test_fetch),
+ cmocka_unit_test(xkey_provider_test_mgmt_sign_cb),
+ };
+
+ int ret = cmocka_run_group_tests_name("xkey provider tests", tests, NULL, NULL);
+
+ uninit_test();
+ return ret;
+}
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (15 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 16/18] Add a unit test for external key provider selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 11:22 ` Arne Schwabe
` (2 more replies)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 18/18] Add xkey_provider sources and includes to MSVC project selva.nair
17 siblings, 3 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
Signed-off-by: Selva Nair <selva.nair@...277...>
---
configure.ac | 2 -
tests/unit_tests/openvpn/Makefile.am | 4 -
tests/unit_tests/openvpn/test_provider.c | 112 +++++++++++++++++++++--
3 files changed, 105 insertions(+), 13 deletions(-)
diff --git a/configure.ac b/configure.ac
index c446f631..e0f9c332 100644
--- a/configure.ac
+++ b/configure.ac
@@ -766,8 +766,6 @@ PKG_CHECK_MODULES(
[]
)
-AM_CONDITIONAL([HAVE_XKEY_PROVIDER], [false])
-
if test "${with_crypto_library}" = "openssl"; then
AC_ARG_VAR([OPENSSL_CFLAGS], [C compiler flags for OpenSSL])
AC_ARG_VAR([OPENSSL_LIBS], [linker flags for OpenSSL])
diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index 96b670ae..6b5c94ab 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -11,9 +11,7 @@ if HAVE_LD_WRAP_SUPPORT
test_binaries += tls_crypt_testdriver
endif
-if HAVE_XKEY_PROVIDER
test_binaries += provider_testdriver
-endif
TESTS = $(test_binaries)
check_PROGRAMS = $(test_binaries)
@@ -99,7 +97,6 @@ networking_testdriver_SOURCES = test_networking.c mock_msg.c \
$(openvpn_srcdir)/platform.c
endif
-if HAVE_XKEY_PROVIDER
provider_testdriver_CFLAGS = @TEST_CFLAGS@ \
-I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) \
$(OPTIONAL_CRYPTO_CFLAGS)
@@ -113,7 +110,6 @@ provider_testdriver_SOURCES = test_provider.c mock_msg.c \
$(openvpn_srcdir)/base64.c \
mock_get_random.c \
$(openvpn_srcdir)/platform.c
-endif
auth_token_testdriver_CFLAGS = @TEST_CFLAGS@ \
-I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) \
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
index dcf39019..0182b3b4 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -29,6 +29,10 @@
#endif
#include "syshead.h"
+#include "manage.h"
+#include "xkey_common.h"
+
+#ifdef HAVE_XKEY_PROVIDER
#include <setjmp.h>
#include <cmocka.h>
@@ -37,9 +41,6 @@
#include <openssl/core_names.h>
#include <openssl/evp.h>
-#include "manage.h"
-#include "xkey_common.h"
-
struct management *management; /* global */
static int mgmt_callback_called;
@@ -91,11 +92,11 @@ static const char *test_digest_b64 = "dzhlAB6WSMZXC67At5b5Zk1f0Lfb8zq/Asx4YYMgIO
* --- the smallest size of the actual signature with the above
* keys.
*/
-const uint8_t good_sig[] =
+static const uint8_t good_sig[] =
{0xd8, 0xa7, 0xd9, 0x81, 0xd8, 0xaa, 0xd8, 0xad, 0x20, 0xd9, 0x8a, 0xd8,
0xa7, 0x20, 0xd8, 0xb3, 0xd9, 0x85, 0xd8, 0xb3, 0xd9, 0x85, 0x0};
-const char *good_sig_b64 = "2KfZgdiq2K0g2YrYpyDYs9mF2LPZhQA=";
+static const char *good_sig_b64 = "2KfZgdiq2K0g2YrYpyDYs9mF2LPZhQA=";
static EVP_PKEY *
load_pubkey(const char *pem)
@@ -155,10 +156,16 @@ management_query_pk_sig(struct management *man, const char *b64_data,
if (strstr(algorithm, "data=message"))
{
expected_tbs = test_msg_b64;
+ assert_non_null(strstr(algorithm, "hashalg=SHA256"));
}
-
assert_string_equal(b64_data, expected_tbs);
+ /* We test using ECDSA or PSS with saltlen = digest */
+ if (!strstr(algorithm, "ECDSA"))
+ {
+ assert_non_null(strstr(algorithm, "RSA_PKCS1_PSS_PADDING,hashalg=SHA256,saltlen=digest"));
+ }
+
/* Return a predefined string as sig so that the caller
* can confirm that this callback was exercised.
*/
@@ -230,7 +237,6 @@ digest_sign(EVP_PKEY *pkey)
goto done;
}
-
/* sign with sig = NULL to get required siglen */
assert_int_equal(EVP_DigestSign(mctx, sig, &siglen, (uint8_t*)test_msg, strlen(test_msg)), 1);
assert_true(siglen > 0);
@@ -288,6 +294,90 @@ again:
}
}
+/* helpers for testing generic key load and sign */
+static int xkey_free_called;
+static int xkey_sign_called;
+static void
+xkey_free(void *handle)
+{
+ xkey_free_called = 1;
+ /* We use a dummy string as handle -- check its value */
+ assert_string_equal(handle, "xkey_handle");
+}
+
+static int
+xkey_sign(void *handle, unsigned char *sig, size_t *siglen,
+ const unsigned char *tbs, size_t tbslen, XKEY_SIGALG s)
+{
+ if (!sig)
+ {
+ *siglen = 256; /* some arbitrary size */
+ return 1;
+ }
+
+ xkey_sign_called = 1; /* called with non-null sig */
+
+ if (!strcmp(s.op, "DigestSign"))
+ {
+ assert_memory_equal(tbs, test_msg, strlen(test_msg));
+ }
+ else
+ {
+ assert_memory_equal(tbs, test_digest, sizeof(test_digest));
+ }
+
+ /* For the test use sha256 and PSS padding for RSA */
+ assert_int_equal(OBJ_sn2nid(s.mdname), NID_sha256);
+ if (!strcmp(s.keytype, "RSA"))
+ {
+ assert_string_equal(s.padmode, "pss"); /* we use PSS for the test */
+ }
+ else if (strcmp(s.keytype, "EC"))
+ {
+ fail_msg("Unknown keytype: %s", s.keytype);
+ }
+
+ /* return a predefined string as sig */
+ memcpy(sig, good_sig, min_int(sizeof(good_sig), *siglen));
+
+ return 1;
+}
+
+/* Load a key as a generic key and check its sign op gets
+ * called for signature.
+ */
+static void
+xkey_provider_test_generic_sign_cb(void **state)
+{
+ EVP_PKEY *pubkey;
+ const char *dummy = "xkey_handle"; /* a dummy handle for the external key */
+
+ for (size_t i = 0; i < _countof(pubkeys); i++)
+ {
+ pubkey = load_pubkey(pubkeys[i]);
+ assert_true(pubkey != NULL);
+
+ EVP_PKEY *privkey = xkey_load_generic_key(NULL, (void*)dummy, pubkey, xkey_sign, xkey_free);
+ assert_true(privkey != NULL);
+
+ xkey_sign_called = 0;
+ xkey_free_called = 0;
+ uint8_t *sig = digest_sign(privkey);
+ assert_non_null(sig);
+
+ /* check callback for signature got exercised */
+ assert_int_equal(xkey_sign_called, 1);
+ assert_memory_equal(sig, good_sig, sizeof(good_sig));
+ test_free(sig);
+
+ EVP_PKEY_free(pubkey);
+ EVP_PKEY_free(privkey);
+
+ /* check key's free-op got called */
+ assert_int_equal(xkey_free_called, 1);
+ }
+}
+
int
main(void)
{
@@ -296,6 +386,7 @@ main(void)
const struct CMUnitTest tests[] = {
cmocka_unit_test(xkey_provider_test_fetch),
cmocka_unit_test(xkey_provider_test_mgmt_sign_cb),
+ cmocka_unit_test(xkey_provider_test_generic_sign_cb),
};
int ret = cmocka_run_group_tests_name("xkey provider tests", tests, NULL, NULL);
@@ -303,3 +394,10 @@ main(void)
uninit_test();
return ret;
}
+#else
+int
+main(void)
+{
+ return 0;
+}
+#endif /* HAVE_XKEY_PROVIDER */
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature selva.nair
@ 2022-01-20 11:22 ` Arne Schwabe
2022-01-20 14:51 ` Gert Doering
2022-01-20 16:16 ` [Openvpn-devel] [PATCH v4 16+17/18] Add a unit test for external key provider selva.nair
2 siblings, 0 replies; 60+ messages in thread
From: Arne Schwabe @ 2022-01-20 11:22 UTC (permalink / raw)
To: selva.nair@
Am 14.12.21 um 17:59 schrieb selva.nair@...277...:
> From: Selva Nair <selva.nair@...277...>
>
> Signed-off-by: Selva Nair <selva.nair@...277...>
> ---
> configure.ac | 2 -
> tests/unit_tests/openvpn/Makefile.am | 4 -
> tests/unit_tests/openvpn/test_provider.c | 112 +++++++++++++++++++++--
> 3 files changed, 105 insertions(+), 13 deletions(-)
Acked-By: Arne Schwabe <arne@...1227...>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature selva.nair
2022-01-20 11:22 ` Arne Schwabe
@ 2022-01-20 14:51 ` Gert Doering
2022-01-20 15:21 ` Selva Nair
2022-01-20 16:16 ` [Openvpn-devel] [PATCH v4 16+17/18] Add a unit test for external key provider selva.nair
2 siblings, 1 reply; 60+ messages in thread
From: Gert Doering @ 2022-01-20 14:51 UTC (permalink / raw)
To: selva.nair@; +Cc: openvpn-devel
[-- Attachment #1: Type: text/plain, Size: 736 bytes --]
Hi,
On Tue, Dec 14, 2021 at 11:59:27AM -0500, selva.nair@...277... wrote:
> From: Selva Nair <selva.nair@...277...>
>
> Signed-off-by: Selva Nair <selva.nair@...277...>
Is it OK if I squash 16+17 together? I dislike the "history churn"
of modifying configure.ac and Makefile.am in 16 just to remove
the AM_CONDITIONAL bits again in 17...
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany gert@...1296...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 630 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature
2022-01-20 14:51 ` Gert Doering
@ 2022-01-20 15:21 ` Selva Nair
2022-01-20 15:44 ` Gert Doering
0 siblings, 1 reply; 60+ messages in thread
From: Selva Nair @ 2022-01-20 15:21 UTC (permalink / raw)
To: Gert Doering <gert@; +Cc: openvpn-devel
[-- Attachment #1: Type: text/plain, Size: 636 bytes --]
Hi
On Thu, Jan 20, 2022 at 9:51 AM Gert Doering <gert@...1296...> wrote:
> Hi,
>
> On Tue, Dec 14, 2021 at 11:59:27AM -0500, selva.nair@...277... wrote:
> > From: Selva Nair <selva.nair@...277...>
> >
> > Signed-off-by: Selva Nair <selva.nair@...277...>
>
> Is it OK if I squash 16+17 together? I dislike the "history churn"
> of modifying configure.ac and Makefile.am in 16 just to remove
> the AM_CONDITIONAL bits again in 17...
>
Yeah, a previous version had checking for OpenSSL version in configure.ac
and the AM_CONDITIONAL made sense only in that case. I can send a
new 16/18 or please do squash 16 with 17.
Thanks,
Selva
[-- Attachment #2: Type: text/html, Size: 1358 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature
2022-01-20 15:21 ` Selva Nair
@ 2022-01-20 15:44 ` Gert Doering
0 siblings, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 15:44 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: Gert Doering <gert@
[-- Attachment #1: Type: text/plain, Size: 779 bytes --]
Hi,
On Thu, Jan 20, 2022 at 10:21:19AM -0500, Selva Nair wrote:
> Yeah, a previous version had checking for OpenSSL version in configure.ac
> and the AM_CONDITIONAL made sense only in that case. I can send a
> new 16/18 or please do squash 16 with 17.
If you could send a new "16+17 v4" that would make my life easier
in not having to think about joint commit messages :-)
thanks,
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany gert@...1296...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 630 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v4 16+17/18] Add a unit test for external key provider
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature selva.nair
2022-01-20 11:22 ` Arne Schwabe
2022-01-20 14:51 ` Gert Doering
@ 2022-01-20 16:16 ` selva.nair
2022-01-20 19:01 ` [Openvpn-devel] [PATCH applied] " Gert Doering
2 siblings, 1 reply; 60+ messages in thread
From: selva.nair @ 2022-01-20 16:16 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
Tests:
- Check SIGNATURE and KEYMGMT methods can be fetched
from the provider
- Load sample RSA and EC keys as management-external-key
and check that their sign callbacks are correctly exercised:
with and without digest support mocked in the client
capability flag.
-Test generic key load and signature
v4: 16/18 and 17/18 of v3 squashed into one patch
Signed-off-by: Selva Nair <selva.nair@...277...>
---
tests/unit_tests/openvpn/Makefile.am | 16 +
tests/unit_tests/openvpn/test_provider.c | 403 +++++++++++++++++++++++
2 files changed, 419 insertions(+)
create mode 100644 tests/unit_tests/openvpn/test_provider.c
diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index 44b77cc5..6b5c94ab 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -11,6 +11,8 @@ if HAVE_LD_WRAP_SUPPORT
test_binaries += tls_crypt_testdriver
endif
+test_binaries += provider_testdriver
+
TESTS = $(test_binaries)
check_PROGRAMS = $(test_binaries)
@@ -95,6 +97,20 @@ networking_testdriver_SOURCES = test_networking.c mock_msg.c \
$(openvpn_srcdir)/platform.c
endif
+provider_testdriver_CFLAGS = @TEST_CFLAGS@ \
+ -I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) \
+ $(OPTIONAL_CRYPTO_CFLAGS)
+provider_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
+ $(OPTIONAL_CRYPTO_LIBS)
+
+provider_testdriver_SOURCES = test_provider.c mock_msg.c \
+ $(openvpn_srcdir)/xkey_helper.c \
+ $(openvpn_srcdir)/xkey_provider.c \
+ $(openvpn_srcdir)/buffer.c \
+ $(openvpn_srcdir)/base64.c \
+ mock_get_random.c \
+ $(openvpn_srcdir)/platform.c
+
auth_token_testdriver_CFLAGS = @TEST_CFLAGS@ \
-I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) \
$(OPTIONAL_CRYPTO_CFLAGS)
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
new file mode 100644
index 00000000..0182b3b4
--- /dev/null
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -0,0 +1,403 @@
+/*
+ * OpenVPN -- An application to securely tunnel IP networks
+ * over a single 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@...277...>
+ *
+ * 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 "manage.h"
+#include "xkey_common.h"
+
+#ifdef HAVE_XKEY_PROVIDER
+
+#include <setjmp.h>
+#include <cmocka.h>
+#include <openssl/bio.h>
+#include <openssl/pem.h>
+#include <openssl/core_names.h>
+#include <openssl/evp.h>
+
+struct management *management; /* global */
+static int mgmt_callback_called;
+
+#ifndef _countof
+#define _countof(x) sizeof((x))/sizeof(*(x))
+#endif
+
+static OSSL_PROVIDER *prov[2];
+
+/* public keys for testing -- RSA and EC */
+static const char * const pubkey1 = "-----BEGIN PUBLIC KEY-----\n"
+ "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA7GWP6RLCGlvmVioIqYI6\n"
+ "LUR4owA7sJ/nJxBAk+/xzD6gqgSigBsTqeb+gdZwkKjY1N4w2DUA0r5i8Eja/BWN\n"
+ "xMZtC5nxK4MACtMqIwvlzfk130NhFXKtlZj2cyFBXqDdRyeg1ZrUQagcHVcgcReP\n"
+ "9yiePgfO7NUOQk8edEeOR53SFCgnLBQQ9dGWtZN0hO/5BN6NSm/fd6vq0VjTRP5a\n"
+ "BAH/BnqX9/3jV0jh8N9AE59mI1rjVVQ9VDnuAPkS8dLfdC661/CNxt0YWByTIgt1\n"
+ "+qjW4LUvLbnU/rlPhuJ1SBZg+z/JtDBCKfs7syu5WYFqRvNFg7/91Rr/NwxvW/1h\n"
+ "8QIDAQAB\n"
+ "-----END PUBLIC KEY-----\n";
+
+static const char * const pubkey2 = "-----BEGIN PUBLIC KEY-----\n"
+ "MFYwEAYHKoZIzj0CAQYFK4EEAAoDQgAEO85iXW+HgnUkwlj1DohNVw0GsnGIh1gZ\n"
+ "u95ff1JiUaJIkYNIkZA+hwIPFVH5aJcSCv3SPIeDS2VUAESNKHZJBQ==\n"
+ "-----END PUBLIC KEY-----\n";
+
+static const char *pubkeys[] = {pubkey1, pubkey2};
+
+static const char *prov_name = "ovpn.xkey";
+
+static const char* test_msg = "Lorem ipsum dolor sit amet, consectetur "
+ "adipisici elit, sed eiusmod tempor incidunt "
+ "ut labore et dolore magna aliqua.";
+
+static const char* test_msg_b64 =
+ "TG9yZW0gaXBzdW0gZG9sb3Igc2l0IGFtZXQsIGNvbnNlY3RldHVyIGFkaXBpc2ljaS"
+ "BlbGl0LCBzZWQgZWl1c21vZCB0ZW1wb3IgaW5jaWR1bnQgdXQgbGFib3JlIGV0IGRv"
+ "bG9yZSBtYWduYSBhbGlxdWEu";
+
+/* Sha256 digest of test_msg excluding NUL terminator */
+static const uint8_t test_digest[] =
+ {0x77, 0x38, 0x65, 0x00, 0x1e, 0x96, 0x48, 0xc6, 0x57, 0x0b, 0xae,
+ 0xc0, 0xb7, 0x96, 0xf9, 0x66, 0x4d, 0x5f, 0xd0, 0xb7, 0xdb, 0xf3,
+ 0x3a, 0xbf, 0x02, 0xcc, 0x78, 0x61, 0x83, 0x20, 0x20, 0xee};
+
+static const char *test_digest_b64 = "dzhlAB6WSMZXC67At5b5Zk1f0Lfb8zq/Asx4YYMgIO4=";
+
+/* Dummy signature used only to check that the expected callback
+ * was successfully exercised. Keep this shorter than 64 bytes
+ * --- the smallest size of the actual signature with the above
+ * keys.
+ */
+static const uint8_t good_sig[] =
+ {0xd8, 0xa7, 0xd9, 0x81, 0xd8, 0xaa, 0xd8, 0xad, 0x20, 0xd9, 0x8a, 0xd8,
+ 0xa7, 0x20, 0xd8, 0xb3, 0xd9, 0x85, 0xd8, 0xb3, 0xd9, 0x85, 0x0};
+
+static const char *good_sig_b64 = "2KfZgdiq2K0g2YrYpyDYs9mF2LPZhQA=";
+
+static EVP_PKEY *
+load_pubkey(const char *pem)
+{
+ BIO *in = BIO_new_mem_buf(pem, -1);
+ assert_non_null(in);
+
+ EVP_PKEY *pkey = PEM_read_bio_PUBKEY(in, NULL, NULL, NULL);
+ assert_non_null(pkey);
+
+ BIO_free(in);
+ return pkey;
+}
+
+static void
+init_test()
+{
+ prov[0] = OSSL_PROVIDER_load(NULL,"default");
+ OSSL_PROVIDER_add_builtin(NULL, prov_name, xkey_provider_init);
+ prov[1] = OSSL_PROVIDER_load(NULL, prov_name);
+
+ /* set default propq matching what we use in ssl_openssl.c */
+ EVP_set_default_properties(NULL, "?provider!=ovpn.xkey");
+
+ management = test_calloc(sizeof(*management), 1);
+}
+
+static void
+uninit_test()
+{
+ for (size_t i = 0; i < _countof(prov); i++)
+ {
+ if (prov[i])
+ {
+ OSSL_PROVIDER_unload(prov[i]);
+ }
+ }
+ test_free(management);
+}
+
+/* Mock management callback for signature.
+ * We check that the received data to sign matches test_msg or
+ * test_digest and return a predefined string as signature so that
+ * the caller can validate all steps up to sending the data to
+ * the management client.
+ */
+char *
+management_query_pk_sig(struct management *man, const char *b64_data,
+ const char *algorithm)
+{
+ char *out = NULL;
+
+ /* indicate entry to the callback */
+ mgmt_callback_called = 1;
+
+ const char *expected_tbs = test_digest_b64;
+ if (strstr(algorithm, "data=message"))
+ {
+ expected_tbs = test_msg_b64;
+ assert_non_null(strstr(algorithm, "hashalg=SHA256"));
+ }
+ assert_string_equal(b64_data, expected_tbs);
+
+ /* We test using ECDSA or PSS with saltlen = digest */
+ if (!strstr(algorithm, "ECDSA"))
+ {
+ assert_non_null(strstr(algorithm, "RSA_PKCS1_PSS_PADDING,hashalg=SHA256,saltlen=digest"));
+ }
+
+ /* Return a predefined string as sig so that the caller
+ * can confirm that this callback was exercised.
+ */
+ out = strdup(good_sig_b64);
+ assert_non_null(out);
+
+ return out;
+}
+
+/* Check signature and keymgmt methods can be fetched from the provider */
+static void
+xkey_provider_test_fetch(void **state)
+{
+ assert_true(OSSL_PROVIDER_available(NULL, prov_name));
+
+ const char *algs[] = {"RSA", "ECDSA"};
+
+ for (size_t i = 0; i < _countof(algs); i++)
+ {
+ EVP_SIGNATURE *sig = EVP_SIGNATURE_fetch(NULL, algs[i], "provider=ovpn.xkey");
+ assert_non_null(sig);
+ assert_string_equal(OSSL_PROVIDER_get0_name(EVP_SIGNATURE_get0_provider(sig)), prov_name);
+
+ EVP_SIGNATURE_free(sig);
+ }
+
+ const char *names[] = {"RSA", "EC"};
+
+ for (size_t i = 0; i < _countof(names); i++)
+ {
+ EVP_KEYMGMT *km = EVP_KEYMGMT_fetch(NULL, names[i], "provider=ovpn.xkey");
+ assert_non_null(km);
+ assert_string_equal(OSSL_PROVIDER_get0_name(EVP_KEYMGMT_get0_provider(km)), prov_name);
+
+ EVP_KEYMGMT_free(km);
+ }
+}
+
+/* sign a test message using pkey -- caller must free the returned sig */
+static uint8_t *
+digest_sign(EVP_PKEY *pkey)
+{
+ uint8_t *sig = NULL;
+ size_t siglen = 0;
+
+ OSSL_PARAM params[6] = {OSSL_PARAM_END};
+
+ const char *mdname = "SHA256";
+ const char *padmode = "pss";
+ const char *saltlen = "digest";
+
+ if (EVP_PKEY_get_id(pkey) == EVP_PKEY_RSA)
+ {
+ params[0] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_DIGEST, (char *)mdname, 0);
+ params[1] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_PAD_MODE, (char *)padmode, 0);
+ params[2] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_PSS_SALTLEN, (char *)saltlen, 0);
+ /* same digest for mgf1 */
+ params[3] = OSSL_PARAM_construct_utf8_string(OSSL_SIGNATURE_PARAM_MGF1_DIGEST, (char *)saltlen, 0);
+ params[4] = OSSL_PARAM_construct_end();
+ }
+
+ EVP_PKEY_CTX *pctx = NULL;
+ EVP_MD_CTX *mctx = EVP_MD_CTX_new();
+
+ if (!mctx
+ || EVP_DigestSignInit_ex(mctx, &pctx, mdname, NULL, NULL, pkey, params) <= 0)
+ {
+ fail_msg("Failed to initialize EVP_DigestSignInit_ex()");
+ goto done;
+ }
+
+ /* sign with sig = NULL to get required siglen */
+ assert_int_equal(EVP_DigestSign(mctx, sig, &siglen, (uint8_t*)test_msg, strlen(test_msg)), 1);
+ assert_true(siglen > 0);
+
+ if ((sig = test_calloc(1, siglen)) == NULL)
+ {
+ fail_msg("Out of memory");
+ }
+ assert_int_equal(EVP_DigestSign(mctx, sig, &siglen, (uint8_t*)test_msg, strlen(test_msg)), 1);
+
+done:
+ if (mctx)
+ {
+ EVP_MD_CTX_free(mctx); /* pctx is internally allocated and freed by mctx */
+ }
+ return sig;
+}
+
+/* Check loading of management external key and have sign callback exercised
+ * for RSA and EC keys with and without digest support in management client.
+ * Sha256 digest used for both cases with pss padding for RSA.
+ */
+static void
+xkey_provider_test_mgmt_sign_cb(void **state)
+{
+ EVP_PKEY *pubkey;
+ for (size_t i = 0; i < _countof(pubkeys); i++)
+ {
+ pubkey = load_pubkey(pubkeys[i]);
+ assert_true(pubkey != NULL);
+ EVP_PKEY *privkey = xkey_load_management_key(NULL, pubkey);
+ assert_true(privkey != NULL);
+
+ management->settings.flags = MF_EXTERNAL_KEY|MF_EXTERNAL_KEY_PSSPAD;
+
+ /* first without digest support in management client */
+again:
+ mgmt_callback_called = 0;
+ uint8_t *sig = digest_sign(privkey);
+ assert_non_null(sig);
+
+ /* check callback for signature got exercised */
+ assert_int_equal(mgmt_callback_called, 1);
+ assert_memory_equal(sig, good_sig, sizeof(good_sig));
+ test_free(sig);
+
+ if (!(management->settings.flags & MF_EXTERNAL_KEY_DIGEST))
+ {
+ management->settings.flags |= MF_EXTERNAL_KEY_DIGEST;
+ goto again; /* this time with digest support announced */
+ }
+
+ EVP_PKEY_free(pubkey);
+ EVP_PKEY_free(privkey);
+ }
+}
+
+/* helpers for testing generic key load and sign */
+static int xkey_free_called;
+static int xkey_sign_called;
+static void
+xkey_free(void *handle)
+{
+ xkey_free_called = 1;
+ /* We use a dummy string as handle -- check its value */
+ assert_string_equal(handle, "xkey_handle");
+}
+
+static int
+xkey_sign(void *handle, unsigned char *sig, size_t *siglen,
+ const unsigned char *tbs, size_t tbslen, XKEY_SIGALG s)
+{
+ if (!sig)
+ {
+ *siglen = 256; /* some arbitrary size */
+ return 1;
+ }
+
+ xkey_sign_called = 1; /* called with non-null sig */
+
+ if (!strcmp(s.op, "DigestSign"))
+ {
+ assert_memory_equal(tbs, test_msg, strlen(test_msg));
+ }
+ else
+ {
+ assert_memory_equal(tbs, test_digest, sizeof(test_digest));
+ }
+
+ /* For the test use sha256 and PSS padding for RSA */
+ assert_int_equal(OBJ_sn2nid(s.mdname), NID_sha256);
+ if (!strcmp(s.keytype, "RSA"))
+ {
+ assert_string_equal(s.padmode, "pss"); /* we use PSS for the test */
+ }
+ else if (strcmp(s.keytype, "EC"))
+ {
+ fail_msg("Unknown keytype: %s", s.keytype);
+ }
+
+ /* return a predefined string as sig */
+ memcpy(sig, good_sig, min_int(sizeof(good_sig), *siglen));
+
+ return 1;
+}
+
+/* Load a key as a generic key and check its sign op gets
+ * called for signature.
+ */
+static void
+xkey_provider_test_generic_sign_cb(void **state)
+{
+ EVP_PKEY *pubkey;
+ const char *dummy = "xkey_handle"; /* a dummy handle for the external key */
+
+ for (size_t i = 0; i < _countof(pubkeys); i++)
+ {
+ pubkey = load_pubkey(pubkeys[i]);
+ assert_true(pubkey != NULL);
+
+ EVP_PKEY *privkey = xkey_load_generic_key(NULL, (void*)dummy, pubkey, xkey_sign, xkey_free);
+ assert_true(privkey != NULL);
+
+ xkey_sign_called = 0;
+ xkey_free_called = 0;
+ uint8_t *sig = digest_sign(privkey);
+ assert_non_null(sig);
+
+ /* check callback for signature got exercised */
+ assert_int_equal(xkey_sign_called, 1);
+ assert_memory_equal(sig, good_sig, sizeof(good_sig));
+ test_free(sig);
+
+ EVP_PKEY_free(pubkey);
+ EVP_PKEY_free(privkey);
+
+ /* check key's free-op got called */
+ assert_int_equal(xkey_free_called, 1);
+ }
+}
+
+int
+main(void)
+{
+ init_test();
+
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(xkey_provider_test_fetch),
+ cmocka_unit_test(xkey_provider_test_mgmt_sign_cb),
+ cmocka_unit_test(xkey_provider_test_generic_sign_cb),
+ };
+
+ int ret = cmocka_run_group_tests_name("xkey provider tests", tests, NULL, NULL);
+
+ uninit_test();
+ return ret;
+}
+#else
+int
+main(void)
+{
+ return 0;
+}
+#endif /* HAVE_XKEY_PROVIDER */
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread* [Openvpn-devel] [PATCH applied] Re: Add a unit test for external key provider
2022-01-20 16:16 ` [Openvpn-devel] [PATCH v4 16+17/18] Add a unit test for external key provider selva.nair
@ 2022-01-20 19:01 ` Gert Doering
0 siblings, 0 replies; 60+ messages in thread
From: Gert Doering @ 2022-01-20 19:01 UTC (permalink / raw)
To: Selva Nair <selva.nair@; +Cc: openvpn-devel
Combining Arne's ACK for 16+17 into this one. As far as I can see
(not checked line-by-line) this is indeed the same code, just squashed
into one commit, not touching configure.ac (thanks).
And indeed, it now tests something :-)
[==========] Running 3 test(s).
[ RUN ] xkey_provider_test_fetch
[ OK ] xkey_provider_test_fetch
[ RUN ] xkey_provider_test_mgmt_sign_cb
[ OK ] xkey_provider_test_mgmt_sign_cb
[ RUN ] xkey_provider_test_generic_sign_cb
[ OK ] xkey_provider_test_generic_sign_cb
[==========] 3 test(s) run.
[ PASSED ] 3 test(s).
PASS: provider_testdriver
and just an empty "PASS: provider_testdriver" when compiled without
XKEY.
Your patch has been applied to the master branch.
commit 4fe50675946f4533a9a373f4332e417f1bbfeabe
Author: Selva Nair
Date: Thu Jan 20 11:16:16 2022 -0500
Add a unit test for external key provider
Signed-off-by: Selva Nair <selva.nair@...277...>
Acked-by: Arne Schwabe <arne@...1227...>
Message-Id: <20220120161616.13447-1-selva.nair@...277...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23608.html
Signed-off-by: Gert Doering <gert@...1296...>
--
kind regards,
Gert Doering
^ permalink raw reply [flat|nested] 60+ messages in thread
* [Openvpn-devel] [PATCH v3 18/18] Add xkey_provider sources and includes to MSVC project
2021-12-14 16:59 [Openvpn-devel] [PATCH v3 00/18] External key provider for use with OpenSSL 3 selva.nair
` (16 preceding siblings ...)
2021-12-14 16:59 ` [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature selva.nair
@ 2021-12-14 16:59 ` selva.nair
2022-01-20 11:23 ` Arne Schwabe
2022-01-20 14:48 ` [Openvpn-devel] [PATCH applied] " Gert Doering
17 siblings, 2 replies; 60+ messages in thread
From: selva.nair @ 2021-12-14 16:59 UTC (permalink / raw)
To: openvpn-devel
From: Selva Nair <selva.nair@...277...>
Signed-off-by: Selva Nair <selva.nair@...277...>
---
src/openvpn/openvpn.vcxproj | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index 65ee6839..2f0cee60 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -316,6 +316,8 @@
<ClCompile Include="vlan.c" />
<ClCompile Include="win32.c" />
<ClCompile Include="win32-util.c" />
+ <ClCompile Include="xkey_helper.c"/>
+ <ClCompile Include="xkey_provider.c"/>
</ItemGroup>
<ItemGroup>
<ClInclude Include="argv.h" />
@@ -407,6 +409,7 @@
<ClInclude Include="vlan.h" />
<ClInclude Include="win32.h" />
<ClInclude Include="win32-util.h" />
+ <ClInclude Include="xkey_common.h"/>
</ItemGroup>
<ItemGroup>
<ResourceCompile Include="openvpn_win32_resources.rc" />
--
2.30.2
^ permalink raw reply related [flat|nested] 60+ messages in thread