diff mbox series

[Openvpn-devel] Fix handling an optional invalid cipher at the end of data-ciphers

Message ID 20211206150852.3142891-1-arne@rfc2549.org
State Accepted
Headers
Series [Openvpn-devel] Fix handling an optional invalid cipher at the end of data-ciphers |

Commit Message

Arne Schwabe Dec. 6, 2021, 4:08 a.m. UTC 
  If an optional cipher was found at the end of --data-cipher that was
not available, it would reset the error and allow non optional ciphers
to be ignored.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/ssl_ncp.c               | 4 ++--
 tests/unit_tests/openvpn/test_ncp.c | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

Gert Doering Dec. 6, 2021, 5:44 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

Sorry for the chaos.  This indeed fixes the behaviour I saw on "1/9 v1"
(and it adds a test case!).  Thanks.

2021-12-06 17:43:08 Unsupported optional cipher in --data-ciphers: Vollbit
2021-12-06 17:43:08 Unsupported optional cipher in --data-ciphers: Littlebit
2021-12-06 17:43:08 Unsupported cipher in --data-ciphers: nixbit
Options error: --data-ciphers list contains unsupported ciphers or is too long.

(and vice versa, all combinations of optional/non-optional unsupported
do what one would expect now)

Your patch has been applied to the master branch.

commit 868433857fbf8d71515ac0ffecb98eae893515dc
Author: Arne Schwabe
Date:   Mon Dec 6 16:08:52 2021 +0100

     Fix handling an optional invalid cipher at the end of data-ciphers

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20211206150852.3142891-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/search?l=mid&q=20211206150852.3142891-1-arne@rfc2549.org
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index 7ad825038..7f5e6fe8b 100644
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -134,7 +134,7 @@  mutate_ncp_cipher_list(const char *list, struct gc_arena *gc)
         {
             const char* optstr = optional ? "optional ": "";
             msg(M_WARN, "Unsupported %scipher in --data-ciphers: %s", optstr, token);
-            error_found = !optional;
+            error_found = error_found || !optional;
         }
         else
         {
@@ -489,4 +489,4 @@  p2p_mode_ncp(struct tls_multi *multi, struct tls_session *session)
         multi->use_peer_id, multi->peer_id, common_cipher);
 
     gc_free(&gc);
-}
\ No newline at end of file
+}
diff --git a/tests/unit_tests/openvpn/test_ncp.c b/tests/unit_tests/openvpn/test_ncp.c
index faf09a36c..6702133ad 100644
--- a/tests/unit_tests/openvpn/test_ncp.c
+++ b/tests/unit_tests/openvpn/test_ncp.c
@@ -95,6 +95,9 @@  test_check_ncp_ciphers_list(void **state)
     /* All unsupported should still yield an empty list */
     assert_ptr_equal(mutate_ncp_cipher_list("?kugelfisch:?grasshopper", &gc), NULL);
 
+    /* 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);
+
     /* 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_kt_get("ChaCha20-Poly1305");