[Openvpn-devel,v3,07/18] Enable signing via provider for management-external-key
Commit Message
From: Selva Nair <selva.nair@gmail.com>
- 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@gmail.com>
---
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(-)
Comments
Am 14.12.21 um 17:59 schrieb selva.nair@gmail.com:
> From: Selva Nair <selva.nair@gmail.com>
>
> - 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@gmail.com>
>
Acked-By: Arne Schwabe <arne@rfc2549.org>
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@gmail.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20211214165928.30676-8-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23428.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
Hi,
On Thu, Jan 20, 2022 at 10:18 AM Gert Doering <gert@greenie.muc.de> 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
<div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 20, 2022 at 10:18 AM Gert Doering <<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Compile and client tested on 1.1.1 and 3.0.1.<br>
<br>
Glancing at the code related to management_external_key() does<br>
not make me very happy... too many build time variants. </blockquote><div><br></div><div>"Happiness" is never a word that comes to mind while reading OpenVPN code :)</div><div>...<br></div><div><br></div><div>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. </div><div><br></div><div>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.</div><div><br></div><div>Selva</div></div></div>
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@greenie.muc.de> 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
@@ -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)
{
@@ -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,
@@ -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 */