Ensure only CBC, CFB, OFB and AEAD ciphers are considered valid data ciphers

Make sure cipher_valid only considers these four operations as valid.
This fixes that something 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 don't support, unlike GCM.

Patch v2: add the indication if the cipher was optional into the message

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>

diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index fe84919257228a3d9958eb73bb152f300ad17ac9..b6884af96d95db7dfe6f04e941b4dc3752569dc8 100644 (file)
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -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);

diff --git a/tests/unit_tests/openvpn/test_ncp.c b/tests/unit_tests/openvpn/test_ncp.c
index 2595d8c4e4ed7f2ed26fccf8792e958c57acdcee..bb9a28244c87bbd67006b50637ef92a72767b1f1 100644 (file)
--- a/tests/unit_tests/openvpn/test_ncp.c
+++ b/tests/unit_tests/openvpn/test_ncp.c
@@ -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");