| Message ID | 20221218192203.1214943-1-selva.nair@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel] Do not include auth-token in pulled option digest | expand |
Am 18.12.22 um 20:22 schrieb selva.nair@gmail.com: > From: Selva Nair <selva.nair@gmail.com> > > As change in auth-token is common on restart and does not > require tun-reopen, exclude it from the "pulled options digest" > calculation. Without this tun is always re-opened on SIGUSR1 > if auth-token is in use which breaks persist-tun. > Makes sense Acked-By: Arne Schwabe <arne@rfc2549.org>
On Sun, Dec 18, 2022 at 02:22:03PM -0500, selva.nair@gmail.com wrote: > From: Selva Nair <selva.nair@gmail.com> > > As change in auth-token is common on restart and does not > require tun-reopen, exclude it from the "pulled options digest" > calculation. Without this tun is always re-opened on SIGUSR1 > if auth-token is in use which breaks persist-tun. > > Fixes #200 > > Signed-off-by: Selva Nair <selva.nair@gmail.com> > --- > src/openvpn/push.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/openvpn/push.c b/src/openvpn/push.c > index ad2f3c65..95e3ae49 100644 > --- a/src/openvpn/push.c > +++ b/src/openvpn/push.c > @@ -989,8 +989,8 @@ push_update_digest(md_ctx_t *ctx, struct buffer *buf, const struct options *opt) > char line[OPTION_PARM_SIZE]; > while (buf_parse(buf, ',', line, sizeof(line))) > { > - /* peer-id might change on restart and this should not trigger reopening tun */ > - if (strprefix(line, "peer-id ")) > + /* peer-id and auth-token might change on restart and this should not trigger reopening tun */ > + if (strprefix(line, "peer-id ") || strprefix(line, "auth-token")) If I interpret this correctly, this will also exclude auth-token-user, because you didn't add the space after the option name like for peer-id. This should either be reflected in the comment above or changed. Regards,
Hi, On Mon, Dec 19, 2022 at 01:21:37PM +0100, Frank Lichtenheld wrote: > > + /* peer-id and auth-token might change on restart and this should not trigger reopening tun */ > > + if (strprefix(line, "peer-id ") || strprefix(line, "auth-token")) > > If I interpret this correctly, this will also exclude auth-token-user, because you didn't > add the space after the option name like for peer-id. This should either be reflected > in the comment above or changed. Well spotted. For "less magic, easier understanding" reasons I'd actually include it explicitly + if (strprefix(line, "peer-id ") || strprefix(line, "auth-token ") || strprefix(line, "auth-token-user ")) (with blank) - yes, Selva's original code does that, but more magically. gert
diff --git a/src/openvpn/push.c b/src/openvpn/push.c index ad2f3c65..95e3ae49 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -989,8 +989,8 @@ push_update_digest(md_ctx_t *ctx, struct buffer *buf, const struct options *opt) char line[OPTION_PARM_SIZE]; while (buf_parse(buf, ',', line, sizeof(line))) { - /* peer-id might change on restart and this should not trigger reopening tun */ - if (strprefix(line, "peer-id ")) + /* peer-id and auth-token might change on restart and this should not trigger reopening tun */ + if (strprefix(line, "peer-id ") || strprefix(line, "auth-token")) { continue; }