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