Skip to content

Commit 0683f33

Browse files
Fix P2P epoch MTU accounting
P2P NCP stores the negotiated epoch flag in the live TLS options, while the MTU calculator reads imported protocol flags from the main options structure. Synchronize that flag for non-pull point-to-point sessions before calculating dynamic framing, and clear it when a later negotiation does not use epoch data keys. Preserve the normal pull-client path. Related: #1074 Signed-off-by: Darren Carreras <carrerasdarren@gmail.com>
1 parent d110697 commit 0683f33

6 files changed

Lines changed: 72 additions & 0 deletions

File tree

CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,8 @@ if (BUILD_TESTING)
842842
src/openvpn/crypto_mbedtls.c
843843
src/openvpn/crypto_openssl.c
844844
src/openvpn/crypto.c
845+
src/openvpn/mss.c
846+
src/openvpn/mtu.c
845847
src/openvpn/otime.c
846848
src/openvpn/packet_id.c
847849
src/openvpn/ssl_util.c

src/openvpn/ssl.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,6 +1651,8 @@ tls_session_update_crypto_params(struct tls_multi *multi, struct tls_session *se
16511651
return false;
16521652
}
16531653

1654+
p2p_ncp_update_options(options, session);
1655+
16541656
/* Import crypto settings that might be set by pull/push */
16551657
session->opt->crypto_flags |= options->imported_protocol_flags;
16561658

src/openvpn/ssl_ncp.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,19 @@ p2p_ncp_set_options(struct tls_multi *multi, struct tls_session *session, const
470470
}
471471
}
472472

473+
void
474+
p2p_ncp_update_options(struct options *options, const struct tls_session *session)
475+
{
476+
if (options->mode != MODE_POINT_TO_POINT || options->pull)
477+
{
478+
return;
479+
}
480+
481+
options->imported_protocol_flags &= ~CO_EPOCH_DATA_KEY_FORMAT;
482+
options->imported_protocol_flags |=
483+
session->opt->crypto_flags & CO_EPOCH_DATA_KEY_FORMAT;
484+
}
485+
473486
void
474487
p2p_mode_ncp(struct tls_multi *multi, struct tls_session *session)
475488
{

src/openvpn/ssl_ncp.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,12 @@ bool tls_item_in_cipher_list(const char *item, const char *list);
130130
*/
131131
void p2p_mode_ncp(struct tls_multi *multi, struct tls_session *session);
132132

133+
/**
134+
* Copy P2P-negotiated protocol flags needed outside the TLS session into the
135+
* main options structure.
136+
*/
137+
void p2p_ncp_update_options(struct options *options, const struct tls_session *session);
138+
133139
/**
134140
* Determines the best common cipher from both peers IV_CIPHER lists. The
135141
* first cipher from the tls-server that is also in the tls-client IV_CIPHER

tests/unit_tests/openvpn/Makefile.am

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,8 @@ ncp_testdriver_SOURCES = test_ncp.c \
341341
$(top_srcdir)/src/openvpn/crypto_mbedtls.c \
342342
$(top_srcdir)/src/openvpn/crypto_mbedtls_legacy.c \
343343
$(top_srcdir)/src/openvpn/crypto_openssl.c \
344+
$(top_srcdir)/src/openvpn/mss.c \
345+
$(top_srcdir)/src/openvpn/mtu.c \
344346
$(top_srcdir)/src/openvpn/otime.c \
345347
$(top_srcdir)/src/openvpn/packet_id.c \
346348
$(top_srcdir)/src/openvpn/platform.c \

tests/unit_tests/openvpn/test_ncp.c

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <cmocka.h>
3535

3636
#include "ssl_ncp.c"
37+
#include "mss.h"
3738
#include "test_common.h"
3839

3940
/* Defines for use in the tests and the mock parse_line() */
@@ -392,6 +393,51 @@ test_ncp_expand(void **state)
392393
gc_free(&gc);
393394
}
394395

396+
static void
397+
test_p2p_epoch_mssfix_mtu(void **state)
398+
{
399+
struct tls_options tls_options = { .data_epoch_supported = true };
400+
struct tls_session session = { .opt = &tls_options };
401+
struct tls_multi multi = { 0 };
402+
struct options o = { .mode = MODE_POINT_TO_POINT };
403+
char peer_info[32];
404+
405+
snprintf(peer_info, sizeof(peer_info), "IV_PROTO=%u",
406+
IV_PROTO_DATA_V2 | IV_PROTO_NCP_P2P | IV_PROTO_DATA_EPOCH);
407+
multi.peer_info = peer_info;
408+
409+
p2p_ncp_set_options(&multi, &session, "AES-256-GCM");
410+
assert_true(session.opt->crypto_flags & CO_EPOCH_DATA_KEY_FORMAT);
411+
p2p_ncp_update_options(&o, &session);
412+
assert_true(o.imported_protocol_flags & CO_EPOCH_DATA_KEY_FORMAT);
413+
414+
o.ce.tun_mtu = 1400;
415+
o.ce.mssfix = 1000;
416+
o.ce.proto = PROTO_UDP;
417+
o.ciphername = "AES-256-GCM";
418+
o.authname = "SHA1";
419+
o.tls_client = true;
420+
o.use_peer_id = true;
421+
422+
struct key_type kt;
423+
init_key_type(&kt, o.ciphername, o.authname, true, false);
424+
425+
struct frame frame = { 0 };
426+
frame_calculate_dynamic(&frame, &kt, &o, NULL);
427+
428+
/* opcode/peer-id + epoch packet-id + tag + TCP/IP headers */
429+
assert_int_equal(frame.mss_fix, 1000 - 4 - 8 - 16 - 40);
430+
431+
session.opt->crypto_flags &= ~CO_EPOCH_DATA_KEY_FORMAT;
432+
p2p_ncp_update_options(&o, &session);
433+
assert_false(o.imported_protocol_flags & CO_EPOCH_DATA_KEY_FORMAT);
434+
435+
o.pull = true;
436+
o.imported_protocol_flags |= CO_EPOCH_DATA_KEY_FORMAT;
437+
p2p_ncp_update_options(&o, &session);
438+
assert_true(o.imported_protocol_flags & CO_EPOCH_DATA_KEY_FORMAT);
439+
}
440+
395441

396442
const struct CMUnitTest ncp_tests[] = {
397443
cmocka_unit_test(test_check_ncp_ciphers_list),
@@ -400,6 +446,7 @@ const struct CMUnitTest ncp_tests[] = {
400446
cmocka_unit_test(test_ncp_best),
401447
cmocka_unit_test(test_ncp_default),
402448
cmocka_unit_test(test_ncp_expand),
449+
cmocka_unit_test(test_p2p_epoch_mssfix_mtu),
403450
};
404451

405452

0 commit comments

Comments
 (0)