1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
|
From fa6a1824b0f37bff137204156a74ca28cf5b6f83 Mon Sep 17 00:00:00 2001
From: Arne Schwabe <arne@rfc2549.org>
Date: Mon, 27 Oct 2025 10:05:55 +0100
Subject: [PATCH] Fix memcmp check for the hmac verification in the 3way
handshake being inverted
This is a stupid mistake but causes all hmac cookies to be accepted,
thus breaking source IP address validation. As a consequence, TLS
sessions can be openend and state can be consumed in the server from
IP addresses that did not initiate an initial connection.
While at it, fix check to only allow [t-2;t] timeslots, disallowing
HMACs coming in from a future timeslot.
Github: OpenVPN/openvpn-private-issues#56
CVE: 2025-13086
Reported-By: Joshua Rogers <contact@joshua.hu>
Found-by: ZeroPath (https://zeropath.com/)
Reported-By: stefan@srlabs.de
Change-Id: I9cbe2bf535575b47ddd7f34e985c5c1c6953a6fc
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Max Fillinger <max@max-fillinger.net>
(cherry picked from commit 68ec931e7fb4af11d5ba0d4283df0350083fd373)
---
src/openvpn/ssl_pkt.c | 7 ++--
tests/unit_tests/openvpn/test_pkt.c | 58 ++++++++++++++++++++++++++++-
2 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 432bb8b8181..cf1ce172732 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -576,13 +576,14 @@ check_session_hmac_and_pkt_id(struct tls_pre_decrypt_state *state,
}
- /* check adjacent timestamps too */
- for (int offset = -2; offset <= 1; offset++)
+ /* check adjacent timestamps too, the handwindow is split in 2 for the
+ * offset, so we check the current timeslot and the two before that */
+ for (int offset = -2; offset <= 0; offset++)
{
struct session_id expected_id =
calculate_session_id_hmac(state->peer_session_id, from, hmac, handwindow, offset);
- if (memcmp_constant_time(&expected_id, &state->server_session_id, SID_SIZE))
+ if (memcmp_constant_time(&expected_id, &state->server_session_id, SID_SIZE) == 0)
{
return true;
}
diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c
index 27f52cf500d..62abafaac99 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -444,6 +444,8 @@ test_verify_hmac_tls_auth(void **ut_state)
hmac_ctx_t *hmac = session_id_hmac_init();
struct link_socket_actual from = { 0 };
+ from.dest.addr.sa.sa_family = AF_INET;
+ from.dest.addr.in4.sin_addr.s_addr = ntohl(0x01020304);
struct tls_auth_standalone tas = { 0 };
struct tls_pre_decrypt_state state = { 0 };
@@ -471,10 +473,12 @@ test_verify_hmac_tls_auth(void **ut_state)
static void
test_verify_hmac_none(void **ut_state)
{
+ now = 1000;
hmac_ctx_t *hmac = session_id_hmac_init();
struct link_socket_actual from = { 0 };
from.dest.addr.sa.sa_family = AF_INET;
+ from.dest.addr.in4.sin_addr.s_addr = ntohl(0x01020304);
struct tls_auth_standalone tas = { 0 };
struct tls_pre_decrypt_state state = { 0 };
@@ -489,9 +493,61 @@ test_verify_hmac_none(void **ut_state)
verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf);
assert_int_equal(verdict, VERDICT_VALID_ACK_V1);
+ /* This packet has a random hmac, so it should fail to validate */
bool valid = check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true);
+ assert_false(valid);
+
+ struct session_id client_id = { { 0xae, 0xb9, 0xaf, 0xe1, 0xf0, 0x1d, 0x79, 0xc8 } };
+ assert_memory_equal(&client_id, &state.peer_session_id, sizeof(struct session_id));
+
+ struct session_id expected_id = calculate_session_id_hmac(client_id, &from.dest, hmac, 30, 0);
+
+ free_tls_pre_decrypt_state(&state);
+ buf_reset_len(&buf);
+
+ /* Write the packet again into the buffer but this time, replacing the peer packet
+ * id with the expected one */
+ buf_write(&buf, client_ack_none_random_id, sizeof(client_ack_none_random_id) - 8);
+ buf_write(&buf, expected_id.id, 8);
+
+ verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf);
+ assert_int_equal(verdict, VERDICT_VALID_ACK_V1);
+ valid = check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true);
+
assert_true(valid);
+ /* Our handwindow is 30 so the slices are half of that, so they are
+ * (975,990), (990, 1005), (1005, 1020), (1020, 1035), (1035, 1050)
+ * So setting time to the two future ones should work
+ */
+ now = 980;
+ assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true));
+ now = 1040;
+ assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true));
+ now = 1002;
+ assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true));
+ now = 1022;
+ assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true));
+ now = 1010;
+ assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true));
+
+ /* Changing the IP address should make this invalid */
+ from.dest.addr.in4.sin_addr.s_addr = ntohl(0x01020305);
+ assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true));
+
+ /* Change to the correct one again */
+ from.dest.addr.in4.sin_addr.s_addr = ntohl(0x01020304);
+ assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true));
+
+ /* Modify the peer id, should now fail hmac verification */
+ buf_inc_len(&buf, -4);
+ buf_write_u32(&buf, 0x12345678);
+
+ free_tls_pre_decrypt_state(&state);
+ verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf);
+ assert_int_equal(verdict, VERDICT_VALID_ACK_V1);
+ assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true));
+
free_tls_pre_decrypt_state(&state);
free_buf(&buf);
hmac_ctx_cleanup(hmac);
@@ -723,12 +779,12 @@ int
main(void)
{
const struct CMUnitTest tests[] = {
+ cmocka_unit_test(test_verify_hmac_none),
cmocka_unit_test(test_tls_decrypt_lite_none),
cmocka_unit_test(test_tls_decrypt_lite_auth),
cmocka_unit_test(test_tls_decrypt_lite_crypt),
cmocka_unit_test(test_parse_ack),
cmocka_unit_test(test_calc_session_id_hmac_static),
- cmocka_unit_test(test_verify_hmac_none),
cmocka_unit_test(test_verify_hmac_tls_auth),
cmocka_unit_test(test_verify_hmac_none_out_of_range_ack),
cmocka_unit_test(test_generate_reset_packet_plain),
|
