Re: [Openvpn-devel] [PATCH v2] Use P_DATA_V2 for server->client packets too

[Gert D. <ge...@gr...>]

Hi,

as just discussed on IRC, I'm not fully happy with this, and want 
a v3...

On Sun, Nov 12, 2017 at 06:22:37PM +0100, Steffan Karger wrote:
[..]
> diff --git a/src/openvpn/push.c b/src/openvpn/push.c
> index 5947a31f..16a4101f 100644
> --- a/src/openvpn/push.c
> +++ b/src/openvpn/push.c
> @@ -366,6 +366,7 @@ prepare_push_reply(struct context *c, struct gc_arena *gc,
>              push_option_fmt(gc, push_list, M_USAGE, "peer-id %d",
>                              tls_multi->peer_id);
>          }
> +        tls_multi->use_peer_id = true;
>      }

While this *works* today, it relies on the fact that only clients
that can do IV_PROTO=2 will ever send IV_PROTO=<anything>.  

But it still creates weird code (more context):

     /* Send peer-id if client supports it */
     optstr = peer_info ? strstr(peer_info, "IV_PROTO=") : NULL;
     if (optstr)
     {   
         int proto = 0;
         int r = sscanf(optstr, "IV_PROTO=%d", &proto);
         if ((r == 1) && (proto >= 2))
         {   
             push_option_fmt(gc, push_list, M_USAGE, "peer-id %d",
                             tls_multi->peer_id);
         }
+        tls_multi->use_peer_id = true;
     }

where we set tls_multi->use_peer_id = true for any client that sends
IV_PROTO=<anything>, but the rules for "push a peer ID to the client"
is much stricter, requiring it to be IV_PROTO=<num> with num>=2 - so
the "tls_multi->use_peer_id = true;" should go inside that block.

gert

-- 
now what should I write here...

Gert Doering - Munich, Germany                             ge...@gr...