[Openvpn-devel,2/3] Fix max saltlen calculation in cryptoapi.c

Message ID 20220125025128.2117-2-selva.nair@gmail.com
State Accepted
Headers show
Series
  • [Openvpn-devel,1/3] xkey: Use a custom error level for debug messages
Related show

Commit Message

Selva Nair Jan. 25, 2022, 2:51 a.m.
From: Selva Nair <selva.nair@gmail.com>

(nbits - 1)/8 should have been rounded up. Fix and move it to
an inlined function for reuse in pkcs11_openssl.c (used in the
next commit).

Note: The error is not triggered in normal use as OpenSSL
always seems to use saltlen="digest" for signing.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpn/cryptoapi.c   |  2 +-
 src/openvpn/xkey_common.h | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

Comments

Arne Schwabe Jan. 26, 2022, 10:27 a.m. | #1
Am 25.01.22 um 03:51 schrieb selva.nair@gmail.com:
> From: Selva Nair <selva.nair@gmail.com>
> 
> (nbits - 1)/8 should have been rounded up. Fix and move it to
> an inlined function for reuse in pkcs11_openssl.c (used in the
> next commit).
> 
> Note: The error is not triggered in normal use as OpenSSL
> always seems to use saltlen="digest" for signing.

That calculation is just too familiar.

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering Jan. 26, 2022, 12:53 p.m. | #2
I won't claim to understand this, but if Arne says the math is (now)
fine, I'm happy to believe this :-) - only compile tested on 3.0.1 / Linux.

Your patch has been applied to the master branch.

commit 72daac6973c304b93e6516879948c5470d0c805a
Author: Selva Nair
Date:   Mon Jan 24 21:51:27 2022 -0500

     Fix max saltlen calculation in cryptoapi.c

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


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index 8e0ceba7..56cab962 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -843,7 +843,7 @@  xkey_cng_rsa_sign(CAPI_DATA *cd, unsigned char *sig, size_t *siglen, const unsig
         int saltlen = tbslen; /* digest size by default */
         if (!strcmp(sigalg.saltlen, "max"))
         {
-            saltlen = (EVP_PKEY_bits(cd->pubkey) - 1)/8 - tbslen - 2;
+            saltlen = xkey_max_saltlen(EVP_PKEY_bits(cd->pubkey), tbslen);
             if (saltlen < 0)
             {
                 msg(M_NONFATAL, "Error in cryptoapicert: invalid salt length (%d)", saltlen);
diff --git a/src/openvpn/xkey_common.h b/src/openvpn/xkey_common.h
index 75ca5011..1e51e672 100644
--- a/src/openvpn/xkey_common.h
+++ b/src/openvpn/xkey_common.h
@@ -153,6 +153,20 @@  xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey,
 
 extern OSSL_LIB_CTX *tls_libctx; /* Global */
 
+/**
+ * Maximum salt length for PSS signature.
+ *
+ * @param modBits    Number of bits in RSA modulus
+ * @param hLen       Length of digest to be signed
+ * @returns the maximum allowed salt length. Caller must check it's not < 0.
+ */
+static inline int
+xkey_max_saltlen(int modBits, int hLen)
+{
+    int emLen = (modBits - 1 + 7)/8; /* ceil((modBits - 1)/8) */
+
+    return emLen - hLen - 2;
+}
 #endif /* HAVE_XKEY_PROVIDER */
 
 #endif /* XKEY_COMMON_H_ */