diff mbox series

[Openvpn-devel] Do not include auth-token in pulled option digest

Message ID 20221218192203.1214943-1-selva.nair@gmail.com
State Superseded
Headers
Series [Openvpn-devel] Do not include auth-token in pulled option digest |

Commit Message

Selva Nair Dec. 18, 2022, 7:22 p.m. UTC 
  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(-)

Comments

Arne Schwabe Dec. 19, 2022, 12:11 p.m. UTC | #1 
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>
Frank Lichtenheld Dec. 19, 2022, 12:21 p.m. UTC | #2 
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,
Gert Doering Dec. 19, 2022, 1:05 p.m. UTC | #3 
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 mbox series

Patch

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;
         }