From 4ee98f284a93c3b855092d35ac21371d9dcad65b Mon Sep 17 00:00:00 2001 From: Bernhard Schmidt Date: Wed, 24 Feb 2021 19:54:12 +0100 Subject: New upstream version 2.5.1 --- src/openvpn/init.c | 4 +- src/openvpn/manage.c | 5 +-- src/openvpn/misc.c | 9 +++- src/openvpn/misc.h | 12 ++++- src/openvpn/options.c | 100 +++++++++++++++++++++--------------------- src/openvpn/pf.c | 13 +++++- src/openvpn/ps.c | 34 +++++++++++--- src/openvpn/ssl.c | 10 ++--- src/openvpn/ssl.h | 5 ++- src/openvpn/tun.c | 47 ++++++++++---------- src/openvpnserv/interactive.c | 2 +- 11 files changed, 143 insertions(+), 98 deletions(-) (limited to 'src') diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 31ecadc..ed7e732 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -1606,7 +1606,7 @@ initialization_sequence_completed(struct context *c, const unsigned int flags) */ if (c->options.mode == MODE_POINT_TO_POINT) { - delayed_auth_pass_purge(); + ssl_clean_user_pass(); } /* Test if errors */ @@ -2552,7 +2552,6 @@ key_schedule_free(struct key_schedule *ks, bool free_ssl_ctx) if (tls_ctx_initialised(&ks->ssl_ctx) && free_ssl_ctx) { tls_ctx_free(&ks->ssl_ctx); - free_key_ctx(&ks->tls_crypt_v2_server_key); } CLEAR(*ks); } @@ -3615,6 +3614,7 @@ do_close_free_key_schedule(struct context *c, bool free_ssl_ctx) * always free the tls_auth/crypt key. If persist_key is true, the key will * be reloaded from memory (pre-cached) */ + free_key_ctx(&c->c1.ks.tls_crypt_v2_server_key); free_key_ctx_bi(&c->c1.ks.tls_wrap_key); CLEAR(c->c1.ks.tls_wrap_key); buf_clear(&c->c1.ks.tls_crypt_v2_wkc); diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index ac14217..d86b6a7 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -2102,7 +2102,7 @@ man_io_error(struct management *man, const char *prefix) static ssize_t man_send_with_fd(int fd, void *ptr, size_t nbytes, int flags, int sendfd) { - struct msghdr msg; + struct msghdr msg = { 0 }; struct iovec iov[1]; union { @@ -2134,7 +2134,7 @@ man_send_with_fd(int fd, void *ptr, size_t nbytes, int flags, int sendfd) static ssize_t man_recv_with_fd(int fd, void *ptr, size_t nbytes, int flags, int *recvfd) { - struct msghdr msghdr; + struct msghdr msghdr = { 0 }; struct iovec iov[1]; ssize_t n; @@ -3613,7 +3613,6 @@ management_query_user_pass(struct management *man, { /* preserve caller's settings */ man->connection.up_query.nocache = up->nocache; - man->connection.up_query.wait_for_push = up->wait_for_push; *up = man->connection.up_query; } secure_memzero(&man->connection.up_query, sizeof(man->connection.up_query)); diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 1038b38..c0c72dd 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -510,10 +510,15 @@ void set_auth_token(struct user_pass *up, struct user_pass *tk, const char *token) { - if (token && strlen(token) && up && up->defined) + if (strlen(token) && (up->defined || tk->defined)) { + /* auth-token has no password, so it needs the username + * either already set or copied from up */ strncpynt(tk->password, token, USER_PASS_LEN); - strncpynt(tk->username, up->username, USER_PASS_LEN); + if (up->defined) + { + strncpynt(tk->username, up->username, USER_PASS_LEN); + } tk->defined = true; } diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index a03d94e..e4342b0 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -64,7 +64,6 @@ struct user_pass { bool defined; bool nocache; - bool wait_for_push; /* true if this object is waiting for a push-reply */ /* max length of username/password */ #ifdef ENABLE_PKCS11 @@ -145,6 +144,17 @@ void fail_user_pass(const char *prefix, void purge_user_pass(struct user_pass *up, const bool force); +/** + * Sets the auth-token to token if a username is available from either + * up or already present in tk. The method will also purge up if + * the auth-nocache option is active. + * + * @param up (non Auth-token) Username/password + * @param tk auth-token userpass to set + * @param token token to use as password for the + * + * @note all parameters to this function must not be null. + */ void set_auth_token(struct user_pass *up, struct user_pass *tk, const char *token); diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 658ca53..0d99e99 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -1982,6 +1982,23 @@ connection_entry_load_re(struct connection_entry *ce, const struct remote_entry } } +static void +connection_entry_preload_key(const char **key_file, bool *key_inline, + struct gc_arena *gc) +{ + if (key_file && *key_file && !(*key_inline)) + { + struct buffer in = buffer_read_from_file(*key_file, gc); + if (!buf_valid(&in)) + { + msg(M_FATAL, "Cannot pre-load keyfile (%s)", *key_file); + } + + *key_file = (const char *) in.data; + *key_inline = true; + } +} + static void options_postprocess_verify_ce(const struct options *options, const struct connection_entry *ce) @@ -2933,36 +2950,17 @@ options_postprocess_mutate_ce(struct options *o, struct connection_entry *ce) ce->tls_crypt_v2_file_inline = o->tls_crypt_v2_file_inline; } - /* pre-cache tls-auth/crypt key file if persist-key was specified and keys - * were not already embedded in the config file + /* Pre-cache tls-auth/crypt(-v2) key file if persist-key was specified and + * keys were not already embedded in the config file. */ if (o->persist_key) { - if (ce->tls_auth_file && !ce->tls_auth_file_inline) - { - struct buffer in = buffer_read_from_file(ce->tls_auth_file, &o->gc); - if (!buf_valid(&in)) - { - msg(M_FATAL, "Cannot pre-load tls-auth keyfile (%s)", - ce->tls_auth_file); - } - - ce->tls_auth_file = (char *)in.data; - ce->tls_auth_file_inline = true; - } - - if (ce->tls_crypt_file && !ce->tls_crypt_file_inline) - { - struct buffer in = buffer_read_from_file(ce->tls_crypt_file, &o->gc); - if (!buf_valid(&in)) - { - msg(M_FATAL, "Cannot pre-load tls-crypt keyfile (%s)", - ce->tls_crypt_file); - } - - ce->tls_crypt_file = (char *)in.data; - ce->tls_crypt_file_inline = true; - } + connection_entry_preload_key(&ce->tls_auth_file, + &ce->tls_auth_file_inline, &o->gc); + connection_entry_preload_key(&ce->tls_crypt_file, + &ce->tls_crypt_file_inline, &o->gc); + connection_entry_preload_key(&ce->tls_crypt_v2_file, + &ce->tls_crypt_v2_file_inline, &o->gc); } } @@ -3966,7 +3964,7 @@ options_warning_safe_scan2(const int msglevel, if (strprefix(p1, "key-method ") || strprefix(p1, "keydir ") || strprefix(p1, "proto ") - || strprefix(p1, "tls-auth ") + || streq(p1, "tls-auth") || strprefix(p1, "tun-ipv6") || strprefix(p1, "cipher ")) { @@ -4661,7 +4659,8 @@ in_src_get(const struct in_src *is, char *line, const int size) } static char * -read_inline_file(struct in_src *is, const char *close_tag, struct gc_arena *gc) +read_inline_file(struct in_src *is, const char *close_tag, + int *num_lines, struct gc_arena *gc) { char line[OPTION_LINE_SIZE]; struct buffer buf = alloc_buf(8*OPTION_LINE_SIZE); @@ -4670,6 +4669,7 @@ read_inline_file(struct in_src *is, const char *close_tag, struct gc_arena *gc) while (in_src_get(is, line, sizeof(line))) { + (*num_lines)++; char *line_ptr = line; /* Remove leading spaces */ while (isspace(*line_ptr)) @@ -4703,10 +4703,10 @@ read_inline_file(struct in_src *is, const char *close_tag, struct gc_arena *gc) return ret; } -static bool +static int check_inline_file(struct in_src *is, char *p[], struct gc_arena *gc) { - bool is_inline = false; + int num_inline_lines = 0; if (p[0] && !p[1]) { @@ -4719,16 +4719,15 @@ check_inline_file(struct in_src *is, char *p[], struct gc_arena *gc) p[0] = string_alloc(arg + 1, gc); close_tag = alloc_buf(strlen(p[0]) + 4); buf_printf(&close_tag, "", p[0]); - p[1] = read_inline_file(is, BSTR(&close_tag), gc); + p[1] = read_inline_file(is, BSTR(&close_tag), &num_inline_lines, gc); p[2] = NULL; free_buf(&close_tag); - is_inline = true; } } - return is_inline; + return num_inline_lines; } -static bool +static int check_inline_file_via_fp(FILE *fp, char *p[], struct gc_arena *gc) { struct in_src is; @@ -4737,7 +4736,7 @@ check_inline_file_via_fp(FILE *fp, char *p[], struct gc_arena *gc) return check_inline_file(&is, p, gc); } -static bool +static int check_inline_file_via_buf(struct buffer *multiline, char *p[], struct gc_arena *gc) { @@ -4808,13 +4807,12 @@ read_config_file(struct options *options, } if (parse_line(line + offset, p, SIZE(p)-1, file, line_num, msglevel, &options->gc)) { - bool is_inline; - bypass_doubledash(&p[0]); - is_inline = check_inline_file_via_fp(fp, p, &options->gc); - add_option(options, p, is_inline, file, line_num, level, + int lines_inline = check_inline_file_via_fp(fp, p, &options->gc); + add_option(options, p, lines_inline, file, line_num, level, msglevel, permission_mask, option_types_found, es); + line_num += lines_inline; } } if (fp != stdin) @@ -4857,12 +4855,11 @@ read_config_string(const char *prefix, ++line_num; if (parse_line(line, p, SIZE(p)-1, prefix, line_num, msglevel, &options->gc)) { - bool is_inline; - bypass_doubledash(&p[0]); - is_inline = check_inline_file_via_buf(&multiline, p, &options->gc); - add_option(options, p, is_inline, prefix, line_num, 0, msglevel, + int lines_inline = check_inline_file_via_buf(&multiline, p, &options->gc); + add_option(options, p, lines_inline, prefix, line_num, 0, msglevel, permission_mask, option_types_found, es); + line_num += lines_inline; } CLEAR(p); } @@ -5311,13 +5308,14 @@ add_option(struct options *options, } if (good) { -#if 0 - /* removed for now since ECHO can potentially include - * security-sensitive strings */ - msg(M_INFO, "%s:%s", - pull_mode ? "ECHO-PULL" : "ECHO", - BSTR(&string)); -#endif + /* only message-related ECHO are logged, since other ECHOs + * can potentially include security-sensitive strings */ + if (strncmp(p[1], "msg", 3) == 0) + { + msg(M_INFO, "%s:%s", + pull_mode ? "ECHO-PULL" : "ECHO", + BSTR(&string)); + } #ifdef ENABLE_MANAGEMENT if (management) { diff --git a/src/openvpn/pf.c b/src/openvpn/pf.c index f9bbfb5..3f472ef 100644 --- a/src/openvpn/pf.c +++ b/src/openvpn/pf.c @@ -639,8 +639,17 @@ pf_init_context(struct context *c) } if (!c->c2.pf.enabled) { - msg(M_WARN, "WARNING: failed to init PF plugin, rejecting client."); - register_signal(c, SIGUSR1, "plugin-pf-init-failed"); + /* At some point in openvpn history, this code just printed a + * warning and signalled itself (SIGUSR1, "plugin-pf-init-failed") + * to terminate the client instance. This got broken at one of + * the client auth state refactorings (leading to SIGSEGV crashes) + * and due to "pf will be removed anyway" reasons the easiest way + * to prevent crashes is to REQUIRE that plugins succeed - so if + * the plugin fails, we cleanly abort OpenVPN + * + * see also: https://community.openvpn.net/openvpn/ticket/1377 + */ + msg(M_FATAL, "FATAL: failed to init PF plugin, must succeed."); return; } } diff --git a/src/openvpn/ps.c b/src/openvpn/ps.c index 2089e6b..5d76078 100644 --- a/src/openvpn/ps.c +++ b/src/openvpn/ps.c @@ -983,14 +983,38 @@ is_openvpn_protocol(const struct buffer *buf) const int len = BLEN(buf); if (len >= 3) { - return p[0] == 0 - && p[1] >= 14 - && (p[2] == (P_CONTROL_HARD_RESET_CLIENT_V2 << P_OPCODE_SHIFT) - || p[2] == (P_CONTROL_HARD_RESET_CLIENT_V3 << P_OPCODE_SHIFT)); + int plen = (p[0] << 8) | p[1]; + + if (p[2] == (P_CONTROL_HARD_RESET_CLIENT_V3 << P_OPCODE_SHIFT)) + { + /* WKc is at least 290 byte (not including metadata): + * + * 16 bit len + 256 bit HMAC + 2048 bit Kc = 2320 bit + * + * This is increased by the normal length of client handshake + + * tls-crypt overhead (32) + * + * For metadata tls-crypt-v2.txt does not explicitly specify + * an upper limit but we also have TLS_CRYPT_V2_MAX_WKC_LEN + * as 1024 bytes. We err on the safe side with 255 extra overhead + * + * We don't do the 2 byte check for tls-crypt-v2 because it is very + * unrealistic to have only 2 bytes available. + */ + return (plen >= 336 && plen < (1024 + 255)); + } + else + { + /* For non tls-crypt2 we assume the packet length to valid between + * 14 and 255 */ + return plen >= 14 && plen <= 255 + && (p[2] == (P_CONTROL_HARD_RESET_CLIENT_V2 << P_OPCODE_SHIFT)); + } } else if (len >= 2) { - return p[0] == 0 && p[1] >= 14; + int plen = (p[0] << 8) | p[1]; + return plen >= 14 && plen <= 255; } else { diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index c6ba812..d7494c2 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -434,8 +434,6 @@ ssl_set_auth_nocache(void) { passbuf.nocache = true; auth_user_pass.nocache = true; - /* wait for push-reply, because auth-token may still need the username */ - auth_user_pass.wait_for_push = true; } /* @@ -2358,14 +2356,15 @@ key_method_2_write(struct buffer *buf, struct tls_session *session) } /* if auth-nocache was specified, the auth_user_pass object reaches * a "complete" state only after having received the push-reply - * message. + * message. The push message might contain an auth-token that needs + * the username of auth_user_pass. * * For this reason, skip the purge operation here if no push-reply * message has been received yet. * * This normally happens upon first negotiation only. */ - if (!auth_user_pass.wait_for_push) + if (!session->opt->pull) { purge_user_pass(&auth_user_pass, false); } @@ -4104,8 +4103,7 @@ done: } void -delayed_auth_pass_purge(void) +ssl_clean_user_pass(void) { - auth_user_pass.wait_for_push = false; purge_user_pass(&auth_user_pass, false); } diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 005628f..97d721b 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -598,7 +598,10 @@ void extract_x509_field_test(void); */ bool is_hard_reset_method2(int op); -void delayed_auth_pass_purge(void); +/** + * Cleans the saved user/password unless auth-nocache is in use. + */ +void ssl_clean_user_pass(void); /* diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 8315a42..1767420 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -5212,7 +5212,7 @@ netsh_command(const struct argv *a, int n, int msglevel) for (i = 0; i < n; ++i) { bool status; - management_sleep(1); + management_sleep(0); netcmd_semaphore_lock(); argv_msg_prefix(M_INFO, a, "NETSH"); status = openvpn_execve_check(a, NULL, 0, "ERROR: netsh command failed"); @@ -5240,7 +5240,6 @@ ipconfig_register_dns(const struct env_set *es) WIN_IPCONFIG_PATH_SUFFIX); argv_msg(D_TUNTAP_INFO, &argv); openvpn_execve_check(&argv, es, 0, err); - argv_free(&argv); argv_printf(&argv, "%s%s /registerdns", get_win_sys_path(), @@ -6160,35 +6159,35 @@ tuntap_set_ip_addr(struct tuntap *tt, status, strerror_win32(status, &gc)); } - } - /* - * If the TAP-Windows driver is masquerading as a DHCP server - * make sure the TCP/IP properties for the adapter are - * set correctly. - */ - if (dhcp_masq_post) - { - /* check dhcp enable status */ - if (dhcp_status(index) == DHCP_STATUS_DISABLED) + /* + * If the TAP-Windows driver is masquerading as a DHCP server + * make sure the TCP/IP properties for the adapter are + * set correctly. + */ + if (dhcp_masq_post) { - msg(M_WARN, "WARNING: You have selected '--ip-win32 dynamic', which will not work unless the TAP-Windows TCP/IP properties are set to 'Obtain an IP address automatically'"); - } + /* check dhcp enable status */ + if (dhcp_status(index) == DHCP_STATUS_DISABLED) + { + msg(M_WARN, "WARNING: You have selected '--ip-win32 dynamic', which will not work unless the TAP-Windows TCP/IP properties are set to 'Obtain an IP address automatically'"); + } - /* force an explicit DHCP lease renewal on TAP adapter? */ - if (tt->options.dhcp_pre_release) - { - dhcp_release(tt); + /* force an explicit DHCP lease renewal on TAP adapter? */ + if (tt->options.dhcp_pre_release) + { + dhcp_release(tt); + } + if (tt->options.dhcp_renew) + { + dhcp_renew(tt); + } } - if (tt->options.dhcp_renew) + else { - dhcp_renew(tt); + fork_dhcp_action(tt); } } - else - { - fork_dhcp_action(tt); - } if (tt->did_ifconfig_setup && tt->options.ip_win32_type == IPW32_SET_IPAPI) { diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index 65bb106..5d5cbfe 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -1104,7 +1104,7 @@ wmic_nicconfig_cmd(const wchar_t *action, const NET_IFINDEX if_index, } else { - fmt = L"wmic nicconfig where (InterfaceIndex=%ld) call %s %s"; + fmt = L"wmic nicconfig where (InterfaceIndex=%ld) call %s \"%s\""; } size_t ncmdline = wcslen(fmt) + 20 + wcslen(action) /* max 20 for ifindex */ -- cgit v1.2.3