Re: [Openvpn-devel] [PATCH 3/3] Introduce dynamic tls-crypt for secure soft_reset/session renegotiation
On Freitag, 9. September 2022 21:59:02 CEST Arne Schwabe wrote:
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -1803,6 +1803,10 @@ multi_client_set_protocol_options(struct context *c)
> {
> o->imported_protocol_flags |= CO_USE_TLS_KEY_MATERIAL_EXPORT;
> }
> +if (proto & IV_PROTO_SECURE_RENOG)
I think any occurrence of "renog" (i.e. short for renegotiation[s]) should be
changed to "reneg" throughout this patch. See --reneg-x options. Could be just
me, so a more opinions would be welcome.
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -8553,6 +8553,10 @@ add_option(struct options *options,
> {
> options->imported_protocol_flags |=
> CO_USE_TLS_KEY_MATERIAL_EXPORT; }
> +else if (streq(p[j], "secure-renog"))
Should be rewritten to use --protocol-flags instead.
> @@ -71,9 +71,77 @@ tls_crypt_init_key(struct key_ctx_bi *key, const char
> *key_file, msg(M_FATAL, "ERROR: --tls-crypt not supported");
> }
> crypto_read_openvpn_key(, key, key_file, key_inline, key_direction,
> -"Control Channel Encryption", "tls-crypt");
> +"Control Channel Encryption", "tls-crypt",
> keydata); }
>
> +/**
> + * Will produce dest = dest XOR data
> + */
> +static void
> +xor_key2_key(struct key2 *dest, const struct key2 *data)
I wonder if this would be more readable as
xor_key(struct key2 *key, const struct key2 *mask)
> +bool
> +tls_session_generate_secure_renegotition_key(struct tls_multi *multi,
> + struct tls_session *session)
typo renegotiation_key
> +struct key2 rengokeys;
typo renogkeys (renegkeys, IMHO)
> @@ -285,7 +354,7 @@ tls_crypt_v2_init_client_key(struct key_ctx_bi *key,
> struct buffer *wkc_buf, }
>
> tls_crypt_v2_load_client_key(key, , false);
> -secure_memzero(, sizeof(key2));
> +*original_key = key2;
We should do the zeroing in tls_session_generate_secure_renegotiation_key()
shortly after we used it to XOR then. And maybe only delay it if we need to
XOR anyways, could use original_key == NULL as indication.
> @@ -587,8 +655,8 @@ tls_crypt_v2_extract_client_key(struct buffer *buf,
> ctx->cleanup_key_ctx = true;
> ctx->opt.flags |= CO_PACKET_ID_LONG_FORM;
> memset(>opt.key_ctx_bi, 0, sizeof(ctx->opt.key_ctx_bi));
> -tls_crypt_v2_load_client_key(>opt.key_ctx_bi, _key, true);
> -secure_memzero(_key, sizeof(client_key));
> +tls_crypt_v2_load_client_key(>opt.key_ctx_bi,
> + >original_tlscrypt_keydata, true);
Same as above for the server side. Could zero here immediately if
original_tlscrypt_keydata == NULL
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/3] Allows renegotiation only to start if session is fully established
On Freitag, 9. September 2022 21:59:00 CEST Arne Schwabe wrote: > This change makes the state machine more strict in terms of transation *transitions > Signed-off-by: Arne Schwabe Acked-by: Heiko Hund For those who wonder what this is/does, my take on it: basically shields the calls to key_state_soft_reset() on both the sending and receiving side, and ensures both ends have completed the control channel negotiation fully (i.e. are in state S_GENERATED_KEYS). At that time pushed options are guaranteed to be processed/active, which was the main motivation for this change, as I understand it. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Allow Authtoken lifetime to be short than renegotiation time
Acked-by: Gert Doering
The feature itself is really in the "we are a swiss army knife and can
do everything" side of things. It does not introduce a new option and
no new #ifdef, and the actual code change is not very intrusive.
I should point out that there is potential for conflict with the
ongoing discussion on username locking - this patch ensures that
the username is locked
+if (!multi->locked_username)
+{
+msg(D_SHOW_KEYS, "username not locked, skipping auth-token renewal.");
+return;
+}
.. so with my open patch ("do not lock empty username") this will lead
to "renewed tokens are only available after first renegotiation", which
is certainly not what we want. But Arne did not like that patch anyway,
so we need a better fix there.
I have submitted the result to my "server side hard core testing" setup,
which does have an --auth-gen-token instance. Without extra options,
this does what it did before - new tokens get pushed at renegotiation
time, and reconnecting with a valid token bypasses (deliberately slow)
auth_pam auth.
With the new option (renewal-time set to 60) I see incoming unsolicited
PUSH_REPLY messages:
2022-10-17 18:29:41 Initialization Sequence Completed
2022-10-17 18:30:50 PUSH: Received control message: 'PUSH_REPLY,
auth-tokenSESS_ID'
2022-10-17 18:31:54 PUSH: Received control message: 'PUSH_REPLY,
auth-tokenSESS_ID'
2022-10-17 18:32:48 PUSH: Received control message: 'PUSH_REPLY,
auth-tokenSESS_ID'
2022-10-17 18:33:43 PUSH: Received control message: 'PUSH_REPLY,
auth-tokenSESS_ID'
(interesting enough, the server claims to have sent these at 18:30:40,
18:31:40, 18:32:40, 18:33:40... - discussed this with Arne: PUSH_REPLY
messages are queued only, but tls_process() isn't scheduled, so the
dangling message are send when check_tls() is called, every 10-ish
seconds. For normal renewal intervals this shouldn't matter, I just
found it interesting)
I also have reneg-sec set to 150 on the client, so there's "intermixed"
reneg/PUSH_REPLY - but this is fine, we just renew "a bit more often"
in this case.
Normal token expiry (600 seconds in my testbed) works as before. "Client
offline for too long" expiry is not so easy to test, so I killed the
server instead, waited, restarted :-) - when restarting the server
quickly, the stored token on the client works perfectly. When waiting
for 120 seconds before restarting, we get a clear indication:
2022-10-17 18:42:19 us=660299 194.97.140.21:58930 Timestamp (1666024749) of
auth-token is out of the renewal window
2022-10-17 18:42:19 us=660325 194.97.140.21:58930 --auth-gen-token: auth-token
from client expired
.. and the client falls back to auth-user-pass, as it should.
I have only tested with a "master" client, but my understanding of the
PUSH_REPLY handler is "every version of OpenVPN since ever" handles
unsolicited PUSH_REPLY just fine. So testing this with 2.3, 2.4, 2.5
was too much for my lazy self.
Your patch has been applied to the master branch.
commit 9a5161704173e31f2510d3f5c29361f76e275d0f
Author: Arne Schwabe
Date: Mon Oct 17 11:51:45 2022 +0200
Allow Authtoken lifetime to be short than renegotiation time
Signed-off-by: Arne Schwabe
Acked-by: Gert Doering
Message-Id: <20221017095145.2580186-1-a...@rfc2549.org>
URL:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25407.html
Signed-off-by: Gert Doering
--
kind regards,
Gert Doering
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH 1/2] FreeBSD: for topology subnet, put tun interface into IFF_BROADCAST mode
Signed-off-by: Kristof Provost
On 12 Oct 2022, at 16:59, Gert Doering wrote:
> For reasons unknown, OpenVPN has always put FreeBSD tun(4) interfaces
> into point-to-point mode (IFF_POINTOPOINT), which means "local and
> remote address, no on-link subnet".
>
> "--topology subnet" was emulated by adding a subnet-route to the "remote"
> (which was just picking a free address from the subnet).
>
> This works well enough for classic tun(4) interfaces that have no
> next-hop resolution, and routes pointing to "that fake remote" only
> (because all routing is done inside OpenVPN and it does not matter how
> packets get there) - but for ovpn(4) interfaces, it breaks iroute setup,
> where the route next-hop must be an on-link address.
>
> Thus, change interface to IFF_BROADCAST mode, and get rid of all the
> special code needed to "fake" subnet mode.
>
> Tested with tun(4) and ovpn(4) on FreeBSD 14, client and server, and
> with tun(4) on FreeBSD 12 and 7.4
>
> To actually work with ovpn(4) / FreeBSD DCO, a followup patch for
> kernel ovpn(4) and OpenVPN dco_freebsd.c is needed.
>
> Signed-off-by: Gert Doering
> ---
> Changes.rst | 5 +
> src/openvpn/tun.c | 37 -
> 2 files changed, 17 insertions(+), 25 deletions(-)
>
> diff --git a/Changes.rst b/Changes.rst
> index df56f76a..0397cb94 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -163,6 +163,11 @@ User-visible Changes
> - :code:`link_mtu` parameter is removed from environment or replaced with 0
> when scripts are
>called with parameters. This parameter is unreliable and no longer
> internally calculated.
>
> +- FreeBSD tun interfaces with ``--topology subnet`` are now put into real
> + subnet mode (IFF_BROADCAST instead of IFF_POINTOPOINT) - this might upset
> + software that enumerates interfaces, looking for "broadcast capable?" and
> + expecting certain results. Normal uses should not see any difference.
> +
> Overview of changes in 2.5
> ==
>
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index ddee49f9..a83ec9e6 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -1479,43 +1479,23 @@ do_ifconfig_ipv4(struct tuntap *tt, const char
> *ifname, int tun_mtu,
>
> #elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY)
>
> -in_addr_t remote_end; /* for "virtual" subnet topology */
> -
> /* example: ifconfig tun2 10.2.0.2 10.2.0.1 mtu 1450 netmask
> 255.255.255.255 up */
> -if (tun)
> +if (tun) /* point-to-point tun */
> {
> argv_printf(, "%s %s %s %s mtu %d netmask 255.255.255.255 up",
> IFCONFIG_PATH, ifname, ifconfig_local,
> ifconfig_remote_netmask, tun_mtu);
> }
> -else if (tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
> -{
> -remote_end = create_arbitrary_remote( tt );
> -argv_printf(, "%s %s %s %s mtu %d netmask %s up", IFCONFIG_PATH,
> -ifname, ifconfig_local, print_in_addr_t(remote_end, 0,
> ),
> -tun_mtu, ifconfig_remote_netmask);
> -}
> -else
> +else/* tun with topology subnet and tap mode (always subnet)
> */
> {
> -argv_printf(, "%s %s %s netmask %s mtu %d up", IFCONFIG_PATH,
> -ifname, ifconfig_local, ifconfig_remote_netmask,
> tun_mtu);
> +int netbits = netmask_to_netbits2(tt->remote_netmask);
> +argv_printf(, "%s %s %s/%d mtu %d up", IFCONFIG_PATH,
> +ifname, ifconfig_local, netbits, tun_mtu );
> }
>
> argv_msg(M_INFO, );
> openvpn_execve_check(, es, S_FATAL, "FreeBSD ifconfig failed");
>
> -/* Add a network route for the local tun interface */
> -if (!tun && tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
> -{
> -struct route_ipv4 r;
> -CLEAR(r);
> -r.flags = RT_DEFINED;
> -r.network = tt->local & tt->remote_netmask;
> -r.netmask = tt->remote_netmask;
> -r.gateway = remote_end;
> -add_route(, tt, 0, NULL, es, NULL);
> -}
> -
> #elif defined(TARGET_AIX)
> {
> /* AIX ifconfig will complain if it can't find ODM path in env */
> @@ -2949,12 +2929,19 @@ open_tun(const char *dev, const char *dev_type, const
> char *dev_node, struct tun
>
> if (tt->fd >= 0 && tt->type == DEV_TYPE_TUN)
> {
> +/* see "Interface Flags" in ifnet(9) */
> int i = IFF_POINTOPOINT | IFF_MULTICAST;
> +if (tt->topology == TOP_SUBNET)
> +{
> +i = IFF_BROADCAST | IFF_MULTICAST;
> +}
>
> if (ioctl(tt->fd, TUNSIFMODE, ) < 0)
> {
> msg(M_WARN | M_ERRNO, "ioctl(TUNSIFMODE)");
> }
> +
> +/* multi_af mode for v4+v6, see "tun(4)" */
> i = 1;
> if (ioctl(tt->fd, TUNSIFHEAD, ) < 0)
> {
> --
> 2.37.3
>
>
>
>
Re: [Openvpn-devel] [PATCH 2/2] FreeBSD DCO: introduce real subnet mode
Signed-off-by: Kristof Provost
On 12 Oct 2022, at 16:59, Gert Doering wrote:
> To be able to configure a FreeBSD interface to "subnet" mode
> (as opposed to point-to-point mode), it needs to have its
> if_iflags set to IFF_BROADCAST. For tun(4) interface this is
> done with the TUNSIFMODE ioctl(), but this does not work for
> more modern interfaces like ovpn(4) which communicate over
> a common SIOCSDRVSPEC ioctl() that contains a "cmd" and a
> "parameter list".
>
> Introduce OVPN_SET_IFMODE cmd, add dco_set_ifmode() function
> to put kernel interface into IFF_BROADCAST or IFF_POINTOPOINT
> as needed.
>
> (This needs a kernel patch to add the OVPN_SET_IFMODE on the
> other side - with an older kernel, OpenVPN will just fail now)
>
> Signed-off-by: Gert Doering
> ---
> src/openvpn/dco_freebsd.c | 36 ++
> src/openvpn/ovpn_dco_freebsd.h | 1 +
> 2 files changed, 37 insertions(+)
>
> diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
> index c6da6ce3..8adbf7f1 100644
> --- a/src/openvpn/dco_freebsd.c
> +++ b/src/openvpn/dco_freebsd.c
> @@ -165,6 +165,34 @@ ovpn_dco_init(int mode, dco_context_t *dco)
> return true;
> }
>
> +static int
> +dco_set_ifmode(dco_context_t *dco, int ifmode)
> +{
> +struct ifdrv drv;
> +nvlist_t *nvl;
> +int ret;
> +
> +msg(M_INFO, "ifmode=%08x", ifmode);
> +nvl = nvlist_create(0);
> +nvlist_add_number(nvl, "ifmode", ifmode);
> +
> +CLEAR(drv);
> +snprintf(drv.ifd_name, IFNAMSIZ, "%s", dco->ifname);
> +drv.ifd_cmd = OVPN_SET_IFMODE;
> +drv.ifd_data = nvlist_pack(nvl, _len);
> +
> +ret = ioctl(dco->fd, SIOCSDRVSPEC, );
> +if (ret)
> +{
> +msg(M_WARN | M_ERRNO, "Failed to set ifmode");
> +}
> +
> +free(drv.ifd_data);
> +nvlist_destroy(nvl);
> +
> +return ret;
> +}
> +
> static int
> create_interface(struct tuntap *tt, const char *dev)
> {
> @@ -205,6 +233,14 @@ create_interface(struct tuntap *tt, const char *dev)
> snprintf(tt->dco.ifname, IFNAMSIZ, "%s", ifr.ifr_data);
> tt->actual_name = string_alloc(tt->dco.ifname, NULL);
>
> +/* see "Interface Flags" in ifnet(9) */
> +int i = IFF_POINTOPOINT | IFF_MULTICAST;
> +if (tt->topology == TOP_SUBNET)
> +{
> +i = IFF_BROADCAST | IFF_MULTICAST;
> +}
> +dco_set_ifmode(>dco, i);
> +
> return 0;
> }
>
> diff --git a/src/openvpn/ovpn_dco_freebsd.h b/src/openvpn/ovpn_dco_freebsd.h
> index 7ceec06e..cf92d597 100644
> --- a/src/openvpn/ovpn_dco_freebsd.h
> +++ b/src/openvpn/ovpn_dco_freebsd.h
> @@ -60,5 +60,6 @@ enum ovpn_key_cipher {
> #define OVPN_SEND_PKT _IO('D', 9)
> #define OVPN_POLL_PKT _IO('D', 10)
> #define OVPN_GET_PKT_IO('D', 11)
> +#define OVPN_SET_IFMODE _IO('D', 12)
>
> #endif /* ifndef _NET_IF_OVPN_H_ */
> --
> 2.37.3
>
>
>
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] route/iroute handling on FreeBSD
On 12 Oct 2022, at 16:38, Gert Doering wrote:
> people have alreadycomplained at me that I write so long e-mails today,
> so I can write more...
>
> On Wed, Oct 12, 2022 at 08:39:31AM +0200, Gert Doering wrote:
>> Factor 1: single-peer (client or p2p) vs. multi-peer
>>
>> single-peer -> DCO has only 1 peer, all packets that go into the
>> tun/dco interface are sent out to the single peer
>> ("dumb pipe mode") - exactly like tun(4) behaves
>>
>> If a subnet is configured on the interface, packets to
>> ALL IPs (!= local) in that subnet are sent to the other
>> side. No next-hop lookup is done.
>
> This is "sort of" handled in if_ovpn.c today
>
> ovpn_route_peer(struct ovpn_softc *sc, struct mbuf **m0,
> const struct sockaddr *dst)
> {
> ...
> /* Shortcut if we're a client (or are a server and have only one
> client). */
> if (sc->peercount == 1)
> return (ovpn_find_only_peer(sc));
>
>
> ... so this works for the client, but has one interesting drawback on the
> server - if there is only a single client connected, the server will send
> ALL to-be-tunneled packets to that client. As soon as client #2 connects,
> packets are properly sorted.
>
> [..]
>> Factor 2: IFF_POINTOPOINT vs. IFF_BROADCAST
>>
>> This seems to be a *BSD-specific thing, aka "there is nothing in the
>> Linux specific code that seems to bother with this".
>
> I've whacked at if_ovpn.c and dco_freebsd.c a bit now, and I seem
> to have working code for both ends. I am not a FreeBSD kernel coder,
> so I have no idea how many behavioural standards I am violating,
> but it makes "real subnet mode in OpenVPN" work for me, with DCO.
>
> Kernel patch attached, OpenVPN patches will follow soonish (outside
> of this e-mail thread).
>
I’m happy with that, and will commit (modulo one small style tweak) that patch
in FreeBSD.
Best regards,
Kristof
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Allow Authtoken lifetime to be short than renegotiation time
Currently the life time of the auth-token is tied to the renegotiation
time. While this is fine for many setups, some setups prefer a user
to be no longer authenticated when the user disconnects from the VPN
for a certain amount of time.
This commit allows to shorten the renewal time of the auth-token and
ensures that the server resends the auth-token often enough over the
existing control channel. This way of updating the auth token is a lot
more lightweight than the alternative (frequent renegotiations).
Patch v2: fix grammar mistakes (thanks Gert), fix unit tests
Signed-off-by: Arne Schwabe
---
doc/man-sections/server-options.rst| 16 ---
src/openvpn/auth_token.c | 50 +++---
src/openvpn/auth_token.h | 14 --
src/openvpn/forward.c | 7 +++
src/openvpn/init.c | 13 ++
src/openvpn/openvpn.h | 3 ++
src/openvpn/options.c | 24 +--
src/openvpn/options.h | 2 +-
src/openvpn/ssl.c | 8 +++-
src/openvpn/ssl_common.h | 3 +-
tests/unit_tests/openvpn/test_auth_token.c | 12 --
11 files changed, 126 insertions(+), 26 deletions(-)
diff --git a/doc/man-sections/server-options.rst
b/doc/man-sections/server-options.rst
index 9d0c73b69..99263fff3 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -14,7 +14,7 @@ fast hardware. SSL/TLS authentication must be used in this
mode.
Valid syntax:
::
- auth-gen-token [lifetime] [external-auth]
+ auth-gen-token [lifetime] [renewal-time] [external-auth]
After successful user/password authentication, the OpenVPN server will
with this option generate a temporary authentication token and push that
@@ -31,13 +31,17 @@ fast hardware. SSL/TLS authentication must be used in this
mode.
The lifetime is defined in seconds. If lifetime is not set or it is set
to :code:`0`, the token will never expire.
+ If ``renewal-time`` is not set it defaults to ``reneg-sec``.
+
+
The token will expire either after the configured ``lifetime`` of the
token is reached or after not being renewed for more than 2 \*
- ``reneg-sec`` seconds. Clients will be sent renewed tokens on every TLS
- renogiation to keep the client's token updated. This is done to
- invalidate a token if a client is disconnected for a sufficiently long
- time, while at the same time permitting much longer token lifetimes for
- active clients.
+ ``renewal-time`` seconds. Clients will be sent renewed tokens on every TLS
+ renegotiation. If ``renewal-time`` is lower than ``reneg-sec`` the server
+ will push an updated temporary authentication token every ``reneweal-time``
+ seconds. This is done to invalidate a token if a client is disconnected for a
+ sufficiently long time, while at the same time permitting much longer token
+ lifetimes for active clients.
This feature is useful for environments which are configured to use One
Time Passwords (OTP) as part of the user/password authentications and
diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index b5f9f6dd7..7b963a9c5 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -174,7 +174,7 @@ generate_auth_token(const struct user_pass *up, struct
tls_multi *multi)
if (multi->auth_token_initial)
{
-/* Just enough space to fit 8 bytes+ 1 extra to decode a non padded
+/* Just enough space to fit 8 bytes+ 1 extra to decode a non-padded
* base64 string (multiple of 3 bytes). 9 bytes => 12 bytes base64
* bytes
*/
@@ -349,11 +349,11 @@ verify_auth_token(struct user_pass *up, struct tls_multi
*multi,
/* Accept session tokens only if their timestamp is in the acceptable range
* for renegotiations */
bool in_renegotiation_time = now >= timestamp
- && now < timestamp + 2 *
session->opt->renegotiate_seconds;
+ && now < timestamp + 2 *
session->opt->auth_token_renewal;
if (!in_renegotiation_time)
{
-msg(M_WARN, "Timestamp (%" PRIu64 ") of auth-token is out of the
renegotiation window",
+msg(M_WARN, "Timestamp (%" PRIu64 ") of auth-token is out of the
renewal window",
timestamp);
ret |= AUTH_TOKEN_EXPIRED;
}
@@ -416,6 +416,44 @@ wipe_auth_token(struct tls_multi *multi)
}
}
+void
+check_send_auth_token(struct context *c)
+{
+struct tls_multi *multi = c->c2.tls_multi;
+struct tls_session *session = >session[TM_ACTIVE];
+
+if (get_primary_key(multi)->state < S_GENERATED_KEYS
+|| get_primary_key(multi)->authenticated != KS_AUTH_TRUE)
+{
+/* the currently active session is still in renegotiation or another
+ * not fully authorized state. We are either very close to a
+ *
Re: [Openvpn-devel] [PATCH] Allow Authtoken lifetime to be short than renegotiation time
Hi, I'm working through this, and have some questions... On Fri, Oct 07, 2022 at 05:38:23PM +0200, Arne Schwabe wrote: > diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c > index 6a45b9e91..eca4a4335 100644 > --- a/src/openvpn/forward.c > +++ b/src/openvpn/forward.c > @@ -195,9 +196,15 @@ check_tls(struct context *c) > > interval_schedule_wakeup(>c2.tmp_int, ); > > -/* Our current code has no good hooks in the TLS machinery to update > +/* > + * Our current code has no good hooks in the TLS machinery to update > * DCO keys. So we check the key status after the whole TLS machinery > * has been completed and potentially update them > + * > + * We have a hidden state transition from secondary to primary key based > + * on ks->auth_deferred_expire that DCO needs to check that the normal > + * TLS state engine does not check. So we call the doc check even if > + * tmp_status does not indicate that something has changed. > */ > check_dco_key_status(c); > This seems unrelated to auth-token, but "escaped from the P2P DCO rework". I suggest to ignore that hunk. > diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h > index f1cade2ef..db0a96cc9 100644 > --- a/src/openvpn/ssl_common.h > +++ b/src/openvpn/ssl_common.h > @@ -624,6 +625,10 @@ struct tls_multi >* user/pass authentications in this session. >*/ > char *auth_token_initial; > +/**< Last time an auth-token was generated, this is strictly speaking > redundant > + * as the auth_token attribute already contains the information but in a > + * highly encoded way */ > +time_t auth_token_lastgenerated; > /**< The first auth-token we sent to a client. We use this to remember > * the session ID and initial timestamp when generating new auth-token. > */ This is not used at all. I suspect this was coming from an initial approach and later all was done based on event handling. I suggest to ignore that hunk as well. The rest looks reasonable (and I'm halfway through applying it, so no need for a v2, just an "yes, please ignore those hunks"). gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Change exit signal in P2P to be a SIGUSR1 and delay CC exit in P2MP
Acked-by: Gert Doering Tested the whole lot again. Only difference to v1 is in p2mp mode with incoming TLS EEN, which now logs 10:15:34 cron2-freebsd-tc-amd64/194.97.140.21:53341 Exit message received by peer 10:15:34 cron2-freebsd-tc-amd64/194.97.140.21:53341 Delayed exit in 5 seconds 10:15:38 read UDPv6 [ECONNREFUSED]: Connection refused (fd=5,code=111) 10:15:39 us=240199 cron2-freebsd-tc-amd64/194.97.140.21:53341 SIGTERM[soft,delayed-exit] received, client-instance exiting (the ECONNREFUSED is "the client exited right away, so never saw the control-plane ACK" - calling "--explicit-exit-notify 10" on the client makes those last packets succeed, with no adverse effects on unwanted restarts) I have worked a bit on the commit message, and a lot on the new comment block - I hope it is now clearer what could happen, and how this change avoids the problem. I have also added a Changes.rst entry (user-visible changes), pointing out that "--remap-usr1 SIGTERM" will bring back the old behaviour, if someone relies on it. Your patch has been applied to the master branch. commit d468dff7bdfd79059818c190ddf41b125bb658de Author: Arne Schwabe Date: Sun Oct 16 17:49:53 2022 +0200 Change exit signal in P2P to be a SIGUSR1 and delay CC exit in P2MP Signed-off-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20221016154953.2483509-1-a...@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25403.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
