From: Steffan Karger Date: Wed, 23 Nov 2016 21:21:44 +0000 (+0100) Subject: Poor man's NCP for non-NCP peers X-Git-Tag: v2.4_beta2~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6e5ad2fa0b1e7ca2e05e0a38f47ed5561cda63b0;p=thirdparty%2Fopenvpn.git Poor man's NCP for non-NCP peers Allows non-NCP peers (<= 2.3, or 2.4+ with --ncp-disable) to specify a --cipher that is different from the one in our config, as long as the new cipher value is allowed (i.e. in --ncp-ciphers at our side). This works both client-to-server and server-to-client. I.e. a 2.4 client with "cipher BF-CBC" and "ncp-ciphers AES-256-GCM:AES-256-CBC" can connect to both a 2.3 server with "cipher BF-CBC" as well as a server with "cipher AES-256-CBC" in its config. The other way around, a 2.3 client with either "cipher BF-CBC" or "cipher AES-256-CBC" can connect to a 2.4 server with e.g. "cipher BF-CBC" and "ncp-ciphers AES-256-GCM:AES-256-CBC" in its config. This patch was inspired by Gert's "Poor man's NCP for 2.3 clients" patch, but takes a different approach to avoid the need for server-side scripts or client-side 'setenv UV_*' tricks. Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <1479936104-4045-1-git-send-email-steffan@karger.me> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13218.html Signed-off-by: Gert Doering --- diff --git a/doc/openvpn.8 b/doc/openvpn.8 index ffb0f7dd8..0aa04449d 100644 --- a/doc/openvpn.8 +++ b/doc/openvpn.8 @@ -4148,9 +4148,9 @@ to disable encryption. As of OpenVPN 2.4, cipher negotiation (NCP) can override the cipher specified by .B \-\-cipher\fR. See -.B \-\-ncp-ciphers +.B \-\-ncp\-ciphers and -.B \-\-ncp-disable +.B \-\-ncp\-disable for more on NCP. .\"********************************************************* @@ -4178,6 +4178,16 @@ If both peers support and do not disable NCP, the negotiated cipher will override the cipher specified by .B \-\-cipher\fR. +Additionally, to allow for more smooth transition, if NCP is enabled, OpenVPN +will inherit the cipher of the peer if that cipher is different from the local +.B \-\-cipher +setting, but the peer cipher is one of the ciphers specified in +.B \-\-ncp\-ciphers\fR. +E.g. a non-NCP client (<=2.3, or with \-\-ncp\-disabled set) connecting to a +NCP server (2.4+) with "\-\-cipher BF-CBC" and "\-\-ncp-ciphers +AES-256-GCM:AES-256-CBC" set can either specify "\-\-cipher BF-CBC" or +"\-\-cipher AES-256-CBC" and both will work. + .\"********************************************************* .TP .B \-\-ncp\-disable diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 470dc89af..2ccbab2f7 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1932,8 +1932,14 @@ do_deferred_options (struct context *c, const unsigned int found) { struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE]; if (found & OPT_P_NCP) - msg (D_PUSH, "OPTIONS IMPORT: data channel crypto options modified"); - /* Do not regenerate keys if server sends an extra push request */ + { + msg (D_PUSH, "OPTIONS IMPORT: data channel crypto options modified"); + } + else if (c->options.ncp_enabled) + { + tls_poor_mans_ncp(&c->options, c->c2.tls_multi->remote_ciphername); + } + /* Do not regenerate keys if server sends an extra push reply */ if (!session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized && !tls_session_update_crypto_params(session, &c->options, &c->c2.frame)) { diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 8e09e4a00..7c2b989d8 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -1643,6 +1643,8 @@ show_settings (const struct options *o) SHOW_STR (shared_secret_file); SHOW_INT (key_direction); SHOW_STR (ciphername); + SHOW_BOOL (ncp_enabled); + SHOW_STR (ncp_ciphers); SHOW_STR (authname); SHOW_STR (prng_hash); SHOW_INT (prng_nonce_secret_len); @@ -3445,6 +3447,36 @@ options_string_version (const char* s, struct gc_arena *gc) #endif /* ENABLE_OCC */ +char * +options_string_extract_option (const char *options_string,const char *opt_name, + struct gc_arena *gc) +{ + char *ret = NULL; + const size_t opt_name_len = strlen(opt_name); + + const char *p = options_string; + while (p) + { + if (0 == strncmp(p, opt_name, opt_name_len) && + strlen(p) > (opt_name_len+1) && p[opt_name_len] == ' ') + { + /* option found, extract value */ + const char *start = &p[opt_name_len+1]; + const char *end = strchr (p, ','); + size_t val_len = end ? end - start : strlen (start); + ret = gc_malloc (val_len+1, true, gc); + memcpy (ret, start, val_len); + break; + } + p = strchr (p, ','); + if (p) + { + p++; /* skip delimiter */ + } + } + return ret; +} + static void foreign_option (struct options *o, char *argv[], int len, struct env_set *es) { diff --git a/src/openvpn/options.h b/src/openvpn/options.h index a02855613..067728a07 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -726,6 +726,20 @@ void options_warning (char *actual, const char *expected); #endif +/** + * Given an OpenVPN options string, extract the value of an option. + * + * @param options_string Zero-terminated, comma-separated options string + * @param opt_name The name of the option to extract + * @param gc The gc to allocate the return value + * + * @return gc-allocated value of option with name opt_name if option was found, + * or NULL otherwise. + */ +char *options_string_extract_option (const char *options_string, + const char *opt_name, struct gc_arena *gc); + + void options_postprocess (struct options *options); void pre_pull_save (struct options *o); diff --git a/src/openvpn/push.c b/src/openvpn/push.c index b7539e6de..9953079c6 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -262,7 +262,7 @@ incoming_push_message (struct context *c, const struct buffer *buffer) !tls_session_update_crypto_params (session, &c->options, &c->c2.frame)) { - msg (D_TLS_ERRORS, "TLS Error: server generate_key_expansion failed"); + msg (D_TLS_ERRORS, "TLS Error: initializing data channel failed"); goto error; } } @@ -370,6 +370,10 @@ prepare_push_reply (struct context *c, struct gc_arena *gc, push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername); } } + else if (o->ncp_enabled) + { + tls_poor_mans_ncp (o, tls_multi->remote_ciphername); + } /* If server uses --auth-gen-token and we have an auth token * to send to the client diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 16b9cb7b0..8259292e7 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1216,6 +1216,8 @@ tls_multi_free (struct tls_multi *multi, bool clear) free (multi->auth_token); } + free (multi->remote_ciphername); + for (i = 0; i < TM_SIZE; ++i) tls_session_free (&multi->session[i], false); @@ -1723,8 +1725,8 @@ key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len) { } } -static bool -item_in_list(const char *item, const char *list) +bool +tls_item_in_cipher_list(const char *item, const char *list) { char *tmp_ciphers = string_alloc (list, NULL); char *tmp_ciphers_orig = tmp_ciphers; @@ -1741,6 +1743,20 @@ item_in_list(const char *item, const char *list) return token != NULL; } +void +tls_poor_mans_ncp(struct options *o, const char *remote_ciphername) +{ + if (o->ncp_enabled && remote_ciphername && + 0 != strcmp(o->ciphername, remote_ciphername)) + { + if (tls_item_in_cipher_list(remote_ciphername, o->ncp_ciphers)) + { + o->ciphername = string_alloc(remote_ciphername, &o->gc); + msg (D_TLS_DEBUG_LOW, "Using peer cipher '%s'", o->ciphername); + } + } +} + bool tls_session_update_crypto_params(struct tls_session *session, const struct options *options, struct frame *frame) @@ -1752,7 +1768,7 @@ tls_session_update_crypto_params(struct tls_session *session, if (!session->opt->server && 0 != strcmp(options->ciphername, session->opt->config_ciphername) && - !item_in_list(options->ciphername, options->ncp_ciphers)) + !tls_item_in_cipher_list(options->ciphername, options->ncp_ciphers)) { msg (D_TLS_ERRORS, "Error: pushed cipher not allowed - %s not in %s or %s", options->ciphername, session->opt->config_ciphername, @@ -2312,10 +2328,20 @@ key_method_2_read (struct buffer *buf, struct tls_multi *multi, struct tls_sessi if ( multi->peer_info ) output_peer_info_env (session->opt->es, multi->peer_info); + free (multi->remote_ciphername); + multi->remote_ciphername = + options_string_extract_option (options, "cipher", NULL); + if (tls_peer_info_ncp_ver (multi->peer_info) < 2) { - /* Peer does not support NCP */ - session->opt->ncp_enabled = false; + /* Peer does not support NCP, but leave NCP enabled if the local and + * remote cipher do not match to attempt 'poor-man's NCP'. + */ + if (multi->remote_ciphername == NULL || + 0 == strcmp(multi->remote_ciphername, multi->opt.config_ciphername)) + { + session->opt->ncp_enabled = false; + } } #endif diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 777b62165..047eb248d 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -489,6 +489,15 @@ void tls_update_remote_addr (struct tls_multi *multi, bool tls_session_update_crypto_params(struct tls_session *session, const struct options *options, struct frame *frame); +/** + * "Poor man's NCP": Use peer cipher if it is an allowed (NCP) cipher. + * Allows non-NCP peers to upgrade their cipher individually. + * + * Make sure to call tls_session_update_crypto_params() after calling this + * function. + */ +void tls_poor_mans_ncp(struct options *o, const char *remote_ciphername); + #ifdef MANAGEMENT_DEF_AUTH static inline char * tls_get_peer_info(const struct tls_multi *multi) @@ -512,6 +521,13 @@ int tls_peer_info_ncp_ver(const char *peer_info); */ bool tls_check_ncp_cipher_list(const char *list); +/** + * Return true iff item is present in the colon-separated zero-terminated + * cipher list. + */ +bool tls_item_in_cipher_list(const char *item, const char *list); + + /* * inline functions */ diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 28702af1d..7938f41f5 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -540,6 +540,8 @@ struct tls_multi uint32_t peer_id; bool use_peer_id; + char *remote_ciphername; /**< cipher specified in peer's config file */ + char *auth_token; /**< If server sends a generated auth-token, * this is the token to use for future * user/pass authentications in this session.