Hi,

reading this more closely at merging/testing time, I do have a change
request...

On Fri, Jun 26, 2020 at 08:49:44PM +0200, Maximilian Wilhelm wrote:
> +#ifdef TARGET_LINUX
> +    else if (streq (p[0], "bind-dev") && p[1])
> +    {
> +        VERIFY_PERMISSION (OPT_P_SOCKFLAGS);
> +        options->bind_dev = p[1];
> +    }
> +#endif

One could argue whether the argument should be changed for IFNAMSIZ here
(so we can error-out right away if it's too long).  But this is just 
something to consider.

> --- a/src/openvpn/socket.c
> +++ b/src/openvpn/socket.c
> @@ -1138,6 +1138,14 @@ create_socket(struct link_socket *sock, struct 
> addrinfo *addr)
>      /* set socket to --mark packets with given value */
>      socket_set_mark(sock->sd, sock->mark);
>  
> +#if defined(TARGET_LINUX)
> +    if (sock->bind_dev)
> +    {
> +        msg (M_INFO, "Using bind-dev %s", sock->bind_dev);
> +        setsockopt (sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev, 
> strlen (sock->bind_dev) + 1);
> +    }
> +#endif

Here, we *must* have a return code check, and logging of an error message
if setsocktopt() fails.

Imagine someone calling "openvpn --bind-dev eht0" (because he has fat
fingers).  The current code will silently fail the setsockopt() - because
there is no such interface name - but nothing in the logs will show a hint
*why* openvpn is just not doing what requested.


Sorry for not spotting this in the first review.

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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to