[Openvpn-devel,v2] Ensure only CBC, CFB, OFB and AEAD ciphers are considered valid data ciphers
Commit Message
Make sure cipher_valid only considered these four operations as valid.
This fixes that somjething like --data-ciphers AES-256-GCM:AES-128-CCM
will start but later fail when trying to use the CCM cipher.
We say "a supported AEAD" mode in our error since CCM is also an AEAD mode
but one we support like GCM.
Patch v2: add the indication if the cipher was optional into the message
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
src/openvpn/ssl_ncp.c | 10 +++++++++-
tests/unit_tests/openvpn/test_ncp.c | 6 ++++++
2 files changed, 15 insertions(+), 1 deletion(-)
Comments
On Mon, Oct 10, 2022 at 05:55:15PM +0200, Arne Schwabe wrote:
> Make sure cipher_valid only considered these four operations as valid.
"considers"
> This fixes that somjething like --data-ciphers AES-256-GCM:AES-128-CCM
"something"
> will start but later fail when trying to use the CCM cipher.
>
> We say "a supported AEAD" mode in our error since CCM is also an AEAD mode
> but one we support like GCM.
"but not one we support, like GCM" or
"but one we don't support, unlike GCM"
Actual code looks good to me, so
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Regards,
Just basic compile + t_client tested (which will excercise --ncp-ciphers
a bit, but not add any "unsupported optionals" today).
Your patch has been applied to the master branch.
commit 6bbd89c5c82e7a2366ecac969b35f88797a73763
Author: Arne Schwabe
Date: Mon Oct 10 17:55:15 2022 +0200
Ensure only CBC, CFB, OFB and AEAD ciphers are considered valid data ciphers
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20221010155515.1687151-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25379.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
@@ -121,6 +121,7 @@ mutate_ncp_cipher_list(const char *list, struct gc_arena *gc)
}
const bool nonecipher = (strcmp(token, "none") == 0);
+ const char *optstr = optional ? "optional " : "";
if (nonecipher)
{
@@ -132,10 +133,17 @@ mutate_ncp_cipher_list(const char *list, struct gc_arena *gc)
}
if (!nonecipher && !cipher_valid(token))
{
- const char *optstr = optional ? "optional " : "";
msg(M_WARN, "Unsupported %scipher in --data-ciphers: %s", optstr, token);
error_found = error_found || !optional;
}
+ else if (!nonecipher && !cipher_kt_mode_aead(token)
+ && !cipher_kt_mode_cbc(token)
+ && !cipher_kt_mode_ofb_cfb(token))
+ {
+ msg(M_WARN, "Unsupported %scipher algorithm '%s'. It does not use "
+ "CFB, OFB, CBC, or a supported AEAD mode", optstr, token);
+ error_found = error_found || !optional;
+ }
else
{
const char *ovpn_cipher_name = cipher_kt_name(token);
@@ -98,6 +98,12 @@ test_check_ncp_ciphers_list(void **state)
/* If the last is optional, previous invalid ciphers should be ignored */
assert_ptr_equal(mutate_ncp_cipher_list("Vollbit:Littlebit:AES-256-CBC:BF-CBC:?nixbit", &gc), NULL);
+ /* We do not support CCM ciphers */
+ assert_ptr_equal(mutate_ncp_cipher_list("AES-256-GCM:AES-128-CCM", &gc), NULL);
+
+ assert_string_equal(mutate_ncp_cipher_list("AES-256-GCM:?AES-128-CCM:AES-128-GCM", &gc),
+ aes_ciphers);
+
/* For testing that with OpenSSL 1.1.0+ that also accepts ciphers in
* a different spelling the normalised cipher output is the same */
bool have_chacha_mixed_case = cipher_valid("ChaCha20-Poly1305");