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 <a...@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,
--
Antonio Quartulli
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel