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 <arne@rfc2549.org>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <
20200312113654.16184-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19546.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
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
}
#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");
options_postprocess_mutate_invariant(o);
+ if (o->ncp_enabled)
+ {
+ o->ncp_ciphers = mutate_ncp_cipher_list(o->ncp_ciphers, &o->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)
{
/*
}
}
-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(&new_list)> 0)
+ {
+ /* The next if condition ensure there is always space for
+ * a :
+ */
+ buf_puts(&new_list, ":");
+ }
+
+ /* Ensure buffer has capacity for cipher name + : + \0 */
+ if (!(buf_forward_capacity(&new_list) >
+ strlen(ovpn_cipher_name) + 2))
+ {
+ msg(M_WARN, "Length of --ncp-ciphers is over the "
+ "limit of 127 chars");
+ error_found = true;
+ }
+ else
+ {
+ buf_puts(&new_list, ovpn_cipher_name);
+ }
}
token = strtok(NULL, ":");
}
+
+
+
+ char *ret = NULL;
+ if (!error_found && buf_len(&new_list) > 0)
+ {
+ buf_null_terminate(&new_list);
+ ret = string_alloc(buf_str(&new_list), gc);
+ }
free(tmp_ciphers);
+ free_buf(&new_list);
- return 0 < strlen(list) && !unsupported_cipher_found;
+ return ret;
}
bool
* Check whether the ciphers in the supplied list are supported.
*
* @param list Colon-separated list of ciphers
+ * @parms gc gc_arena to allocate the returned string
*
- * @returns true iff all ciphers in list are supported.
+ * @returns colon separated string of normalised (via
+ * translate_cipher_name_from_openvpn) and
+ * zero terminated string iff all ciphers
+ * in list are supported and the total length
+ * is short than MAX_NCP_CIPHERS_LENGTH. NULL
+ * otherwise.
*/
-bool tls_check_ncp_cipher_list(const char *list);
+char *
+mutate_ncp_cipher_list(const char *list, struct gc_arena *gc);
/**
* Return true iff item is present in the colon-separated zero-terminated
*/
bool tls_item_in_cipher_list(const char *item, const char *list);
+/**
+ * The maximum length of a ncp-cipher string that is accepted.
+ *
+ * Since this list needs to be pushed as IV_CIPHERS, we are conservative
+ * about its length.
+ */
+#define MAX_NCP_CIPHERS_LENGTH 127
+
#endif /* ifndef OPENVPN_SSL_NCP_H */
static void
test_check_ncp_ciphers_list(void **state)
{
+ struct gc_arena gc = gc_new();
bool have_chacha = cipher_kt_get("CHACHA20-POLY1305");
- assert_true(tls_check_ncp_cipher_list(aes_ciphers));
- assert_true(have_chacha == tls_check_ncp_cipher_list(bf_chacha));
- assert_false(tls_check_ncp_cipher_list("vollbit"));
- assert_false(tls_check_ncp_cipher_list("AES-256-GCM:vollbit"));
+
+
+ assert_string_equal(mutate_ncp_cipher_list(aes_ciphers, &gc), aes_ciphers);
+
+ if (have_chacha)
+ {
+ assert_string_equal(mutate_ncp_cipher_list(bf_chacha, &gc), bf_chacha);
+ assert_string_equal(mutate_ncp_cipher_list("BF-CBC:CHACHA20-POLY1305", &gc),
+ bf_chacha);
+ }
+ else
+ {
+ assert_ptr_equal(mutate_ncp_cipher_list(bf_chacha, &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");
+ if (have_chacha_mixed_case)
+ {
+ assert_string_equal(mutate_ncp_cipher_list("BF-CBC:ChaCha20-Poly1305", &gc),
+ bf_chacha);
+ }
+
+ assert_ptr_equal(mutate_ncp_cipher_list("vollbit", &gc), NULL);
+ assert_ptr_equal(mutate_ncp_cipher_list("AES-256-GCM:vollbit", &gc), NULL);
+ assert_ptr_equal(mutate_ncp_cipher_list("", &gc), NULL);
+
+ assert_ptr_equal(mutate_ncp_cipher_list(
+ "ChaCha20-Poly1305:ChaCha20-Poly1305:ChaCha20-Poly1305:"
+ "ChaCha20-Poly1305:ChaCha20-Poly1305:ChaCha20-Poly1305:"
+ "ChaCha20-Poly1305", &gc), NULL);
+
+#ifdef ENABLE_CRYPTO_OPENSSL
+ assert_string_equal(mutate_ncp_cipher_list("id-aes128-GCM:id-aes256-GCM",
+ &gc), "AES-128-GCM:AES-256-GCM");
+#else
+ assert_string_equal(mutate_ncp_cipher_list("BLOWFISH-CBC",
+ &gc), "BF-CBC");
+#endif
+ gc_free(&gc);
}
static void
struct gc_arena gc = gc_new();
char *best_cipher;
- const char *serverlist="CHACHA20_POLY1305:AES-128-GCM";
+ const char *serverlist = "CHACHA20_POLY1305:AES-128-GCM";
best_cipher = ncp_get_best_cipher(serverlist, "BF-CBC",
"IV_YOLO=NO\nIV_BAR=7",
struct gc_arena gc = gc_new();
char *best_cipher;
- const char *serverlist="CHACHA20_POLY1305:AES-128-GCM:AES-256-GCM";
+ const char *serverlist = "CHACHA20_POLY1305:AES-128-GCM:AES-256-GCM";
best_cipher = ncp_get_best_cipher(serverlist, "BF-CBC",
"IV_YOLO=NO\nIV_NCP=2\nIV_BAR=7",
-
const struct CMUnitTest ncp_tests[] = {
cmocka_unit_test(test_check_ncp_ciphers_list),
cmocka_unit_test(test_extract_client_ciphers),
};
-int main(void)
+int
+main(void)
{
#if defined(ENABLE_CRYPTO_OPENSSL)
OpenSSL_add_all_algorithms();
projects / thirdparty / openvpn.git / commitdiff
