From 87c2fd4310e5b345102d7a4915dc5e3a65052305 Mon Sep 17 00:00:00 2001 From: Bernhard Schmidt Date: Sun, 14 Oct 2018 22:51:08 +0200 Subject: Revert "Merge branch 'stretch' of ssh://git.debian.org/git/collab-maint/openvpn into stretch" This reverts commit 3804bc2606a92e2f2f4b3a2b043af0d77d92b386, reversing changes made to 678cfd249add7ca758e4c41933c7b730132c99f4. --- src/openvpn/crypto.c | 31 +++++++++++++---------------- src/openvpn/forward.c | 7 ------- src/openvpn/init.c | 2 -- src/openvpn/lladdr.c | 2 +- src/openvpn/openvpn.h | 3 +-- src/openvpn/options.c | 16 +-------------- src/openvpn/packet_id.c | 34 +++----------------------------- src/openvpn/packet_id.h | 36 ++++++++++++++++++++++------------ src/openvpn/route.c | 15 +++++++------- src/openvpn/ssl.c | 18 +++++++++-------- src/openvpn/ssl.h | 8 -------- src/openvpn/ssl_verify.c | 51 +++++++++++++++++++----------------------------- src/openvpn/syshead.h | 2 +- src/openvpn/tls_crypt.c | 6 +++--- src/openvpn/tun.c | 6 +++--- 15 files changed, 87 insertions(+), 150 deletions(-) (limited to 'src/openvpn') diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 0dba7ca..7119abc 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -85,6 +85,7 @@ openvpn_encrypt_aead(struct buffer *buf, struct buffer work, /* Prepare IV */ { struct buffer iv_buffer; + struct packet_id_net pin; uint8_t iv[OPENVPN_MAX_IV_LENGTH] = {0}; const int iv_len = cipher_ctx_iv_length(ctx->cipher); @@ -93,11 +94,8 @@ openvpn_encrypt_aead(struct buffer *buf, struct buffer work, buf_set_write(&iv_buffer, iv, iv_len); /* IV starts with packet id to make the IV unique for packet */ - if (!packet_id_write(&opt->packet_id.send, &iv_buffer, false, false)) - { - msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over"); - goto err; - } + packet_id_alloc_outgoing(&opt->packet_id.send, &pin, false); + ASSERT(packet_id_write(&pin, &iv_buffer, false, false)); /* Remainder of IV consists of implicit part (unique per session) */ ASSERT(buf_write(&iv_buffer, ctx->implicit_iv, ctx->implicit_iv_len)); @@ -198,25 +196,25 @@ openvpn_encrypt_v1(struct buffer *buf, struct buffer work, } /* Put packet ID in plaintext buffer */ - if (packet_id_initialized(&opt->packet_id) - && !packet_id_write(&opt->packet_id.send, buf, - opt->flags & CO_PACKET_ID_LONG_FORM, - true)) + if (packet_id_initialized(&opt->packet_id)) { - msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over"); - goto err; + struct packet_id_net pin; + packet_id_alloc_outgoing(&opt->packet_id.send, &pin, BOOL_CAST(opt->flags & CO_PACKET_ID_LONG_FORM)); + ASSERT(packet_id_write(&pin, buf, BOOL_CAST(opt->flags & CO_PACKET_ID_LONG_FORM), true)); } } else if (cipher_kt_mode_ofb_cfb(cipher_kt)) { + struct packet_id_net pin; struct buffer b; /* IV and packet-ID required for this mode. */ ASSERT(opt->flags & CO_USE_IV); ASSERT(packet_id_initialized(&opt->packet_id)); + packet_id_alloc_outgoing(&opt->packet_id.send, &pin, true); buf_set_write(&b, iv_buf, iv_size); - ASSERT(packet_id_write(&opt->packet_id.send, &b, true, false)); + ASSERT(packet_id_write(&pin, &b, true, false)); } else /* We only support CBC, CFB, or OFB modes right now */ { @@ -264,12 +262,11 @@ openvpn_encrypt_v1(struct buffer *buf, struct buffer work, } else /* No Encryption */ { - if (packet_id_initialized(&opt->packet_id) - && !packet_id_write(&opt->packet_id.send, buf, - opt->flags & CO_PACKET_ID_LONG_FORM, true)) + if (packet_id_initialized(&opt->packet_id)) { - msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over"); - goto err; + struct packet_id_net pin; + packet_id_alloc_outgoing(&opt->packet_id.send, &pin, BOOL_CAST(opt->flags & CO_PACKET_ID_LONG_FORM)); + ASSERT(packet_id_write(&pin, buf, BOOL_CAST(opt->flags & CO_PACKET_ID_LONG_FORM), true)); } if (ctx->hmac) { diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 2f3f3c5..8102e94 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -866,16 +866,9 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo * will load crypto_options with the correct encryption key * and return false. */ - uint8_t opcode = *BPTR(&c->c2.buf) >> P_OPCODE_SHIFT; if (tls_pre_decrypt(c->c2.tls_multi, &c->c2.from, &c->c2.buf, &co, floated, &ad_start)) { - /* Restore pre-NCP frame parameters */ - if (is_hard_reset(opcode, c->options.key_method)) - { - c->c2.frame = c->c2.frame_initial; - } - interval_action(&c->c2.tmp_int); /* reset packet received timer if TLS packet */ diff --git a/src/openvpn/init.c b/src/openvpn/init.c index cf4a64c..9a3e29d 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -4055,8 +4055,6 @@ init_instance(struct context *c, const struct env_set *env, const unsigned int f c->c2.did_open_tun = do_open_tun(c); } - c->c2.frame_initial = c->c2.frame; - /* print MTU info */ do_print_data_channel_mtu_parms(c); diff --git a/src/openvpn/lladdr.c b/src/openvpn/lladdr.c index dce05ad..ff71e48 100644 --- a/src/openvpn/lladdr.c +++ b/src/openvpn/lladdr.c @@ -50,7 +50,7 @@ set_lladdr(const char *ifname, const char *lladdr, "%s %s lladdr %s", IFCONFIG_PATH, ifname, lladdr); -#elif defined(TARGET_FREEBSD) || defined(__FreeBSD_kernel__) +#elif defined(TARGET_FREEBSD) argv_printf(&argv, "%s %s ether %s", IFCONFIG_PATH, diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index 7ea0d17..37edec4 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -263,8 +263,7 @@ struct context_2 struct link_socket_actual from; /* address of incoming datagram */ /* MTU frame parameters */ - struct frame frame; /* Active frame parameters */ - struct frame frame_initial; /* Restored on new session */ + struct frame frame; #ifdef ENABLE_FRAGMENT /* Object to handle advanced MTU negotiation and datagram fragmentation */ diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 2f1b298..bfedb6a 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -198,7 +198,7 @@ static const char usage_message[] = " is established. Multiple routes can be specified.\n" " netmask default: 255.255.255.255\n" " gateway default: taken from --route-gateway or --ifconfig\n" - " Specify default by leaving blank or setting to \"default\".\n" + " Specify default by leaving blank or setting to \"nil\".\n" "--route-ipv6 network/bits [gateway] [metric] :\n" " Add IPv6 route to routing table after connection\n" " is established. Multiple routes can be specified.\n" @@ -6789,20 +6789,6 @@ add_option(struct options *options, options->port_share_port = p[2]; options->port_share_journal_dir = p[3]; } - else if (streq (p[0], "pkcs11-id-type") || - streq (p[0], "pkcs11-sign-mode") || - streq (p[0], "pkcs11-slot") || - streq (p[0], "pkcs11-slot-type") || - streq (p[0], "show-pkcs11-objects") || - streq (p[0], "show-pkcs11-slots")) - { - if (file) - msg (msglevel, "You are using an obsolete parameter in %s:%d: %s (%s).\nPlease see /usr/share/doc/openvpn/NEWS.Debian.gz for details.", - file, line, p[0], PACKAGE_VERSION); - else - msg (msglevel, "You are using an obsolete parameter: --%s (%s).\nPlease see /usr/share/doc/openvpn/NEWS.Debian.gz for details.", - p[0], PACKAGE_VERSION); - } #endif else if (streq(p[0], "client-to-client") && !p[1]) { diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c index 6f70c5d..fe13e1d 100644 --- a/src/openvpn/packet_id.c +++ b/src/openvpn/packet_id.c @@ -325,40 +325,12 @@ packet_id_read(struct packet_id_net *pin, struct buffer *buf, bool long_form) return true; } -static bool -packet_id_send_update(struct packet_id_send *p, bool long_form) -{ - if (!p->time) - { - p->time = now; - } - if (p->id == PACKET_ID_MAX) - { - /* Packet ID only allowed to roll over if using long form and time has - * moved forward since last roll over. - */ - if (!long_form || now <= p->time) - { - return false; - } - p->time = now; - p->id = 0; - } - p->id++; - return true; -} - bool -packet_id_write(struct packet_id_send *p, struct buffer *buf, bool long_form, - bool prepend) +packet_id_write(const struct packet_id_net *pin, struct buffer *buf, bool long_form, bool prepend) { - if (!packet_id_send_update(p, long_form)) - { - return false; - } + packet_id_type net_id = htonpid(pin->id); + net_time_t net_time = htontime(pin->time); - const packet_id_type net_id = htonpid(p->id); - const net_time_t net_time = htontime(p->time); if (prepend) { if (long_form) diff --git a/src/openvpn/packet_id.h b/src/openvpn/packet_id.h index aceacf8..ecc25a6 100644 --- a/src/openvpn/packet_id.h +++ b/src/openvpn/packet_id.h @@ -50,7 +50,6 @@ * to for network transmission. */ typedef uint32_t packet_id_type; -#define PACKET_ID_MAX UINT32_MAX typedef uint32_t net_time_t; /* @@ -255,18 +254,7 @@ const char *packet_id_persist_print(const struct packet_id_persist *p, struct gc bool packet_id_read(struct packet_id_net *pin, struct buffer *buf, bool long_form); -/** - * Write a packet ID to buf, and update the packet ID state. - * - * @param p Packet ID state. - * @param buf Buffer to write the packet ID too - * @param long_form If true, also update and write time_t to buf - * @param prepend If true, prepend to buffer, otherwise apppend. - * - * @return true if successful, false otherwise. - */ -bool packet_id_write(struct packet_id_send *p, struct buffer *buf, - bool long_form, bool prepend); +bool packet_id_write(const struct packet_id_net *pin, struct buffer *buf, bool long_form, bool prepend); /* * Inline functions. @@ -316,6 +304,28 @@ packet_id_close_to_wrapping(const struct packet_id_send *p) return p->id >= PACKET_ID_WRAP_TRIGGER; } +/* + * Allocate an outgoing packet id. + * Sequence number ranges from 1 to 2^32-1. + * In long_form, a time_t is added as well. + */ +static inline void +packet_id_alloc_outgoing(struct packet_id_send *p, struct packet_id_net *pin, bool long_form) +{ + if (!p->time) + { + p->time = now; + } + pin->id = ++p->id; + if (!pin->id) + { + ASSERT(long_form); + p->time = now; + pin->id = p->id = 1; + } + pin->time = p->time; +} + static inline bool check_timestamp_delta(time_t remote, unsigned int max_delta) { diff --git a/src/openvpn/route.c b/src/openvpn/route.c index ea09d71..0c93dcd 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -1670,7 +1670,7 @@ add_route(struct route_ipv4 *r, argv_msg(D_ROUTE, &argv); status = openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route add command failed"); -#elif defined(TARGET_FREEBSD) || defined(__FreeBSD_kernel__) +#elif defined(TARGET_FREEBSD) argv_printf(&argv, "%s add", ROUTE_PATH); @@ -1856,7 +1856,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag network = print_in6_addr( r6->network, 0, &gc); gateway = print_in6_addr( r6->gateway, 0, &gc); -#if defined(TARGET_DARWIN) || defined(__FreeBSD_kernel__) \ +#if defined(TARGET_DARWIN) \ || defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) \ || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) @@ -2032,7 +2032,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag argv_msg(D_ROUTE, &argv); status = openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route add -inet6 command failed"); -#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) || defined(__FreeBSD_kernel__) +#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) argv_printf(&argv, "%s add -inet6 %s/%d", ROUTE_PATH, @@ -2216,7 +2216,7 @@ delete_route(struct route_ipv4 *r, argv_msg(D_ROUTE, &argv); openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route delete command failed"); -#elif defined(TARGET_FREEBSD) || defined(__FreeBSD_kernel__) +#elif defined(TARGET_FREEBSD) argv_printf(&argv, "%s delete -net %s %s %s", ROUTE_PATH, @@ -2323,7 +2323,7 @@ delete_route_ipv6(const struct route_ipv6 *r6, const struct tuntap *tt, unsigned network = print_in6_addr( r6->network, 0, &gc); gateway = print_in6_addr( r6->gateway, 0, &gc); -#if defined(TARGET_DARWIN) || defined(__FreeBSD_kernel__) \ +#if defined(TARGET_DARWIN) \ || defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) \ || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) @@ -2458,7 +2458,7 @@ delete_route_ipv6(const struct route_ipv6 *r6, const struct tuntap *tt, unsigned argv_msg(D_ROUTE, &argv); openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route delete -inet6 command failed"); -#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) || defined(__FreeBSD_kernel__) +#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) argv_printf(&argv, "%s delete -inet6 %s/%d", ROUTE_PATH, @@ -3499,8 +3499,7 @@ done: #elif defined(TARGET_DARWIN) || defined(TARGET_SOLARIS) \ || defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) \ - || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) \ - || defined(__FreeBSD_kernel__) + || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) #include #include diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index d94a421..cff4052 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -830,7 +830,14 @@ print_key_id(struct tls_multi *multi, struct gc_arena *gc) return BSTR(&out); } -bool +/* + * Given a key_method, return true if op + * represents the required form of hard_reset. + * + * If key_method = 0, return true if any + * form of hard reset is used. + */ +static bool is_hard_reset(int op, int key_method) { if (!key_method || key_method == 1) @@ -2240,7 +2247,7 @@ push_peer_info(struct buffer *buf, struct tls_session *session) buf_printf(&out, "IV_PLAT=mac\n"); #elif defined(TARGET_NETBSD) buf_printf(&out, "IV_PLAT=netbsd\n"); -#elif defined(TARGET_FREEBSD) || defined(__FreeBSD_kernel__) +#elif defined(TARGET_FREEBSD) buf_printf(&out, "IV_PLAT=freebsd\n"); #elif defined(TARGET_ANDROID) buf_printf(&out, "IV_PLAT=android\n"); @@ -3701,12 +3708,7 @@ tls_pre_decrypt(struct tls_multi *multi, /* Save incoming ciphertext packet to reliable buffer */ struct buffer *in = reliable_get_buf(ks->rec_reliable); ASSERT(in); - if(!buf_copy(in, buf)) - { - msg(D_MULTI_DROPPED, - "Incoming control channel packet too big, dropping."); - goto error; - } + ASSERT(buf_copy(in, buf)); reliable_mark_active_incoming(ks->rec_reliable, in, id, op); } diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 03688ca..ed1344e 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -591,14 +591,6 @@ void show_tls_performance_stats(void); /*#define EXTRACT_X509_FIELD_TEST*/ void extract_x509_field_test(void); -/** - * Given a key_method, return true if opcode represents the required form of - * hard_reset. - * - * If key_method == 0, return true if any form of hard reset is used. - */ -bool is_hard_reset(int op, int key_method); - #endif /* ENABLE_CRYPTO */ #endif /* ifndef OPENVPN_SSL_H */ diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index c553484..334eb29 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -80,28 +80,6 @@ setenv_untrusted(struct tls_session *session) setenv_link_socket_actual(session->opt->es, "untrusted", &session->untrusted_addr, SA_IP_PORT); } - -/** - * Wipes the authentication token out of the memory, frees and cleans up related buffers and flags - * - * @param multi Pointer to a multi object holding the auth_token variables - */ -static void -wipe_auth_token(struct tls_multi *multi) -{ - if(multi) - { - if (multi->auth_token) - { - secure_memzero(multi->auth_token, AUTH_TOKEN_SIZE); - free(multi->auth_token); - } - multi->auth_token = NULL; - multi->auth_token_sent = false; - } -} - - /* * Remove authenticated state from all sessions in the given tunnel */ @@ -110,14 +88,10 @@ tls_deauthenticate(struct tls_multi *multi) { if (multi) { - wipe_auth_token(multi); - for (int i = 0; i < TM_SIZE; ++i) - { - for (int j = 0; j < KS_SIZE; ++j) - { + int i, j; + for (i = 0; i < TM_SIZE; ++i) + for (j = 0; j < KS_SIZE; ++j) multi->session[i].key[j].authenticated = false; - } - } } } @@ -1239,6 +1213,21 @@ verify_user_pass_management(struct tls_session *session, const struct user_pass } #endif /* ifdef MANAGEMENT_DEF_AUTH */ +/** + * Wipes the authentication token out of the memory, frees and cleans up related buffers and flags + * + * @param multi Pointer to a multi object holding the auth_token variables + */ +static void +wipe_auth_token(struct tls_multi *multi) +{ + secure_memzero(multi->auth_token, AUTH_TOKEN_SIZE); + free(multi->auth_token); + multi->auth_token = NULL; + multi->auth_token_sent = false; +} + + /* * Main username/password verification entry point */ @@ -1290,7 +1279,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi, /* Ensure that the username has not changed */ if (!tls_lock_username(multi, up->username)) { - /* auth-token cleared in tls_lock_username() on failure */ + wipe_auth_token(multi); ks->authenticated = false; goto done; } @@ -1311,6 +1300,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi, if (memcmp_constant_time(multi->auth_token, up->password, strlen(multi->auth_token)) != 0) { + wipe_auth_token(multi); ks->authenticated = false; tls_deauthenticate(multi); @@ -1482,7 +1472,6 @@ verify_final_auth_checks(struct tls_multi *multi, struct tls_session *session) if (!cn || !strcmp(cn, CCD_DEFAULT) || !test_file(path)) { ks->authenticated = false; - wipe_auth_token(multi); msg(D_TLS_ERRORS, "TLS Auth Error: --client-config-dir authentication failed for common name '%s' file='%s'", session->common_name, path ? path : "UNDEF"); diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h index 078ed3a..a1b6047 100644 --- a/src/openvpn/syshead.h +++ b/src/openvpn/syshead.h @@ -294,7 +294,7 @@ #endif /* TARGET_OPENBSD */ -#if defined(TARGET_FREEBSD) || defined(__FreeBSD_kernel__) +#ifdef TARGET_FREEBSD #ifdef HAVE_SYS_UIO_H #include diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c index 4c7170f..c227b09 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -95,10 +95,10 @@ tls_crypt_wrap(const struct buffer *src, struct buffer *dst, format_hex(BPTR(src), BLEN(src), 80, &gc)); /* Get packet ID */ - if (!packet_id_write(&opt->packet_id.send, dst, true, false)) { - msg(D_CRYPT_ERRORS, "TLS-CRYPT ERROR: packet ID roll over."); - goto err; + struct packet_id_net pin; + packet_id_alloc_outgoing(&opt->packet_id.send, &pin, true); + packet_id_write(&pin, dst, true, false); } dmsg(D_PACKET_CONTENT, "TLS-CRYPT WRAP AD: %s", diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index a4f7779..f812844 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -840,7 +840,7 @@ delete_route_connected_v6_net(struct tuntap *tt, #endif /* if defined(_WIN32) || defined(TARGET_DARWIN) || defined(TARGET_NETBSD) || defined(TARGET_OPENBSD) */ #if defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) \ - || defined(TARGET_OPENBSD) || defined(__FreeBSD_kernel__) + || defined(TARGET_OPENBSD) /* we can't use true subnet mode on tun on all platforms, as that * conflicts with IPv6 (wants to use ND then, which we don't do), * but the OSes want "a remote address that is different from ours" @@ -1408,7 +1408,7 @@ do_ifconfig(struct tuntap *tt, add_route_connected_v6_net(tt, es); } -#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) || defined(__FreeBSD_kernel__) +#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) in_addr_t remote_end; /* for "virtual" subnet topology */ @@ -2762,7 +2762,7 @@ read_tun(struct tuntap *tt, uint8_t *buf, int len) } } -#elif defined(TARGET_FREEBSD)||defined(__FreeBSD_kernel__) +#elif defined(TARGET_FREEBSD) static inline int freebsd_modify_read_write_return(int len) -- cgit v1.2.3