Skip to content

Commit 7dea53d

Browse files
committed
Add lookup of multi session by session id
TODO: We seem to support that a peer changes it session id. Is that really supported/how to reproduce that? Change-Id: Idb59ecd119331b198792ad1379bec8600211651b Signed-off-by: Arne Schwabe <arne@rfc2549.org>
1 parent 5057163 commit 7dea53d

File tree

10 files changed

+247
-19
lines changed

10 files changed

+247
-19
lines changed

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,7 @@ if (BUILD_TESTING)
826826
src/openvpn/siphash.c
827827
src/openvpn/siphash_reference.c
828828
src/openvpn/siphash_openssl.c
829+
src/openvpn/session_id.c
829830
)
830831

831832
target_sources(test_ncp PRIVATE

doc/man-sections/advanced-options.rst

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,19 @@ used when debugging or testing out special usage scenarios.
2929

3030
--hash-size args
3131
Set the size of the real address hash table to ``r`` and the virtual
32-
address table to ``v``.
32+
address table to ``v``. And if specified the session id hash table to
33+
``s``.
3334

3435
Valid syntax:
3536
::
3637

3738
hash-size r v
3839

3940
By default, both tables are sized at 1024 buckets.
41+
hash-size r v s
42+
43+
By default, both address tables are sized at 1024 buckets and session id
44+
table is sized at double the real address table if not specified (2048).
4045

4146
--bcast-buffers n
4247
Allocate ``n`` buffers for broadcast datagrams (default :code:`256`).

src/openvpn/mudp.c

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
#include "memdbg.h"
3434
#include "ssl_pkt.h"
35+
#include "sid_hash.h"
3536

3637
#ifdef HAVE_SYS_INOTIFY_H
3738
#include <sys/inotify.h>
@@ -166,7 +167,7 @@ do_pre_decrypt_check(struct multi_context *m, struct tls_pre_decrypt_state *stat
166167
}
167168
else
168169
{
169-
msg(D_MULTI_DEBUG,
170+
msg(D_MULTI_MEDIUM,
170171
"Valid packet (%s) with HMAC challenge from peer (%s), "
171172
"accepting new connection.",
172173
packet_opcode_name(op), peer);
@@ -180,6 +181,7 @@ do_pre_decrypt_check(struct multi_context *m, struct tls_pre_decrypt_state *stat
180181
return false;
181182
}
182183

184+
183185
#if defined(__GNUC__) || defined(__clang__)
184186
#pragma GCC diagnostic push
185187
#pragma GCC diagnostic ignored "-Wsign-compare"
@@ -235,6 +237,7 @@ handle_connection_attempt(struct multi_context *m,
235237
struct tls_session *session =
236238
&mi->context.c2.tls_multi->session[TM_INITIAL];
237239
session_skip_to_pre_start(session, &state, &m->top.c2.from);
240+
multi_hash_sid_add(m, &state.peer_session_id, mi);
238241
}
239242
}
240243
}
@@ -254,17 +257,43 @@ handle_connection_attempt(struct multi_context *m,
254257
struct multi_instance *
255258
multi_get_instance_udp_control(struct multi_context *m, struct link_socket *sock)
256259
{
260+
struct hash *addr_hash = m->hash;
261+
262+
/* Copy buffer, to a tmp buffer, so that reading the sesison does not
263+
* modify the internal pointers */
264+
struct buffer tmp = m->top.c2.buf;
265+
266+
/* op code */
267+
uint8_t op = (uint8_t)buf_read_u8(&tmp);
268+
269+
struct session_id sid = { 0 };
270+
session_id_read(&sid, &tmp);
271+
272+
struct hash_element *he_sid = multi_hash_sid_lookup(m, &sid);
273+
274+
if (he_sid)
275+
{
276+
return he_sid->value;
277+
}
278+
279+
/* If we cannot find the session the only valid case where we assign
280+
* this to an existing session is the case when this is a soft reset.
281+
* In this case we fall back to the lookup via IP address */
282+
if (op != P_CONTROL_SOFT_RESET_V1)
283+
{
284+
return NULL;
285+
}
286+
257287
struct mroute_addr real = { 0 };
258-
struct hash *hash = m->hash;
259288
real.proto = sock->info.proto;
260289

261-
if (mroute_extract_openvpn_sockaddr(&real, &m->top.c2.from.dest, true) && m->top.c2.buf.len > 0)
290+
if (mroute_extract_openvpn_sockaddr(&real, &m->top.c2.from.dest, true))
262291
{
263292
struct hash_element *he;
264-
const uint64_t hv = hash_value(hash, &real);
265-
struct hash_bucket *bucket = hash_bucket(hash, hv);
293+
const uint64_t addr_hv = hash_value(addr_hash, &real);
294+
struct hash_bucket *bucket = hash_bucket(addr_hash, addr_hv);
266295

267-
he = hash_lookup_fast(hash, bucket, &real, hv);
296+
he = hash_lookup_fast(addr_hash, bucket, &real, addr_hv);
268297
if (he)
269298
{
270299
return he->value;
@@ -357,6 +386,13 @@ multi_get_create_instance_udp(struct multi_context *m, bool *floated, struct lin
357386
}
358387
else
359388
{
389+
if (m->top.c2.buf.len < 9)
390+
{
391+
/* control packets must be at least the opcode byte + session id
392+
* (8 byte) long, otherwise they are not valid packets */
393+
return NULL;
394+
}
395+
360396
mi = multi_get_instance_udp_control(m, sock);
361397

362398
/* we have no existing multi instance for this connection, control

src/openvpn/multi.c

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#ifdef HAVE_CONFIG_H
2424
#include "config.h"
2525
#endif
26+
#include "sid_hash.h"
2627
#include "siphash.h"
2728

2829
#ifdef HAVE_SYS_INOTIFY_H
@@ -272,8 +273,8 @@ multi_init(struct context *t)
272273
struct multi_context *m = t->multi;
273274
int dev = DEV_TYPE_UNDEF;
274275

275-
msg(D_MULTI_LOW, "MULTI: multi_init called, r=%d v=%d", t->options.real_hash_size,
276-
t->options.virtual_hash_size);
276+
msg(D_MULTI_LOW, "MULTI: multi_init called, r=%d v=%d s=%d", t->options.real_hash_size,
277+
t->options.virtual_hash_size, t->options.sid_hash_size);
277278

278279
/*
279280
* Get tun/tap/null device type
@@ -301,6 +302,12 @@ multi_init(struct context *t)
301302
m->vhash = hash_init(t->options.virtual_hash_size, siphash_new_context(), siphash_free_context,
302303
mroute_addr_hash_function, mroute_addr_compare_function);
303304

305+
/*
306+
* Peer session id hash table. Used to lookup a session by the session
307+
* id of one of its active sessions */
308+
m->sid_hash = hash_init(t->options.sid_hash_size, siphash_new_context(), siphash_free_context,
309+
session_id_hash_function, session_id_hash_equal);
310+
304311
#ifdef ENABLE_MANAGEMENT
305312
m->cid_hash = hash_init(t->options.real_hash_size, NULL, free, cid_hash_function, cid_compare_function);
306313
#endif
@@ -599,6 +606,15 @@ multi_close_instance(struct multi_context *m, struct multi_instance *mi, bool sh
599606
}
600607
#endif
601608

609+
for (int i = 0; i < SIZE(mi->sid_hashed_values); i++)
610+
{
611+
if (session_id_defined(&mi->sid_hashed_values[i]))
612+
{
613+
multi_hash_sid_remove(m, &mi->sid_hashed_values[i]);
614+
CLEAR(mi->sid_hashed_values[i]);
615+
}
616+
}
617+
602618
if (mi->context.c2.tls_multi->peer_id != MAX_PEER_ID)
603619
{
604620
m->instances[mi->context.c2.tls_multi->peer_id] = NULL;
@@ -673,6 +689,7 @@ multi_uninit(struct multi_context *m)
673689

674690
hash_free(m->hash);
675691
hash_free(m->vhash);
692+
hash_free(m->sid_hash);
676693
#ifdef ENABLE_MANAGEMENT
677694
hash_free(m->cid_hash);
678695
#endif

src/openvpn/multi.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,10 @@ struct multi_instance
134134
bool did_real_hash;
135135
#ifdef ENABLE_MANAGEMENT
136136
bool did_cid_hash;
137+
/* The values we put into the multi_context->sid_hash to be able to
138+
* update the sid_hash without putting updates deep into the tls
139+
* methods, the key pointers of the hash map also point here */
140+
struct session_id sid_hashed_values[2];
137141
struct buffer_list *cc_config;
138142
#endif
139143
bool did_iroutes;
@@ -170,6 +174,11 @@ struct multi_context
170174
* address of the remote peer. */
171175
struct hash *vhash; /**< VPN tunnel instances indexed by
172176
* virtual address of remote hosts. */
177+
struct hash *sid_hash; /**< TLS sessions indexed by the peer's
178+
session id. We do not care collisions here
179+
as clients should have unique ids and
180+
supporting clients with identical SIDs
181+
is not needed */
173182
struct schedule *schedule;
174183
struct mbuf_set *mbuf; /**< Set of buffers for passing data
175184
* channel packets between VPN tunnel

src/openvpn/options.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,7 @@ init_options(struct options *o)
851851
o->vlan_pvid = 1;
852852
o->real_hash_size = 1024;
853853
o->virtual_hash_size = 1024;
854+
o->sid_hash_size = 2048;
854855
o->n_bcast_buf = 256;
855856
o->tcp_queue_limit = 64;
856857
o->max_clients = 1024;
@@ -7312,7 +7313,7 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
73127313
options->ifconfig_ipv6_pool_base = network;
73137314
options->ifconfig_ipv6_pool_netbits = netbits;
73147315
}
7315-
else if (streq(p[0], "hash-size") && p[1] && p[2] && !p[3])
7316+
else if (streq(p[0], "hash-size") && p[1] && p[2] && !p[4])
73167317
{
73177318
int real, virtual;
73187319

@@ -7324,6 +7325,21 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
73247325
}
73257326
options->real_hash_size = (uint32_t)real;
73267327
options->virtual_hash_size = (uint32_t)virtual;
7328+
7329+
if (p[3])
7330+
{
7331+
int sid;
7332+
if (!atoi_constrained(p[2], &sid, "hash-size sid", 1, INT_MAX, msglevel))
7333+
{
7334+
goto err;
7335+
}
7336+
options->sid_hash_size = (uint32_t)sid;
7337+
}
7338+
else
7339+
{
7340+
options->sid_hash_size = (uint32_t)(2 * real);
7341+
}
7342+
73277343
}
73287344
else if (streq(p[0], "connect-freq") && p[1] && p[2] && !p[3])
73297345
{

src/openvpn/options.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,7 @@ struct options
496496

497497
uint32_t real_hash_size;
498498
uint32_t virtual_hash_size;
499+
uint32_t sid_hash_size;
499500
const char *client_connect_script;
500501
const char *client_disconnect_script;
501502
const char *learn_address_script;

src/openvpn/sid_hash.h

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* OpenVPN -- An application to securely tunnel IP networks
3+
* over a single TCP/UDP port, with support for SSL/TLS-based
4+
* session authentication and key exchange,
5+
* packet encryption, packet authentication, and
6+
* packet compression.
7+
*
8+
* Copyright (C) 2026 OpenVPN Inc <sales@openvpn.net>
9+
* Copyright (C) 2026 Arne Schwabe <arne@rfc2549.org>
10+
*
11+
*
12+
* This program is free software; you can redistribute it and/or modify
13+
* it under the terms of the GNU General Public License version 2
14+
* as published by the Free Software Foundation.
15+
*
16+
* This program is distributed in the hope that it will be useful,
17+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
18+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
19+
* GNU General Public License for more details.
20+
*
21+
* You should have received a copy of the GNU General Public License along
22+
* with this program; if not, write to the Free Software Foundation, Inc.,
23+
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
24+
*/
25+
26+
#include "session_id.h"
27+
#include "multi.h"
28+
#include "list.h"
29+
#include "siphash.h"
30+
31+
inline static void
32+
multi_hash_sid_add(struct multi_context *m, struct session_id *sid,
33+
struct multi_instance *mi)
34+
{
35+
struct session_id *stored_sid = NULL;
36+
for (int i = 0; i < 2; i++)
37+
{
38+
if (!session_id_defined(mi->sid_hashed_values))
39+
{
40+
stored_sid = mi->sid_hashed_values;
41+
break;
42+
}
43+
}
44+
/* there should be always a free slot if we try to add to map */
45+
ASSERT(stored_sid);
46+
47+
*stored_sid = *sid;
48+
49+
const uint64_t hv = hash_value(m->sid_hash, stored_sid);
50+
struct hash_bucket *bucket = hash_bucket(m->sid_hash, hv);
51+
hash_add_fast(m->sid_hash, bucket, stored_sid, hv, mi);
52+
multi_instance_inc_refcount(mi);
53+
}
54+
55+
inline static bool
56+
multi_hash_sid_remove(struct multi_context *m, struct session_id *sid)
57+
{
58+
const uint64_t hv = hash_value(m->sid_hash, sid);
59+
struct hash_bucket *bucket = hash_bucket(m->sid_hash, hv);
60+
struct hash_element *he = hash_lookup_fast(m->sid_hash, bucket, sid, hv);
61+
if (he)
62+
{
63+
struct multi_instance *mi = he->value;
64+
ASSERT(hash_remove_fast(m->sid_hash, bucket, sid, hv));
65+
multi_instance_dec_refcount(mi);
66+
return true;
67+
}
68+
else
69+
{
70+
return false;
71+
}
72+
}
73+
74+
static inline struct hash_element *
75+
multi_hash_sid_lookup(struct multi_context *m, struct session_id *sid)
76+
{
77+
const uint64_t sid_hv = hash_value(m->sid_hash, sid);
78+
struct hash_bucket *sid_bucket = hash_bucket(m->sid_hash, sid_hv);
79+
struct hash_element *he_sid = hash_lookup_fast(m->sid_hash, sid_bucket, sid, sid_hv);
80+
return he_sid;
81+
}
82+
83+
/* hashing the session. As the struct is just an 8 byte array
84+
* hashing is straight forward */
85+
static inline uint64_t
86+
session_id_hash_function(const void *key, void *ctx)
87+
{
88+
return siphash_hash_func(key, sizeof(struct session_id), ctx);
89+
}
90+
91+
/* wrapper for session_id_equal to have the void* arguments that the
92+
* hash map requires */
93+
static inline bool
94+
session_id_hash_equal(const void *sid1, const void *sid2)
95+
{
96+
return session_id_equal((struct session_id *)sid1, (struct session_id *)sid2);
97+
}

tests/unit_tests/openvpn/Makefile.am

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,8 @@ misc_testdriver_SOURCES = test_misc.c \
382382
$(top_srcdir)/src/openvpn/list.c \
383383
$(top_srcdir)/src/openvpn/siphash.c \
384384
$(top_srcdir)/src/openvpn/siphash_openssl.c \
385-
$(top_srcdir)/src/openvpn/siphash_reference.c
385+
$(top_srcdir)/src/openvpn/siphash_reference.c \
386+
$(top_srcdir)/src/openvpn/session_id.c
386387

387388
push_update_msg_testdriver_CFLAGS = -I$(top_srcdir)/src/openvpn \
388389
-I$(top_srcdir)/src/compat \

0 commit comments

Comments
 (0)