You can subscribe to this list here.
| 2002 |
Jan
|
Feb
|
Mar
|
Apr
(24) |
May
(14) |
Jun
(29) |
Jul
(33) |
Aug
(3) |
Sep
(8) |
Oct
(18) |
Nov
(1) |
Dec
(10) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2003 |
Jan
(3) |
Feb
(33) |
Mar
(7) |
Apr
(28) |
May
(30) |
Jun
(5) |
Jul
(10) |
Aug
(7) |
Sep
(32) |
Oct
(41) |
Nov
(20) |
Dec
(10) |
| 2004 |
Jan
(24) |
Feb
(18) |
Mar
(57) |
Apr
(40) |
May
(55) |
Jun
(48) |
Jul
(77) |
Aug
(15) |
Sep
(56) |
Oct
(80) |
Nov
(74) |
Dec
(52) |
| 2005 |
Jan
(38) |
Feb
(42) |
Mar
(39) |
Apr
(56) |
May
(79) |
Jun
(73) |
Jul
(16) |
Aug
(23) |
Sep
(68) |
Oct
(77) |
Nov
(52) |
Dec
(27) |
| 2006 |
Jan
(27) |
Feb
(18) |
Mar
(51) |
Apr
(62) |
May
(28) |
Jun
(50) |
Jul
(36) |
Aug
(33) |
Sep
(47) |
Oct
(50) |
Nov
(77) |
Dec
(13) |
| 2007 |
Jan
(15) |
Feb
(8) |
Mar
(14) |
Apr
(18) |
May
(25) |
Jun
(16) |
Jul
(16) |
Aug
(19) |
Sep
(32) |
Oct
(17) |
Nov
(5) |
Dec
(5) |
| 2008 |
Jan
(64) |
Feb
(25) |
Mar
(25) |
Apr
(6) |
May
(28) |
Jun
(20) |
Jul
(10) |
Aug
(27) |
Sep
(28) |
Oct
(59) |
Nov
(37) |
Dec
(43) |
| 2009 |
Jan
(40) |
Feb
(25) |
Mar
(12) |
Apr
(57) |
May
(46) |
Jun
(29) |
Jul
(39) |
Aug
(10) |
Sep
(20) |
Oct
(42) |
Nov
(50) |
Dec
(57) |
| 2010 |
Jan
(82) |
Feb
(165) |
Mar
(256) |
Apr
(260) |
May
(36) |
Jun
(87) |
Jul
(53) |
Aug
(89) |
Sep
(107) |
Oct
(51) |
Nov
(88) |
Dec
(117) |
| 2011 |
Jan
(69) |
Feb
(60) |
Mar
(113) |
Apr
(71) |
May
(67) |
Jun
(90) |
Jul
(88) |
Aug
(90) |
Sep
(48) |
Oct
(64) |
Nov
(69) |
Dec
(118) |
| 2012 |
Jan
(49) |
Feb
(528) |
Mar
(351) |
Apr
(190) |
May
(238) |
Jun
(193) |
Jul
(104) |
Aug
(100) |
Sep
(57) |
Oct
(41) |
Nov
(47) |
Dec
(51) |
| 2013 |
Jan
(94) |
Feb
(57) |
Mar
(96) |
Apr
(105) |
May
(77) |
Jun
(102) |
Jul
(27) |
Aug
(81) |
Sep
(32) |
Oct
(53) |
Nov
(127) |
Dec
(65) |
| 2014 |
Jan
(113) |
Feb
(59) |
Mar
(104) |
Apr
(259) |
May
(70) |
Jun
(70) |
Jul
(146) |
Aug
(45) |
Sep
(58) |
Oct
(149) |
Nov
(77) |
Dec
(83) |
| 2015 |
Jan
(53) |
Feb
(66) |
Mar
(86) |
Apr
(50) |
May
(135) |
Jun
(76) |
Jul
(151) |
Aug
(83) |
Sep
(97) |
Oct
(262) |
Nov
(245) |
Dec
(231) |
| 2016 |
Jan
(131) |
Feb
(233) |
Mar
(97) |
Apr
(138) |
May
(221) |
Jun
(254) |
Jul
(92) |
Aug
(248) |
Sep
(168) |
Oct
(275) |
Nov
(477) |
Dec
(445) |
| 2017 |
Jan
(218) |
Feb
(217) |
Mar
(146) |
Apr
(172) |
May
(216) |
Jun
(252) |
Jul
(164) |
Aug
(192) |
Sep
(190) |
Oct
(143) |
Nov
(255) |
Dec
(182) |
| 2018 |
Jan
(295) |
Feb
(164) |
Mar
(113) |
Apr
(147) |
May
(64) |
Jun
(262) |
Jul
(184) |
Aug
(90) |
Sep
(69) |
Oct
(364) |
Nov
(102) |
Dec
(101) |
| 2019 |
Jan
(119) |
Feb
(64) |
Mar
(64) |
Apr
(102) |
May
(57) |
Jun
(154) |
Jul
(84) |
Aug
(81) |
Sep
(76) |
Oct
(102) |
Nov
(233) |
Dec
(89) |
| 2020 |
Jan
(38) |
Feb
(170) |
Mar
(155) |
Apr
(172) |
May
(120) |
Jun
(223) |
Jul
(461) |
Aug
(227) |
Sep
(268) |
Oct
(113) |
Nov
(56) |
Dec
(124) |
| 2021 |
Jan
(121) |
Feb
(48) |
Mar
(334) |
Apr
(345) |
May
(207) |
Jun
(136) |
Jul
(71) |
Aug
(112) |
Sep
(122) |
Oct
(173) |
Nov
(184) |
Dec
(223) |
| 2022 |
Jan
(197) |
Feb
(206) |
Mar
(156) |
Apr
(212) |
May
(192) |
Jun
(170) |
Jul
(143) |
Aug
(380) |
Sep
(182) |
Oct
(148) |
Nov
(128) |
Dec
(269) |
| 2023 |
Jan
(248) |
Feb
(196) |
Mar
(264) |
Apr
(36) |
May
(123) |
Jun
(66) |
Jul
(120) |
Aug
(48) |
Sep
(157) |
Oct
(198) |
Nov
(300) |
Dec
(273) |
| 2024 |
Jan
(271) |
Feb
(147) |
Mar
(207) |
Apr
(78) |
May
(107) |
Jun
(168) |
Jul
(151) |
Aug
(51) |
Sep
(438) |
Oct
(221) |
Nov
(302) |
Dec
(357) |
| 2025 |
Jan
(451) |
Feb
(219) |
Mar
(326) |
Apr
(232) |
May
(306) |
Jun
(181) |
Jul
(452) |
Aug
(282) |
Sep
(620) |
Oct
(697) |
Nov
|
Dec
|
|
From: MaxF (C. Review) <ge...@op...> - 2025-10-27 15:56:44
|
Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1315?usp=email
to review the following change.
Change subject: Zeroize tls-crypt-v2 client keys
......................................................................
Zeroize tls-crypt-v2 client keys
Joshua Rogers sent in a bug report generated with ZeroPath that the
tls-crypt-v2 client key is loaded before running the verify script. If
the verify script fails, the key is not zeroized.
While investigating this report, I found that free_tls_pre_decrypt_state
never zeroizes tls_wrap_tmp.original_wrap_keydata. So also when the
check is successful, key data will remain in memory when it is no longer
needed.
This commit moves the tls-crypt-v2-verify check before loading the key.
If it fails, original_wrap_keydata is zeroized. Also, in
free_tls_pre_decrypt_state, if a key has been loaded,
original_wrap_keydata is zeroized.
Change-Id: Icfcbf8ee20c1c0016eb98b570f24b9325b157c5c
Signed-off-by: Max Fillinger <ma...@ma...>
---
M src/openvpn/ssl_pkt.c
M src/openvpn/tls_crypt.c
2 files changed, 7 insertions(+), 5 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/15/1315/1
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 825719c..d7f7ac3 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -280,6 +280,7 @@
if (state->tls_wrap_tmp.cleanup_key_ctx)
{
free_key_ctx_bi(&state->tls_wrap_tmp.opt.key_ctx_bi);
+ secure_memzero(&state->tls_wrap_tmp.original_wrap_keydata, sizeof(state->tls_wrap_tmp.original_wrap_keydata));
}
}
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 51b4eb3..a808de3 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -642,6 +642,12 @@
return false;
}
+ if (opt && opt->tls_crypt_v2_verify_script && !tls_crypt_v2_verify_metadata(ctx, opt))
+ {
+ secure_memzero(&ctx->original_wrap_keydata, sizeof(ctx->original_wrap_keydata));
+ return false;
+ }
+
/* Load the decrypted key */
ctx->mode = TLS_WRAP_CRYPT;
ctx->cleanup_key_ctx = true;
@@ -652,11 +658,6 @@
/* Remove client key from buffer so tls-crypt code can unwrap message */
ASSERT(buf_inc_len(buf, -(BLEN(&wrapped_client_key))));
- if (opt && opt->tls_crypt_v2_verify_script)
- {
- return tls_crypt_v2_verify_metadata(ctx, opt);
- }
-
return true;
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1315?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Icfcbf8ee20c1c0016eb98b570f24b9325b157c5c
Gerrit-Change-Number: 1315
Gerrit-PatchSet: 1
Gerrit-Owner: MaxF <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: ordex (C. Review) <ge...@op...> - 2025-10-27 15:19:09
|
Attention is currently required from: flichtenheld, plaisthos.
Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1314?usp=email
to look at the new patch set (#2).
Change subject: sitnl: set FD_CLOEXEC on socket to prevent abuse
......................................................................
sitnl: set FD_CLOEXEC on socket to prevent abuse
Since OpenVPN spawns various child processes, it is important
that sockets are closed after calling exec.
The sitnl socket didn't have the right flag set, resulting
in it surviving in, for example, connect/disconnect scripts
and giving the latter a chance to abuse the socket.
Ensure this doesn't happen by setting FD_CLOEXEC on
this socket right after creation.
Reported-by: ZeroPath (https://zeropath.com/)
Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e
Signed-off-by: Antonio Quartulli <an...@ma...>
---
M src/openvpn/networking_sitnl.c
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/14/1314/2
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index b3adb16..3e20b70 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -27,6 +27,7 @@
#include "dco.h"
#include "errlevel.h"
+#include "fdmisc.h"
#include "buffer.h"
#include "misc.h"
#include "networking.h"
@@ -181,6 +182,9 @@
return fd;
}
+ /* set close on exec to avoid child processes access the socket */
+ set_cloexec(fd);
+
if (setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &sndbuf, sizeof(sndbuf)) < 0)
{
msg(M_WARN | M_ERRNO, "%s: SO_SNDBUF", __func__);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1314?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e
Gerrit-Change-Number: 1314
Gerrit-PatchSet: 2
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: ordex (C. Review) <ge...@op...> - 2025-10-27 13:55:33
|
Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1314?usp=email
to review the following change.
Change subject: sitnl: set FD_CLOEXEC on socket to prevent abuse
......................................................................
sitnl: set FD_CLOEXEC on socket to prevent abuse
Since OpenVPN spawns various child processes, it is important
that sockets are closed after calling exec.
The sitnl socket didn't have the right flag set, resulting
in it surviving in, for example, connect/disconnect scripts
and giving the latter a chance to abuse the socket.
Ensure this doesn't happen by setting FD_CLOEXEC on
this socket right after creation.
Reported-by: ZeroPath (https://zeropath.com/)
Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e
Signed-off-by: Antonio Quartulli <an...@ma...>
---
M src/openvpn/networking_sitnl.c
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/14/1314/1
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index b3adb16..a959fa8 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -181,6 +181,9 @@
return fd;
}
+ /* set close on exec to avoid child processes access the socket */
+ set_cloexec(fd);
+
if (setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &sndbuf, sizeof(sndbuf)) < 0)
{
msg(M_WARN | M_ERRNO, "%s: SO_SNDBUF", __func__);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1314?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e
Gerrit-Change-Number: 1314
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <an...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: its_Giaan (C. Review) <ge...@op...> - 2025-10-27 13:47:12
|
Attention is currently required from: cron2, flichtenheld, plaisthos. its_Giaan has posted comments on this change by its_Giaan. ( http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email ) Change subject: multipeer: introduce asymmetric peer-id ...................................................................... Patch Set 5: (8 comments) Patchset: PS4: > The part that picks the "peer-id" pushed and parsed options. […] Done File src/openvpn/ssl.c: http://gerrit.openvpn.net/c/openvpn/+/1089/comment/483a5681_27b0022b?usp=email : PS4, Line 1179: ret->rx_peer_id = MAX_PEER_ID; > Add comment here that we also use the rx peer id to identify DCO clients as this has become now a im […] Done http://gerrit.openvpn.net/c/openvpn/+/1089/comment/d7cabc51_e6990ae0?usp=email : PS4, Line 1982: } > This is still not guarded by DCO capability. […] Done http://gerrit.openvpn.net/c/openvpn/+/1089/comment/c0fec251_e0eada21?usp=email : PS4, Line 2165: if (multi->rx_peer_id == MAX_PEER_ID && session->opt->mode != MODE_SERVER) > This feel be a very hacky place to set the multi rx peer id. […] I moved this into tls_multi_init_finalize(), hope that's fine. File src/openvpn/ssl_ncp.c: http://gerrit.openvpn.net/c/openvpn/+/1089/comment/74331eed_c592a014?usp=email : PS4, Line 425: if (tx_peer_id) > This also need to take DCO capability into account. Done http://gerrit.openvpn.net/c/openvpn/+/1089/comment/c5e954b4_8a6a0a93?usp=email : PS4, Line 450: if (multi->use_peer_id) > I think this parts needs to be skipped if we are using/negotiated asymmetric peer-id as it would ove […] Done File src/openvpn/ssl_util.h: http://gerrit.openvpn.net/c/openvpn/+/1089/comment/be89150e_3068de2f?usp=email : PS4, Line 56: uint32_t extract_asymmetric_peer_id(const char *peer_info); > Add doxygen please Done File src/openvpn/ssl_util.c: http://gerrit.openvpn.net/c/openvpn/+/1089/comment/c1989b67_8f07aa92?usp=email : PS4, Line 90: return 0; > 0 is a valid peer id. […] Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9 Gerrit-Change-Number: 1089 Gerrit-PatchSet: 5 Gerrit-Owner: its_Giaan <gia...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: cron2 <ge...@gr...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Mon, 27 Oct 2025 13:47:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: plaisthos <arn...@rf...> |
|
From: its_Giaan (C. Review) <ge...@op...> - 2025-10-27 13:44:53
|
Attention is currently required from: cron2, flichtenheld, its_Giaan.
Hello cron2, flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email
to look at the new patch set (#5).
Change subject: multipeer: introduce asymmetric peer-id
......................................................................
multipeer: introduce asymmetric peer-id
In order to achieve a multipeer functionality, peers now
use separate IDs for sending (tx_peer_id) and receiving
(rx_peer_id).
Each peer announces its own ID through pushing peer-info
using 'ID=7f1' hex format so identification can still
happen even if IP/port changes.
In P2P mode, peer switch to using the announced IDs after
mutual exchange.
In P2MP mode, clients always announce their ID, and servers
can optionally respond with their own to enable the same
behavior.
Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9
Signed-off-by: Gianmarco De Gregori <gia...@ma...>
---
M src/openvpn/dco.c
M src/openvpn/init.c
M src/openvpn/misc.c
M src/openvpn/multi.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/push.c
M src/openvpn/push_util.c
M src/openvpn/ssl.c
M src/openvpn/ssl_common.h
M src/openvpn/ssl_ncp.c
M src/openvpn/ssl_util.c
M src/openvpn/ssl_util.h
M tests/unit_tests/openvpn/test_crypto.c
14 files changed, 189 insertions(+), 47 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/89/1089/5
diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 8fb4662..af1b599 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -515,14 +515,15 @@
c->c2.tls_multi->dco_peer_id = -1;
}
#endif
- int ret = dco_new_peer(&c->c1.tuntap->dco, multi->peer_id, sock->sd, NULL,
- proto_is_dgram(sock->info.proto) ? remoteaddr : NULL, NULL, NULL);
+ int ret = dco_new_peer(&c->c1.tuntap->dco, multi->rx_peer_id, sock->sd, NULL,
+ proto_is_dgram(sock->info.proto) ? remoteaddr : NULL,
+ NULL, NULL);
if (ret < 0)
{
return ret;
}
- c->c2.tls_multi->dco_peer_id = multi->peer_id;
+ c->c2.tls_multi->dco_peer_id = multi->rx_peer_id;
return 0;
}
@@ -597,7 +598,7 @@
{
struct context *c = &mi->context;
- int peer_id = c->c2.tls_multi->peer_id;
+ int peer_id = c->c2.tls_multi->rx_peer_id;
struct sockaddr *remoteaddr, *localaddr = NULL;
struct sockaddr_storage local = { 0 };
const socket_descriptor_t sd = c->c2.link_sockets[0]->sd;
@@ -668,8 +669,7 @@
if (addrtype == MR_ADDR_IPV6)
{
#if defined(_WIN32)
- dco_win_add_iroute_ipv6(&c->c1.tuntap->dco, addr->v6.addr, addr->netbits,
- c->c2.tls_multi->peer_id);
+ dco_win_add_iroute_ipv6(&c->c1.tuntap->dco, addr->v6.addr, addr->netbits, c->c2.tls_multi->rx_peer_id);
#else
net_route_v6_add(&m->top.net_ctx, &addr->v6.addr, addr->netbits,
&mi->context.c2.push_ifconfig_ipv6_local, c->c1.tuntap->actual_name, 0,
@@ -679,8 +679,7 @@
else if (addrtype == MR_ADDR_IPV4)
{
#if defined(_WIN32)
- dco_win_add_iroute_ipv4(&c->c1.tuntap->dco, addr->v4.addr, addr->netbits,
- c->c2.tls_multi->peer_id);
+ dco_win_add_iroute_ipv4(&c->c1.tuntap->dco, addr->v4.addr, addr->netbits, c->c2.tls_multi->rx_peer_id);
#else
in_addr_t dest = htonl(addr->v4.addr);
net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits, &mi->context.c2.push_ifconfig_local,
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index aa2611d..8a148c6 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2223,7 +2223,7 @@
if (o->use_peer_id)
{
- buf_printf(&out, ", peer-id: %d", o->peer_id);
+ buf_printf(&out, ", rx_peer-id: %u, tx_peer-id: %u", c->c2.tls_multi->rx_peer_id, c->c2.tls_multi->tx_peer_id);
}
#ifdef USE_COMP
@@ -2702,7 +2702,12 @@
{
msg(D_PUSH_DEBUG, "OPTIONS IMPORT: peer-id set");
c->c2.tls_multi->use_peer_id = true;
- c->c2.tls_multi->peer_id = c->options.peer_id;
+ c->c2.tls_multi->tx_peer_id = c->options.rx_peer_id;
+ if (!c->c2.tls_multi->use_asymmetric_peer_id)
+ {
+ c->c2.tls_multi->rx_peer_id = c->options.rx_peer_id;
+ c->c2.tls_multi->tx_peer_id = c->options.rx_peer_id;
+ }
}
/* process (potentially) pushed options */
@@ -2729,7 +2734,7 @@
/* Ensure that for epoch data format is only enabled if also data v2
* is enabled */
bool epoch_data = c->options.imported_protocol_flags & CO_EPOCH_DATA_KEY_FORMAT;
- bool datav2_enabled = c->options.use_peer_id && c->options.peer_id < MAX_PEER_ID;
+ bool datav2_enabled = c->options.use_peer_id && c->options.rx_peer_id < MAX_PEER_ID;
if (epoch_data && !datav2_enabled)
{
@@ -3478,6 +3483,10 @@
if (c->c2.tls_multi)
{
tls_multi_init_finalize(c->c2.tls_multi, c->options.ce.tls_mtu);
+ if (c->c2.tls_multi->rx_peer_id != MAX_PEER_ID)
+ {
+ c->options.use_peer_id = true;
+ }
ASSERT(c->c2.tls_multi->opt.frame.buf.payload_size <= c->c2.frame.buf.payload_size);
frame_print(&c->c2.tls_multi->opt.frame, D_MTU_INFO, "Control Channel MTU parms");
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 188f44e..061c573 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -761,14 +761,15 @@
{
chomp(line);
if (validate_peer_info_line(line)
- && (strncmp(line, "IV_", 3) == 0 || strncmp(line, "UV_", 3) == 0))
+ && (strncmp(line, "IV_", 3) == 0 || strncmp(line, "UV_", 3) == 0
+ || strncmp(line, "ID", 2) == 0))
{
msg(M_INFO, "peer info: %s", line);
env_set_add(es, line);
}
else
{
- msg(M_WARN, "validation failed on peer_info line received from client");
+ msg(M_WARN, "validation failed on peer_info line received");
}
}
}
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f60944d..ae5c3ed 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -453,7 +453,7 @@
if (mi->context.c2.tls_multi && check_debug_level(D_DCO_DEBUG)
&& dco_enabled(&mi->context.options))
{
- buf_printf(&out, " peer-id=%d", mi->context.c2.tls_multi->peer_id);
+ buf_printf(&out, " rx_peer-id=%d", mi->context.c2.tls_multi->rx_peer_id);
}
return BSTR(&out);
}
@@ -626,9 +626,9 @@
}
#endif
- if (mi->context.c2.tls_multi->peer_id != MAX_PEER_ID)
+ if (mi->context.c2.tls_multi->rx_peer_id != MAX_PEER_ID)
{
- m->instances[mi->context.c2.tls_multi->peer_id] = NULL;
+ m->instances[mi->context.c2.tls_multi->rx_peer_id] = NULL;
}
schedule_remove_entry(m->schedule, (struct schedule_entry *)mi);
@@ -941,8 +941,7 @@
#else
sep,
#endif
- sep,
- mi->context.c2.tls_multi ? mi->context.c2.tls_multi->peer_id : UINT32_MAX,
+ sep, mi->context.c2.tls_multi ? mi->context.c2.tls_multi->rx_peer_id : UINT32_MAX,
sep, translate_cipher_name_to_openvpn(mi->context.options.ciphername));
}
gc_free(&gc);
@@ -1749,6 +1748,7 @@
tls_multi->use_peer_id = true;
o->use_peer_id = true;
}
+
else if (dco_enabled(o))
{
msg(M_INFO, "Client does not support DATA_V2. Data channel offloading "
@@ -3143,12 +3143,12 @@
* has, so we disallow it. This can happen if a DCO netlink notification
* gets lost and we miss a floating step.
*/
- if (m1->peer_id == m2->peer_id)
+ if (m1->rx_peer_id == m2->rx_peer_id)
{
msg(M_WARN,
"disallowing peer %" PRIu32 " (%s) from floating to "
"its own address (%s)",
- m1->peer_id, tls_common_name(mi->context.c2.tls_multi, false),
+ m1->rx_peer_id, tls_common_name(mi->context.c2.tls_multi, false),
mroute_addr_print(&mi->real, &gc));
goto done;
}
@@ -3161,9 +3161,10 @@
}
msg(D_MULTI_MEDIUM, "peer %" PRIu32 " (%s) floated from %s to %s",
- mi->context.c2.tls_multi->peer_id, tls_common_name(mi->context.c2.tls_multi, false),
- mroute_addr_print_ex(&mi->real, MAPF_SHOW_FAMILY, &gc),
- mroute_addr_print_ex(&real, MAPF_SHOW_FAMILY, &gc));
+ mi->context.c2.tls_multi->rx_peer_id,
+ tls_common_name(mi->context.c2.tls_multi, false),
+ mroute_addr_print(&mi->real, &gc),
+ print_link_socket_actual(&m->top.c2.from, &gc));
/* remove old address from hash table before changing address */
ASSERT(hash_remove(m->hash, &mi->real));
@@ -4143,7 +4144,7 @@
{
if (!m->instances[i])
{
- mi->context.c2.tls_multi->peer_id = i;
+ mi->context.c2.tls_multi->rx_peer_id = i;
m->instances[i] = mi;
break;
}
@@ -4151,7 +4152,7 @@
/* should not really end up here, since multi_create_instance returns null
* if amount of clients exceeds max_clients */
- ASSERT(mi->context.c2.tls_multi->peer_id < m->max_clients);
+ ASSERT(mi->context.c2.tls_multi->rx_peer_id < m->max_clients);
}
/**************************************************************************/
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 65c6b3b..3878026 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3885,6 +3885,7 @@
"incompatible with each other.");
}
+
if (dco_enabled(o))
{
/* check if any option should force disabling DCO */
@@ -9222,7 +9223,8 @@
{
VERIFY_PERMISSION(OPT_P_PEER_ID);
options->use_peer_id = true;
- options->peer_id = atoi_warn(p[1], msglevel);
+ options->rx_peer_id = atoi_warn(p[1], msglevel);
+ options->tx_peer_id = atoi_warn(p[1], msglevel);
}
else if (streq(p[0], "keying-material-exporter") && p[1] && p[2])
{
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 009904a..5803d46 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -705,7 +705,8 @@
#endif
bool use_peer_id;
- uint32_t peer_id;
+ uint32_t rx_peer_id;
+ uint32_t tx_peer_id;
/* Keying Material Exporters [RFC 5705] */
const char *keying_material_exporter_label;
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 2c717c7..963593c 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -659,9 +659,10 @@
print_in_addr_t(c->c2.push_ifconfig_remote_netmask, 0, gc));
}
- if (tls_multi->use_peer_id)
+ if (!tls_multi->use_asymmetric_peer_id)
{
- push_option_fmt(gc, push_list, M_USAGE, "peer-id %d", tls_multi->peer_id);
+ push_option_fmt(gc, push_list, M_USAGE, "peer-id %d",
+ tls_multi->rx_peer_id);
}
/*
* If server uses --auth-gen-token and we have an auth token
diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c
index 552679a..de4f0db 100644
--- a/src/openvpn/push_util.c
+++ b/src/openvpn/push_util.c
@@ -187,7 +187,7 @@
unsigned int permission_mask = pull_permission_mask(c);
if (process_push_update(c, &o, permission_mask, &option_types_found, &tmp_msg, true) == PUSH_MSG_ERROR)
{
- msg(M_WARN, "Failed to process push update message sent to client ID: %u", c->c2.tls_multi->peer_id);
+ msg(M_WARN, "Failed to process push update message sent to client ID: %u", c->c2.tls_multi->rx_peer_id);
}
}
@@ -292,7 +292,7 @@
if (!support_push_update(mi))
{
- msg(M_CLIENT, "PUSH_UPDATE: not sending message to unsupported peer with ID: %u", mi->context.c2.tls_multi->peer_id);
+ msg(M_CLIENT, "PUSH_UPDATE: not sending message to unsupported peer with ID: %u", mi->context.c2.tls_multi->rx_peer_id);
gc_free(&gc);
return 0;
}
@@ -327,7 +327,7 @@
/* Type is UPT_BROADCAST so we update every client */
if (!send_single_push_update(m, curr_mi, msgs))
{
- msg(M_CLIENT, "ERROR: Peer ID: %u has not been updated", curr_mi->context.c2.tls_multi->peer_id);
+ msg(M_CLIENT, "ERROR: Peer ID: %u has not been updated", curr_mi->context.c2.tls_multi->rx_peer_id);
continue;
}
count++;
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 908854a..a841354 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1174,7 +1174,10 @@
/* get command line derived options */
ret->opt = *tls_options;
ret->dco_peer_id = -1;
- ret->peer_id = MAX_PEER_ID;
+ ret->use_asymmetric_peer_id = false;
+ /* The rx_peer_id is also used to identify DCO clients */
+ ret->rx_peer_id = MAX_PEER_ID;
+ ret->tx_peer_id = MAX_PEER_ID;
return ret;
}
@@ -1187,6 +1190,22 @@
tls_session_init(multi, &multi->session[TM_ACTIVE]);
tls_session_init(multi, &multi->session[TM_INITIAL]);
+
+ if (!multi->opt.dco_enabled)
+ {
+ /* Calculate the asymmetric peer-id */
+ if (multi->rx_peer_id == MAX_PEER_ID && multi->session[TM_INITIAL].opt->mode != MODE_SERVER)
+ {
+ uint8_t peerid[3];
+ srand((unsigned)time(NULL));
+ for (int i = 0; i < 3; i++)
+ {
+ peerid[i] = rand();
+ }
+
+ multi->rx_peer_id = (peerid[0] << 16) + (peerid[1] << 8) + peerid[2];
+ }
+ }
}
/*
@@ -1863,6 +1882,28 @@
return str;
}
+static bool
+push_peer_info_server(struct buffer *buf, struct tls_session *session, uint32_t peer_id)
+{
+ struct gc_arena gc = gc_new();
+ bool ret = false;
+ struct buffer out = alloc_buf_gc(64, &gc);
+
+ if (peer_id != MAX_PEER_ID && (!session->opt->dco_enabled))
+ {
+ buf_printf(&out, "ID=%x\n", peer_id);
+ }
+ if (!write_string(buf, BSTR(&out), -1))
+ {
+ goto error;
+ }
+ ret = true;
+
+error:
+ gc_free(&gc);
+ return ret;
+}
+
/**
* Prepares the IV_ and UV_ variables that are part of the
* exchange to signal the peer's capabilities. The amount
@@ -1879,7 +1920,7 @@
* @return true if no error was encountered
*/
static bool
-push_peer_info(struct buffer *buf, struct tls_session *session)
+push_peer_info(struct buffer *buf, struct tls_session *session, uint32_t peer_id)
{
struct gc_arena gc = gc_new();
bool ret = false;
@@ -1977,6 +2018,11 @@
buf_printf(&out, "IV_PROTO=%d\n", iv_proto);
+ if (peer_id != MAX_PEER_ID && (!session->opt->dco_enabled))
+ {
+ buf_printf(&out, "ID=%x\n", peer_id);
+ }
+
if (session->opt->push_peer_info_detail > 1)
{
/* push compression status */
@@ -2157,11 +2203,16 @@
}
}
- if (!push_peer_info(buf, session))
+ if (!push_peer_info(buf, session, multi->rx_peer_id))
{
goto error;
}
+ if (session->opt->mode == MODE_SERVER && multi->use_asymmetric_peer_id && !session->opt->push_peer_info_detail)
+ {
+ push_peer_info_server(buf, session, multi->rx_peer_id);
+ }
+
if (session->opt->server && session->opt->mode != MODE_SERVER && ks->key_id == 0)
{
/* tls-server option set and not P2MP server, so we
@@ -2270,6 +2321,44 @@
if (multi->peer_info)
{
output_peer_info_env(session->opt->es, multi->peer_info);
+ if (session->opt->mode == MODE_SERVER)
+ {
+ if (!session->opt->dco_enabled)
+ {
+ uint32_t peer_id = extract_asymmetric_peer_id(multi->peer_info);
+ if (peer_id != UINT32_MAX)
+ {
+ multi->tx_peer_id = peer_id;
+ multi->use_asymmetric_peer_id = true;
+ multi->use_peer_id = true;
+ }
+ else
+ {
+ /* Client has no asymmetric peer-id capability */
+ multi->tx_peer_id = multi->rx_peer_id;
+ }
+ }
+ else
+ {
+ /* With DCO we don't need the tx_peer_id atm */
+ multi->tx_peer_id = multi->rx_peer_id;
+ }
+ }
+ }
+ else
+ {
+ free(multi->peer_info);
+ multi->peer_info = read_string_alloc(buf);
+ }
+ if (session->opt->mode == MODE_POINT_TO_POINT && !session->opt->dco_enabled)
+ {
+ uint32_t peer_id = extract_asymmetric_peer_id(multi->peer_info);
+ if (peer_id != UINT32_MAX)
+ {
+ multi->tx_peer_id = peer_id;
+ multi->use_asymmetric_peer_id = true;
+ multi->use_peer_id = true;
+ }
}
free(multi->remote_ciphername);
@@ -4006,8 +4095,8 @@
msg(D_TLS_DEBUG, __func__);
ASSERT(ks);
-
- peer = htonl(((P_DATA_V2 << P_OPCODE_SHIFT) | ks->key_id) << 24 | (multi->peer_id & 0xFFFFFF));
+ peer = htonl(((P_DATA_V2 << P_OPCODE_SHIFT) | ks->key_id) << 24
+ | (multi->tx_peer_id & 0xFFFFFF));
ASSERT(buf_write_prepend(buf, &peer, 4));
}
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index de89d30..c9818d4 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -697,8 +697,10 @@
#define AUTH_TOKEN_VALID_EMPTYUSER (1 << 2)
/* For P_DATA_V2 */
- uint32_t peer_id;
+ uint32_t rx_peer_id;
+ uint32_t tx_peer_id;
bool use_peer_id;
+ bool use_asymmetric_peer_id;
char *remote_ciphername; /**< cipher specified in peer's config file */
bool remote_usescomp; /**< remote announced comp-lzo in OCC string */
diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index 790e50f..a5c0bd6 100644
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -406,6 +406,7 @@
{
/* will return 0 if peer_info is null */
const unsigned int iv_proto_peer = extract_iv_proto(multi->peer_info);
+ const unsigned int tx_peer_id = extract_asymmetric_peer_id(multi->peer_info);
/* The other peer does not support P2P NCP */
if (!(iv_proto_peer & IV_PROTO_NCP_P2P))
@@ -416,7 +417,9 @@
if (iv_proto_peer & IV_PROTO_DATA_V2)
{
multi->use_peer_id = true;
- multi->peer_id = 0x76706e; /* 'v' 'p' 'n' */
+ multi->use_asymmetric_peer_id = true;
+ multi->rx_peer_id = 0x76706e; /* 'v' 'p' 'n' */
+ multi->tx_peer_id = 0x76706e; /* 'v' 'p' 'n' */
}
if (iv_proto_peer & IV_PROTO_CC_EXIT_NOTIFY)
@@ -440,7 +443,12 @@
{
session->opt->crypto_flags |= CO_USE_TLS_KEY_MATERIAL_EXPORT;
- if (multi->use_peer_id)
+ /* The asymmetric peer-id trumps on the EKM generated ones */
+ if ((tx_peer_id != UINT32_MAX) && (!multi->opt.dco_enabled))
+ {
+ multi->tx_peer_id = tx_peer_id;
+ }
+ else
{
/* Using a non hardcoded peer-id makes a tiny bit harder to
* fingerprint packets and also gives each connection a unique
@@ -458,7 +466,7 @@
}
else
{
- multi->peer_id = (peerid[0] << 16) + (peerid[1] << 8) + peerid[2];
+ multi->rx_peer_id = (peerid[0] << 16) + (peerid[1] << 8) + peerid[2];
}
}
}
@@ -500,11 +508,13 @@
common_cipher = BSTR(&out);
}
- msg(D_TLS_DEBUG_LOW,
- "P2P mode NCP negotiation result: "
- "TLS_export=%d, DATA_v2=%d, peer-id %d, epoch=%d, cipher=%s",
- (bool)(session->opt->crypto_flags & CO_USE_TLS_KEY_MATERIAL_EXPORT), multi->use_peer_id,
- multi->peer_id, (bool)(session->opt->crypto_flags & CO_EPOCH_DATA_KEY_FORMAT),
+ msg(D_TLS_DEBUG_LOW, "P2P mode NCP negotiation result: "
+ "TLS_export=%d, DATA_v2=%d, rx-peer-id %d, tx-peer-id %d, epoch=%d, cipher=%s",
+ (bool)(session->opt->crypto_flags & CO_USE_TLS_KEY_MATERIAL_EXPORT),
+ multi->use_peer_id,
+ multi->rx_peer_id,
+ multi->tx_peer_id,
+ (bool)(session->opt->crypto_flags & CO_EPOCH_DATA_KEY_FORMAT),
common_cipher);
gc_free(&gc);
diff --git a/src/openvpn/ssl_util.c b/src/openvpn/ssl_util.c
index fb7cf3e..af074c7 100644
--- a/src/openvpn/ssl_util.c
+++ b/src/openvpn/ssl_util.c
@@ -72,6 +72,24 @@
return 0;
}
+uint32_t
+extract_asymmetric_peer_id(const char *peer_info)
+{
+ const char *optstr = peer_info ? strstr(peer_info, "ID=") : NULL;
+ if (optstr)
+ {
+ uint32_t peer_id = 0;
+ int r = sscanf(optstr, "ID=%x", &peer_id);
+ {
+ if (r == 1 && peer_id >= 0)
+ {
+ return peer_id;
+ }
+ }
+ }
+ return UINT32_MAX;
+}
+
const char *
options_string_compat_lzo(const char *options, struct gc_arena *gc)
{
diff --git a/src/openvpn/ssl_util.h b/src/openvpn/ssl_util.h
index 007ed69..ec3c85a 100644
--- a/src/openvpn/ssl_util.h
+++ b/src/openvpn/ssl_util.h
@@ -53,6 +53,15 @@
*/
unsigned int extract_iv_proto(const char *peer_info);
+
+/**
+ * Extracts the ID variable and returns its value or
+ * UINT32_MAX if it cannot be extracted.
+ *
+ * @param peer_info peer info string to search for ID
+ */
+uint32_t extract_asymmetric_peer_id(const char *peer_info);
+
/**
* Takes a locally produced OCC string for TLS server mode and modifies as
* if the option comp-lzo was enabled. This is to send a client in
diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c
index 93dfa42..3cfe531 100644
--- a/tests/unit_tests/openvpn/test_crypto.c
+++ b/tests/unit_tests/openvpn/test_crypto.c
@@ -434,7 +434,7 @@
o.authname = "SHA1";
o.ciphername = "AES-256-GCM";
o.tls_client = true;
- o.peer_id = 77;
+ o.rx_peer_id = 77;
o.use_peer_id = true;
init_key_type(&kt, o.ciphername, o.authname, true, false);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9
Gerrit-Change-Number: 1089
Gerrit-PatchSet: 5
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: its_Giaan <gia...@ma...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-10-27 13:08:36
|
Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1312?usp=email
to review the following change.
Change subject: multi: Fix type handling for hashes, mostly inotify_watchers
......................................................................
multi: Fix type handling for hashes, mostly inotify_watchers
Change-Id: Idede28c850def5e3b4a17dcbd0a5771f15cfc668
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/multi.c
1 file changed, 15 insertions(+), 14 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/12/1312/1
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 74cc866..f9f6b16 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -268,7 +268,7 @@
*/
int_hash_function(const void *key, uint32_t iv)
{
- return (unsigned long)key;
+ return (uint32_t)(uintptr_t)key;
}
static bool
@@ -306,22 +306,23 @@
* to determine which client sent an incoming packet
* which is seen on the TCP/UDP socket.
*/
- m->hash = hash_init(t->options.real_hash_size, get_random(), mroute_addr_hash_function,
- mroute_addr_compare_function);
+ m->hash = hash_init(t->options.real_hash_size, (uint32_t)get_random(),
+ mroute_addr_hash_function, mroute_addr_compare_function);
/*
* Virtual address hash table. Used to determine
* which client to route a packet to.
*/
- m->vhash = hash_init(t->options.virtual_hash_size, get_random(), mroute_addr_hash_function,
- mroute_addr_compare_function);
+ m->vhash = hash_init(t->options.virtual_hash_size, (uint32_t)get_random(),
+ mroute_addr_hash_function, mroute_addr_compare_function);
/*
* This hash table is a clone of m->hash but with a
* bucket size of one so that it can be used
* for fast iteration through the list.
*/
- m->iter = hash_init(1, get_random(), mroute_addr_hash_function, mroute_addr_compare_function);
+ m->iter = hash_init(1, (uint32_t)get_random(), mroute_addr_hash_function,
+ mroute_addr_compare_function);
#ifdef ENABLE_MANAGEMENT
m->cid_hash = hash_init(t->options.real_hash_size, 0, cid_hash_function, cid_compare_function);
@@ -332,8 +333,8 @@
* Mapping between inotify watch descriptors and
* multi_instances.
*/
- m->inotify_watchers =
- hash_init(t->options.real_hash_size, get_random(), int_hash_function, int_compare_function);
+ m->inotify_watchers = hash_init(t->options.real_hash_size, (uint32_t)get_random(),
+ int_hash_function, int_compare_function);
#endif
/*
@@ -623,7 +624,7 @@
#ifdef ENABLE_ASYNC_PUSH
if (mi->inotify_watch != -1)
{
- hash_remove(m->inotify_watchers, (void *)(unsigned long)mi->inotify_watch);
+ hash_remove(m->inotify_watchers, (void *)(uintptr_t)mi->inotify_watch);
mi->inotify_watch = -1;
}
#endif
@@ -2825,7 +2826,7 @@
msg(D_MULTI_DEBUG, "MULTI: modified fd %d, mask %d", pevent->wd, pevent->mask);
struct multi_instance *mi =
- hash_lookup(m->inotify_watchers, (void *)(unsigned long)pevent->wd);
+ hash_lookup(m->inotify_watchers, (void *)(uintptr_t)pevent->wd);
if (pevent->mask & IN_CLOSE_WRITE)
{
@@ -2844,7 +2845,7 @@
/* this event is _always_ fired when watch is removed or file is deleted */
if (mi)
{
- hash_remove(m->inotify_watchers, (void *)(unsigned long)pevent->wd);
+ hash_remove(m->inotify_watchers, (void *)(uintptr_t)pevent->wd);
mi->inotify_watch = -1;
}
}
@@ -2984,14 +2985,14 @@
const char *file)
{
/* watch acf file */
- long watch_descriptor = inotify_add_watch(inotify_fd, file, IN_CLOSE_WRITE | IN_ONESHOT);
+ int watch_descriptor = inotify_add_watch(inotify_fd, file, IN_CLOSE_WRITE | IN_ONESHOT);
if (watch_descriptor >= 0)
{
if (mi->inotify_watch != -1)
{
- hash_remove(m->inotify_watchers, (void *)(unsigned long)mi->inotify_watch);
+ hash_remove(m->inotify_watchers, (void *)(uintptr_t)mi->inotify_watch);
}
- hash_add(m->inotify_watchers, (const uintptr_t *)watch_descriptor, mi, true);
+ hash_add(m->inotify_watchers, (void *)(uintptr_t)watch_descriptor, mi, true);
mi->inotify_watch = watch_descriptor;
}
else
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1312?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idede28c850def5e3b4a17dcbd0a5771f15cfc668
Gerrit-Change-Number: 1312
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-10-27 13:08:34
|
Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1310?usp=email
to review the following change.
Change subject: manage: Change kill_by_addr to use better types for port/proto
......................................................................
manage: Change kill_by_addr to use better types for port/proto
Change-Id: I750a0b8107baa77fb14558d0c8e4ef8020d62efd
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/manage.c
M src/openvpn/manage.h
M src/openvpn/multi.c
M src/openvpn/socket.h
4 files changed, 8 insertions(+), 8 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/10/1310/1
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 1cb5c63..823e46a 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -608,14 +608,14 @@
if (status)
{
const int port = atoi(p3);
- const int proto = (streq(p1, "tcp")) ? PROTO_TCP_SERVER
- : (streq(p1, "udp")) ? PROTO_UDP
- : PROTO_NONE;
+ const uint8_t proto = (streq(p1, "tcp")) ? PROTO_TCP_SERVER
+ : (streq(p1, "udp")) ? PROTO_UDP
+ : PROTO_NONE;
- if ((port > 0 && port < 65536) && (proto != PROTO_NONE))
+ if ((port > 0 && port < UINT16_MAX) && (proto != PROTO_NONE))
{
n_killed = (*man->persist.callback.kill_by_addr)(man->persist.callback.arg,
- addr, port, proto);
+ addr, (uint16_t)port, proto);
if (n_killed > 0)
{
msg(M_CLIENT, "SUCCESS: %d client(s) at address %s:%s:%d killed", n_killed,
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index a31eb06..1a14c9d 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -178,7 +178,7 @@
void (*status)(void *arg, const int version, struct status_output *so);
void (*show_net)(void *arg, const msglvl_t msglevel);
int (*kill_by_cn)(void *arg, const char *common_name);
- int (*kill_by_addr)(void *arg, const in_addr_t addr, const int port, const int proto);
+ int (*kill_by_addr)(void *arg, const in_addr_t addr, const uint16_t port, const uint8_t proto);
void (*delete_event)(void *arg, event_t event);
int (*n_clients)(void *arg);
bool (*send_cc_message)(void *arg, const char *message, const char *parameter);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index fa9c654..ad5004e 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3952,7 +3952,7 @@
}
static int
-management_callback_kill_by_addr(void *arg, const in_addr_t addr, const int port, const int proto)
+management_callback_kill_by_addr(void *arg, const in_addr_t addr, const uint16_t port, const uint8_t proto)
{
struct multi_context *m = (struct multi_context *)arg;
struct hash_iterator hi;
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index e986c9c..832d62e 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -89,7 +89,7 @@
const char *ipchange_command;
const struct plugin_list *plugins;
bool remote_float;
- int proto; /* Protocol (PROTO_x defined below) */
+ uint8_t proto; /* Protocol (PROTO_x defined below) */
sa_family_t af; /* Address family like AF_INET, AF_INET6 or AF_UNSPEC*/
bool bind_ipv6_only;
int mtu_changed; /* Set to true when mtu value is changed */
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1310?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I750a0b8107baa77fb14558d0c8e4ef8020d62efd
Gerrit-Change-Number: 1310
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-10-27 13:08:34
|
Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1313?usp=email
to review the following change.
Change subject: multi: Fix various conversion warnings
......................................................................
multi: Fix various conversion warnings
Mostly make required casts explicit.
Change-Id: I88cd7e1ebb49e97db33bad75c4fbbe23d196e964
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/multi.c
1 file changed, 8 insertions(+), 15 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/13/1313/1
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f9f6b16..50f83cb 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -256,11 +256,6 @@
#endif
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
#ifdef ENABLE_ASYNC_PUSH
static uint32_t
/*
@@ -2813,8 +2808,8 @@
multi_process_file_closed(struct multi_context *m, const unsigned int mpp_flags)
{
char buffer[INOTIFY_EVENT_BUFFER_SIZE];
- size_t buffer_i = 0;
- int r = read(m->top.c2.inotify_fd, buffer, INOTIFY_EVENT_BUFFER_SIZE);
+ ssize_t buffer_i = 0;
+ ssize_t r = read(m->top.c2.inotify_fd, buffer, INOTIFY_EVENT_BUFFER_SIZE);
while (buffer_i < r)
{
@@ -2948,21 +2943,23 @@
static inline unsigned int
compute_wakeup_sigma(const struct timeval *delta)
{
+ ASSERT(delta->tv_sec >= 0);
+ ASSERT(delta->tv_usec >= 0);
if (delta->tv_sec < 1)
{
/* if < 1 sec, fuzz = # of microseconds / 8 */
- return delta->tv_usec >> 3;
+ return (unsigned int)(delta->tv_usec >> 3);
}
else
{
/* if < 10 minutes, fuzz = 13.1% of timeout */
if (delta->tv_sec < 600)
{
- return delta->tv_sec << 17;
+ return (unsigned int)(delta->tv_sec << 17);
}
else
{
- return 120000000; /* if >= 10 minutes, fuzz = 2 minutes */
+ return 120 * 1000000; /* if >= 10 minutes, fuzz = 2 minutes */
}
}
}
@@ -3759,7 +3756,7 @@
for (i = 0; i < parm.packet_size; ++i)
{
- ASSERT(buf_write_u8(&buf, get_random() & 0xFF));
+ ASSERT(buf_write_u8(&buf, (uint8_t)(get_random() & 0xFF)));
}
for (i = 0; i < parm.n_packets; ++i)
@@ -3982,10 +3979,6 @@
return count;
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
static void
management_delete_event(void *arg, event_t event)
{
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1313?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I88cd7e1ebb49e97db33bad75c4fbbe23d196e964
Gerrit-Change-Number: 1313
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-10-27 13:08:34
|
Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1311?usp=email
to review the following change.
Change subject: multi: Fix wrong sigma value in multi_push_restart_schedule_exit
......................................................................
multi: Fix wrong sigma value in multi_push_restart_schedule_exit
Sigma was computed based on the absolute time and
not the delta.
Change-Id: I62b8263f18c4e2e7f5ecacb4616737f5ba836303
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/multi.c
1 file changed, 2 insertions(+), 4 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/11/1311/1
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index ad5004e..74cc866 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3843,7 +3843,6 @@
{
struct hash_iterator hi;
struct hash_element *he;
- struct timeval tv;
/* tell all clients to restart */
hash_iterator_init(m->iter, &hi);
@@ -3861,15 +3860,14 @@
/* reschedule signal */
ASSERT(!openvpn_gettimeofday(&m->deferred_shutdown_signal.wakeup, NULL));
- tv.tv_sec = 2;
- tv.tv_usec = 0;
+ struct timeval tv = { .tv_sec = 2, .tv_usec = 0 };
tv_add(&m->deferred_shutdown_signal.wakeup, &tv);
m->deferred_shutdown_signal.signal_received = m->top.sig->signal_received;
schedule_add_entry(m->schedule, (struct schedule_entry *)&m->deferred_shutdown_signal,
&m->deferred_shutdown_signal.wakeup,
- compute_wakeup_sigma(&m->deferred_shutdown_signal.wakeup));
+ compute_wakeup_sigma(&tv));
signal_reset(m->top.sig, 0);
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1311?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I62b8263f18c4e2e7f5ecacb4616737f5ba836303
Gerrit-Change-Number: 1311
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-10-27 13:08:23
|
Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1309?usp=email
to review the following change.
Change subject: mroute: Remove unused mask argument of mroute_get_in*
......................................................................
mroute: Remove unused mask argument of mroute_get_in*
These are obsolete since the removal of pf feature.
Avoids spurious conversion warnings.
Change-Id: I501bf780957a9c685eed5994a15de09c28efc3f0
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/mroute.c
1 file changed, 9 insertions(+), 18 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/09/1309/1
diff --git a/src/openvpn/mroute.c b/src/openvpn/mroute.c
index 88ea647..b50d48f 100644
--- a/src/openvpn/mroute.c
+++ b/src/openvpn/mroute.c
@@ -103,17 +103,12 @@
return true;
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
static inline void
-mroute_get_in_addr_t(struct mroute_addr *ma, const in_addr_t src, unsigned int mask)
+mroute_get_in_addr_t(struct mroute_addr *ma, const in_addr_t src)
{
if (ma)
{
- ma->type = MR_ADDR_IPV4 | mask;
+ ma->type = MR_ADDR_IPV4;
ma->netbits = 0;
ma->len = 4;
ma->v4.addr = src;
@@ -121,11 +116,11 @@
}
static inline void
-mroute_get_in6_addr(struct mroute_addr *ma, const struct in6_addr src, unsigned int mask)
+mroute_get_in6_addr(struct mroute_addr *ma, const struct in6_addr src)
{
if (ma)
{
- ma->type = MR_ADDR_IPV6 | mask;
+ ma->type = MR_ADDR_IPV6;
ma->netbits = 0;
ma->len = 16;
ma->v6.addr = src;
@@ -161,8 +156,8 @@
{
const struct openvpn_iphdr *ip = (const struct openvpn_iphdr *)BPTR(buf);
- mroute_get_in_addr_t(src, ip->saddr, 0);
- mroute_get_in_addr_t(dest, ip->daddr, 0);
+ mroute_get_in_addr_t(src, ip->saddr);
+ mroute_get_in_addr_t(dest, ip->daddr);
/* multicast packet? */
if (mroute_is_mcast(ip->daddr))
@@ -192,8 +187,8 @@
gc_free(&gc);
#endif
- mroute_get_in6_addr(src, ipv6->saddr, 0);
- mroute_get_in6_addr(dest, ipv6->daddr, 0);
+ mroute_get_in6_addr(src, ipv6->saddr);
+ mroute_get_in6_addr(dest, ipv6->daddr);
if (mroute_is_mcast_ipv6(ipv6->daddr))
{
@@ -342,7 +337,7 @@
}
else
{
- ma->v6.addr.s6_addr[byte--] &= (IPV4_NETMASK_HOST << bits_to_clear);
+ ma->v6.addr.s6_addr[byte--] &= (0xFF << bits_to_clear);
bits_to_clear = 0;
}
}
@@ -552,10 +547,6 @@
}
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
void
mroute_helper_free(struct mroute_helper *mh)
{
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1309?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I501bf780957a9c685eed5994a15de09c28efc3f0
Gerrit-Change-Number: 1309
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
|
|
From: plaisthos (C. Review) <ge...@op...> - 2025-10-27 12:20:05
|
Attention is currently required from: d12fk, flichtenheld, ordex, plaisthos.
Hello cron2, d12fk, flichtenheld, ordex,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1192?usp=email
to look at the new patch set (#13).
Change subject: Install host routes for ifconfig-push routes when DCO is enabled
......................................................................
Install host routes for ifconfig-push routes when DCO is enabled
ifconfig-push and ifconfig-ipv6-push can configure the IP address of a
client. If this IP address lies inside the network that is configured
on the ovpn/tun device this works as expected as the routing table point to
the ovpn/tun interface. However, if the IP address
is outside that range, the IP packets are not forwarded to the ovpn/tun
interface.
This patch adds logic to add host routes for these
ifconfig-push/ifconfig-ipv6-push addresses to ensure that traffic for
these IP addresses is also directed to the VPN.
For Linux it is important that these extra routes are routes using scope link
rather than static since otherwise routes via these IP addresses, like
iroute, will not work. On FreeBSD we also use interface routes as works and
routes that target interfaces instead of IP addresses are less brittle.
Tested using a server with ccd:
openvpn --server 10.33.0.0 255.255.192.0 --server-ipv6 fd00:f00f::1/64 --client-config-dir ~/ccd [...]
and a client with lwipvonpn and the following ccd file:
iroute-ipv6 FD00:F00F:CAFE::1001/64
ifconfig-ipv6-push FD00:F00F:D00D::77/64
push "setenv-safe ifconfig_ipv6_local_2 FD00:F00F:CAFE::1001"
push "setenv-safe ifconfig_ipv6_netbits_2 64"
iroute 10.234.234.0 255.255.255.0
ifconfig-push 10.11.12.13 255.255.255.0
push "setenv-safe ifconfig_local_2 10.234.234.12"
push "setenv-safe ifconfig_netmask_2 255.255.255.0"
This setups an ifconfig-push addresses outside the --server/--server-ipv6
network and additionally configures a iroute behind that client. The
setenv-safe configure lwipovpn to use that additional IP addresses to allow
testing via ping.
Windows behaves like the user space implementation. It does require these
special routes but instead (like user space) needs static routes to redirect
IP traffic for these IP addresses to the tunnel interface. E.g. in the example
above the server config needs to have:
route 10.234.234.0 255.255.255.0
route 10.11.12.0 255.255.255.0
route-ipv6 FD00:F00F:CAFE::1001/64
route-ipv6 FD00:F00F:D00D::77/64
Change-Id: I83295e00d1a756dfa44050b0a4493095fb050fff
Signed-off-by: Arne Schwabe <ar...@rf...>
---
M doc/man-sections/server-options.rst
M src/openvpn/dco.c
M src/openvpn/mroute.h
M src/openvpn/multi.c
M src/openvpn/multi.h
5 files changed, 156 insertions(+), 7 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/92/1192/13
diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst
index ccc1374..9c7fa46 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -314,6 +314,10 @@
3. Use ``--ifconfig-pool`` allocation for dynamic IP (last
choice).
+ When DCO is enabled and the IP is not in contained in the network specified
+ by ``--ifconfig``, OpenVPN will install a /32 host route for the ``local``
+ IP address.
+
--ifconfig-ipv6-push args
for ``--client-config-dir`` per-client static IPv6 interface
configuration, see ``--client-config-dir`` and ``--ifconfig-push`` for
@@ -324,6 +328,10 @@
ifconfig-ipv6-push ipv6addr/bits ipv6remote
+ When DCO is enabled and the IP is not in contained in the network specified
+ by ``--ifconfig-ipv6``, OpenVPN will install a /128 host route for the
+ ``ipv6addr`` IP address.
+
--multihome
Configure a multi-homed UDP server. This option needs to be used when a
server has more than one IP address (e.g. multiple interfaces, or
diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 8fb4662..d8ea269 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -664,6 +664,14 @@
return;
}
+#if defined(_WIN32)
+ if (addr->type & MR_ONLINK_DCO_ADDR)
+ {
+ /* Windows does not need these extra routes, so we ignore/skip them */
+ return;
+ }
+#endif
+
struct context *c = &mi->context;
if (addrtype == MR_ADDR_IPV6)
{
@@ -671,8 +679,14 @@
dco_win_add_iroute_ipv6(&c->c1.tuntap->dco, addr->v6.addr, addr->netbits,
c->c2.tls_multi->peer_id);
#else
+ const struct in6_addr *gateway = &mi->context.c2.push_ifconfig_ipv6_local;
+ if (addr->type & MR_ONLINK_DCO_ADDR)
+ {
+ gateway = NULL;
+ }
+
net_route_v6_add(&m->top.net_ctx, &addr->v6.addr, addr->netbits,
- &mi->context.c2.push_ifconfig_ipv6_local, c->c1.tuntap->actual_name, 0,
+ gateway, c->c1.tuntap->actual_name, 0,
DCO_IROUTE_METRIC);
#endif
}
@@ -683,7 +697,13 @@
c->c2.tls_multi->peer_id);
#else
in_addr_t dest = htonl(addr->v4.addr);
- net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits, &mi->context.c2.push_ifconfig_local,
+ const in_addr_t *gateway = &mi->context.c2.push_ifconfig_local;
+ if (addr->type & MR_ONLINK_DCO_ADDR)
+ {
+ gateway = NULL;
+ }
+
+ net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits, gateway,
c->c1.tuntap->actual_name, 0, DCO_IROUTE_METRIC);
#endif
}
@@ -714,6 +734,20 @@
DCO_IROUTE_METRIC);
#endif
}
+
+#if !defined(_WIN32)
+ /* Check if we added a host route as the assigned client IP address was
+ * not in the on link scope defined by --ifconfig */
+ in_addr_t ifconfig_local = mi->context.c2.push_ifconfig_local;
+
+ if (multi_check_push_ifconfig_extra_route(mi, ifconfig_local))
+ {
+ /* On windows we do not install these routes, so we also do not need to delete them */
+ net_route_v4_del(&m->top.net_ctx, &ifconfig_local,
+ 32, NULL, c->c1.tuntap->actual_name, 0,
+ DCO_IROUTE_METRIC);
+ }
+#endif
}
if (mi->context.c2.push_ifconfig_ipv6_defined)
@@ -728,6 +762,18 @@
DCO_IROUTE_METRIC);
#endif
}
+
+ /* Checked if we added a host route as the assigned client IP address was
+ * outside the --ifconfig-ipv6 tun interface config */
+#if !defined(_WIN32)
+ struct in6_addr *dest = &mi->context.c2.push_ifconfig_ipv6_local;
+ if (multi_check_push_ifconfig_ipv6_extra_route(mi, dest))
+ {
+ /* On windows we do not install these routes, so we also do not need to delete them */
+ net_route_v6_del(&m->top.net_ctx, dest, 128, NULL,
+ c->c1.tuntap->actual_name, 0, DCO_IROUTE_METRIC);
+ }
+#endif
}
#endif /* if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) || defined(_WIN32) */
}
diff --git a/src/openvpn/mroute.h b/src/openvpn/mroute.h
index 5b0c694..afd2e6c 100644
--- a/src/openvpn/mroute.h
+++ b/src/openvpn/mroute.h
@@ -20,6 +20,7 @@
* with this program; if not, see <https://www.gnu.org/licenses/>.
*/
+
#ifndef MROUTE_H
#define MROUTE_H
@@ -74,6 +75,9 @@
/* Address type mask indicating that proto # is part of address */
#define MR_WITH_PROTO 32
+/* MRoute is an on link/scope address needed for DCO on Unix platforms */
+#define MR_ONLINK_DCO_ADDR 64
+
struct mroute_addr
{
uint8_t len; /* length of address */
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f60944d..ba35556 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -42,6 +42,7 @@
#include "ssl_ncp.h"
#include "vlan.h"
#include "auth_token.h"
+#include "route.h"
#include <inttypes.h>
#include <string.h>
@@ -1231,11 +1232,18 @@
management_learn_addr(management, &mi->context.c2.mda_context, &addr, primary);
}
#endif
- if (!primary)
+ if (primary && multi_check_push_ifconfig_extra_route(mi, addr.v4.addr))
{
- /* "primary" is the VPN ifconfig address of the peer and already
- * known to DCO, so only install "extra" iroutes (primary = false)
- */
+ /* "primary" is the VPN ifconfig address of the peer */
+ /* if it does not fall into the network defined by ifconfig_local
+ * we install this as extra onscope address on the interface */
+ addr.netbits = 32;
+ addr.type |= MR_ONLINK_DCO_ADDR;
+
+ dco_install_iroute(m, mi, &addr);
+ }
+ else if (!primary)
+ {
ASSERT(netbits >= 0); /* DCO requires populated netbits */
dco_install_iroute(m, mi, &addr);
}
@@ -1269,7 +1277,17 @@
management_learn_addr(management, &mi->context.c2.mda_context, &addr, primary);
}
#endif
- if (!primary)
+ if (primary && multi_check_push_ifconfig_ipv6_extra_route(mi, &addr.v6.addr))
+ {
+ /* "primary" is the VPN ifconfig address of the peer */
+ /* if it does not fall into the network defined by ifconfig_local
+ * we install this as extra onscope address on the interface */
+ addr.netbits = 128;
+ addr.type |= MR_ONLINK_DCO_ADDR;
+
+ dco_install_iroute(m, mi, &addr);
+ }
+ else if (!primary)
{
/* "primary" is the VPN ifconfig address of the peer and already
* known to DCO, so only install "extra" iroutes (primary = false)
@@ -4391,3 +4409,49 @@
}
}
}
+
+bool
+multi_check_push_ifconfig_extra_route(struct multi_instance *mi, in_addr_t dest)
+{
+ struct options *o = &mi->context.options;
+ in_addr_t local_addr, local_netmask;
+
+ if (!o->ifconfig_local || !o->ifconfig_remote_netmask)
+ {
+ /* If we do not have a local address, we just return false as
+ * this check doesn't make sense. */
+ return false;
+ }
+
+ /* if it falls into the network defined by ifconfig_local we assume
+ * it is already known to DCO and only install "extra" iroutes */
+ inet_pton(AF_INET, o->ifconfig_local, &local_addr);
+ inet_pton(AF_INET, o->ifconfig_remote_netmask, &local_netmask);
+
+ return (local_addr & local_netmask) != (dest & local_netmask);
+}
+
+bool
+multi_check_push_ifconfig_ipv6_extra_route(struct multi_instance *mi,
+ struct in6_addr *dest)
+{
+ struct options *o = &mi->context.options;
+
+ if (!o->ifconfig_ipv6_local || !o->ifconfig_ipv6_netbits)
+ {
+ /* If we do not have a local address, we just return false as
+ * this check doesn't make sense. */
+ return false;
+ }
+
+ /* if it falls into the network defined by ifconfig_local we assume
+ * it is already known to DCO and only install "extra" iroutes */
+ struct in6_addr ifconfig_local;
+ if (inet_pton(AF_INET6, o->ifconfig_ipv6_local, &ifconfig_local) != 1)
+ {
+ return false;
+ }
+
+ return (!ipv6_net_contains_host(&ifconfig_local, o->ifconfig_ipv6_netbits,
+ dest));
+}
\ No newline at end of file
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 594ea3a..2fbb433 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -665,6 +665,33 @@
return ret;
}
+/**
+ * Determines if the ifconfig_push_local address falls into the range of the local
+ * IP addresses of the VPN interface (ifconfig_local with ifconfig_remote_netmask)
+ *
+ * @param mi The multi-instance to check this condition for
+ * @param dest The destination IP address to check
+ *
+ * @return Returns true if ifconfig_push is outside that range and requires an extra
+ * route to be installed.
+ */
+bool
+multi_check_push_ifconfig_extra_route(struct multi_instance *mi, in_addr_t dest);
+
+/**
+ * Determines if the ifconfig_ipv6_local address falls into the range of the local
+ * IP addresses of the VPN interface (ifconfig_local with ifconfig_remote_netmask)
+ *
+ * @param mi The multi-instance to check this condition for
+ * @param dest The destination IPv6 address to check
+ *
+ * @return Returns true if ifconfig_push is outside that range and requires an extra
+ * route to be installed.
+ */
+bool
+multi_check_push_ifconfig_ipv6_extra_route(struct multi_instance *mi,
+ struct in6_addr *dest);
+
/*
* Check for signals.
*/
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1192?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I83295e00d1a756dfa44050b0a4493095fb050fff
Gerrit-Change-Number: 1192
Gerrit-PatchSet: 13
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: ordex <an...@ma...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ordex <an...@ma...>
Gerrit-Attention: d12fk <he...@op...>
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-10-27 11:52:59
|
Attention is currently required from: cron2, plaisthos. flichtenheld has posted comments on this change by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1259?usp=email ) Change subject: init: Fix conversion warnings ...................................................................... Patch Set 6: (1 comment) Patchset: PS1: > I am not happy with this - there's a number of clear "positive integer values only" variables involv […] I have rechecked my changes. I really think this is the best possible code without changing the types of struct frame and struct buffer. Both of which would have widespread repercussions (obviously more for buffer than frame). So I would recommend we go with this for now. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1259?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I41b9f7686180f2549bd4984cbbf66059f0ba2b15 Gerrit-Change-Number: 1259 Gerrit-PatchSet: 6 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: cron2 <ge...@gr...> Gerrit-Comment-Date: Mon, 27 Oct 2025 11:52:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: cron2 <ge...@gr...> |
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-10-27 11:48:51
|
Attention is currently required from: MaxF, plaisthos. flichtenheld has posted comments on this change by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1304?usp=email ) Change subject: Add option to check tls-crypt-v2 key timestamps ...................................................................... Patch Set 3: -Code-Review Copied votes on follow-up patch sets have been updated: * Copied Code-Review vote has been removed from patch set 4 (was Code-Review-2) since the new Code-Review=0 vote is not copyable (copy condition: "changekind:NO_CHANGE OR changekind:TRIVIAL_REBASE OR is:MIN"). (1 comment) File src/openvpn/tls_crypt.c: http://gerrit.openvpn.net/c/openvpn/+/1304/comment/f76fba9a_587eaa9c?usp=email : PS2, Line 679: return tls_crypt_v2_check_client_key_age(ctx, opt->tls_crypt_v2_max_age); > That was bad, thanks for catching that! Should be fixed now. Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1304?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I0579d18c784e2ac16973d5553992c28f281a0900 Gerrit-Change-Number: 1304 Gerrit-PatchSet: 3 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: MaxF <ma...@ma...> Gerrit-Comment-Date: Mon, 27 Oct 2025 11:48:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: flichtenheld <fr...@li...> Comment-In-Reply-To: MaxF <ma...@ma...> |
|
From: d12fk (C. Review) <ge...@op...> - 2025-10-27 11:41:29
|
Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1308?usp=email
to review the following change.
Change subject: iservice: use interface index with netsh
......................................................................
iservice: use interface index with netsh
We use the interface index with netsh everywhere else, so convert this
invocation of netsh to index use as well.
Change-Id: I329b407693b97dd048bd3788ecad345d6e25dab2
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpnserv/interactive.c
1 file changed, 25 insertions(+), 37 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/08/1308/1
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index ce0d4dd..029c1d7 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -995,16 +995,16 @@
}
/**
- * Run the command: netsh interface ip $action wins $if_name [static] $addr
+ * Run the command: netsh interface ip $action wins $if_index [static] $addr
* @param action "delete", "add" or "set"
- * @param if_name "name_of_interface"
+ * @param if_index index of the interface to modify WINS for
* @param addr IPv4 address as a string
*
* If addr is null and action = "delete" all addresses are deleted.
* if action = "set" then "static" is added before $addr
*/
static DWORD
-netsh_wins_cmd(const wchar_t *action, const wchar_t *if_name, const wchar_t *addr)
+netsh_wins_cmd(const wchar_t *action, int if_index, const wchar_t *addr)
{
DWORD err = 0;
int timeout = 30000; /* in msec */
@@ -1030,10 +1030,10 @@
/* cmd template:
* netsh interface ip $action wins $if_name $static $addr
*/
- const wchar_t *fmt = L"netsh interface ip %ls wins \"%ls\" %ls %ls";
+ const wchar_t *fmt = L"netsh interface ip %ls wins %d %ls %ls";
/* max cmdline length in wchars -- include room for worst case and some */
- size_t ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(action) + wcslen(addr)
+ size_t ncmdline = wcslen(fmt) + 11 /*if_index*/ + wcslen(action) + wcslen(addr)
+ wcslen(addr_static) + 32 + 1;
cmdline = malloc(ncmdline * sizeof(wchar_t));
if (!cmdline)
@@ -1042,7 +1042,7 @@
goto out;
}
- swprintf(cmdline, ncmdline, fmt, action, if_name, addr_static, addr);
+ swprintf(cmdline, ncmdline, fmt, action, if_index, addr_static, addr);
err = ExecCommand(argv0, cmdline, timeout);
@@ -2882,7 +2882,7 @@
static DWORD
HandleWINSConfigMessage(const wins_cfg_message_t *msg, undo_lists_t *lists)
{
- DWORD err = 0;
+ DWORD err = NO_ERROR;
wchar_t addr[16]; /* large enough to hold string representation of an ipv4 */
int addr_len = msg->addr_len;
@@ -2892,38 +2892,25 @@
addr_len = _countof(msg->addr);
}
- if (!msg->iface.name[0]) /* interface name is required */
+ if (!msg->iface.index) /* interface index is required */
{
return ERROR_MESSAGE_DATA;
}
- /* use a non-const reference with limited scope to enforce null-termination of strings from
- * client */
- {
- wins_cfg_message_t *msgptr = (wins_cfg_message_t *)msg;
- msgptr->iface.name[_countof(msg->iface.name) - 1] = '\0';
- }
-
- wchar_t *wide_name = utf8to16(msg->iface.name); /* utf8 to wide-char */
- if (!wide_name)
- {
- return ERROR_OUTOFMEMORY;
- }
-
/* We delete all current addresses before adding any
* OR if the message type is del_wins_cfg
*/
if (addr_len > 0 || msg->header.type == msg_del_wins_cfg)
{
- err = netsh_wins_cmd(L"delete", wide_name, NULL);
+ err = netsh_wins_cmd(L"delete", msg->iface.index, NULL);
if (err)
{
goto out;
}
- free(RemoveListItem(&(*lists)[undo_wins], CmpWString, wide_name));
+ free(RemoveListItem(&(*lists)[undo_wins], CmpAny, NULL));
}
- if (msg->header.type == msg_del_wins_cfg)
+ if (addr_len == 0 || msg->header.type == msg_del_wins_cfg)
{
goto out; /* job done */
}
@@ -2931,7 +2918,7 @@
for (int i = 0; i < addr_len; ++i)
{
RtlIpv4AddressToStringW(&msg->addr[i].ipv4, addr);
- err = netsh_wins_cmd(i == 0 ? L"set" : L"add", wide_name, addr);
+ err = netsh_wins_cmd(i == 0 ? L"set" : L"add", msg->iface.index, addr);
if (i == 0 && err)
{
goto out;
@@ -2941,22 +2928,23 @@
*/
}
- err = 0;
-
- if (addr_len > 0)
+ int *if_index = malloc(sizeof(msg->iface.index));
+ if (if_index)
{
- wchar_t *tmp_name = _wcsdup(wide_name);
- if (!tmp_name || AddListItem(&(*lists)[undo_wins], tmp_name))
- {
- free(tmp_name);
- netsh_wins_cmd(L"delete", wide_name, NULL);
- err = ERROR_OUTOFMEMORY;
- goto out;
- }
+ *if_index = msg->iface.index;
}
+ if (!if_index || AddListItem(&(*lists)[undo_wins], if_index))
+ {
+ free(if_index);
+ netsh_wins_cmd(L"delete", msg->iface.index, NULL);
+ err = ERROR_OUTOFMEMORY;
+ goto out;
+ }
+
+ err = 0;
+
out:
- free(wide_name);
return err;
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1308?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I329b407693b97dd048bd3788ecad345d6e25dab2
Gerrit-Change-Number: 1308
Gerrit-PatchSet: 1
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: plaisthos (C. Review) <ge...@op...> - 2025-10-27 10:35:38
|
Attention is currently required from: d12fk, flichtenheld, ordex, plaisthos.
Hello cron2, d12fk, flichtenheld, ordex,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1192?usp=email
to look at the new patch set (#12).
Change subject: Install host routes for ifconfig-push routes when DCO is enabled
......................................................................
Install host routes for ifconfig-push routes when DCO is enabled
ifconfig-push and ifconfig-ipv6-push can configure the IP address of a
client. If this IP address lies inside the network that is configured
on the ovpn/tun device this works as expected as the routing table point to
the ovpn/tun interface. However, if the IP address
is outside that range, the IP packets are not forwarded to the ovpn/tun
interface.
This patch adds logic to add host routes for these
ifconfig-push/ifconfig-ipv6-push addresses to ensure that traffic for
these IP addresses is also directed to the VPN.
For Linux it is important that these extra routes are routes using scope link
rather than static since otherwise routes via these IP addresses, like
iroute, will not work. On FreeBSD we also use interface routes as works and
routes that target interfaces instead of IP addresses are less brittle.
Tested using a server with ccd:
openvpn --server 10.33.0.0 255.255.192.0 --server-ipv6 fd00:f00f::1/64 --client-config-dir ~/ccd [...]
and a client with lwipvonpn and the following ccd file:
iroute-ipv6 FD00:F00F:CAFE::1001/64
ifconfig-ipv6-push FD00:F00F:D00D::77/64
push "setenv-safe ifconfig_ipv6_local_2 FD00:F00F:CAFE::1001"
push "setenv-safe ifconfig_ipv6_netbits_2 64"
iroute 10.234.234.0 255.255.255.0
ifconfig-push 10.11.12.13 255.255.255.0
push "setenv-safe ifconfig_local_2 10.234.234.12"
push "setenv-safe ifconfig_netmask_2 255.255.255.0"
This setups an ifconfig-push addresses outside the --server/--server-ipv6
network and additionally configures a iroute behind that client. The
setenv-safe configure lwipovpn to use that additional IP addresses to allow
testing via ping.
Windows behaves like the user space implementation. It does require these
special routes but instead (like user space) needs static routes to redirect
IP traffic for these IP addresses to the tunnel interface. E.g. in the example
above the server config needs to have:
route 10.234.234.0 255.255.255.0
route 10.11.12.0 255.255.255.0
route-ipv6 FD00:F00F:CAFE::1001/64
route-ipv6 FD00:F00F:D00D::77/64
Change-Id: I83295e00d1a756dfa44050b0a4493095fb050fff
Signed-off-by: Arne Schwabe <ar...@rf...>
---
M doc/man-sections/server-options.rst
M src/openvpn/dco.c
M src/openvpn/mroute.h
M src/openvpn/multi.c
M src/openvpn/multi.h
5 files changed, 152 insertions(+), 7 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/92/1192/12
diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst
index ccc1374..9c7fa46 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -314,6 +314,10 @@
3. Use ``--ifconfig-pool`` allocation for dynamic IP (last
choice).
+ When DCO is enabled and the IP is not in contained in the network specified
+ by ``--ifconfig``, OpenVPN will install a /32 host route for the ``local``
+ IP address.
+
--ifconfig-ipv6-push args
for ``--client-config-dir`` per-client static IPv6 interface
configuration, see ``--client-config-dir`` and ``--ifconfig-push`` for
@@ -324,6 +328,10 @@
ifconfig-ipv6-push ipv6addr/bits ipv6remote
+ When DCO is enabled and the IP is not in contained in the network specified
+ by ``--ifconfig-ipv6``, OpenVPN will install a /128 host route for the
+ ``ipv6addr`` IP address.
+
--multihome
Configure a multi-homed UDP server. This option needs to be used when a
server has more than one IP address (e.g. multiple interfaces, or
diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 8fb4662..d8ea269 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -664,6 +664,14 @@
return;
}
+#if defined(_WIN32)
+ if (addr->type & MR_ONLINK_DCO_ADDR)
+ {
+ /* Windows does not need these extra routes, so we ignore/skip them */
+ return;
+ }
+#endif
+
struct context *c = &mi->context;
if (addrtype == MR_ADDR_IPV6)
{
@@ -671,8 +679,14 @@
dco_win_add_iroute_ipv6(&c->c1.tuntap->dco, addr->v6.addr, addr->netbits,
c->c2.tls_multi->peer_id);
#else
+ const struct in6_addr *gateway = &mi->context.c2.push_ifconfig_ipv6_local;
+ if (addr->type & MR_ONLINK_DCO_ADDR)
+ {
+ gateway = NULL;
+ }
+
net_route_v6_add(&m->top.net_ctx, &addr->v6.addr, addr->netbits,
- &mi->context.c2.push_ifconfig_ipv6_local, c->c1.tuntap->actual_name, 0,
+ gateway, c->c1.tuntap->actual_name, 0,
DCO_IROUTE_METRIC);
#endif
}
@@ -683,7 +697,13 @@
c->c2.tls_multi->peer_id);
#else
in_addr_t dest = htonl(addr->v4.addr);
- net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits, &mi->context.c2.push_ifconfig_local,
+ const in_addr_t *gateway = &mi->context.c2.push_ifconfig_local;
+ if (addr->type & MR_ONLINK_DCO_ADDR)
+ {
+ gateway = NULL;
+ }
+
+ net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits, gateway,
c->c1.tuntap->actual_name, 0, DCO_IROUTE_METRIC);
#endif
}
@@ -714,6 +734,20 @@
DCO_IROUTE_METRIC);
#endif
}
+
+#if !defined(_WIN32)
+ /* Check if we added a host route as the assigned client IP address was
+ * not in the on link scope defined by --ifconfig */
+ in_addr_t ifconfig_local = mi->context.c2.push_ifconfig_local;
+
+ if (multi_check_push_ifconfig_extra_route(mi, ifconfig_local))
+ {
+ /* On windows we do not install these routes, so we also do not need to delete them */
+ net_route_v4_del(&m->top.net_ctx, &ifconfig_local,
+ 32, NULL, c->c1.tuntap->actual_name, 0,
+ DCO_IROUTE_METRIC);
+ }
+#endif
}
if (mi->context.c2.push_ifconfig_ipv6_defined)
@@ -728,6 +762,18 @@
DCO_IROUTE_METRIC);
#endif
}
+
+ /* Checked if we added a host route as the assigned client IP address was
+ * outside the --ifconfig-ipv6 tun interface config */
+#if !defined(_WIN32)
+ struct in6_addr *dest = &mi->context.c2.push_ifconfig_ipv6_local;
+ if (multi_check_push_ifconfig_ipv6_extra_route(mi, dest))
+ {
+ /* On windows we do not install these routes, so we also do not need to delete them */
+ net_route_v6_del(&m->top.net_ctx, dest, 128, NULL,
+ c->c1.tuntap->actual_name, 0, DCO_IROUTE_METRIC);
+ }
+#endif
}
#endif /* if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) || defined(_WIN32) */
}
diff --git a/src/openvpn/mroute.h b/src/openvpn/mroute.h
index 5b0c694..afd2e6c 100644
--- a/src/openvpn/mroute.h
+++ b/src/openvpn/mroute.h
@@ -20,6 +20,7 @@
* with this program; if not, see <https://www.gnu.org/licenses/>.
*/
+
#ifndef MROUTE_H
#define MROUTE_H
@@ -74,6 +75,9 @@
/* Address type mask indicating that proto # is part of address */
#define MR_WITH_PROTO 32
+/* MRoute is an on link/scope address needed for DCO on Unix platforms */
+#define MR_ONLINK_DCO_ADDR 64
+
struct mroute_addr
{
uint8_t len; /* length of address */
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f60944d..ba35556 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -42,6 +42,7 @@
#include "ssl_ncp.h"
#include "vlan.h"
#include "auth_token.h"
+#include "route.h"
#include <inttypes.h>
#include <string.h>
@@ -1231,11 +1232,18 @@
management_learn_addr(management, &mi->context.c2.mda_context, &addr, primary);
}
#endif
- if (!primary)
+ if (primary && multi_check_push_ifconfig_extra_route(mi, addr.v4.addr))
{
- /* "primary" is the VPN ifconfig address of the peer and already
- * known to DCO, so only install "extra" iroutes (primary = false)
- */
+ /* "primary" is the VPN ifconfig address of the peer */
+ /* if it does not fall into the network defined by ifconfig_local
+ * we install this as extra onscope address on the interface */
+ addr.netbits = 32;
+ addr.type |= MR_ONLINK_DCO_ADDR;
+
+ dco_install_iroute(m, mi, &addr);
+ }
+ else if (!primary)
+ {
ASSERT(netbits >= 0); /* DCO requires populated netbits */
dco_install_iroute(m, mi, &addr);
}
@@ -1269,7 +1277,17 @@
management_learn_addr(management, &mi->context.c2.mda_context, &addr, primary);
}
#endif
- if (!primary)
+ if (primary && multi_check_push_ifconfig_ipv6_extra_route(mi, &addr.v6.addr))
+ {
+ /* "primary" is the VPN ifconfig address of the peer */
+ /* if it does not fall into the network defined by ifconfig_local
+ * we install this as extra onscope address on the interface */
+ addr.netbits = 128;
+ addr.type |= MR_ONLINK_DCO_ADDR;
+
+ dco_install_iroute(m, mi, &addr);
+ }
+ else if (!primary)
{
/* "primary" is the VPN ifconfig address of the peer and already
* known to DCO, so only install "extra" iroutes (primary = false)
@@ -4391,3 +4409,49 @@
}
}
}
+
+bool
+multi_check_push_ifconfig_extra_route(struct multi_instance *mi, in_addr_t dest)
+{
+ struct options *o = &mi->context.options;
+ in_addr_t local_addr, local_netmask;
+
+ if (!o->ifconfig_local || !o->ifconfig_remote_netmask)
+ {
+ /* If we do not have a local address, we just return false as
+ * this check doesn't make sense. */
+ return false;
+ }
+
+ /* if it falls into the network defined by ifconfig_local we assume
+ * it is already known to DCO and only install "extra" iroutes */
+ inet_pton(AF_INET, o->ifconfig_local, &local_addr);
+ inet_pton(AF_INET, o->ifconfig_remote_netmask, &local_netmask);
+
+ return (local_addr & local_netmask) != (dest & local_netmask);
+}
+
+bool
+multi_check_push_ifconfig_ipv6_extra_route(struct multi_instance *mi,
+ struct in6_addr *dest)
+{
+ struct options *o = &mi->context.options;
+
+ if (!o->ifconfig_ipv6_local || !o->ifconfig_ipv6_netbits)
+ {
+ /* If we do not have a local address, we just return false as
+ * this check doesn't make sense. */
+ return false;
+ }
+
+ /* if it falls into the network defined by ifconfig_local we assume
+ * it is already known to DCO and only install "extra" iroutes */
+ struct in6_addr ifconfig_local;
+ if (inet_pton(AF_INET6, o->ifconfig_ipv6_local, &ifconfig_local) != 1)
+ {
+ return false;
+ }
+
+ return (!ipv6_net_contains_host(&ifconfig_local, o->ifconfig_ipv6_netbits,
+ dest));
+}
\ No newline at end of file
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 594ea3a..a26514d 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -665,6 +665,29 @@
return ret;
}
+/**
+ * Determines if the ifconfig_push_local address falls into the range of the local
+ * IP addresses of the VPN interface (ifconfig_local with ifconfig_remote_netmask)
+ *
+ * @param mi The multi-instance to check this condition for
+ *
+ * @return Returns true if ifconfig_push is outside that range and requires an extra
+ * route to be installed.
+ */
+bool
+multi_check_push_ifconfig_extra_route(struct multi_instance *mi, in_addr_t dest);
+
+/**
+ * Determines if the ifconfig_ipv6_local address falls into the range of the local
+ * IP addresses of the VPN interface (ifconfig_local with ifconfig_remote_netmask)
+ *
+ * @return Returns true if ifconfig_push is outside that range and requires an extra
+ * route to be installed.
+ */
+bool
+multi_check_push_ifconfig_ipv6_extra_route(struct multi_instance *mi,
+ struct in6_addr *dest);
+
/*
* Check for signals.
*/
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1192?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I83295e00d1a756dfa44050b0a4493095fb050fff
Gerrit-Change-Number: 1192
Gerrit-PatchSet: 12
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: ordex <an...@ma...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: ordex <an...@ma...>
Gerrit-Attention: d12fk <he...@op...>
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-27 10:02:59
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email ) Change subject: ssl_mbedtls: fix missing perf_pop() call ...................................................................... ssl_mbedtls: fix missing perf_pop() call This was triggered by a bug report submitted by Joshua Rogers, who used ZeroPath to discover we missed a perf_pop() call in one of the error paths of ssl_mbedtls.c. Move an existing perf_pop call a bit upwards to fix that. The perf code is always disabled by ENABLE_PERFORMANCE_METRICS being commented out in perf.h. There is no configure flag. None of the active developers remembers using it and the git log shows no actual code changes since at least the project structure overhaul of 2012. So this has no real-world impact. Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0 Signed-off-by: Steffan Karger <st...@ka...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1305 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33870.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/ssl_mbedtls.c 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 8fb69c3..2862989 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1489,13 +1489,13 @@ /* Error during read, check for retry error */ if (retval < 0) { + perf_pop(); if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; } mbed_log_err(D_TLS_ERRORS, retval, "TLS_ERROR: read tls_read_plaintext error"); buf->len = 0; - perf_pop(); return -1; } /* Nothing read, try again */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: release/2.6 Gerrit-Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0 Gerrit-Change-Number: 1305 Gerrit-PatchSet: 2 Gerrit-Owner: syzzer <st...@ka...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-27 10:02:58
|
cron2 has uploaded a new patch set (#2) to the change originally created by syzzer. ( http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: ssl_mbedtls: fix missing perf_pop() call ...................................................................... ssl_mbedtls: fix missing perf_pop() call This was triggered by a bug report submitted by Joshua Rogers, who used ZeroPath to discover we missed a perf_pop() call in one of the error paths of ssl_mbedtls.c. Move an existing perf_pop call a bit upwards to fix that. The perf code is always disabled by ENABLE_PERFORMANCE_METRICS being commented out in perf.h. There is no configure flag. None of the active developers remembers using it and the git log shows no actual code changes since at least the project structure overhaul of 2012. So this has no real-world impact. Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0 Signed-off-by: Steffan Karger <st...@ka...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1305 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33870.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/ssl_mbedtls.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/05/1305/2 diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 8fb69c3..2862989 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1489,13 +1489,13 @@ /* Error during read, check for retry error */ if (retval < 0) { + perf_pop(); if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; } mbed_log_err(D_TLS_ERRORS, retval, "TLS_ERROR: read tls_read_plaintext error"); buf->len = 0; - perf_pop(); return -1; } /* Nothing read, try again */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1305?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: release/2.6 Gerrit-Change-Id: I5b6881dc73358c8d1249ee2ceb968ede295105b0 Gerrit-Change-Number: 1305 Gerrit-PatchSet: 2 Gerrit-Owner: syzzer <st...@ka...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: Gert D. <ge...@gr...> - 2025-10-27 10:02:06
|
While this is sort-of a NOP in 2.6, at least it's a consistent NOP now :-)
Your patch has been applied to the release/2.6 branch.
commit e83c63f58135913bb2ca2f4345da8cd77098474b
Author: Steffan Karger
Date: Sun Oct 26 15:35:14 2025 +0100
ssl_mbedtls: fix missing perf_pop() call
Signed-off-by: Steffan Karger <st...@ka...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1305
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33870.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: MaxF (C. Review) <ge...@op...> - 2025-10-27 09:55:30
|
Attention is currently required from: flichtenheld, plaisthos.
Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1304?usp=email
to look at the new patch set (#4).
Change subject: Add option to check tls-crypt-v2 key timestamps
......................................................................
Add option to check tls-crypt-v2 key timestamps
This commit adds the option --tls-crypt-v2-max-age n. When a client key
is older than n days or has no timestamp, the server rejects it.
Based on work by Rein van Baaren for Sentyron.
Co-authored-by: Rein van Baaren <rev...@pr...>
Change-Id: I0579d18c784e2ac16973d5553992c28f281a0900
Signed-off-by: Max Fillinger <ma...@ma...>
---
M doc/man-sections/tls-options.rst
M doc/tls-crypt-v2.txt
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/ssl_common.h
M src/openvpn/tls_crypt.c
7 files changed, 49 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/1304/4
diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst
index db107e6..63cb32f 100644
--- a/doc/man-sections/tls-options.rst
+++ b/doc/man-sections/tls-options.rst
@@ -568,6 +568,10 @@
The command can reject the connection by exiting with a non-zero exit
code.
+--tls-crypt-v2-max-age n
+ Reject tls-crypt-v2 client keys that are older than n days or have
+ no timestamp.
+
--tls-exit
Exit on TLS negotiation failure. This option can be useful when you only
want to make one attempt at connecting, e.g. in a test or monitoring script.
diff --git a/doc/tls-crypt-v2.txt b/doc/tls-crypt-v2.txt
index 7dcd041..c2e9deb 100644
--- a/doc/tls-crypt-v2.txt
+++ b/doc/tls-crypt-v2.txt
@@ -139,7 +139,10 @@
The message is dropped and no error response is sent when either 3.1, 3.2 or
3.3 fails (DoS protection).
-4. Server optionally checks metadata using a --tls-crypt-v2-verify script
+4. The server optionally checks if the client key contains a timestamp that is
+ below a maximum age configured with the --tls-crypt-v2-max-age option.
+
+5. Server optionally checks metadata using a --tls-crypt-v2-verify script
This allows early abort of connection, *before* we expose any of the
notoriously dangerous TLS, X.509 and ASN.1 parsers and thereby reduces the
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index aa2611d..a6331dc 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3442,6 +3442,7 @@
{
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;
+ to.tls_crypt_v2_max_age = c->options.tls_crypt_v2_max_age;
if (options->ce.tls_crypt_v2_force_cookie)
{
to.tls_wrap.opt.flags |= CO_FORCE_TLSCRYPTV2_COOKIE;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 65c6b3b..2cdf58f 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -653,6 +653,8 @@
" fresh tls-crypt-v2 server key, and store to keyfile\n"
"--tls-crypt-v2-verify cmd : Run command cmd to verify the metadata of the\n"
" client-supplied tls-crypt-v2 client key\n"
+ "--tls-crypt-v2-max-age n : Only accept tls-crypt-v2 client keys that have a\n"
+ " timestamp which is at most n days old.\n"
"--askpass [file]: Get PEM password from controlling tty before we daemonize.\n"
"--auth-nocache : Don't cache --askpass or --auth-user-pass passwords.\n"
"--crl-verify crl ['dir']: Check peer certificate against a CRL.\n"
@@ -9087,6 +9089,14 @@
VERIFY_PERMISSION(OPT_P_GENERAL);
options->tls_crypt_v2_verify_script = p[1];
}
+ else if (streq(p[0], "tls-crypt-v2-max-age") && p[1])
+ {
+ VERIFY_PERMISSION(OPT_P_GENERAL);
+ if (!atoi_constrained(p[1], &options->tls_crypt_v2_max_age, "tls-crypt-v2-max-age", 1, INT_MAX, msglevel))
+ {
+ goto err;
+ }
+ }
else if (streq(p[0], "x509-track") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_GENERAL);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 009904a..9329030 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -682,6 +682,8 @@
const char *tls_crypt_v2_verify_script;
+ int tls_crypt_v2_max_age;
+
/* Allow only one session */
bool single_session;
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index de89d30..0402a6a 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -383,6 +383,7 @@
bool tls_crypt_v2;
const char *tls_crypt_v2_verify_script;
+ int tls_crypt_v2_max_age;
/** TLS handshake wrapping state */
struct tls_wrap_ctx tls_wrap;
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 51b4eb3..83355f9 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -29,6 +29,7 @@
#include "argv.h"
#include "base64.h"
#include "crypto.h"
+#include "integer.h"
#include "platform.h"
#include "run_command.h"
#include "session_id.h"
@@ -528,6 +529,27 @@
}
static bool
+tls_crypt_v2_check_client_key_age(const struct tls_wrap_ctx *ctx, int max_days)
+{
+ const uint8_t *metadata = ctx->tls_crypt_v2_metadata.data;
+ if (*metadata != TLS_CRYPT_METADATA_TYPE_TIMESTAMP)
+ {
+ msg(M_WARN, "ERROR: Client key doesn't have a timestamp.");
+ return false;
+ }
+ int64_t timestamp;
+ memcpy(×tamp, metadata + 1, sizeof(int64_t));
+ timestamp = (int64_t)ntohll((uint64_t)timestamp);
+ int64_t max_age_in_seconds = max_days * 24 * 60 * 60;
+ if (now - timestamp > max_age_in_seconds)
+ {
+ msg(M_WARN, "ERROR: Client key is too old.");
+ return false;
+ }
+ return true;
+}
+
+static bool
tls_crypt_v2_verify_metadata(const struct tls_wrap_ctx *ctx, const struct tls_options *opt)
{
bool ret = false;
@@ -652,6 +674,11 @@
/* Remove client key from buffer so tls-crypt code can unwrap message */
ASSERT(buf_inc_len(buf, -(BLEN(&wrapped_client_key))));
+ if (opt && opt->tls_crypt_v2_max_age > 0 && !tls_crypt_v2_check_client_key_age(ctx, opt->tls_crypt_v2_max_age))
+ {
+ return false;
+ }
+
if (opt && opt->tls_crypt_v2_verify_script)
{
return tls_crypt_v2_verify_metadata(ctx, opt);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1304?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0579d18c784e2ac16973d5553992c28f281a0900
Gerrit-Change-Number: 1304
Gerrit-PatchSet: 4
Gerrit-Owner: MaxF <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: MaxF (C. Review) <ge...@op...> - 2025-10-27 08:51:46
|
Attention is currently required from: flichtenheld, plaisthos. MaxF has posted comments on this change by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1304?usp=email ) Change subject: Add option to check tls-crypt-v2 key timestamps ...................................................................... Patch Set 3: (1 comment) File src/openvpn/tls_crypt.c: http://gerrit.openvpn.net/c/openvpn/+/1304/comment/906940de_55b6796f?usp=email : PS2, Line 679: return tls_crypt_v2_check_client_key_age(ctx, opt->tls_crypt_v2_max_age); > that is logically wrong. […] That was bad, thanks for catching that! Should be fixed now. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1304?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I0579d18c784e2ac16973d5553992c28f281a0900 Gerrit-Change-Number: 1304 Gerrit-PatchSet: 3 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Mon, 27 Oct 2025 08:51:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: flichtenheld <fr...@li...> |
|
From: MaxF (C. Review) <ge...@op...> - 2025-10-27 08:48:51
|
Attention is currently required from: MaxF, plaisthos.
Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1304?usp=email
to look at the new patch set (#3).
Change subject: Add option to check tls-crypt-v2 key timestamps
......................................................................
Add option to check tls-crypt-v2 key timestamps
This commit adds the option --tls-crypt-v2-max-age n. When a client key
is older than n days or has no timestamp, the server rejects it.
Based on work by Rein van Baaren for Sentyron.
Co-authored-by: Rein van Baaren <rev...@pr...>
Change-Id: I0579d18c784e2ac16973d5553992c28f281a0900
---
M doc/man-sections/tls-options.rst
M doc/tls-crypt-v2.txt
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/ssl_common.h
M src/openvpn/tls_crypt.c
7 files changed, 49 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/1304/3
diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst
index db107e6..63cb32f 100644
--- a/doc/man-sections/tls-options.rst
+++ b/doc/man-sections/tls-options.rst
@@ -568,6 +568,10 @@
The command can reject the connection by exiting with a non-zero exit
code.
+--tls-crypt-v2-max-age n
+ Reject tls-crypt-v2 client keys that are older than n days or have
+ no timestamp.
+
--tls-exit
Exit on TLS negotiation failure. This option can be useful when you only
want to make one attempt at connecting, e.g. in a test or monitoring script.
diff --git a/doc/tls-crypt-v2.txt b/doc/tls-crypt-v2.txt
index 7dcd041..c2e9deb 100644
--- a/doc/tls-crypt-v2.txt
+++ b/doc/tls-crypt-v2.txt
@@ -139,7 +139,10 @@
The message is dropped and no error response is sent when either 3.1, 3.2 or
3.3 fails (DoS protection).
-4. Server optionally checks metadata using a --tls-crypt-v2-verify script
+4. The server optionally checks if the client key contains a timestamp that is
+ below a maximum age configured with the --tls-crypt-v2-max-age option.
+
+5. Server optionally checks metadata using a --tls-crypt-v2-verify script
This allows early abort of connection, *before* we expose any of the
notoriously dangerous TLS, X.509 and ASN.1 parsers and thereby reduces the
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index aa2611d..a6331dc 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3442,6 +3442,7 @@
{
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;
+ to.tls_crypt_v2_max_age = c->options.tls_crypt_v2_max_age;
if (options->ce.tls_crypt_v2_force_cookie)
{
to.tls_wrap.opt.flags |= CO_FORCE_TLSCRYPTV2_COOKIE;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 65c6b3b..2cdf58f 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -653,6 +653,8 @@
" fresh tls-crypt-v2 server key, and store to keyfile\n"
"--tls-crypt-v2-verify cmd : Run command cmd to verify the metadata of the\n"
" client-supplied tls-crypt-v2 client key\n"
+ "--tls-crypt-v2-max-age n : Only accept tls-crypt-v2 client keys that have a\n"
+ " timestamp which is at most n days old.\n"
"--askpass [file]: Get PEM password from controlling tty before we daemonize.\n"
"--auth-nocache : Don't cache --askpass or --auth-user-pass passwords.\n"
"--crl-verify crl ['dir']: Check peer certificate against a CRL.\n"
@@ -9087,6 +9089,14 @@
VERIFY_PERMISSION(OPT_P_GENERAL);
options->tls_crypt_v2_verify_script = p[1];
}
+ else if (streq(p[0], "tls-crypt-v2-max-age") && p[1])
+ {
+ VERIFY_PERMISSION(OPT_P_GENERAL);
+ if (!atoi_constrained(p[1], &options->tls_crypt_v2_max_age, "tls-crypt-v2-max-age", 1, INT_MAX, msglevel))
+ {
+ goto err;
+ }
+ }
else if (streq(p[0], "x509-track") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_GENERAL);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 009904a..9329030 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -682,6 +682,8 @@
const char *tls_crypt_v2_verify_script;
+ int tls_crypt_v2_max_age;
+
/* Allow only one session */
bool single_session;
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index de89d30..0402a6a 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -383,6 +383,7 @@
bool tls_crypt_v2;
const char *tls_crypt_v2_verify_script;
+ int tls_crypt_v2_max_age;
/** TLS handshake wrapping state */
struct tls_wrap_ctx tls_wrap;
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 51b4eb3..83355f9 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -29,6 +29,7 @@
#include "argv.h"
#include "base64.h"
#include "crypto.h"
+#include "integer.h"
#include "platform.h"
#include "run_command.h"
#include "session_id.h"
@@ -528,6 +529,27 @@
}
static bool
+tls_crypt_v2_check_client_key_age(const struct tls_wrap_ctx *ctx, int max_days)
+{
+ const uint8_t *metadata = ctx->tls_crypt_v2_metadata.data;
+ if (*metadata != TLS_CRYPT_METADATA_TYPE_TIMESTAMP)
+ {
+ msg(M_WARN, "ERROR: Client key doesn't have a timestamp.");
+ return false;
+ }
+ int64_t timestamp;
+ memcpy(×tamp, metadata + 1, sizeof(int64_t));
+ timestamp = (int64_t)ntohll((uint64_t)timestamp);
+ int64_t max_age_in_seconds = max_days * 24 * 60 * 60;
+ if (now - timestamp > max_age_in_seconds)
+ {
+ msg(M_WARN, "ERROR: Client key is too old.");
+ return false;
+ }
+ return true;
+}
+
+static bool
tls_crypt_v2_verify_metadata(const struct tls_wrap_ctx *ctx, const struct tls_options *opt)
{
bool ret = false;
@@ -652,6 +674,11 @@
/* Remove client key from buffer so tls-crypt code can unwrap message */
ASSERT(buf_inc_len(buf, -(BLEN(&wrapped_client_key))));
+ if (opt && opt->tls_crypt_v2_max_age > 0 && !tls_crypt_v2_check_client_key_age(ctx, opt->tls_crypt_v2_max_age))
+ {
+ return false;
+ }
+
if (opt && opt->tls_crypt_v2_verify_script)
{
return tls_crypt_v2_verify_metadata(ctx, opt);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1304?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0579d18c784e2ac16973d5553992c28f281a0900
Gerrit-Change-Number: 1304
Gerrit-PatchSet: 3
Gerrit-Owner: MaxF <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: MaxF <ma...@ma...>
|
|
From: d12fk (C. Review) <ge...@op...> - 2025-10-26 22:14:38
|
Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1307?usp=email
to review the following change.
Change subject: iservice: validate config path better
......................................................................
iservice: validate config path better
Instead of just rejecting any path that contains ".." use some WIN32 API
functions to combine, canonicalize and then check if the resulting
path is located under the config directory. Makes the code prettier
and more correct.
Change-Id: I0e94068f467f2899daf133b032a785d2d7fc05e4
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpnserv/CMakeLists.txt
M src/openvpnserv/validate.c
2 files changed, 10 insertions(+), 19 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/07/1307/1
diff --git a/src/openvpnserv/CMakeLists.txt b/src/openvpnserv/CMakeLists.txt
index 340b904..a30b164 100644
--- a/src/openvpnserv/CMakeLists.txt
+++ b/src/openvpnserv/CMakeLists.txt
@@ -31,7 +31,7 @@
)
target_link_libraries(openvpnserv
advapi32.lib userenv.lib iphlpapi.lib fwpuclnt.lib rpcrt4.lib
- shlwapi.lib netapi32.lib ws2_32.lib ntdll.lib ole32.lib)
+ shlwapi.lib netapi32.lib ws2_32.lib ntdll.lib ole32.lib pathcch.lib)
if (MINGW)
target_compile_options(openvpnserv PRIVATE -municode)
target_link_options(openvpnserv PRIVATE -municode)
diff --git a/src/openvpnserv/validate.c b/src/openvpnserv/validate.c
index 59d5b86..675f8a8 100644
--- a/src/openvpnserv/validate.c
+++ b/src/openvpnserv/validate.c
@@ -24,6 +24,7 @@
#include <lmaccess.h>
#include <shlwapi.h>
+#include <pathcch.h>
#include <lm.h>
static const WCHAR *white_list[] = {
@@ -53,36 +54,26 @@
static PTOKEN_GROUPS GetTokenGroups(const HANDLE token);
/*
- * Check workdir\fname is inside config_dir
- * The logic here is simple: we may reject some valid paths if ..\ is in any of the strings
+ * Check that config path is inside config_dir
+ * The logic here is simple: if the path isn't prefixed with config_dir it's rejected
*/
static BOOL
CheckConfigPath(const WCHAR *workdir, const WCHAR *fname, const settings_t *s)
{
- WCHAR tmp[MAX_PATH];
- const WCHAR *config_file = NULL;
- const WCHAR *config_dir = NULL;
+ HRESULT res;
+ WCHAR config_path[MAX_PATH];
- /* convert fname to full path */
+ /* convert fname to full canonical path */
if (PathIsRelativeW(fname))
{
- swprintf(tmp, _countof(tmp), L"%ls\\%ls", workdir, fname);
- config_file = tmp;
+ res = PathCchCombine(config_path, sizeof(config_path), workdir, fname);
}
else
{
- config_file = fname;
+ res = PathCchCanonicalize(config_path, sizeof(config_path), fname);
}
- config_dir = s->config_dir;
-
- if (wcsncmp(config_dir, config_file, wcslen(config_dir)) == 0
- && wcsstr(config_file + wcslen(config_dir), L"..") == NULL)
- {
- return TRUE;
- }
-
- return FALSE;
+ return res == S_OK && wcsncmp(config_path, s->config_dir, wcslen(s->config_dir)) == 0;
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1307?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0e94068f467f2899daf133b032a785d2d7fc05e4
Gerrit-Change-Number: 1307
Gerrit-PatchSet: 1
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-26 21:50:42
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1303?usp=email ) Change subject: Remove perf.c/perf.h ...................................................................... Remove perf.c/perf.h This code was always disabled by ENABLE_PERFORMANCE_METRICS being commented out in perf.h. There was no configure flag. None of the active developers remembers using it, the git log shows no actual code changes since at least the project structure overhaul of 2012, and tools like gprof are nowadays the go-to tool for performance profiling. So, out with our custom implementation. This was triggered by a bug report submitted by Joshua Rogers, who used ZeroPath to discover we missed a perf_pop() call in one of the error paths of ssl_mbedtls.c. This commit resolves that using git rm. Change-Id: I5bb666a73b4381066e86f53d957e1987fa07303b Signed-off-by: Steffan Karger <st...@ka...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1303 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33868.html Signed-off-by: Gert Doering <ge...@gr...> --- M CMakeLists.txt M src/openvpn/Makefile.am M src/openvpn/error.c M src/openvpn/event.h M src/openvpn/forward.c M src/openvpn/multi.c M src/openvpn/multi.h M src/openvpn/multi_io.c M src/openvpn/openvpn.c M src/openvpn/openvpn.h D src/openvpn/perf.c D src/openvpn/perf.h M src/openvpn/ssl.c M src/openvpn/ssl_mbedtls.c M src/openvpn/ssl_openssl.c M src/openvpn/status.c 16 files changed, 0 insertions(+), 496 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 23fb4a5..5954a6e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -519,8 +519,6 @@ src/openvpn/ovpn_dco_win.h src/openvpn/packet_id.c src/openvpn/packet_id.h - src/openvpn/perf.c - src/openvpn/perf.h src/openvpn/ping.c src/openvpn/ping.h src/openvpn/pkcs11.c diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am index e44fb2b..dc58cd1 100644 --- a/src/openvpn/Makefile.am +++ b/src/openvpn/Makefile.am @@ -112,7 +112,6 @@ options_parse.c \ otime.c otime.h \ packet_id.c packet_id.h \ - perf.c perf.h \ ping.c ping.h \ plugin.c plugin.h \ pool.c pool.h \ diff --git a/src/openvpn/error.c b/src/openvpn/error.c index 58c2fd1..735d259 100644 --- a/src/openvpn/error.c +++ b/src/openvpn/error.c @@ -34,7 +34,6 @@ #include "socket.h" #include "tun.h" #include "otime.h" -#include "perf.h" #include "status.h" #include "integer.h" #include "ps.h" @@ -734,11 +733,6 @@ abort(); } #endif - - if (status == OPENVPN_EXIT_STATUS_GOOD) - { - perf_output_results(); - } } exit(status); diff --git a/src/openvpn/event.h b/src/openvpn/event.h index 8a89a25..f6aa9c4 100644 --- a/src/openvpn/event.h +++ b/src/openvpn/event.h @@ -25,7 +25,6 @@ #include "win32.h" #include "sig.h" -#include "perf.h" /* * rwflags passed to event_ctl and returned by @@ -189,9 +188,7 @@ event_wait(struct event_set *es, const struct timeval *tv, struct event_set_return *out, int outlen) { int ret; - perf_push(PERF_IO_WAIT); ret = (*es->func.wait)(es, tv, out, outlen); - perf_pop(); return ret; } diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 79a6fc7..7f72000 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -932,8 +932,6 @@ /*ASSERT (!c->c2.to_tun.len);*/ - perf_push(PERF_READ_IN_LINK); - c->c2.buf = c->c2.buffers->read_link_buf; ASSERT(buf_init(&c->c2.buf, c->c2.frame.buf.headroom)); @@ -966,7 +964,6 @@ msg(D_STREAM_ERRORS, "Connection reset, restarting [%d]", status); } } - perf_pop(); return; } @@ -983,8 +980,6 @@ /* Remove socks header if applicable */ socks_postprocess_incoming_link(c, sock); - - perf_pop(); } bool @@ -1212,15 +1207,11 @@ static void process_incoming_link(struct context *c, struct link_socket *sock) { - perf_push(PERF_PROC_IN_LINK); - struct link_socket_info *lsi = &sock->info; const uint8_t *orig_buf = c->c2.buf.data; process_incoming_link_part1(c, lsi, false); process_incoming_link_part2(c, lsi, orig_buf); - - perf_pop(); } void @@ -1326,8 +1317,6 @@ */ /*ASSERT (!c->c2.to_link.len);*/ - perf_push(PERF_READ_IN_TUN); - c->c2.buf = c->c2.buffers->read_tun_buf; #ifdef _WIN32 @@ -1360,7 +1349,6 @@ { register_signal(c->sig, SIGTERM, "tun-stop"); msg(M_INFO, "TUN/TAP interface has been stopped, exiting"); - perf_pop(); return; } @@ -1370,14 +1358,11 @@ register_signal(c->sig, SIGHUP, "tun-abort"); c->persist.restart_sleep_seconds = 10; msg(M_INFO, "TUN/TAP I/O operation aborted, restarting"); - perf_pop(); return; } /* Check the status return from read() */ check_status(c->c2.buf.len, "read from TUN/TAP", NULL, c->c1.tuntap); - - perf_pop(); } /** @@ -1497,8 +1482,6 @@ { struct gc_arena gc = gc_new(); - perf_push(PERF_PROC_IN_TUN); - if (c->c2.buf.len > 0) { c->c2.tun_read_bytes += c->c2.buf.len; @@ -1542,7 +1525,6 @@ { buf_reset(&c->c2.to_link); } - perf_pop(); gc_free(&gc); } @@ -1770,8 +1752,6 @@ struct gc_arena gc = gc_new(); int error_code = 0; - perf_push(PERF_PROC_OUT_LINK); - if (c->c2.to_link.len > 0 && c->c2.to_link.len <= c->c2.frame.buf.payload_size) { /* @@ -1899,7 +1879,6 @@ buf_reset(&c->c2.to_link); - perf_pop(); gc_free(&gc); } @@ -1919,8 +1898,6 @@ return; } - perf_push(PERF_PROC_OUT_TUN); - /* * The --mssfix option requires * us to examine the IP header (IPv4 or IPv6). @@ -1993,8 +1970,6 @@ } buf_reset(&c->c2.to_tun); - - perf_pop(); } #if defined(__GNUC__) || defined(__clang__) diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index fa9c654..f60944d 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -580,8 +580,6 @@ void multi_close_instance(struct multi_context *m, struct multi_instance *mi, bool shutdown) { - perf_push(PERF_MULTI_CLOSE_INSTANCE); - ASSERT(!mi->halt); mi->halt = true; bool is_dgram = proto_is_dgram(mi->context.c2.link_sockets[0]->info.proto); @@ -672,8 +670,6 @@ * vhash reaper deal with it. */ multi_instance_dec_refcount(mi); - - perf_pop(); } /* @@ -734,8 +730,6 @@ struct gc_arena gc = gc_new(); struct multi_instance *mi; - perf_push(PERF_MULTI_CREATE_INSTANCE); - msg(D_MULTI_MEDIUM, "MULTI: multi_create_instance called"); ALLOC_OBJ_CLEAR(mi, struct multi_instance); @@ -807,13 +801,11 @@ mi->ev_arg.type = EVENT_ARG_MULTI_INSTANCE; mi->ev_arg.u.mi = mi; - perf_pop(); gc_free(&gc); return mi; err: multi_close_instance(m, mi, false); - perf_pop(); gc_free(&gc); return NULL; } @@ -2907,7 +2899,6 @@ if (BLEN(buf) > 0) { - perf_push(PERF_MULTI_BCAST); #ifdef MULTI_DEBUG_EVENT_LOOP printf("BCAST len=%d\n", BLEN(buf)); #endif @@ -2929,7 +2920,6 @@ hash_iterator_free(&hi); mbuf_free_buf(mb); - perf_pop(); } } @@ -3399,7 +3389,6 @@ /* decrypt in instance context */ - perf_push(PERF_PROC_IN_LINK); lsi = &sock->info; orig_buf = c->c2.buf.data; if (process_incoming_link_part1(c, lsi, floated)) @@ -3412,7 +3401,6 @@ process_incoming_link_part2(c, lsi, orig_buf); } - perf_pop(); if (TUNNEL_TYPE(m->top.c1.tuntap) == DEV_TYPE_TUN) { @@ -4180,8 +4168,6 @@ while (true) { - perf_push(PERF_EVENT_LOOP); - /* wait on tun/socket list */ multi_get_timeout(multi, &multi->top.c2.timeval); status = multi_io_wait(multi); @@ -4202,7 +4188,6 @@ } MULTI_CHECK_SIG(multi); - perf_pop(); } } diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index 97bbc4a..594ea3a 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -38,7 +38,6 @@ #include "mudp.h" #include "mtcp.h" #include "multi_io.h" -#include "perf.h" #include "vlan.h" #include "reflect_filter.h" diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c index 0bfbb63..6e31687 100644 --- a/src/openvpn/multi_io.c +++ b/src/openvpn/multi_io.c @@ -242,9 +242,7 @@ tun_input_pending = NULL; /* For some reason, the Linux 2.2 TUN/TAP driver hits this timeout */ c->c2.timeval.tv_sec = 1; - perf_push(PERF_PROC_OUT_TUN_MTCP); io_wait(c, IOW_TO_TUN); - perf_pop(); break; case TA_SOCKET_WRITE: diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c index 64b4f8c..eaaa59b 100644 --- a/src/openvpn/openvpn.c +++ b/src/openvpn/openvpn.c @@ -72,8 +72,6 @@ /* main event loop */ while (true) { - perf_push(PERF_EVENT_LOOP); - /* process timers, TLS, etc. */ pre_select(c); P2P_CHECK_SIG(); @@ -85,15 +83,12 @@ /* timeout? */ if (c->c2.event_set_status == ES_TIMEOUT) { - perf_pop(); continue; } /* process the I/O which triggered select */ process_io(c, c->c2.link_sockets[0]); P2P_CHECK_SIG(); - - perf_pop(); } persist_client_stats(c); diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index cd99cd4..a198fcf 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -524,7 +524,6 @@ if (IS_SIG(c)) \ { \ const int brk = func(arg); \ - perf_pop(); \ if (brk) \ { \ break; \ diff --git a/src/openvpn/perf.c b/src/openvpn/perf.c deleted file mode 100644 index 51c1a97..0000000 --- a/src/openvpn/perf.c +++ /dev/null @@ -1,306 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single TCP/UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2002-2025 OpenVPN Inc <sa...@op...> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, see <https://www.gnu.org/licenses/>. - */ - -#ifdef HAVE_CONFIG_H -#include "config.h" -#endif - -#include "syshead.h" - -#include "perf.h" - -#ifdef ENABLE_PERFORMANCE_METRICS - -#include "error.h" -#include "otime.h" - -#include "memdbg.h" - -static const char *metric_names[] = { "PERF_BIO_READ_PLAINTEXT", - "PERF_BIO_WRITE_PLAINTEXT", - "PERF_BIO_READ_CIPHERTEXT", - "PERF_BIO_WRITE_CIPHERTEXT", - "PERF_TLS_MULTI_PROCESS", - "PERF_IO_WAIT", - "PERF_EVENT_LOOP", - "PERF_MULTI_CREATE_INSTANCE", - "PERF_MULTI_CLOSE_INSTANCE", - "PERF_MULTI_SHOW_STATS", - "PERF_MULTI_BCAST", - "PERF_MULTI_MCAST", - "PERF_SCRIPT", - "PERF_READ_IN_LINK", - "PERF_PROC_IN_LINK", - "PERF_READ_IN_TUN", - "PERF_PROC_IN_TUN", - "PERF_PROC_OUT_LINK", - "PERF_PROC_OUT_TUN", - "PERF_PROC_OUT_TUN_MTCP" }; - -struct perf -{ -#define PS_INITIAL 0 -#define PS_METER_RUNNING 1 -#define PS_METER_INTERRUPTED 2 - int state; - - struct timeval start; - double sofar; - double sum; - double max; - double count; -}; - -struct perf_set -{ - int stack_len; - int stack[STACK_N]; - struct perf perf[PERF_N]; -}; - -static struct perf_set perf_set; - -static void perf_print_state(int lev); - -static inline int -get_stack_index(int sdelta) -{ - const int sindex = perf_set.stack_len + sdelta; - if (sindex >= 0 && sindex < STACK_N) - { - return sindex; - } - else - { - return -1; - } -} - -static int -get_perf_index(int sdelta) -{ - const int sindex = get_stack_index(sdelta); - if (sindex >= 0) - { - const int pindex = perf_set.stack[sindex]; - if (pindex >= 0 && pindex < PERF_N) - { - return pindex; - } - else - { - return -1; - } - } - else - { - return -1; - } -} - -static struct perf * -get_perf(int sdelta) -{ - const int pindex = get_perf_index(sdelta); - if (pindex >= 0) - { - return &perf_set.perf[pindex]; - } - else - { - return NULL; - } -} - -static void -push_perf_index(int pindex) -{ - const int sindex = get_stack_index(0); - const int newlen = get_stack_index(1); - if (sindex >= 0 && newlen >= 0 && pindex >= 0 && pindex < PERF_N) - { - int i; - for (i = 0; i < sindex; ++i) - { - if (perf_set.stack[i] == pindex) - { - perf_print_state(M_INFO); - msg(M_FATAL, "PERF: push_perf_index %s failed", metric_names[pindex]); - } - } - - perf_set.stack[sindex] = pindex; - perf_set.stack_len = newlen; - } - else - { - msg(M_FATAL, "PERF: push_perf_index: stack push error"); - } -} - -static void -pop_perf_index(void) -{ - const int newlen = get_stack_index(-1); - if (newlen >= 0) - { - perf_set.stack_len = newlen; - } - else - { - msg(M_FATAL, "PERF: pop_perf_index: stack pop error"); - } -} - -static void -state_must_be(const struct perf *p, const int wanted) -{ - if (p->state != wanted) - { - msg(M_FATAL, "PERF: bad state actual=%d wanted=%d", p->state, wanted); - } -} - -static void -update_sofar(struct perf *p) -{ - struct timeval current; - ASSERT(!gettimeofday(¤t, NULL)); - p->sofar += (double)tv_subtract(¤t, &p->start, 600) / 1000000.0; - tv_clear(&p->start); -} - -static void -perf_start(struct perf *p) -{ - state_must_be(p, PS_INITIAL); - ASSERT(!gettimeofday(&p->start, NULL)); - p->sofar = 0.0; - p->state = PS_METER_RUNNING; -} - -static void -perf_stop(struct perf *p) -{ - state_must_be(p, PS_METER_RUNNING); - update_sofar(p); - p->sum += p->sofar; - if (p->sofar > p->max) - { - p->max = p->sofar; - } - p->count += 1.0; - p->sofar = 0.0; - p->state = PS_INITIAL; -} - -static void -perf_interrupt(struct perf *p) -{ - state_must_be(p, PS_METER_RUNNING); - update_sofar(p); - p->state = PS_METER_INTERRUPTED; -} - -static void -perf_resume(struct perf *p) -{ - state_must_be(p, PS_METER_INTERRUPTED); - ASSERT(!gettimeofday(&p->start, NULL)); - p->state = PS_METER_RUNNING; -} - -void -perf_push(int type) -{ - struct perf *prev; - struct perf *cur; - - ASSERT(SIZE(metric_names) == PERF_N); - push_perf_index(type); - - prev = get_perf(-2); - cur = get_perf(-1); - - ASSERT(cur); - - if (prev) - { - perf_interrupt(prev); - } - perf_start(cur); -} - -void -perf_pop(void) -{ - struct perf *prev; - struct perf *cur; - - prev = get_perf(-2); - cur = get_perf(-1); - - ASSERT(cur); - perf_stop(cur); - - if (prev) - { - perf_resume(prev); - } - - pop_perf_index(); -} - -void -perf_output_results(void) -{ - int i; - msg(M_INFO, "LATENCY PROFILE (mean and max are in milliseconds)"); - for (i = 0; i < PERF_N; ++i) - { - struct perf *p = &perf_set.perf[i]; - if (p->count > 0.0) - { - const double mean = p->sum / p->count; - msg(M_INFO, "%s n=%.0f mean=%.3f max=%.3f", metric_names[i], p->count, mean * 1000.0, - p->max * 1000.0); - } - } -} - -static void -perf_print_state(int lev) -{ - struct gc_arena gc = gc_new(); - int i; - msg(lev, "PERF STATE"); - msg(lev, "Stack:"); - for (i = 0; i < perf_set.stack_len; ++i) - { - const int j = perf_set.stack[i]; - const struct perf *p = &perf_set.perf[j]; - msg(lev, "[%d] %s state=%d start=%s sofar=%f sum=%f max=%f count=%f", i, metric_names[j], - p->state, tv_string(&p->start, &gc), p->sofar, p->sum, p->max, p->count); - } - gc_free(&gc); -} -#endif /* ifdef ENABLE_PERFORMANCE_METRICS */ diff --git a/src/openvpn/perf.h b/src/openvpn/perf.h deleted file mode 100644 index 2a178a1..0000000 --- a/src/openvpn/perf.h +++ /dev/null @@ -1,91 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single TCP/UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2002-2025 OpenVPN Inc <sa...@op...> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, see <https://www.gnu.org/licenses/>. - */ - -/* - * The interval_ routines are designed to optimize the calling of a routine - * (normally tls_multi_process()) which can be called less frequently - * between triggers. - */ - -#ifndef PERF_H -#define PERF_H - -/*#define ENABLE_PERFORMANCE_METRICS*/ - -/* - * Metrics - */ -#define PERF_BIO_READ_PLAINTEXT 0 -#define PERF_BIO_WRITE_PLAINTEXT 1 -#define PERF_BIO_READ_CIPHERTEXT 2 -#define PERF_BIO_WRITE_CIPHERTEXT 3 -#define PERF_TLS_MULTI_PROCESS 4 -#define PERF_IO_WAIT 5 -#define PERF_EVENT_LOOP 6 -#define PERF_MULTI_CREATE_INSTANCE 7 -#define PERF_MULTI_CLOSE_INSTANCE 8 -#define PERF_MULTI_SHOW_STATS 9 -#define PERF_MULTI_BCAST 10 -#define PERF_MULTI_MCAST 11 -#define PERF_SCRIPT 12 -#define PERF_READ_IN_LINK 13 -#define PERF_PROC_IN_LINK 14 -#define PERF_READ_IN_TUN 15 -#define PERF_PROC_IN_TUN 16 -#define PERF_PROC_OUT_LINK 17 -#define PERF_PROC_OUT_TUN 18 -#define PERF_PROC_OUT_TUN_MTCP 19 -#define PERF_N 20 - -#ifdef ENABLE_PERFORMANCE_METRICS - -#include "basic.h" - -/* - * Stack size - */ -#define STACK_N 64 - -void perf_push(int type); - -void perf_pop(void); - -void perf_output_results(void); - -#else /* ifdef ENABLE_PERFORMANCE_METRICS */ - -static inline void -perf_push(int type) -{ -} -static inline void -perf_pop(void) -{ -} -static inline void -perf_output_results(void) -{ -} - -#endif /* ifdef ENABLE_PERFORMANCE_METRICS */ - -#endif /* ifndef PERF_H */ diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 567560f..908854a 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -47,7 +47,6 @@ #include "misc.h" #include "fdmisc.h" #include "interval.h" -#include "perf.h" #include "status.h" #include "gremlin.h" #include "pkcs11.h" @@ -3220,8 +3219,6 @@ int active = TLSMP_INACTIVE; bool error = false; - perf_push(PERF_TLS_MULTI_PROCESS); - tls_clear_error(); /* @@ -3413,7 +3410,6 @@ } #endif - perf_pop(); gc_free(&gc); return (tas == TLS_AUTHENTICATION_FAILED) ? TLSMP_KILL : active; diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 80eb51b..488f9b9 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1286,14 +1286,12 @@ key_state_write_plaintext_const(struct key_state_ssl *ks, const uint8_t *data, int len) { int retval = 0; - perf_push(PERF_BIO_WRITE_PLAINTEXT); ASSERT(NULL != ks); ASSERT(len >= 0); if (0 == len) { - perf_pop(); return 0; } @@ -1303,7 +1301,6 @@ if (retval < 0) { - perf_pop(); if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; @@ -1316,14 +1313,12 @@ { msg(D_TLS_ERRORS, "TLS ERROR: write tls_write_plaintext_const incomplete %d/%d", retval, len); - perf_pop(); return -1; } /* successful write */ dmsg(D_HANDSHAKE_VERBOSE, "write tls_write_plaintext_const %d bytes", retval); - perf_pop(); return 1; } @@ -1333,15 +1328,12 @@ int retval = 0; int len = 0; - perf_push(PERF_BIO_READ_CIPHERTEXT); - ASSERT(NULL != ks); ASSERT(buf); ASSERT(buf->len >= 0); if (buf->len) { - perf_pop(); return 0; } @@ -1352,7 +1344,6 @@ /* Error during read, check for retry error */ if (retval < 0) { - perf_pop(); if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; @@ -1365,14 +1356,12 @@ if (0 == retval) { buf->len = 0; - perf_pop(); return 0; } /* successful read */ dmsg(D_HANDSHAKE_VERBOSE, "read tls_read_ciphertext %d bytes", retval); buf->len = retval; - perf_pop(); return 1; } @@ -1380,7 +1369,6 @@ key_state_write_ciphertext(struct key_state_ssl *ks, struct buffer *buf) { int retval = 0; - perf_push(PERF_BIO_WRITE_CIPHERTEXT); ASSERT(NULL != ks); ASSERT(buf); @@ -1388,7 +1376,6 @@ if (0 == buf->len) { - perf_pop(); return 0; } @@ -1396,8 +1383,6 @@ if (retval < 0) { - perf_pop(); - if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; @@ -1410,7 +1395,6 @@ { msg(D_TLS_ERRORS, "TLS ERROR: write tls_write_ciphertext incomplete %d/%d", retval, buf->len); - perf_pop(); return -1; } @@ -1420,7 +1404,6 @@ memset(BPTR(buf), 0, BLEN(buf)); /* erase data just written */ buf->len = 0; - perf_pop(); return 1; } @@ -1430,15 +1413,12 @@ int retval = 0; int len = 0; - perf_push(PERF_BIO_READ_PLAINTEXT); - ASSERT(NULL != ks); ASSERT(buf); ASSERT(buf->len >= 0); if (buf->len) { - perf_pop(); return 0; } @@ -1455,14 +1435,12 @@ } mbed_log_err(D_TLS_ERRORS, retval, "TLS_ERROR: read tls_read_plaintext error"); buf->len = 0; - perf_pop(); return -1; } /* Nothing read, try again */ if (0 == retval) { buf->len = 0; - perf_pop(); return 0; } @@ -1470,7 +1448,6 @@ dmsg(D_HANDSHAKE_VERBOSE, "read tls_read_plaintext %d bytes", retval); buf->len = retval; - perf_pop(); return 1; } diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 434df7d..d997141 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -2208,14 +2208,12 @@ key_state_write_plaintext(struct key_state_ssl *ks_ssl, struct buffer *buf) { int ret = 0; - perf_push(PERF_BIO_WRITE_PLAINTEXT); ASSERT(NULL != ks_ssl); ret = bio_write(ks_ssl->ssl_bio, BPTR(buf), BLEN(buf), "tls_write_plaintext"); bio_write_post(ret, buf); - perf_pop(); return ret; } @@ -2223,13 +2221,11 @@ key_state_write_plaintext_const(struct key_state_ssl *ks_ssl, const uint8_t *data, int len) { int ret = 0; - perf_push(PERF_BIO_WRITE_PLAINTEXT); ASSERT(NULL != ks_ssl); ret = bio_write(ks_ssl->ssl_bio, data, len, "tls_write_plaintext_const"); - perf_pop(); return ret; } @@ -2237,13 +2233,11 @@ key_state_read_ciphertext(struct key_state_ssl *ks_ssl, struct buffer *buf) { int ret = 0; - perf_push(PERF_BIO_READ_CIPHERTEXT); ASSERT(NULL != ks_ssl); ret = bio_read(ks_ssl->ct_out, buf, "tls_read_ciphertext"); - perf_pop(); return ret; } @@ -2251,14 +2245,12 @@ key_state_write_ciphertext(struct key_state_ssl *ks_ssl, struct buffer *buf) { int ret = 0; - perf_push(PERF_BIO_WRITE_CIPHERTEXT); ASSERT(NULL != ks_ssl); ret = bio_write(ks_ssl->ct_in, BPTR(buf), BLEN(buf), "tls_write_ciphertext"); bio_write_post(ret, buf); - perf_pop(); return ret; } @@ -2266,13 +2258,11 @@ key_state_read_plaintext(struct key_state_ssl *ks_ssl, struct buffer *buf) { int ret = 0; - perf_push(PERF_BIO_READ_PLAINTEXT); ASSERT(NULL != ks_ssl); ret = bio_read(ks_ssl->ssl_bio, buf, "tls_read_plaintext"); - perf_pop(); return ret; } diff --git a/src/openvpn/status.c b/src/openvpn/status.c index 1e1e3fb..5ca33cb 100644 --- a/src/openvpn/status.c +++ b/src/openvpn/status.c @@ -27,7 +27,6 @@ #include "syshead.h" #include "status.h" -#include "perf.h" #include "misc.h" #include "fdmisc.h" -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1303?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I5bb666a73b4381066e86f53d957e1987fa07303b Gerrit-Change-Number: 1303 Gerrit-PatchSet: 2 Gerrit-Owner: syzzer <st...@ka...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-26 21:50:40
|
cron2 has uploaded a new patch set (#2) to the change originally created by syzzer. ( http://gerrit.openvpn.net/c/openvpn/+/1303?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: Remove perf.c/perf.h ...................................................................... Remove perf.c/perf.h This code was always disabled by ENABLE_PERFORMANCE_METRICS being commented out in perf.h. There was no configure flag. None of the active developers remembers using it, the git log shows no actual code changes since at least the project structure overhaul of 2012, and tools like gprof are nowadays the go-to tool for performance profiling. So, out with our custom implementation. This was triggered by a bug report submitted by Joshua Rogers, who used ZeroPath to discover we missed a perf_pop() call in one of the error paths of ssl_mbedtls.c. This commit resolves that using git rm. Change-Id: I5bb666a73b4381066e86f53d957e1987fa07303b Signed-off-by: Steffan Karger <st...@ka...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1303 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33868.html Signed-off-by: Gert Doering <ge...@gr...> --- M CMakeLists.txt M src/openvpn/Makefile.am M src/openvpn/error.c M src/openvpn/event.h M src/openvpn/forward.c M src/openvpn/multi.c M src/openvpn/multi.h M src/openvpn/multi_io.c M src/openvpn/openvpn.c M src/openvpn/openvpn.h D src/openvpn/perf.c D src/openvpn/perf.h M src/openvpn/ssl.c M src/openvpn/ssl_mbedtls.c M src/openvpn/ssl_openssl.c M src/openvpn/status.c 16 files changed, 0 insertions(+), 496 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/03/1303/2 diff --git a/CMakeLists.txt b/CMakeLists.txt index 23fb4a5..5954a6e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -519,8 +519,6 @@ src/openvpn/ovpn_dco_win.h src/openvpn/packet_id.c src/openvpn/packet_id.h - src/openvpn/perf.c - src/openvpn/perf.h src/openvpn/ping.c src/openvpn/ping.h src/openvpn/pkcs11.c diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am index e44fb2b..dc58cd1 100644 --- a/src/openvpn/Makefile.am +++ b/src/openvpn/Makefile.am @@ -112,7 +112,6 @@ options_parse.c \ otime.c otime.h \ packet_id.c packet_id.h \ - perf.c perf.h \ ping.c ping.h \ plugin.c plugin.h \ pool.c pool.h \ diff --git a/src/openvpn/error.c b/src/openvpn/error.c index 58c2fd1..735d259 100644 --- a/src/openvpn/error.c +++ b/src/openvpn/error.c @@ -34,7 +34,6 @@ #include "socket.h" #include "tun.h" #include "otime.h" -#include "perf.h" #include "status.h" #include "integer.h" #include "ps.h" @@ -734,11 +733,6 @@ abort(); } #endif - - if (status == OPENVPN_EXIT_STATUS_GOOD) - { - perf_output_results(); - } } exit(status); diff --git a/src/openvpn/event.h b/src/openvpn/event.h index 8a89a25..f6aa9c4 100644 --- a/src/openvpn/event.h +++ b/src/openvpn/event.h @@ -25,7 +25,6 @@ #include "win32.h" #include "sig.h" -#include "perf.h" /* * rwflags passed to event_ctl and returned by @@ -189,9 +188,7 @@ event_wait(struct event_set *es, const struct timeval *tv, struct event_set_return *out, int outlen) { int ret; - perf_push(PERF_IO_WAIT); ret = (*es->func.wait)(es, tv, out, outlen); - perf_pop(); return ret; } diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 79a6fc7..7f72000 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -932,8 +932,6 @@ /*ASSERT (!c->c2.to_tun.len);*/ - perf_push(PERF_READ_IN_LINK); - c->c2.buf = c->c2.buffers->read_link_buf; ASSERT(buf_init(&c->c2.buf, c->c2.frame.buf.headroom)); @@ -966,7 +964,6 @@ msg(D_STREAM_ERRORS, "Connection reset, restarting [%d]", status); } } - perf_pop(); return; } @@ -983,8 +980,6 @@ /* Remove socks header if applicable */ socks_postprocess_incoming_link(c, sock); - - perf_pop(); } bool @@ -1212,15 +1207,11 @@ static void process_incoming_link(struct context *c, struct link_socket *sock) { - perf_push(PERF_PROC_IN_LINK); - struct link_socket_info *lsi = &sock->info; const uint8_t *orig_buf = c->c2.buf.data; process_incoming_link_part1(c, lsi, false); process_incoming_link_part2(c, lsi, orig_buf); - - perf_pop(); } void @@ -1326,8 +1317,6 @@ */ /*ASSERT (!c->c2.to_link.len);*/ - perf_push(PERF_READ_IN_TUN); - c->c2.buf = c->c2.buffers->read_tun_buf; #ifdef _WIN32 @@ -1360,7 +1349,6 @@ { register_signal(c->sig, SIGTERM, "tun-stop"); msg(M_INFO, "TUN/TAP interface has been stopped, exiting"); - perf_pop(); return; } @@ -1370,14 +1358,11 @@ register_signal(c->sig, SIGHUP, "tun-abort"); c->persist.restart_sleep_seconds = 10; msg(M_INFO, "TUN/TAP I/O operation aborted, restarting"); - perf_pop(); return; } /* Check the status return from read() */ check_status(c->c2.buf.len, "read from TUN/TAP", NULL, c->c1.tuntap); - - perf_pop(); } /** @@ -1497,8 +1482,6 @@ { struct gc_arena gc = gc_new(); - perf_push(PERF_PROC_IN_TUN); - if (c->c2.buf.len > 0) { c->c2.tun_read_bytes += c->c2.buf.len; @@ -1542,7 +1525,6 @@ { buf_reset(&c->c2.to_link); } - perf_pop(); gc_free(&gc); } @@ -1770,8 +1752,6 @@ struct gc_arena gc = gc_new(); int error_code = 0; - perf_push(PERF_PROC_OUT_LINK); - if (c->c2.to_link.len > 0 && c->c2.to_link.len <= c->c2.frame.buf.payload_size) { /* @@ -1899,7 +1879,6 @@ buf_reset(&c->c2.to_link); - perf_pop(); gc_free(&gc); } @@ -1919,8 +1898,6 @@ return; } - perf_push(PERF_PROC_OUT_TUN); - /* * The --mssfix option requires * us to examine the IP header (IPv4 or IPv6). @@ -1993,8 +1970,6 @@ } buf_reset(&c->c2.to_tun); - - perf_pop(); } #if defined(__GNUC__) || defined(__clang__) diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index fa9c654..f60944d 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -580,8 +580,6 @@ void multi_close_instance(struct multi_context *m, struct multi_instance *mi, bool shutdown) { - perf_push(PERF_MULTI_CLOSE_INSTANCE); - ASSERT(!mi->halt); mi->halt = true; bool is_dgram = proto_is_dgram(mi->context.c2.link_sockets[0]->info.proto); @@ -672,8 +670,6 @@ * vhash reaper deal with it. */ multi_instance_dec_refcount(mi); - - perf_pop(); } /* @@ -734,8 +730,6 @@ struct gc_arena gc = gc_new(); struct multi_instance *mi; - perf_push(PERF_MULTI_CREATE_INSTANCE); - msg(D_MULTI_MEDIUM, "MULTI: multi_create_instance called"); ALLOC_OBJ_CLEAR(mi, struct multi_instance); @@ -807,13 +801,11 @@ mi->ev_arg.type = EVENT_ARG_MULTI_INSTANCE; mi->ev_arg.u.mi = mi; - perf_pop(); gc_free(&gc); return mi; err: multi_close_instance(m, mi, false); - perf_pop(); gc_free(&gc); return NULL; } @@ -2907,7 +2899,6 @@ if (BLEN(buf) > 0) { - perf_push(PERF_MULTI_BCAST); #ifdef MULTI_DEBUG_EVENT_LOOP printf("BCAST len=%d\n", BLEN(buf)); #endif @@ -2929,7 +2920,6 @@ hash_iterator_free(&hi); mbuf_free_buf(mb); - perf_pop(); } } @@ -3399,7 +3389,6 @@ /* decrypt in instance context */ - perf_push(PERF_PROC_IN_LINK); lsi = &sock->info; orig_buf = c->c2.buf.data; if (process_incoming_link_part1(c, lsi, floated)) @@ -3412,7 +3401,6 @@ process_incoming_link_part2(c, lsi, orig_buf); } - perf_pop(); if (TUNNEL_TYPE(m->top.c1.tuntap) == DEV_TYPE_TUN) { @@ -4180,8 +4168,6 @@ while (true) { - perf_push(PERF_EVENT_LOOP); - /* wait on tun/socket list */ multi_get_timeout(multi, &multi->top.c2.timeval); status = multi_io_wait(multi); @@ -4202,7 +4188,6 @@ } MULTI_CHECK_SIG(multi); - perf_pop(); } } diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index 97bbc4a..594ea3a 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -38,7 +38,6 @@ #include "mudp.h" #include "mtcp.h" #include "multi_io.h" -#include "perf.h" #include "vlan.h" #include "reflect_filter.h" diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c index 0bfbb63..6e31687 100644 --- a/src/openvpn/multi_io.c +++ b/src/openvpn/multi_io.c @@ -242,9 +242,7 @@ tun_input_pending = NULL; /* For some reason, the Linux 2.2 TUN/TAP driver hits this timeout */ c->c2.timeval.tv_sec = 1; - perf_push(PERF_PROC_OUT_TUN_MTCP); io_wait(c, IOW_TO_TUN); - perf_pop(); break; case TA_SOCKET_WRITE: diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c index 64b4f8c..eaaa59b 100644 --- a/src/openvpn/openvpn.c +++ b/src/openvpn/openvpn.c @@ -72,8 +72,6 @@ /* main event loop */ while (true) { - perf_push(PERF_EVENT_LOOP); - /* process timers, TLS, etc. */ pre_select(c); P2P_CHECK_SIG(); @@ -85,15 +83,12 @@ /* timeout? */ if (c->c2.event_set_status == ES_TIMEOUT) { - perf_pop(); continue; } /* process the I/O which triggered select */ process_io(c, c->c2.link_sockets[0]); P2P_CHECK_SIG(); - - perf_pop(); } persist_client_stats(c); diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index cd99cd4..a198fcf 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -524,7 +524,6 @@ if (IS_SIG(c)) \ { \ const int brk = func(arg); \ - perf_pop(); \ if (brk) \ { \ break; \ diff --git a/src/openvpn/perf.c b/src/openvpn/perf.c deleted file mode 100644 index 51c1a97..0000000 --- a/src/openvpn/perf.c +++ /dev/null @@ -1,306 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single TCP/UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2002-2025 OpenVPN Inc <sa...@op...> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, see <https://www.gnu.org/licenses/>. - */ - -#ifdef HAVE_CONFIG_H -#include "config.h" -#endif - -#include "syshead.h" - -#include "perf.h" - -#ifdef ENABLE_PERFORMANCE_METRICS - -#include "error.h" -#include "otime.h" - -#include "memdbg.h" - -static const char *metric_names[] = { "PERF_BIO_READ_PLAINTEXT", - "PERF_BIO_WRITE_PLAINTEXT", - "PERF_BIO_READ_CIPHERTEXT", - "PERF_BIO_WRITE_CIPHERTEXT", - "PERF_TLS_MULTI_PROCESS", - "PERF_IO_WAIT", - "PERF_EVENT_LOOP", - "PERF_MULTI_CREATE_INSTANCE", - "PERF_MULTI_CLOSE_INSTANCE", - "PERF_MULTI_SHOW_STATS", - "PERF_MULTI_BCAST", - "PERF_MULTI_MCAST", - "PERF_SCRIPT", - "PERF_READ_IN_LINK", - "PERF_PROC_IN_LINK", - "PERF_READ_IN_TUN", - "PERF_PROC_IN_TUN", - "PERF_PROC_OUT_LINK", - "PERF_PROC_OUT_TUN", - "PERF_PROC_OUT_TUN_MTCP" }; - -struct perf -{ -#define PS_INITIAL 0 -#define PS_METER_RUNNING 1 -#define PS_METER_INTERRUPTED 2 - int state; - - struct timeval start; - double sofar; - double sum; - double max; - double count; -}; - -struct perf_set -{ - int stack_len; - int stack[STACK_N]; - struct perf perf[PERF_N]; -}; - -static struct perf_set perf_set; - -static void perf_print_state(int lev); - -static inline int -get_stack_index(int sdelta) -{ - const int sindex = perf_set.stack_len + sdelta; - if (sindex >= 0 && sindex < STACK_N) - { - return sindex; - } - else - { - return -1; - } -} - -static int -get_perf_index(int sdelta) -{ - const int sindex = get_stack_index(sdelta); - if (sindex >= 0) - { - const int pindex = perf_set.stack[sindex]; - if (pindex >= 0 && pindex < PERF_N) - { - return pindex; - } - else - { - return -1; - } - } - else - { - return -1; - } -} - -static struct perf * -get_perf(int sdelta) -{ - const int pindex = get_perf_index(sdelta); - if (pindex >= 0) - { - return &perf_set.perf[pindex]; - } - else - { - return NULL; - } -} - -static void -push_perf_index(int pindex) -{ - const int sindex = get_stack_index(0); - const int newlen = get_stack_index(1); - if (sindex >= 0 && newlen >= 0 && pindex >= 0 && pindex < PERF_N) - { - int i; - for (i = 0; i < sindex; ++i) - { - if (perf_set.stack[i] == pindex) - { - perf_print_state(M_INFO); - msg(M_FATAL, "PERF: push_perf_index %s failed", metric_names[pindex]); - } - } - - perf_set.stack[sindex] = pindex; - perf_set.stack_len = newlen; - } - else - { - msg(M_FATAL, "PERF: push_perf_index: stack push error"); - } -} - -static void -pop_perf_index(void) -{ - const int newlen = get_stack_index(-1); - if (newlen >= 0) - { - perf_set.stack_len = newlen; - } - else - { - msg(M_FATAL, "PERF: pop_perf_index: stack pop error"); - } -} - -static void -state_must_be(const struct perf *p, const int wanted) -{ - if (p->state != wanted) - { - msg(M_FATAL, "PERF: bad state actual=%d wanted=%d", p->state, wanted); - } -} - -static void -update_sofar(struct perf *p) -{ - struct timeval current; - ASSERT(!gettimeofday(¤t, NULL)); - p->sofar += (double)tv_subtract(¤t, &p->start, 600) / 1000000.0; - tv_clear(&p->start); -} - -static void -perf_start(struct perf *p) -{ - state_must_be(p, PS_INITIAL); - ASSERT(!gettimeofday(&p->start, NULL)); - p->sofar = 0.0; - p->state = PS_METER_RUNNING; -} - -static void -perf_stop(struct perf *p) -{ - state_must_be(p, PS_METER_RUNNING); - update_sofar(p); - p->sum += p->sofar; - if (p->sofar > p->max) - { - p->max = p->sofar; - } - p->count += 1.0; - p->sofar = 0.0; - p->state = PS_INITIAL; -} - -static void -perf_interrupt(struct perf *p) -{ - state_must_be(p, PS_METER_RUNNING); - update_sofar(p); - p->state = PS_METER_INTERRUPTED; -} - -static void -perf_resume(struct perf *p) -{ - state_must_be(p, PS_METER_INTERRUPTED); - ASSERT(!gettimeofday(&p->start, NULL)); - p->state = PS_METER_RUNNING; -} - -void -perf_push(int type) -{ - struct perf *prev; - struct perf *cur; - - ASSERT(SIZE(metric_names) == PERF_N); - push_perf_index(type); - - prev = get_perf(-2); - cur = get_perf(-1); - - ASSERT(cur); - - if (prev) - { - perf_interrupt(prev); - } - perf_start(cur); -} - -void -perf_pop(void) -{ - struct perf *prev; - struct perf *cur; - - prev = get_perf(-2); - cur = get_perf(-1); - - ASSERT(cur); - perf_stop(cur); - - if (prev) - { - perf_resume(prev); - } - - pop_perf_index(); -} - -void -perf_output_results(void) -{ - int i; - msg(M_INFO, "LATENCY PROFILE (mean and max are in milliseconds)"); - for (i = 0; i < PERF_N; ++i) - { - struct perf *p = &perf_set.perf[i]; - if (p->count > 0.0) - { - const double mean = p->sum / p->count; - msg(M_INFO, "%s n=%.0f mean=%.3f max=%.3f", metric_names[i], p->count, mean * 1000.0, - p->max * 1000.0); - } - } -} - -static void -perf_print_state(int lev) -{ - struct gc_arena gc = gc_new(); - int i; - msg(lev, "PERF STATE"); - msg(lev, "Stack:"); - for (i = 0; i < perf_set.stack_len; ++i) - { - const int j = perf_set.stack[i]; - const struct perf *p = &perf_set.perf[j]; - msg(lev, "[%d] %s state=%d start=%s sofar=%f sum=%f max=%f count=%f", i, metric_names[j], - p->state, tv_string(&p->start, &gc), p->sofar, p->sum, p->max, p->count); - } - gc_free(&gc); -} -#endif /* ifdef ENABLE_PERFORMANCE_METRICS */ diff --git a/src/openvpn/perf.h b/src/openvpn/perf.h deleted file mode 100644 index 2a178a1..0000000 --- a/src/openvpn/perf.h +++ /dev/null @@ -1,91 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single TCP/UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2002-2025 OpenVPN Inc <sa...@op...> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, see <https://www.gnu.org/licenses/>. - */ - -/* - * The interval_ routines are designed to optimize the calling of a routine - * (normally tls_multi_process()) which can be called less frequently - * between triggers. - */ - -#ifndef PERF_H -#define PERF_H - -/*#define ENABLE_PERFORMANCE_METRICS*/ - -/* - * Metrics - */ -#define PERF_BIO_READ_PLAINTEXT 0 -#define PERF_BIO_WRITE_PLAINTEXT 1 -#define PERF_BIO_READ_CIPHERTEXT 2 -#define PERF_BIO_WRITE_CIPHERTEXT 3 -#define PERF_TLS_MULTI_PROCESS 4 -#define PERF_IO_WAIT 5 -#define PERF_EVENT_LOOP 6 -#define PERF_MULTI_CREATE_INSTANCE 7 -#define PERF_MULTI_CLOSE_INSTANCE 8 -#define PERF_MULTI_SHOW_STATS 9 -#define PERF_MULTI_BCAST 10 -#define PERF_MULTI_MCAST 11 -#define PERF_SCRIPT 12 -#define PERF_READ_IN_LINK 13 -#define PERF_PROC_IN_LINK 14 -#define PERF_READ_IN_TUN 15 -#define PERF_PROC_IN_TUN 16 -#define PERF_PROC_OUT_LINK 17 -#define PERF_PROC_OUT_TUN 18 -#define PERF_PROC_OUT_TUN_MTCP 19 -#define PERF_N 20 - -#ifdef ENABLE_PERFORMANCE_METRICS - -#include "basic.h" - -/* - * Stack size - */ -#define STACK_N 64 - -void perf_push(int type); - -void perf_pop(void); - -void perf_output_results(void); - -#else /* ifdef ENABLE_PERFORMANCE_METRICS */ - -static inline void -perf_push(int type) -{ -} -static inline void -perf_pop(void) -{ -} -static inline void -perf_output_results(void) -{ -} - -#endif /* ifdef ENABLE_PERFORMANCE_METRICS */ - -#endif /* ifndef PERF_H */ diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 567560f..908854a 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -47,7 +47,6 @@ #include "misc.h" #include "fdmisc.h" #include "interval.h" -#include "perf.h" #include "status.h" #include "gremlin.h" #include "pkcs11.h" @@ -3220,8 +3219,6 @@ int active = TLSMP_INACTIVE; bool error = false; - perf_push(PERF_TLS_MULTI_PROCESS); - tls_clear_error(); /* @@ -3413,7 +3410,6 @@ } #endif - perf_pop(); gc_free(&gc); return (tas == TLS_AUTHENTICATION_FAILED) ? TLSMP_KILL : active; diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 80eb51b..488f9b9 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1286,14 +1286,12 @@ key_state_write_plaintext_const(struct key_state_ssl *ks, const uint8_t *data, int len) { int retval = 0; - perf_push(PERF_BIO_WRITE_PLAINTEXT); ASSERT(NULL != ks); ASSERT(len >= 0); if (0 == len) { - perf_pop(); return 0; } @@ -1303,7 +1301,6 @@ if (retval < 0) { - perf_pop(); if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; @@ -1316,14 +1313,12 @@ { msg(D_TLS_ERRORS, "TLS ERROR: write tls_write_plaintext_const incomplete %d/%d", retval, len); - perf_pop(); return -1; } /* successful write */ dmsg(D_HANDSHAKE_VERBOSE, "write tls_write_plaintext_const %d bytes", retval); - perf_pop(); return 1; } @@ -1333,15 +1328,12 @@ int retval = 0; int len = 0; - perf_push(PERF_BIO_READ_CIPHERTEXT); - ASSERT(NULL != ks); ASSERT(buf); ASSERT(buf->len >= 0); if (buf->len) { - perf_pop(); return 0; } @@ -1352,7 +1344,6 @@ /* Error during read, check for retry error */ if (retval < 0) { - perf_pop(); if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; @@ -1365,14 +1356,12 @@ if (0 == retval) { buf->len = 0; - perf_pop(); return 0; } /* successful read */ dmsg(D_HANDSHAKE_VERBOSE, "read tls_read_ciphertext %d bytes", retval); buf->len = retval; - perf_pop(); return 1; } @@ -1380,7 +1369,6 @@ key_state_write_ciphertext(struct key_state_ssl *ks, struct buffer *buf) { int retval = 0; - perf_push(PERF_BIO_WRITE_CIPHERTEXT); ASSERT(NULL != ks); ASSERT(buf); @@ -1388,7 +1376,6 @@ if (0 == buf->len) { - perf_pop(); return 0; } @@ -1396,8 +1383,6 @@ if (retval < 0) { - perf_pop(); - if (MBEDTLS_ERR_SSL_WANT_WRITE == retval || MBEDTLS_ERR_SSL_WANT_READ == retval) { return 0; @@ -1410,7 +1395,6 @@ { msg(D_TLS_ERRORS, "TLS ERROR: write tls_write_ciphertext incomplete %d/%d", retval, buf->len); - perf_pop(); return -1; } @@ -1420,7 +1404,6 @@ memset(BPTR(buf), 0, BLEN(buf)); /* erase data just written */ buf->len = 0; - perf_pop(); return 1; } @@ -1430,15 +1413,12 @@ int retval = 0; int len = 0; - perf_push(PERF_BIO_READ_PLAINTEXT); - ASSERT(NULL != ks); ASSERT(buf); ASSERT(buf->len >= 0); if (buf->len) { - perf_pop(); return 0; } @@ -1455,14 +1435,12 @@ } mbed_log_err(D_TLS_ERRORS, retval, "TLS_ERROR: read tls_read_plaintext error"); buf->len = 0; - perf_pop(); return -1; } /* Nothing read, try again */ if (0 == retval) { buf->len = 0; - perf_pop(); return 0; } @@ -1470,7 +1448,6 @@ dmsg(D_HANDSHAKE_VERBOSE, "read tls_read_plaintext %d bytes", retval); buf->len = retval; - perf_pop(); return 1; } diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 434df7d..d997141 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -2208,14 +2208,12 @@ key_state_write_plaintext(struct key_state_ssl *ks_ssl, struct buffer *buf) { int ret = 0; - perf_push(PERF_BIO_WRITE_PLAINTEXT); ASSERT(NULL != ks_ssl); ret = bio_write(ks_ssl->ssl_bio, BPTR(buf), BLEN(buf), "tls_write_plaintext"); bio_write_post(ret, buf); - perf_pop(); return ret; } @@ -2223,13 +2221,11 @@ key_state_write_plaintext_const(struct key_state_ssl *ks_ssl, const uint8_t *data, int len) { int ret = 0; - perf_push(PERF_BIO_WRITE_PLAINTEXT); ASSERT(NULL != ks_ssl); ret = bio_write(ks_ssl->ssl_bio, data, len, "tls_write_plaintext_const"); - perf_pop(); return ret; } @@ -2237,13 +2233,11 @@ key_state_read_ciphertext(struct key_state_ssl *ks_ssl, struct buffer *buf) { int ret = 0; - perf_push(PERF_BIO_READ_CIPHERTEXT); ASSERT(NULL != ks_ssl); ret = bio_read(ks_ssl->ct_out, buf, "tls_read_ciphertext"); - perf_pop(); return ret; } @@ -2251,14 +2245,12 @@ key_state_write_ciphertext(struct key_state_ssl *ks_ssl, struct buffer *buf) { int ret = 0; - perf_push(PERF_BIO_WRITE_CIPHERTEXT); ASSERT(NULL != ks_ssl); ret = bio_write(ks_ssl->ct_in, BPTR(buf), BLEN(buf), "tls_write_ciphertext"); bio_write_post(ret, buf); - perf_pop(); return ret; } @@ -2266,13 +2258,11 @@ key_state_read_plaintext(struct key_state_ssl *ks_ssl, struct buffer *buf) { int ret = 0; - perf_push(PERF_BIO_READ_PLAINTEXT); ASSERT(NULL != ks_ssl); ret = bio_read(ks_ssl->ssl_bio, buf, "tls_read_plaintext"); - perf_pop(); return ret; } diff --git a/src/openvpn/status.c b/src/openvpn/status.c index 1e1e3fb..5ca33cb 100644 --- a/src/openvpn/status.c +++ b/src/openvpn/status.c @@ -27,7 +27,6 @@ #include "syshead.h" #include "status.h" -#include "perf.h" #include "misc.h" #include "fdmisc.h" -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1303?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I5bb666a73b4381066e86f53d957e1987fa07303b Gerrit-Change-Number: 1303 Gerrit-PatchSet: 2 Gerrit-Owner: syzzer <st...@ka...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: Gert D. <ge...@gr...> - 2025-10-26 21:50:23
|
Out it goes! The less magical stuff we need to maintain and understand,
the better (especially if it really does not actually work anymore).
I have stared-at-code, the buildbots have built & t_client tested it,
and the remaining failing testcase (1h) is due to external circumstances
(buildbot environment / DNS, "it's always DNS").
Your patch has been applied to the master branch.
commit 9c55e84eea01b1f3ddabae82c7df8adaac7b8c35
Author: Steffan Karger
Date: Sun Oct 26 15:20:52 2025 +0100
Remove perf.c/perf.h
Signed-off-by: Steffan Karger <st...@ka...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1303
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33868.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|