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
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
|
From ac08b27cfa693d9be592bb2597c260635aee9e68 Mon Sep 17 00:00:00 2001
From: Steffan Karger <steffan.karger@fox-it.com>
Date: Tue, 25 Apr 2017 10:00:44 +0200
Subject: [PATCH 2/2] Drop packets instead of asserting out if packet id rolls
over
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously, if a mode was selected where packet ids are not allowed to roll
over, but renegotiation does not succeed for some reason (e.g. no password
entered in time, certificate expired or a malicious peer that refuses the
renegotiaion on purpose) we would continue to use the old keys. Until the
packet ID would roll over and we would ASSERT() out.
Given that this can be triggered on purpose by an authenticated peer, this
is a fix for an authenticated remote DoS vulnerability. An attack is
rather inefficient though; a peer would need to get us to send 2^32
packets (min-size packet is IP+UDP+OPCODE+PID+TAG (no payload), results in
(20+8+1+4+16)×2^32 bytes, or approx. 196 GB).
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
CVE-2017-7479
---
src/openvpn/crypto.c | 25 ++++++++++++++++---------
src/openvpn/packet_id.c | 22 ++++++++++++++++------
src/openvpn/packet_id.h | 1 +
src/openvpn/tls_crypt.c | 6 +++++-
tests/unit_tests/openvpn/test_packet_id.c | 11 +++++++++--
5 files changed, 47 insertions(+), 18 deletions(-)
Index: openvpn-2.4.0/src/openvpn/crypto.c
===================================================================
--- openvpn-2.4.0.orig/src/openvpn/crypto.c
+++ openvpn-2.4.0/src/openvpn/crypto.c
@@ -93,7 +93,11 @@ openvpn_encrypt_aead(struct buffer *buf,
buf_set_write(&iv_buffer, iv, iv_len);
/* IV starts with packet id to make the IV unique for packet */
- ASSERT(packet_id_write(&opt->packet_id.send, &iv_buffer, false, false));
+ if (!packet_id_write(&opt->packet_id.send, &iv_buffer, false, false))
+ {
+ msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over");
+ goto err;
+ }
/* Remainder of IV consists of implicit part (unique per session) */
ASSERT(buf_write(&iv_buffer, ctx->implicit_iv, ctx->implicit_iv_len));
@@ -194,11 +198,13 @@ openvpn_encrypt_v1(struct buffer *buf, s
}
/* Put packet ID in plaintext buffer */
- if (packet_id_initialized(&opt->packet_id))
+ if (packet_id_initialized(&opt->packet_id)
+ && !packet_id_write(&opt->packet_id.send, buf,
+ opt->flags & CO_PACKET_ID_LONG_FORM,
+ true))
{
- ASSERT(packet_id_write(&opt->packet_id.send, buf,
- opt->flags & CO_PACKET_ID_LONG_FORM,
- true));
+ msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over");
+ goto err;
}
}
else if (cipher_kt_mode_ofb_cfb(cipher_kt))
@@ -258,11 +264,12 @@ openvpn_encrypt_v1(struct buffer *buf, s
}
else /* No Encryption */
{
- if (packet_id_initialized(&opt->packet_id))
+ if (packet_id_initialized(&opt->packet_id)
+ && !packet_id_write(&opt->packet_id.send, buf,
+ opt->flags & CO_PACKET_ID_LONG_FORM, true))
{
- ASSERT(packet_id_write(&opt->packet_id.send, buf,
- BOOL_CAST(opt->flags & CO_PACKET_ID_LONG_FORM),
- true));
+ msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over");
+ goto err;
}
if (ctx->hmac)
{
Index: openvpn-2.4.0/src/openvpn/packet_id.c
===================================================================
--- openvpn-2.4.0.orig/src/openvpn/packet_id.c
+++ openvpn-2.4.0/src/openvpn/packet_id.c
@@ -325,27 +325,37 @@ packet_id_read(struct packet_id_net *pin
return true;
}
-static void
+static bool
packet_id_send_update(struct packet_id_send *p, bool long_form)
{
if (!p->time)
{
p->time = now;
}
- p->id++;
- if (!p->id)
+ if (p->id == PACKET_ID_MAX)
{
- ASSERT(long_form);
+ /* Packet ID only allowed to roll over if using long form and time has
+ * moved forward since last roll over.
+ */
+ if (!long_form || now <= p->time)
+ {
+ return false;
+ }
p->time = now;
- p->id = 1;
+ p->id = 0;
}
+ p->id++;
+ return true;
}
bool
packet_id_write(struct packet_id_send *p, struct buffer *buf, bool long_form,
bool prepend)
{
- packet_id_send_update(p, long_form);
+ if (!packet_id_send_update(p, long_form))
+ {
+ return false;
+ }
const packet_id_type net_id = htonpid(p->id);
const net_time_t net_time = htontime(p->time);
Index: openvpn-2.4.0/src/openvpn/packet_id.h
===================================================================
--- openvpn-2.4.0.orig/src/openvpn/packet_id.h
+++ openvpn-2.4.0/src/openvpn/packet_id.h
@@ -50,6 +50,7 @@
* to for network transmission.
*/
typedef uint32_t packet_id_type;
+#define PACKET_ID_MAX UINT32_MAX
typedef uint32_t net_time_t;
/*
Index: openvpn-2.4.0/src/openvpn/tls_crypt.c
===================================================================
--- openvpn-2.4.0.orig/src/openvpn/tls_crypt.c
+++ openvpn-2.4.0/src/openvpn/tls_crypt.c
@@ -95,7 +95,11 @@ tls_crypt_wrap(const struct buffer *src,
format_hex(BPTR(src), BLEN(src), 80, &gc));
/* Get packet ID */
- ASSERT(packet_id_write(&opt->packet_id.send, dst, true, false));
+ if (!packet_id_write(&opt->packet_id.send, dst, true, false))
+ {
+ msg(D_CRYPT_ERRORS, "TLS-CRYPT ERROR: packet ID roll over.");
+ goto err;
+ }
dmsg(D_PACKET_CONTENT, "TLS-CRYPT WRAP AD: %s",
format_hex(BPTR(dst), BLEN(dst), 0, &gc));
Index: openvpn-2.4.0/tests/unit_tests/openvpn/test_packet_id.c
===================================================================
--- openvpn-2.4.0.orig/tests/unit_tests/openvpn/test_packet_id.c
+++ openvpn-2.4.0/tests/unit_tests/openvpn/test_packet_id.c
@@ -129,8 +129,7 @@ test_packet_id_write_short_wrap(void **s
struct test_packet_id_write_data *data = *state;
data->pis.id = ~0;
- expect_assert_failure(
- packet_id_write(&data->pis, &data->test_buf, false, false));
+ assert_false(packet_id_write(&data->pis, &data->test_buf, false, false));
}
static void
@@ -139,8 +138,16 @@ test_packet_id_write_long_wrap(void **st
struct test_packet_id_write_data *data = *state;
data->pis.id = ~0;
+ data->pis.time = 5006;
+
+ /* Write fails if time did not change */
+ now = 5006;
+ assert_false(packet_id_write(&data->pis, &data->test_buf, true, false));
+
+ /* Write succeeds if time moved forward */
now = 5010;
assert_true(packet_id_write(&data->pis, &data->test_buf, true, false));
+
assert(data->pis.id == 1);
assert(data->pis.time == now);
assert_true(data->test_buf_data.buf_id == htonl(1));
|
