[Openvpn-devel,v2] auth_token/tls_crypt: fix usage of md_valid()

Message ID 20220215123157.10615-1-a@unstable.cc
State Accepted
Headers show
Series [Openvpn-devel,v2] auth_token/tls_crypt: fix usage of md_valid() | expand

Commit Message

Antonio Quartulli Feb. 15, 2022, 1:31 a.m. UTC
With b39725cf ("Remove md_kt_t and change crypto API to use const char*")
the logic for validating ciphers and md algorithms has been changed.

We should now *always* use md_valid() when validating a digest alg.

At the same time, add '!' (negation) when validating the digest algorithm
in the tls-crypt code, in order to restore the proper logic.

Cc: Arne Schwabe <arne@rfc2549.org>
Fixes: b39725cf ("Remove md_kt_t and change crypto API to use const char*")
Reported-by: Richard T Bonhomme <tincantech@protonmail.com>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
---

Changes from v1:
* fixed doc for md_valid()


 src/openvpn/auth_token.c     | 2 +-
 src/openvpn/crypto_backend.h | 3 +--
 src/openvpn/tls_crypt.c      | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

Comments

Arne Schwabe Feb. 15, 2022, 1:39 a.m. UTC | #1
Am 15.02.22 um 13:31 schrieb Antonio Quartulli:
> With b39725cf ("Remove md_kt_t and change crypto API to use const char*")
> the logic for validating ciphers and md algorithms has been changed.
> 
> We should now *always* use md_valid() when validating a digest alg.
> 
> At the same time, add '!' (negation) when validating the digest algorithm
> in the tls-crypt code, in order to restore the proper logic.
> 
> Cc: Arne Schwabe <arne@rfc2549.org>
> Fixes: b39725cf ("Remove md_kt_t and change crypto API to use const char*")
> Reported-by: Richard T Bonhomme <tincantech@protonmail.com>
> Signed-off-by: Antonio Quartulli <a@unstable.cc>

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering Feb. 15, 2022, 9:02 p.m. UTC | #2
Stared at code (looks good).

Tested with Richard's config (no more warnings about SHA256, good), and
ran my usual set of client tests (no issues).

I did notice a long standing copy-paste thing in auth_token.c...

         msg(M_WARN, "ERROR: --tls-crypt requires HMAC-SHA-256 support.");

.. this should be "--auth-gen-token" not "--tls-crypt", I think...  for 
the next round of cleanups.

Your patch has been applied to the master branch.

commit af695b53e01035a9137bc78a868cd5410be817f4
Author: Antonio Quartulli
Date:   Tue Feb 15 13:31:57 2022 +0100

     auth_token/tls_crypt: fix usage of md_valid()

     Signed-off-by: Antonio Quartulli <a@unstable.cc>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20220215123157.10615-1-a@unstable.cc>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23793.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index ceae68f6..10c9dde6 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -38,7 +38,7 @@  auth_token_kt(void)
     kt.cipher = "none";
     kt.digest = "SHA256";
 
-    if (!kt.digest)
+    if (!md_valid(kt.digest))
     {
         msg(M_WARN, "ERROR: --tls-crypt requires HMAC-SHA-256 support.");
         return (struct key_type) { 0 };
diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index abf1b876..6d89b9e5 100644
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -520,8 +520,7 @@  static inline bool md_defined(const char* mdname)
  *
  * @param digest        Name of the digest to verify, e.g. \c MD5).
  *
- * @return              A statically allocated structure containing parameters
- *                      for the given message digest.
+ * @return              Whether a digest of the given name is available
  */
 bool md_valid(const char *digest);
 
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 610168b0..aae2a917 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -59,7 +59,7 @@  tls_crypt_kt(void)
         msg(M_WARN, "ERROR: --tls-crypt requires AES-256-CTR support.");
         return (struct key_type) { 0 };
     }
-    if (cipher_valid(kt.digest))
+    if (!md_valid(kt.digest))
     {
         msg(M_WARN, "ERROR: --tls-crypt requires HMAC-SHA-256 support.");
         return (struct key_type) { 0 };