| Message ID | 20220502160945.839283-1-arne@rfc2549.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel,v3] Implement HMAC based session id for tls-crypt v2 | expand |
Still found a few typos and whitespace issues, but no code issues. Overall this looks okay to me but I'm hesitant to actually ack it because I'm not sure of the potential implications of all the protocol hacks (i.e. the packet id flags and the TLV payload). > Arne Schwabe <arne@rfc2549.org> hat am 02.05.2022 18:09 geschrieben: [...] > @@ -58,37 +82,62 @@ do_pre_decrypt_check(struct multi_context *m, > struct openvpn_sockaddr *from = &m->top.c2.from.dest; > int handwindow = m->top.options.handshake_window; > > - > if (verdict == VERDICT_VALID_RESET_V3) > { > - /* For tls-crypt-v2 we need to keep the state of the first packet to > - * store the unwrapped key */ > - return true; > + /* Extract the packet id to check if it has the special format that > + * indicates early negotiation support */ > + struct packet_id_net pin; > + struct buffer tmp = m->top.c2.buf; > + ASSERT(buf_advance(&tmp, 1 + SID_SIZE)); > + ASSERT(packet_id_read(&pin, &tmp, true)); > + > + /* The most significant byte ist 0x0f if early negotiation is supported */ Your German is showing ;) "is" [...] > diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c > index 9c8154b12..2376446ee 100644 > --- a/src/openvpn/ssl_pkt.c > +++ b/src/openvpn/ssl_pkt.c [...] > @@ -434,8 +443,17 @@ tls_reset_standalone(struct tls_auth_standalone *tas, > > ASSERT(buf_write(&buf, &net_pid, sizeof(net_pid))); > > + /* Add indication for tls-crypt-v2 to resend the packet with the with Redundant "with" > + * reply */ [...] > diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h > index ae92f6b33..9426fb1d8 100644 > --- a/src/openvpn/ssl_pkt.h > +++ b/src/openvpn/ssl_pkt.h [...] > @@ -261,4 +272,23 @@ packet_opcode_name(int op) > return "P_???"; > } > } > + > +/* initial packet id (instead of 0) that indicates that the peer supports > + * early protocol negotiation. This will make the packet id turn a bit faster > + * but the network time part of the packet id could take care of that. And > + * this is also a rather theoretical scenario as it still needs more than > + * 2^31 control channel packets to happen */ > +#define EARLY_NEG_MASK 0xff000000 > +#define EARLY_NEG_START 0x0a000000 > + > +#define EARLY_NEG_RESENDWKC 0x00010000 > + > + > +/* Early negotiation that part of the server response in the RESET_V2 packet. > + * Since clients that announce early negotiation support will treat the payload > + * of reset packets special and parse it as TLV messages. > + * as TLV (type, length, value) */ > +#define EARLY_NEG_TLV_FLAG 0x01 > +#define EARLY_NEG_FLAG_RESEND_WKC 0x01 > #endif /* ifndef SSL_PKT_H */ > + new blank line at EOF. > diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c > index 36812628e..f3fddf870 100644 > --- a/tests/unit_tests/openvpn/test_pkt.c > +++ b/tests/unit_tests/openvpn/test_pkt.c [...] > @@ -580,11 +580,11 @@ test_generate_reset_packet_tls_auth(void **ut_state) > > /* Assure repeated generation of reset is deterministic/stateless*/ > reset_packet_id_send(&tas_client.tls_wrap.opt.packet_id.send); > - struct buffer buf2 = tls_reset_standalone(&tas_client, &client_id, &server_id, header); > + struct buffer buf2 = tls_reset_standalone(&tas_client.tls_wrap, &tas_client, &client_id, &server_id, header,false); > assert_int_equal(BLEN(&buf), BLEN(&buf2)); > assert_memory_equal(BPTR(&buf), BPTR(&buf2), BLEN(&buf)); > + Trailing whitespace: --- "a/tests/unit_tests/openvpn/test_pkt.c"^I2022-05-03 11:17:01.179037300 +0200$ +++ "b/tests/unit_tests/openvpn/test_pkt.c"^I2022-05-03 11:17:02.013292900 +0200$ @@ -583,7 +583,7 @@$ struct buffer buf2 = tls_reset_standalone(&tas_client.tls_wrap, &tas_client, &client_id, &server_id, header,false);$ assert_int_equal(BLEN(&buf), BLEN(&buf2));$ assert_memory_equal(BPTR(&buf), BPTR(&buf2), BLEN(&buf));$ - $ +$ free_buf(&buf2);$ free_tls_pre_decrypt_state(&state);$ > free_buf(&buf2); > - > free_tls_pre_decrypt_state(&state); > > packet_id_free(&tas_client.tls_wrap.opt.packet_id); Regards, -- Frank Lichtenheld
Hi, On 02/05/2022 18:09, Arne Schwabe wrote: > Tls-crypt v2 is more complicated to implement a proper stateless > handshake. To allow state handshake this commit does > > - introduce a new packet CONTROL_WKC_V1 that repeats the wrapped > client key. > - introduce a way to negotiate the support for this packet in the > three way handshake > > Details about the protocol changes are in tls-crypt-v2.txt. Optional > arguments to the tls-crypt-v2 option have been added to explicitly > allow or disallow client that do not support the stateless handshake. > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > > Patch v3: improve grammar, style, comments, fix unit tests > --- > Changes.rst | 10 +++ > doc/man-sections/tls-options.rst | 14 ++++ > doc/tls-crypt-v2.txt | 41 +++++++++ > src/openvpn/crypto.h | 8 ++ > src/openvpn/init.c | 4 + > src/openvpn/mudp.c | 85 ++++++++++++++----- > src/openvpn/options.c | 13 +++ > src/openvpn/options.h | 3 + > src/openvpn/reliable.h | 2 - > src/openvpn/ssl.c | 125 +++++++++++++++++++++++++--- > src/openvpn/ssl_pkt.c | 28 +++++-- > src/openvpn/ssl_pkt.h | 36 +++++++- > tests/unit_tests/openvpn/test_pkt.c | 10 +-- > 13 files changed, 334 insertions(+), 45 deletions(-) > > diff --git a/Changes.rst b/Changes.rst > index ceb0b2680..67a23c792 100644 > --- a/Changes.rst > +++ b/Changes.rst > @@ -69,6 +69,16 @@ Improved ``--mssfix`` and ``--fragment`` calculation > account and the resulting size is specified as the total size of the VPN packets > including IP and UDP headers. > > +Cookie based handshake for UDP server > + Instead of allocating a connection for each client on the initial packet > + OpenVPN server will now use an HMAC based cookie as its session id. This > + way the server can verify it on completing the handshake without keeping > + state. This eliminates the amplification and resource exhaustion attacks. > + For tls-crypt-v2 clients, this requires OpenVPN 2.6 clients or later > + because the client needs to resend its client key on completing the hand > + shake. The tls-crypt-v2 option allows controlling if older clients are > + accepted. > + > Deprecated features > ------------------- > ``inetd`` has been removed > diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst > index ac5756034..c06ee3354 100644 > --- a/doc/man-sections/tls-options.rst > +++ b/doc/man-sections/tls-options.rst > @@ -486,6 +486,13 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa > 8000 years'. > > --tls-crypt-v2 keyfile > + > + Valid syntax: > + :: > + tls-crypt-v2 keyfile > + tls-crypt-v2 keyfile force-cookie > + tls-crypt-v2 keyfile allow-noncookie > + > Use client-specific tls-crypt keys. > > For clients, ``keyfile`` is a client-specific tls-crypt key. Such a key > @@ -501,6 +508,13 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa > client is using client-specific keys, and automatically select the right > mode. > > + The optional parameters :code:`force-cookie` allows only tls-crypt-v2 > + clients that support a cookie based stateless three way handshake that > + avoids replay attacks and state exhaustion on the server side (OpenVPN > + 2.6 and later). The option :code:`allow-noncookie` explicitly allows > + older tls-crypt-v2 clients. The default is (currently) > + :code:`allow-noncookie`. > + > --tls-crypt-v2-verify cmd > Run command ``cmd`` to verify the metadata of the client-specific > tls-crypt-v2 key of a connecting client. This allows server > diff --git a/doc/tls-crypt-v2.txt b/doc/tls-crypt-v2.txt > index f6a6a1395..eb7b7138b 100644 > --- a/doc/tls-crypt-v2.txt > +++ b/doc/tls-crypt-v2.txt > @@ -157,6 +157,47 @@ When setting up the openvpn connection: > messages. > > > +HMAC Cookie support > +------------------- > +To avoid exhaustion attack and keeping state for connections that fail to > +complete the three-way handshake, the OpenVPN server will use its own session > +id as challenge that the client must repeat in the third packet of the > +handshake. This introduces a problem. If the server does not keep the wrapped > +client key from the initial packet, the server cannot decode the third packet. > +Therefore, tls-crypt-v2 in 2.6 allows resending the wrapped key in the third > +packet of the handshake with the P_CONTROL_WKC_V1 message. The modified > +handshake is as follows (the rest of the handshake is unmodified): > + > +1. The client creates the P_CONTROL_HARD_RESET_CLIENT_V3 message as before > + but indicates that it supports resending the wrapped key. This is done > + by setting the packet id of the replay id to 0x0f010000. The first byte Here you say that the replay id starts with 0x0f, but later you set EARLY_NEG_START to 0x0a.... Either the doc is wrong or the value of that constant should be changed. > + indicates the early negotiation support and the next byte the flags. > + > +2. The server responds with a P_CONTROL_HARD_RESET_V2 message. Instead of having > + an empty payload like normally, the payload consists of TLV (type (uint16), > + length (uint16), value) packets. TLV was chosen > + to allow extensibility in the future. Currently only the following TLV is > + defined: > + > + flags - type 0x01, length 2. > + > + Bit 1 indicates that the client needs to resend the WKc in the third packet. > + > +3. Instead of normal P_ACK_V1 or P_CONTROL_V1 packet, the client will send a > + P_CONTROL_WKC_V1 packet. The P_CONTROL_WKC_V1 is identical to a normal > + P_CONTROL_V1 packet but with the WKc appended. > + > + Normally the first message of the client is either P_ACK_V1, directly > + followed by a P_CONTROL_V1 message that contains the TLS Client Hello or > + just a P_CONTROL_V1 message. Instead of a P_ACK_V1 message the client should > + send a P_CONTROL_WKC_V1 message with an empty payload. This message must > + also include an ACK for the P_CONTROL_HARD_RESET_V2 message. > + > + When directly sending the TLS Client Hello message in the P_CONTROL_WKC_V1 > + message, the client must ensure that the resulting P_CONTROL_WKC_V1 message > + with the appended WKc does not extend the control message length. > + > + > Considerations > -------------- > > diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h > index 806632edf..98e2c7664 100644 > --- a/src/openvpn/crypto.h > +++ b/src/openvpn/crypto.h > @@ -256,6 +256,14 @@ struct crypto_options > /**< Bit-flag indicating that data channel key derivation > * is done using TLS keying material export [RFC5705] > */ > +#define CO_RESEND_WKC (1<<4) > + /**< Bit-flag indicating that the client is expected to > + * resend the wrapped client key with the 2nd packet (packet-id 1) > + * like with the HARD_RESET_CLIENT_V3 packet */ > +#define CO_FORCE_TLSCRYPTV2_COOKIE (1<<5) > + /**< Bit-flag indicating that we do not allow clients that do > + * not support resending the wrapped client key (WKc) with the > + * third packet of the three-way handshake */ > unsigned int flags; /**< Bit-flags determining behavior of > * security operation functions. */ > }; > diff --git a/src/openvpn/init.c b/src/openvpn/init.c > index 7e7041a8e..85e146bf2 100644 > --- a/src/openvpn/init.c > +++ b/src/openvpn/init.c > @@ -2960,6 +2960,10 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) > { > to.tls_wrap.tls_crypt_v2_server_key = c->c1.ks.tls_crypt_v2_server_key; > to.tls_crypt_v2_verify_script = c->options.tls_crypt_v2_verify_script; > + if (options->ce.tls_crypt_v2_force_cookie) > + { > + to.tls_wrap.opt.flags |= CO_FORCE_TLSCRYPTV2_COOKIE; > + } > } > } > > diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c > index ead61e827..88cd6a497 100644 > --- a/src/openvpn/mudp.c > +++ b/src/openvpn/mudp.c > @@ -40,6 +40,30 @@ > #include <sys/inotify.h> > #endif > > +static void > +send_hmac_reset_packet(struct multi_context *m, > + struct tls_pre_decrypt_state *state, > + struct tls_auth_standalone *tas, > + struct session_id *sid, > + bool tlscryptv2) > +{ > + reset_packet_id_send(&state->tls_wrap_tmp.opt.packet_id.send); > + state->tls_wrap_tmp.opt.packet_id.rec.initialized = true; > + uint8_t header = 0 | (P_CONTROL_HARD_RESET_SERVER_V2 << P_OPCODE_SHIFT); > + struct buffer buf = tls_reset_standalone(&state->tls_wrap_tmp, tas, sid, > + &state->peer_session_id, header, > + tlscryptv2); > + > + struct context *c = &m->top; > + > + buf_reset_len(&c->c2.buffers->aux_buf); > + buf_copy(&c->c2.buffers->aux_buf, &buf); > + m->hmac_reply = c->c2.buffers->aux_buf; > + m->hmac_reply_dest = &m->top.c2.from; > + msg(D_MULTI_DEBUG, "Reset packet from client, sending HMAC based reset challenge"); > +} > + > + > /* Return if this packet should create a new session */ > static bool > do_pre_decrypt_check(struct multi_context *m, > @@ -58,37 +82,62 @@ do_pre_decrypt_check(struct multi_context *m, > struct openvpn_sockaddr *from = &m->top.c2.from.dest; > int handwindow = m->top.options.handshake_window; > > - > if (verdict == VERDICT_VALID_RESET_V3) > { > - /* For tls-crypt-v2 we need to keep the state of the first packet to > - * store the unwrapped key */ > - return true; > + /* Extract the packet id to check if it has the special format that > + * indicates early negotiation support */ > + struct packet_id_net pin; > + struct buffer tmp = m->top.c2.buf; > + ASSERT(buf_advance(&tmp, 1 + SID_SIZE)); > + ASSERT(packet_id_read(&pin, &tmp, true)); > + > + /* The most significant byte ist 0x0f if early negotiation is supported */ > + bool early_neg_support = (pin.id & EARLY_NEG_MASK) == EARLY_NEG_START; > + > + if (early_neg_support && (pin.id & EARLY_NEG_RESENDWKC)) EARLY_NEG_RESENDWKC is always set when EARLY_NEG_START is set too - the two features basically always exist at the same time. I suggest to remove EARLY_NEG_RESENDWKC and just rely on EARLY_NEG_START. A client sending the latter will always support the former. And this is specific to tls-crypt-v2 anyway. > + { > + /* Calculate the session ID HMAC for our reply and create reset packet */ > + struct session_id sid = calculate_session_id_hmac(state->peer_session_id, > + from, hmac, handwindow, 0); > + send_hmac_reset_packet(m, state, tas, &sid, true); > + > + return false; > + } > + else > + { > + /* For tls-crypt-v2 we need to keep the state of the first packet > + * to store the unwrapped key if the client doesn't support resending > + * the wrapped key */ to make this comment more comprehensive of what follows I'd add: ", unless the user specifically disallowed such clients" > + if (tas->tls_wrap.opt.flags & CO_FORCE_TLSCRYPTV2_COOKIE) > + { > + struct gc_arena gc = gc_new(); > + const char *peer = print_link_socket_actual(&m->top.c2.from, &gc); > + msg(D_MULTI_DEBUG, "tls-crypt-v2 force-cookie is enabled," > + "ignoring connection attempt from old client" > + " (%s)", peer); > + gc_free(&gc); > + return false; > + } > + else > + { > + return true; > + } > + } > } > else if (verdict == VERDICT_VALID_RESET_V2) > { > /* Calculate the session ID HMAC for our reply and create reset packet */ > struct session_id sid = calculate_session_id_hmac(state->peer_session_id, > from, hmac, handwindow, 0); > - reset_packet_id_send(&tas->tls_wrap.opt.packet_id.send); > - tas->tls_wrap.opt.packet_id.rec.initialized = true; > - uint8_t header = 0 | (P_CONTROL_HARD_RESET_SERVER_V2 << P_OPCODE_SHIFT); > - struct buffer buf = tls_reset_standalone(tas, &sid, > - &state->peer_session_id, header); > - > > - struct context *c = &m->top; > + send_hmac_reset_packet(m, state, tas, &sid, false); > > - buf_reset_len(&c->c2.buffers->aux_buf); > - buf_copy(&c->c2.buffers->aux_buf, &buf); > - m->hmac_reply = c->c2.buffers->aux_buf; > - m->hmac_reply_dest = &m->top.c2.from; > - msg(D_MULTI_DEBUG, "Reset packet from client, sending HMAC based reset challenge"); > /* We have a reply do not create a new session */ > return false; > > } > - else if (verdict == VERDICT_VALID_CONTROL_V1 || verdict == VERDICT_VALID_ACK_V1) > + else if (verdict == VERDICT_VALID_CONTROL_V1 || verdict == VERDICT_VALID_ACK_V1 > + || verdict == VERDICT_VALID_WKC_V1) > { > /* ACK_V1 contains the peer id (our id) while CONTROL_V1 can but does not > * need to contain the peer id */ > @@ -99,9 +148,7 @@ do_pre_decrypt_check(struct multi_context *m, > const char *peer = print_link_socket_actual(&m->top.c2.from, &gc); > if (!ret) > { > - > msg(D_MULTI_MEDIUM, "Packet with invalid or missing SID from %s", peer); > - > } > else > { > diff --git a/src/openvpn/options.c b/src/openvpn/options.c > index 7f5c903d1..9ff384d09 100644 > --- a/src/openvpn/options.c > +++ b/src/openvpn/options.c > @@ -8896,6 +8896,19 @@ add_option(struct options *options, > options->ce.tls_crypt_v2_file = p[1]; > options->ce.tls_crypt_v2_file_inline = is_inline; > } > + > + if (p[2] && streq(p[2], "force-cookie")) > + { > + options->ce.tls_crypt_v2_force_cookie = true; > + } > + else if (p[2] && streq(p[2], "allow-noncookie")) > + { > + options->ce.tls_crypt_v2_force_cookie = false; > + } > + else if (p[2]) > + { > + msg(msglevel, "Unsupported tls-crypt-v2 argument: %s", p[2]); > + } > } > else if (streq(p[0], "tls-crypt-v2-verify") && p[1] && !p[2]) > { > diff --git a/src/openvpn/options.h b/src/openvpn/options.h > index 055789b3b..c2937dc37 100644 > --- a/src/openvpn/options.h > +++ b/src/openvpn/options.h > @@ -162,6 +162,9 @@ struct connection_entry > * authenticated encryption v2 */ > const char *tls_crypt_v2_file; > bool tls_crypt_v2_file_inline; > + > + /* Allow only client that support resending the wrapped client key */ > + bool tls_crypt_v2_force_cookie; > }; > > struct remote_entry > diff --git a/src/openvpn/reliable.h b/src/openvpn/reliable.h > index 8da2c0729..b9863efe3 100644 > --- a/src/openvpn/reliable.h > +++ b/src/openvpn/reliable.h > @@ -361,8 +361,6 @@ struct reliable_entry *reliable_get_entry_sequenced(struct reliable *rel); > * > * @param rel The reliable structure associated with the given buffer. > * @param buf The buffer of the reliable entry which is to be removed. > - * @param inc_pid If true, the reliable structure's packet ID counter > - * will be incremented. > */ > void reliable_mark_deleted(struct reliable *rel, struct buffer *buf); > > diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c > index 8ea7c06fa..5e1a23ccd 100644 > --- a/src/openvpn/ssl.c > +++ b/src/openvpn/ssl.c > @@ -1099,6 +1099,14 @@ tls_session_init(struct tls_multi *multi, struct tls_session *session) > session->opt->replay_time, > "TLS_WRAP", session->key_id); > > + /* If we are using tls-crypt-v2 we manipulate the packet id to be (ab)used > + * to indicate early protocol negotiation */ > + if (session->opt->tls_crypt_v2) > + { > + session->tls_wrap.opt.packet_id.send.time = now; > + session->tls_wrap.opt.packet_id.send.id = EARLY_NEG_START | EARLY_NEG_RESENDWKC; > + } > + > /* load most recent packet-id to replay protect on --tls-auth */ > packet_id_persist_load_obj(session->tls_wrap.opt.pid_persist, > &session->tls_wrap.opt.packet_id); > @@ -2525,6 +2533,53 @@ session_skip_to_pre_start(struct tls_session *session, > return session_move_pre_start(session, ks, true); > } > > +/** > + * Parses the TLVs (type, length, value) in the early negotiation > + */ > +static bool > +parse_early_negotiation_tlvs(struct buffer *buf, struct key_state *ks) > +{ > + while (buf->len > 0) > + { > + if (buf_len(buf) < 4) > + { > + goto error; > + } > + /* read type */ > + uint16_t type = buf_read_u16(buf); > + uint16_t len = buf_read_u16(buf); > + if (buf_len(buf) < len) > + { > + goto error; > + } > + > + if (type == EARLY_NEG_TLV_FLAG) > + { > + if (len != sizeof(uint16_t)) > + { > + goto error; > + } > + uint16_t flags = buf_read_u16(buf); > + > + if (flags & EARLY_NEG_FLAG_RESEND_WKC) > + { > + ks->crypto_options.flags |= CO_RESEND_WKC; > + } > + } > + else > + { > + /* Skip types we do not parse */ > + buf_advance(buf, len); > + } IMHO a switch/case would be more elegant..but we can do that when we'll add more TLV types. > + } > + reliable_mark_deleted(ks->rec_reliable, buf); > + > + return true; > +error: > + msg(D_TLS_ERRORS, "TLS Error: Early negotiation malformed packet"); > + return false; > +} > + > /** > * Read incoming ciphertext and passes it to the buffer of the SSL library. > * Returns false if an error is encountered that should abort the session. > @@ -2557,6 +2612,13 @@ read_incoming_tls_ciphertext(struct buffer *buf, struct key_state *ks, > return true; > } > > +static bool > +control_packet_needs_wkc(const struct key_state *ks) > +{ > + return (ks->crypto_options.flags & CO_RESEND_WKC) > + && (ks->send_reliable->packet_id == 1); > +} > + > > static bool > tls_process_state(struct tls_multi *multi, > @@ -2626,9 +2688,21 @@ tls_process_state(struct tls_multi *multi, > struct reliable_entry *entry = reliable_get_entry_sequenced(ks->rec_reliable); > if (entry) > { > - if (!read_incoming_tls_ciphertext(&entry->buf, ks, &state_change)) > + /* The first packet from the peer (the reset packet) is special and > + * contains early protocol negotiation */ > + if (entry->packet_id == 0 && is_hard_reset_method2(entry->opcode)) > { > - goto error; > + if (!parse_early_negotiation_tlvs(&entry->buf, ks)) > + { > + goto error; > + } > + } > + else > + { > + if (!read_incoming_tls_ciphertext(&entry->buf, ks, &state_change)) > + { > + goto error; > + } > } > } > > @@ -2721,7 +2795,12 @@ tls_process_state(struct tls_multi *multi, > } > if (status == 1) > { > - reliable_mark_active_outgoing(ks->send_reliable, buf, P_CONTROL_V1); > + int opcode = P_CONTROL_V1; > + if (control_packet_needs_wkc(ks)) > + { > + opcode = P_CONTROL_WKC_V1; > + } > + reliable_mark_active_outgoing(ks->send_reliable, buf, opcode); > INCR_GENERATED; > state_change = true; > dmsg(D_TLS_DEBUG, "Outgoing Ciphertext -> Reliable"); > @@ -2811,15 +2890,38 @@ tls_process(struct tls_multi *multi, > > update_time(); > > + /* We often send acks back to back to a following control packet. This > + * normally does not create a problem (apart from an extra packet. However, > + * with the P_CONTROL_WKC_V1 we need to ensure that the packet gets resend > + * if not received by remote, so instead we use an empty control packet in > + * this special case */ > + > + one empty line should be enough > /* Send 1 or more ACKs (each received control packet gets one ACK) */ > if (!to_link->len && !reliable_ack_empty(ks->rec_ack)) > { > - struct buffer buf = ks->ack_write_buf; > - ASSERT(buf_init(&buf, multi->opt.frame.buf.headroom)); > - write_control_auth(session, ks, &buf, to_link_addr, P_ACK_V1, > - RELIABLE_ACK_SIZE, false); > - *to_link = buf; > - dmsg(D_TLS_DEBUG, "Dedicated ACK -> TCP/UDP"); > + if (control_packet_needs_wkc(ks)) > + { > + struct buffer *buf = reliable_get_buf_output_sequenced(ks->send_reliable); > + if (!buf) > + { > + return false; > + } > + > + /* We do not write anything to the buffer, this way this will be > + * an empty control packet that gets the ack piggybacked and > + * also appended the wrapped client key since it has a WCK opcode */ > + reliable_mark_active_outgoing(ks->send_reliable, buf, P_CONTROL_WKC_V1); > + } > + else > + { > + struct buffer buf = ks->ack_write_buf; > + ASSERT(buf_init(&buf, multi->opt.frame.buf.headroom)); > + write_control_auth(session, ks, &buf, to_link_addr, P_ACK_V1, > + RELIABLE_ACK_SIZE, false); > + *to_link = buf; > + dmsg(D_TLS_DEBUG, "Dedicated ACK -> TCP/UDP"); > + } > } > > /* When should we wake up again? */ > @@ -3464,7 +3566,8 @@ tls_pre_decrypt(struct tls_multi *multi, > } > > /* > - * We have an authenticated control channel packet (if --tls-auth was set). > + * We have an authenticated control channel packet (if --tls-auth/tls-crypt > + * or tls-crypt-v2 was set). > * Now pass to our reliability layer which deals with > * packet acknowledgements, retransmits, sequencing, etc. > */ > @@ -3893,7 +3996,7 @@ protocol_dump(struct buffer *buffer, unsigned int flags, struct gc_arena *gc) > > if (op == P_ACK_V1) > { > - goto done; > + goto print_data; > } > > /* > diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c > index 9c8154b12..2376446ee 100644 > --- a/src/openvpn/ssl_pkt.c > +++ b/src/openvpn/ssl_pkt.c > @@ -148,7 +148,8 @@ tls_wrap_control(struct tls_wrap_ctx *ctx, uint8_t header, struct buffer *buf, > return; > } > > - if ((header >> P_OPCODE_SHIFT) == P_CONTROL_HARD_RESET_CLIENT_V3) > + if ((header >> P_OPCODE_SHIFT) == P_CONTROL_HARD_RESET_CLIENT_V3 > + || (header >> P_OPCODE_SHIFT) == P_CONTROL_WKC_V1) > { > if (!buf_copy(&ctx->work, > ctx->tls_crypt_v2_wkc)) > @@ -197,7 +198,8 @@ read_control_auth(struct buffer *buf, > bool ret = false; > > const uint8_t opcode = *(BPTR(buf)) >> P_OPCODE_SHIFT; > - if (opcode == P_CONTROL_HARD_RESET_CLIENT_V3 > + if ((opcode == P_CONTROL_HARD_RESET_CLIENT_V3 > + || opcode == P_CONTROL_WKC_V1) > && !tls_crypt_v2_extract_client_key(buf, ctx, opt)) > { > msg(D_TLS_ERRORS, > @@ -321,6 +323,7 @@ tls_pre_decrypt_lite(const struct tls_auth_standalone *tas, > if (op != P_CONTROL_HARD_RESET_CLIENT_V2 > && op != P_CONTROL_HARD_RESET_CLIENT_V3 > && op != P_CONTROL_V1 > + && op != P_CONTROL_WKC_V1 > && op != P_ACK_V1) > { > /* > @@ -397,6 +400,10 @@ tls_pre_decrypt_lite(const struct tls_auth_standalone *tas, > { > return VERDICT_VALID_RESET_V3; > } > + else if (op == P_CONTROL_WKC_V1) > + { > + return VERDICT_VALID_WKC_V1; > + } > else > { > return VERDICT_VALID_RESET_V2; > @@ -410,10 +417,12 @@ error: > > > struct buffer > -tls_reset_standalone(struct tls_auth_standalone *tas, > +tls_reset_standalone(struct tls_wrap_ctx *ctx, > + struct tls_auth_standalone *tas, > struct session_id *own_sid, > struct session_id *remote_sid, > - uint8_t header) > + uint8_t header, > + bool tlscryptv2) I know this bool is used only by tlscryptv2, but what this function does is kinda unrelated to having tlscryptv2 or not. Should we rather call it "bool early_neg_resend_wkc" or "bool request_resend_wkc"? > { > struct buffer buf = alloc_buf(tas->frame.buf.payload_size); > ASSERT(buf_init(&buf, tas->frame.buf.headroom)); > @@ -434,8 +443,17 @@ tls_reset_standalone(struct tls_auth_standalone *tas, > > ASSERT(buf_write(&buf, &net_pid, sizeof(net_pid))); > > + /* Add indication for tls-crypt-v2 to resend the packet with the with > + * reply */ > + if (tlscryptv2) > + { > + buf_write_u16(&buf, EARLY_NEG_TLV_FLAG); /* TYPE: flags */ > + buf_write_u16(&buf, sizeof(uint16_t)); > + buf_write_u16(&buf, EARLY_NEG_FLAG_RESEND_WKC); > + } > + > /* Add tls-auth/tls-crypt wrapping, this might replace buf */ > - tls_wrap_control(&tas->tls_wrap, header, &buf, own_sid); > + tls_wrap_control(ctx, header, &buf, own_sid); > > return buf; > } > diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h > index ae92f6b33..9426fb1d8 100644 > --- a/src/openvpn/ssl_pkt.h > +++ b/src/openvpn/ssl_pkt.h > @@ -54,11 +54,15 @@ > /* indicates key_method >= 2 and client-specific tls-crypt key */ > #define P_CONTROL_HARD_RESET_CLIENT_V3 10 /* initial key from client, forget previous state */ > > +/* Variant of P_CONTROL_V1 but with appended wrapped key > + * like P_CONTROL_HARD_RESET_CLIENT_V3 */ > +#define P_CONTROL_WKC_V1 11 > + > /* define the range of legal opcodes > * Since we do no longer support key-method 1 we consider > * the v1 op codes invalid */ > #define P_FIRST_OPCODE 3 > -#define P_LAST_OPCODE 10 > +#define P_LAST_OPCODE 11 > > /* > * Define number of buffers for send and receive in the reliability layer. > @@ -86,6 +90,8 @@ enum first_packet_verdict { > /** This packet is a valid ACK control packet from the peer, > * i.e. it has a valid session id hmac in it */ > VERDICT_VALID_ACK_V1, > + /** The packet is a valid control packet with appended wrapped client key */ > + VERDICT_VALID_WKC_V1, > /** the packet failed on of the various checks */ > VERDICT_INVALID > }; > @@ -217,10 +223,12 @@ read_control_auth(struct buffer *buf, > * The returned buf needs to be free with \c free_buf > */ > struct buffer > -tls_reset_standalone(struct tls_auth_standalone *tas, > +tls_reset_standalone(struct tls_wrap_ctx *ctx, > + struct tls_auth_standalone *tas, > struct session_id *own_sid, > struct session_id *remote_sid, > - uint8_t header); > + uint8_t header, > + bool tlscryptv2); > > static inline const char * > packet_opcode_name(int op) > @@ -248,6 +256,9 @@ packet_opcode_name(int op) > case P_CONTROL_V1: > return "P_CONTROL_V1"; > > + case P_CONTROL_WKC_V1: > + return "P_CONTROL_WKC_V1"; > + > case P_ACK_V1: > return "P_ACK_V1"; > > @@ -261,4 +272,23 @@ packet_opcode_name(int op) > return "P_???"; > } > } > + > +/* initial packet id (instead of 0) that indicates that the peer supports > + * early protocol negotiation. This will make the packet id turn a bit faster > + * but the network time part of the packet id could take care of that. And > + * this is also a rather theoretical scenario as it still needs more than > + * 2^31 control channel packets to happen */ > +#define EARLY_NEG_MASK 0xff000000 > +#define EARLY_NEG_START 0x0a000000 > + > +#define EARLY_NEG_RESENDWKC 0x00010000 > + > + > +/* Early negotiation that part of the server response in the RESET_V2 packet. > + * Since clients that announce early negotiation support will treat the payload > + * of reset packets special and parse it as TLV messages. > + * as TLV (type, length, value) */ > +#define EARLY_NEG_TLV_FLAG 0x01 > +#define EARLY_NEG_FLAG_RESEND_WKC 0x01 The two constants above are used to fill uint16_t fields, therefore I suggest to represent the full form, for clarity. I.e. 0x0001 Moreover, these constants belong to the TLV space and we may add more in the future, therefore I suggest changing the name format to something like "TLV_TYPE_*" and "TLV_VALUE_*". So they'll become: TLV_TYPE_EARLY_NEG_FLAG TLV_VALUE_EARLY_NEG_FLAG_RESEND_WKC In the fuure we will have more TLV_TYPE_* and TLV_VALUE_* and so they'll be properly namescope'd. > #endif /* ifndef SSL_PKT_H */ > + Spurious empty line at the end of the file > diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c > index 36812628e..f3fddf870 100644 > --- a/tests/unit_tests/openvpn/test_pkt.c > +++ b/tests/unit_tests/openvpn/test_pkt.c > @@ -536,7 +536,7 @@ test_generate_reset_packet_plain(void **ut_state) > > uint8_t header = 0 | (P_CONTROL_HARD_RESET_CLIENT_V2 << P_OPCODE_SHIFT); > > - struct buffer buf = tls_reset_standalone(&tas, &client_id, &server_id, header); > + struct buffer buf = tls_reset_standalone(&tas.tls_wrap, &tas, &client_id, &server_id, header, false); > > > verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf); > @@ -544,7 +544,7 @@ test_generate_reset_packet_plain(void **ut_state) > > /* Assure repeated generation of reset is deterministic/stateless*/ > assert_memory_equal(state.peer_session_id.id, client_id.id, SID_SIZE); > - struct buffer buf2 = tls_reset_standalone(&tas, &client_id, &server_id, header); > + struct buffer buf2 = tls_reset_standalone(&tas.tls_wrap, &tas, &client_id, &server_id, header, false); > assert_int_equal(BLEN(&buf), BLEN(&buf2)); > assert_memory_equal(BPTR(&buf), BPTR(&buf2), BLEN(&buf)); > free_buf(&buf2); > @@ -571,7 +571,7 @@ test_generate_reset_packet_tls_auth(void **ut_state) > > now = 0x22446688; > reset_packet_id_send(&tas_client.tls_wrap.opt.packet_id.send); > - struct buffer buf = tls_reset_standalone(&tas_client, &client_id, &server_id, header); > + struct buffer buf = tls_reset_standalone(&tas_client.tls_wrap, &tas_client, &client_id, &server_id, header, false); > > enum first_packet_verdict verdict = tls_pre_decrypt_lite(&tas_server, &state, &from, &buf); > assert_int_equal(verdict, VERDICT_VALID_RESET_V2); > @@ -580,11 +580,11 @@ test_generate_reset_packet_tls_auth(void **ut_state) > > /* Assure repeated generation of reset is deterministic/stateless*/ > reset_packet_id_send(&tas_client.tls_wrap.opt.packet_id.send); > - struct buffer buf2 = tls_reset_standalone(&tas_client, &client_id, &server_id, header); > + struct buffer buf2 = tls_reset_standalone(&tas_client.tls_wrap, &tas_client, &client_id, &server_id, header,false); > assert_int_equal(BLEN(&buf), BLEN(&buf2)); > assert_memory_equal(BPTR(&buf), BPTR(&buf2), BLEN(&buf)); > + > free_buf(&buf2); > - Not sure why you are changing this in this patch? Seems irrelevant, but if you really want to, please remove the empty spaces in the new empty line. > free_tls_pre_decrypt_state(&state); > > packet_id_free(&tas_client.tls_wrap.opt.packet_id); Rest looks good and the TLV is a very nice thing to have! Regards,
diff --git a/Changes.rst b/Changes.rst index ceb0b2680..67a23c792 100644 --- a/Changes.rst +++ b/Changes.rst @@ -69,6 +69,16 @@ Improved ``--mssfix`` and ``--fragment`` calculation account and the resulting size is specified as the total size of the VPN packets including IP and UDP headers. +Cookie based handshake for UDP server + Instead of allocating a connection for each client on the initial packet + OpenVPN server will now use an HMAC based cookie as its session id. This + way the server can verify it on completing the handshake without keeping + state. This eliminates the amplification and resource exhaustion attacks. + For tls-crypt-v2 clients, this requires OpenVPN 2.6 clients or later + because the client needs to resend its client key on completing the hand + shake. The tls-crypt-v2 option allows controlling if older clients are + accepted. + Deprecated features ------------------- ``inetd`` has been removed diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst index ac5756034..c06ee3354 100644 --- a/doc/man-sections/tls-options.rst +++ b/doc/man-sections/tls-options.rst @@ -486,6 +486,13 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa 8000 years'. --tls-crypt-v2 keyfile + + Valid syntax: + :: + tls-crypt-v2 keyfile + tls-crypt-v2 keyfile force-cookie + tls-crypt-v2 keyfile allow-noncookie + Use client-specific tls-crypt keys. For clients, ``keyfile`` is a client-specific tls-crypt key. Such a key @@ -501,6 +508,13 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa client is using client-specific keys, and automatically select the right mode. + The optional parameters :code:`force-cookie` allows only tls-crypt-v2 + clients that support a cookie based stateless three way handshake that + avoids replay attacks and state exhaustion on the server side (OpenVPN + 2.6 and later). The option :code:`allow-noncookie` explicitly allows + older tls-crypt-v2 clients. The default is (currently) + :code:`allow-noncookie`. + --tls-crypt-v2-verify cmd Run command ``cmd`` to verify the metadata of the client-specific tls-crypt-v2 key of a connecting client. This allows server diff --git a/doc/tls-crypt-v2.txt b/doc/tls-crypt-v2.txt index f6a6a1395..eb7b7138b 100644 --- a/doc/tls-crypt-v2.txt +++ b/doc/tls-crypt-v2.txt @@ -157,6 +157,47 @@ When setting up the openvpn connection: messages. +HMAC Cookie support +------------------- +To avoid exhaustion attack and keeping state for connections that fail to +complete the three-way handshake, the OpenVPN server will use its own session +id as challenge that the client must repeat in the third packet of the +handshake. This introduces a problem. If the server does not keep the wrapped +client key from the initial packet, the server cannot decode the third packet. +Therefore, tls-crypt-v2 in 2.6 allows resending the wrapped key in the third +packet of the handshake with the P_CONTROL_WKC_V1 message. The modified +handshake is as follows (the rest of the handshake is unmodified): + +1. The client creates the P_CONTROL_HARD_RESET_CLIENT_V3 message as before + but indicates that it supports resending the wrapped key. This is done + by setting the packet id of the replay id to 0x0f010000. The first byte + indicates the early negotiation support and the next byte the flags. + +2. The server responds with a P_CONTROL_HARD_RESET_V2 message. Instead of having + an empty payload like normally, the payload consists of TLV (type (uint16), + length (uint16), value) packets. TLV was chosen + to allow extensibility in the future. Currently only the following TLV is + defined: + + flags - type 0x01, length 2. + + Bit 1 indicates that the client needs to resend the WKc in the third packet. + +3. Instead of normal P_ACK_V1 or P_CONTROL_V1 packet, the client will send a + P_CONTROL_WKC_V1 packet. The P_CONTROL_WKC_V1 is identical to a normal + P_CONTROL_V1 packet but with the WKc appended. + + Normally the first message of the client is either P_ACK_V1, directly + followed by a P_CONTROL_V1 message that contains the TLS Client Hello or + just a P_CONTROL_V1 message. Instead of a P_ACK_V1 message the client should + send a P_CONTROL_WKC_V1 message with an empty payload. This message must + also include an ACK for the P_CONTROL_HARD_RESET_V2 message. + + When directly sending the TLS Client Hello message in the P_CONTROL_WKC_V1 + message, the client must ensure that the resulting P_CONTROL_WKC_V1 message + with the appended WKc does not extend the control message length. + + Considerations -------------- diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index 806632edf..98e2c7664 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -256,6 +256,14 @@ struct crypto_options /**< Bit-flag indicating that data channel key derivation * is done using TLS keying material export [RFC5705] */ +#define CO_RESEND_WKC (1<<4) + /**< Bit-flag indicating that the client is expected to + * resend the wrapped client key with the 2nd packet (packet-id 1) + * like with the HARD_RESET_CLIENT_V3 packet */ +#define CO_FORCE_TLSCRYPTV2_COOKIE (1<<5) + /**< Bit-flag indicating that we do not allow clients that do + * not support resending the wrapped client key (WKc) with the + * third packet of the three-way handshake */ unsigned int flags; /**< Bit-flags determining behavior of * security operation functions. */ }; diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 7e7041a8e..85e146bf2 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2960,6 +2960,10 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) { to.tls_wrap.tls_crypt_v2_server_key = c->c1.ks.tls_crypt_v2_server_key; to.tls_crypt_v2_verify_script = c->options.tls_crypt_v2_verify_script; + if (options->ce.tls_crypt_v2_force_cookie) + { + to.tls_wrap.opt.flags |= CO_FORCE_TLSCRYPTV2_COOKIE; + } } } diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c index ead61e827..88cd6a497 100644 --- a/src/openvpn/mudp.c +++ b/src/openvpn/mudp.c @@ -40,6 +40,30 @@ #include <sys/inotify.h> #endif +static void +send_hmac_reset_packet(struct multi_context *m, + struct tls_pre_decrypt_state *state, + struct tls_auth_standalone *tas, + struct session_id *sid, + bool tlscryptv2) +{ + reset_packet_id_send(&state->tls_wrap_tmp.opt.packet_id.send); + state->tls_wrap_tmp.opt.packet_id.rec.initialized = true; + uint8_t header = 0 | (P_CONTROL_HARD_RESET_SERVER_V2 << P_OPCODE_SHIFT); + struct buffer buf = tls_reset_standalone(&state->tls_wrap_tmp, tas, sid, + &state->peer_session_id, header, + tlscryptv2); + + struct context *c = &m->top; + + buf_reset_len(&c->c2.buffers->aux_buf); + buf_copy(&c->c2.buffers->aux_buf, &buf); + m->hmac_reply = c->c2.buffers->aux_buf; + m->hmac_reply_dest = &m->top.c2.from; + msg(D_MULTI_DEBUG, "Reset packet from client, sending HMAC based reset challenge"); +} + + /* Return if this packet should create a new session */ static bool do_pre_decrypt_check(struct multi_context *m, @@ -58,37 +82,62 @@ do_pre_decrypt_check(struct multi_context *m, struct openvpn_sockaddr *from = &m->top.c2.from.dest; int handwindow = m->top.options.handshake_window; - if (verdict == VERDICT_VALID_RESET_V3) { - /* For tls-crypt-v2 we need to keep the state of the first packet to - * store the unwrapped key */ - return true; + /* Extract the packet id to check if it has the special format that + * indicates early negotiation support */ + struct packet_id_net pin; + struct buffer tmp = m->top.c2.buf; + ASSERT(buf_advance(&tmp, 1 + SID_SIZE)); + ASSERT(packet_id_read(&pin, &tmp, true)); + + /* The most significant byte ist 0x0f if early negotiation is supported */ + bool early_neg_support = (pin.id & EARLY_NEG_MASK) == EARLY_NEG_START; + + if (early_neg_support && (pin.id & EARLY_NEG_RESENDWKC)) + { + /* Calculate the session ID HMAC for our reply and create reset packet */ + struct session_id sid = calculate_session_id_hmac(state->peer_session_id, + from, hmac, handwindow, 0); + send_hmac_reset_packet(m, state, tas, &sid, true); + + return false; + } + else + { + /* For tls-crypt-v2 we need to keep the state of the first packet + * to store the unwrapped key if the client doesn't support resending + * the wrapped key */ + if (tas->tls_wrap.opt.flags & CO_FORCE_TLSCRYPTV2_COOKIE) + { + struct gc_arena gc = gc_new(); + const char *peer = print_link_socket_actual(&m->top.c2.from, &gc); + msg(D_MULTI_DEBUG, "tls-crypt-v2 force-cookie is enabled," + "ignoring connection attempt from old client" + " (%s)", peer); + gc_free(&gc); + return false; + } + else + { + return true; + } + } } else if (verdict == VERDICT_VALID_RESET_V2) { /* Calculate the session ID HMAC for our reply and create reset packet */ struct session_id sid = calculate_session_id_hmac(state->peer_session_id, from, hmac, handwindow, 0); - reset_packet_id_send(&tas->tls_wrap.opt.packet_id.send); - tas->tls_wrap.opt.packet_id.rec.initialized = true; - uint8_t header = 0 | (P_CONTROL_HARD_RESET_SERVER_V2 << P_OPCODE_SHIFT); - struct buffer buf = tls_reset_standalone(tas, &sid, - &state->peer_session_id, header); - - struct context *c = &m->top; + send_hmac_reset_packet(m, state, tas, &sid, false); - buf_reset_len(&c->c2.buffers->aux_buf); - buf_copy(&c->c2.buffers->aux_buf, &buf); - m->hmac_reply = c->c2.buffers->aux_buf; - m->hmac_reply_dest = &m->top.c2.from; - msg(D_MULTI_DEBUG, "Reset packet from client, sending HMAC based reset challenge"); /* We have a reply do not create a new session */ return false; } - else if (verdict == VERDICT_VALID_CONTROL_V1 || verdict == VERDICT_VALID_ACK_V1) + else if (verdict == VERDICT_VALID_CONTROL_V1 || verdict == VERDICT_VALID_ACK_V1 + || verdict == VERDICT_VALID_WKC_V1) { /* ACK_V1 contains the peer id (our id) while CONTROL_V1 can but does not * need to contain the peer id */ @@ -99,9 +148,7 @@ do_pre_decrypt_check(struct multi_context *m, const char *peer = print_link_socket_actual(&m->top.c2.from, &gc); if (!ret) { - msg(D_MULTI_MEDIUM, "Packet with invalid or missing SID from %s", peer); - } else { diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 7f5c903d1..9ff384d09 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -8896,6 +8896,19 @@ add_option(struct options *options, options->ce.tls_crypt_v2_file = p[1]; options->ce.tls_crypt_v2_file_inline = is_inline; } + + if (p[2] && streq(p[2], "force-cookie")) + { + options->ce.tls_crypt_v2_force_cookie = true; + } + else if (p[2] && streq(p[2], "allow-noncookie")) + { + options->ce.tls_crypt_v2_force_cookie = false; + } + else if (p[2]) + { + msg(msglevel, "Unsupported tls-crypt-v2 argument: %s", p[2]); + } } else if (streq(p[0], "tls-crypt-v2-verify") && p[1] && !p[2]) { diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 055789b3b..c2937dc37 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -162,6 +162,9 @@ struct connection_entry * authenticated encryption v2 */ const char *tls_crypt_v2_file; bool tls_crypt_v2_file_inline; + + /* Allow only client that support resending the wrapped client key */ + bool tls_crypt_v2_force_cookie; }; struct remote_entry diff --git a/src/openvpn/reliable.h b/src/openvpn/reliable.h index 8da2c0729..b9863efe3 100644 --- a/src/openvpn/reliable.h +++ b/src/openvpn/reliable.h @@ -361,8 +361,6 @@ struct reliable_entry *reliable_get_entry_sequenced(struct reliable *rel); * * @param rel The reliable structure associated with the given buffer. * @param buf The buffer of the reliable entry which is to be removed. - * @param inc_pid If true, the reliable structure's packet ID counter - * will be incremented. */ void reliable_mark_deleted(struct reliable *rel, struct buffer *buf); diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 8ea7c06fa..5e1a23ccd 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1099,6 +1099,14 @@ tls_session_init(struct tls_multi *multi, struct tls_session *session) session->opt->replay_time, "TLS_WRAP", session->key_id); + /* If we are using tls-crypt-v2 we manipulate the packet id to be (ab)used + * to indicate early protocol negotiation */ + if (session->opt->tls_crypt_v2) + { + session->tls_wrap.opt.packet_id.send.time = now; + session->tls_wrap.opt.packet_id.send.id = EARLY_NEG_START | EARLY_NEG_RESENDWKC; + } + /* load most recent packet-id to replay protect on --tls-auth */ packet_id_persist_load_obj(session->tls_wrap.opt.pid_persist, &session->tls_wrap.opt.packet_id); @@ -2525,6 +2533,53 @@ session_skip_to_pre_start(struct tls_session *session, return session_move_pre_start(session, ks, true); } +/** + * Parses the TLVs (type, length, value) in the early negotiation + */ +static bool +parse_early_negotiation_tlvs(struct buffer *buf, struct key_state *ks) +{ + while (buf->len > 0) + { + if (buf_len(buf) < 4) + { + goto error; + } + /* read type */ + uint16_t type = buf_read_u16(buf); + uint16_t len = buf_read_u16(buf); + if (buf_len(buf) < len) + { + goto error; + } + + if (type == EARLY_NEG_TLV_FLAG) + { + if (len != sizeof(uint16_t)) + { + goto error; + } + uint16_t flags = buf_read_u16(buf); + + if (flags & EARLY_NEG_FLAG_RESEND_WKC) + { + ks->crypto_options.flags |= CO_RESEND_WKC; + } + } + else + { + /* Skip types we do not parse */ + buf_advance(buf, len); + } + } + reliable_mark_deleted(ks->rec_reliable, buf); + + return true; +error: + msg(D_TLS_ERRORS, "TLS Error: Early negotiation malformed packet"); + return false; +} + /** * Read incoming ciphertext and passes it to the buffer of the SSL library. * Returns false if an error is encountered that should abort the session. @@ -2557,6 +2612,13 @@ read_incoming_tls_ciphertext(struct buffer *buf, struct key_state *ks, return true; } +static bool +control_packet_needs_wkc(const struct key_state *ks) +{ + return (ks->crypto_options.flags & CO_RESEND_WKC) + && (ks->send_reliable->packet_id == 1); +} + static bool tls_process_state(struct tls_multi *multi, @@ -2626,9 +2688,21 @@ tls_process_state(struct tls_multi *multi, struct reliable_entry *entry = reliable_get_entry_sequenced(ks->rec_reliable); if (entry) { - if (!read_incoming_tls_ciphertext(&entry->buf, ks, &state_change)) + /* The first packet from the peer (the reset packet) is special and + * contains early protocol negotiation */ + if (entry->packet_id == 0 && is_hard_reset_method2(entry->opcode)) { - goto error; + if (!parse_early_negotiation_tlvs(&entry->buf, ks)) + { + goto error; + } + } + else + { + if (!read_incoming_tls_ciphertext(&entry->buf, ks, &state_change)) + { + goto error; + } } } @@ -2721,7 +2795,12 @@ tls_process_state(struct tls_multi *multi, } if (status == 1) { - reliable_mark_active_outgoing(ks->send_reliable, buf, P_CONTROL_V1); + int opcode = P_CONTROL_V1; + if (control_packet_needs_wkc(ks)) + { + opcode = P_CONTROL_WKC_V1; + } + reliable_mark_active_outgoing(ks->send_reliable, buf, opcode); INCR_GENERATED; state_change = true; dmsg(D_TLS_DEBUG, "Outgoing Ciphertext -> Reliable"); @@ -2811,15 +2890,38 @@ tls_process(struct tls_multi *multi, update_time(); + /* We often send acks back to back to a following control packet. This + * normally does not create a problem (apart from an extra packet. However, + * with the P_CONTROL_WKC_V1 we need to ensure that the packet gets resend + * if not received by remote, so instead we use an empty control packet in + * this special case */ + + /* Send 1 or more ACKs (each received control packet gets one ACK) */ if (!to_link->len && !reliable_ack_empty(ks->rec_ack)) { - struct buffer buf = ks->ack_write_buf; - ASSERT(buf_init(&buf, multi->opt.frame.buf.headroom)); - write_control_auth(session, ks, &buf, to_link_addr, P_ACK_V1, - RELIABLE_ACK_SIZE, false); - *to_link = buf; - dmsg(D_TLS_DEBUG, "Dedicated ACK -> TCP/UDP"); + if (control_packet_needs_wkc(ks)) + { + struct buffer *buf = reliable_get_buf_output_sequenced(ks->send_reliable); + if (!buf) + { + return false; + } + + /* We do not write anything to the buffer, this way this will be + * an empty control packet that gets the ack piggybacked and + * also appended the wrapped client key since it has a WCK opcode */ + reliable_mark_active_outgoing(ks->send_reliable, buf, P_CONTROL_WKC_V1); + } + else + { + struct buffer buf = ks->ack_write_buf; + ASSERT(buf_init(&buf, multi->opt.frame.buf.headroom)); + write_control_auth(session, ks, &buf, to_link_addr, P_ACK_V1, + RELIABLE_ACK_SIZE, false); + *to_link = buf; + dmsg(D_TLS_DEBUG, "Dedicated ACK -> TCP/UDP"); + } } /* When should we wake up again? */ @@ -3464,7 +3566,8 @@ tls_pre_decrypt(struct tls_multi *multi, } /* - * We have an authenticated control channel packet (if --tls-auth was set). + * We have an authenticated control channel packet (if --tls-auth/tls-crypt + * or tls-crypt-v2 was set). * Now pass to our reliability layer which deals with * packet acknowledgements, retransmits, sequencing, etc. */ @@ -3893,7 +3996,7 @@ protocol_dump(struct buffer *buffer, unsigned int flags, struct gc_arena *gc) if (op == P_ACK_V1) { - goto done; + goto print_data; } /* diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c index 9c8154b12..2376446ee 100644 --- a/src/openvpn/ssl_pkt.c +++ b/src/openvpn/ssl_pkt.c @@ -148,7 +148,8 @@ tls_wrap_control(struct tls_wrap_ctx *ctx, uint8_t header, struct buffer *buf, return; } - if ((header >> P_OPCODE_SHIFT) == P_CONTROL_HARD_RESET_CLIENT_V3) + if ((header >> P_OPCODE_SHIFT) == P_CONTROL_HARD_RESET_CLIENT_V3 + || (header >> P_OPCODE_SHIFT) == P_CONTROL_WKC_V1) { if (!buf_copy(&ctx->work, ctx->tls_crypt_v2_wkc)) @@ -197,7 +198,8 @@ read_control_auth(struct buffer *buf, bool ret = false; const uint8_t opcode = *(BPTR(buf)) >> P_OPCODE_SHIFT; - if (opcode == P_CONTROL_HARD_RESET_CLIENT_V3 + if ((opcode == P_CONTROL_HARD_RESET_CLIENT_V3 + || opcode == P_CONTROL_WKC_V1) && !tls_crypt_v2_extract_client_key(buf, ctx, opt)) { msg(D_TLS_ERRORS, @@ -321,6 +323,7 @@ tls_pre_decrypt_lite(const struct tls_auth_standalone *tas, if (op != P_CONTROL_HARD_RESET_CLIENT_V2 && op != P_CONTROL_HARD_RESET_CLIENT_V3 && op != P_CONTROL_V1 + && op != P_CONTROL_WKC_V1 && op != P_ACK_V1) { /* @@ -397,6 +400,10 @@ tls_pre_decrypt_lite(const struct tls_auth_standalone *tas, { return VERDICT_VALID_RESET_V3; } + else if (op == P_CONTROL_WKC_V1) + { + return VERDICT_VALID_WKC_V1; + } else { return VERDICT_VALID_RESET_V2; @@ -410,10 +417,12 @@ error: struct buffer -tls_reset_standalone(struct tls_auth_standalone *tas, +tls_reset_standalone(struct tls_wrap_ctx *ctx, + struct tls_auth_standalone *tas, struct session_id *own_sid, struct session_id *remote_sid, - uint8_t header) + uint8_t header, + bool tlscryptv2) { struct buffer buf = alloc_buf(tas->frame.buf.payload_size); ASSERT(buf_init(&buf, tas->frame.buf.headroom)); @@ -434,8 +443,17 @@ tls_reset_standalone(struct tls_auth_standalone *tas, ASSERT(buf_write(&buf, &net_pid, sizeof(net_pid))); + /* Add indication for tls-crypt-v2 to resend the packet with the with + * reply */ + if (tlscryptv2) + { + buf_write_u16(&buf, EARLY_NEG_TLV_FLAG); /* TYPE: flags */ + buf_write_u16(&buf, sizeof(uint16_t)); + buf_write_u16(&buf, EARLY_NEG_FLAG_RESEND_WKC); + } + /* Add tls-auth/tls-crypt wrapping, this might replace buf */ - tls_wrap_control(&tas->tls_wrap, header, &buf, own_sid); + tls_wrap_control(ctx, header, &buf, own_sid); return buf; } diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h index ae92f6b33..9426fb1d8 100644 --- a/src/openvpn/ssl_pkt.h +++ b/src/openvpn/ssl_pkt.h @@ -54,11 +54,15 @@ /* indicates key_method >= 2 and client-specific tls-crypt key */ #define P_CONTROL_HARD_RESET_CLIENT_V3 10 /* initial key from client, forget previous state */ +/* Variant of P_CONTROL_V1 but with appended wrapped key + * like P_CONTROL_HARD_RESET_CLIENT_V3 */ +#define P_CONTROL_WKC_V1 11 + /* define the range of legal opcodes * Since we do no longer support key-method 1 we consider * the v1 op codes invalid */ #define P_FIRST_OPCODE 3 -#define P_LAST_OPCODE 10 +#define P_LAST_OPCODE 11 /* * Define number of buffers for send and receive in the reliability layer. @@ -86,6 +90,8 @@ enum first_packet_verdict { /** This packet is a valid ACK control packet from the peer, * i.e. it has a valid session id hmac in it */ VERDICT_VALID_ACK_V1, + /** The packet is a valid control packet with appended wrapped client key */ + VERDICT_VALID_WKC_V1, /** the packet failed on of the various checks */ VERDICT_INVALID }; @@ -217,10 +223,12 @@ read_control_auth(struct buffer *buf, * The returned buf needs to be free with \c free_buf */ struct buffer -tls_reset_standalone(struct tls_auth_standalone *tas, +tls_reset_standalone(struct tls_wrap_ctx *ctx, + struct tls_auth_standalone *tas, struct session_id *own_sid, struct session_id *remote_sid, - uint8_t header); + uint8_t header, + bool tlscryptv2); static inline const char * packet_opcode_name(int op) @@ -248,6 +256,9 @@ packet_opcode_name(int op) case P_CONTROL_V1: return "P_CONTROL_V1"; + case P_CONTROL_WKC_V1: + return "P_CONTROL_WKC_V1"; + case P_ACK_V1: return "P_ACK_V1"; @@ -261,4 +272,23 @@ packet_opcode_name(int op) return "P_???"; } } + +/* initial packet id (instead of 0) that indicates that the peer supports + * early protocol negotiation. This will make the packet id turn a bit faster + * but the network time part of the packet id could take care of that. And + * this is also a rather theoretical scenario as it still needs more than + * 2^31 control channel packets to happen */ +#define EARLY_NEG_MASK 0xff000000 +#define EARLY_NEG_START 0x0a000000 + +#define EARLY_NEG_RESENDWKC 0x00010000 + + +/* Early negotiation that part of the server response in the RESET_V2 packet. + * Since clients that announce early negotiation support will treat the payload + * of reset packets special and parse it as TLV messages. + * as TLV (type, length, value) */ +#define EARLY_NEG_TLV_FLAG 0x01 +#define EARLY_NEG_FLAG_RESEND_WKC 0x01 #endif /* ifndef SSL_PKT_H */ + diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c index 36812628e..f3fddf870 100644 --- a/tests/unit_tests/openvpn/test_pkt.c +++ b/tests/unit_tests/openvpn/test_pkt.c @@ -536,7 +536,7 @@ test_generate_reset_packet_plain(void **ut_state) uint8_t header = 0 | (P_CONTROL_HARD_RESET_CLIENT_V2 << P_OPCODE_SHIFT); - struct buffer buf = tls_reset_standalone(&tas, &client_id, &server_id, header); + struct buffer buf = tls_reset_standalone(&tas.tls_wrap, &tas, &client_id, &server_id, header, false); verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf); @@ -544,7 +544,7 @@ test_generate_reset_packet_plain(void **ut_state) /* Assure repeated generation of reset is deterministic/stateless*/ assert_memory_equal(state.peer_session_id.id, client_id.id, SID_SIZE); - struct buffer buf2 = tls_reset_standalone(&tas, &client_id, &server_id, header); + struct buffer buf2 = tls_reset_standalone(&tas.tls_wrap, &tas, &client_id, &server_id, header, false); assert_int_equal(BLEN(&buf), BLEN(&buf2)); assert_memory_equal(BPTR(&buf), BPTR(&buf2), BLEN(&buf)); free_buf(&buf2); @@ -571,7 +571,7 @@ test_generate_reset_packet_tls_auth(void **ut_state) now = 0x22446688; reset_packet_id_send(&tas_client.tls_wrap.opt.packet_id.send); - struct buffer buf = tls_reset_standalone(&tas_client, &client_id, &server_id, header); + struct buffer buf = tls_reset_standalone(&tas_client.tls_wrap, &tas_client, &client_id, &server_id, header, false); enum first_packet_verdict verdict = tls_pre_decrypt_lite(&tas_server, &state, &from, &buf); assert_int_equal(verdict, VERDICT_VALID_RESET_V2); @@ -580,11 +580,11 @@ test_generate_reset_packet_tls_auth(void **ut_state) /* Assure repeated generation of reset is deterministic/stateless*/ reset_packet_id_send(&tas_client.tls_wrap.opt.packet_id.send); - struct buffer buf2 = tls_reset_standalone(&tas_client, &client_id, &server_id, header); + struct buffer buf2 = tls_reset_standalone(&tas_client.tls_wrap, &tas_client, &client_id, &server_id, header,false); assert_int_equal(BLEN(&buf), BLEN(&buf2)); assert_memory_equal(BPTR(&buf), BPTR(&buf2), BLEN(&buf)); + free_buf(&buf2); - free_tls_pre_decrypt_state(&state); packet_id_free(&tas_client.tls_wrap.opt.packet_id);
Tls-crypt v2 is more complicated to implement a proper stateless handshake. To allow state handshake this commit does - introduce a new packet CONTROL_WKC_V1 that repeats the wrapped client key. - introduce a way to negotiate the support for this packet in the three way handshake Details about the protocol changes are in tls-crypt-v2.txt. Optional arguments to the tls-crypt-v2 option have been added to explicitly allow or disallow client that do not support the stateless handshake. Signed-off-by: Arne Schwabe <arne@rfc2549.org> Patch v3: improve grammar, style, comments, fix unit tests --- Changes.rst | 10 +++ doc/man-sections/tls-options.rst | 14 ++++ doc/tls-crypt-v2.txt | 41 +++++++++ src/openvpn/crypto.h | 8 ++ src/openvpn/init.c | 4 + src/openvpn/mudp.c | 85 ++++++++++++++----- src/openvpn/options.c | 13 +++ src/openvpn/options.h | 3 + src/openvpn/reliable.h | 2 - src/openvpn/ssl.c | 125 +++++++++++++++++++++++++--- src/openvpn/ssl_pkt.c | 28 +++++-- src/openvpn/ssl_pkt.h | 36 +++++++- tests/unit_tests/openvpn/test_pkt.c | 10 +-- 13 files changed, 334 insertions(+), 45 deletions(-)