diff mbox series

[Openvpn-devel,v3,10/18] Respect algorithm support announced by management client

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

Commit Message

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

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@gmail.com>
---
 src/openvpn/xkey_helper.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Arne Schwabe Jan. 19, 2022, 11:55 p.m. UTC | #1 
Am 14.12.21 um 17:59 schrieb selva.nair@gmail.com:
> From: Selva Nair <selva.nair@gmail.com>
> 
> 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@rfc2549.org>
Gert Doering Jan. 20, 2022, 4:42 a.m. UTC | #2 
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@gmail.com>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20211214165928.30676-11-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23441.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

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;
     }