In scenarios of mbed TLS vs OpenSSL we already normalise the ciphers
that are send via the wire protocol via OCC to not have a mismatch
warning between server and client. This is done by
translate_cipher_name_from_openvpn. The same applies also to the
ncp-ciphers list. Specifying non normalised names in ncp-ciphers will
cause negotation not to succeed if ciphers are not in the same form.
Therefore we will normalise the ciphers in options_postmutate.
The alternative and a lot less user friendly alternative would be to
bail if on of the ciphers in ncp-ciphers is not in its normalised form.
Also restrict the ncp-ciphers list to 127. This is somewhat arbitrary
but should prevent too large IV_CIPHER messages and problems sending
those. The server will accept also large IV_CIPHER values from clients.
Patch V2: Correct comment about normalising ciphers
Patch V3: Correct #ifdef statement
Patch V5: Fix tests with OpenSSL 1.0.2 and libraries missing Chacha
Patch V6: Fix unit tests for mbed tls, which recognises ChaCha20-Poly1305
only when used with all uppercase, fix missing space in message
Signed-off-by: Arne Schwabe
---
doc/openvpn.8 | 3 ++
src/openvpn/options.c | 14 ---
src/openvpn/ssl_ncp.c | 57 +
src/openvpn/ssl_ncp.h | 19 +-
tests/unit_tests/openvpn/test_ncp.c | 54 +++
5 files changed, 125 insertions(+), 22 deletions(-)
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 864f94e8..c5b16981 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -4406,6 +4406,9 @@ AES\-256\-GCM:AES\-256\-CBC" set can either specify
"\-\-cipher BF\-CBC" or
Note, for using NCP with a OpenVPN 2.4 peer this list must include
the AES\-256\-GCM and AES\-128\-GCM ciphers.
+
+This list is restricted to be 127 chars long after conversion to OpenVPN
+ciphers.
.\"*
.TP
.B \-\-ncp\-disable
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e79a1215..5127f653 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2516,11 +2516,6 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
}
#endif /* P2MP_SERVER */
-if (options->ncp_enabled &&
!tls_check_ncp_cipher_list(options->ncp_ciphers))
-{
-msg(M_USAGE, "NCP cipher list contains unsupported ciphers.");
-}
-
if (options->keysize)
{
msg(M_WARN, "WARNING: --keysize is DEPRECATED and will be removed in
OpenVPN 2.6");
@@ -3098,6 +3093,15 @@ options_postprocess_mutate(struct options *o)
options_postprocess_mutate_invariant(o);
+if (o->ncp_enabled)
+{
+o->ncp_ciphers = mutate_ncp_cipher_list(o->ncp_ciphers, >gc);
+if (o->ncp_ciphers == NULL)
+{
+msg(M_USAGE, "NCP cipher list contains unsupported ciphers or is
too long.");
+}
+}
+
if (o->remote_list && !o->connection_list)
{
/*
diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index d884e2b5..9ed6ff5f 100644
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -91,27 +91,70 @@ tls_peer_supports_ncp(const char *peer_info)
}
}
-bool
-tls_check_ncp_cipher_list(const char *list)
+char *
+mutate_ncp_cipher_list(const char *list, struct gc_arena *gc)
{
-bool unsupported_cipher_found = false;
+bool error_found = false;
-ASSERT(list);
+struct buffer new_list = alloc_buf(MAX_NCP_CIPHERS_LENGTH);
char *const tmp_ciphers = string_alloc(list, NULL);
const char *token = strtok(tmp_ciphers, ":");
while (token)
{
-if (!cipher_kt_get(translate_cipher_name_from_openvpn(token)))
+/*
+ * Going through a roundtrip by using
translate_cipher_name_from_openvpn
+ * and translate_cipher_name_to_openvpn also normalises the cipher
name,
+ * e.g. replacing AeS-128-gCm with AES-128-GCM
+ */
+const char *cipher_name = translate_cipher_name_from_openvpn(token);
+const cipher_kt_t *ktc = cipher_kt_get(cipher_name);
+if (!ktc)
{
msg(M_WARN, "Unsupported cipher in --ncp-ciphers: %s", token);
-unsupported_cipher_found = true;
+error_found = true;
+}
+else
+{
+const char *ovpn_cipher_name =
+translate_cipher_name_to_openvpn(cipher_kt_name(ktc));
+
+if (buf_len(_list)> 0)
+{
+/* The next if condition ensure there is always space for
+ * a :
+ */
+buf_puts(_list, ":");
+}
+
+/* Ensure buffer has capacity for cipher name + : + \0 */
+if (!(buf_forward_capacity(_list) >
+ strlen(ovpn_cipher_name) + 2))
+{
+msg(M_WARN, "Length of --ncp-ciphers is over the "
+"limit of 127 chars");
