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 [email protected]
signature.asc
Description: PGP signature
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
