Am 30.12.18 um 12:28 schrieb Antonio Quartulli:
> From: Robin Tarsiger <r...@dasyatidae.com>
>
> This new transport protocol is used to tell the core code that traffic
> should not be directly processed, but should rather be rerouted to a
> transport plugin. It is basically an abstraction as it does not say tell
> the code how to process the data, but simply forces its redirection to
> the external code.
>
> Signed-off-by: Robin Tarsiger <r...@dasyatidae.com>
> [anto...@openvpn.net: refactored commits, restyled code]
> ---
> src/openvpn/forward.c | 5 ++
> src/openvpn/socket.c | 146 ++++++++++++++++++++++++++++++++++++++--
> src/openvpn/socket.h | 70 +++++++++++++++++++
> src/openvpn/transport.h | 5 ++
> 4 files changed, 222 insertions(+), 4 deletions(-)
>
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index 0a90fff0..a7092c7e 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -2150,6 +2150,11 @@ io_wait_dowork(struct context *c, const unsigned int
> flags)
> {
> int i;
> c->c2.event_set_status = 0;
> +#ifdef ENABLE_PLUGIN
> + c->c2.event_set_status |=
> + (socket_indirect_pump(c->c2.link_socket, esr, &status) &
> 3)
The & 3 looks like some defined should be used instead.
> if (addr->ai_protocol == IPPROTO_UDP || addr->ai_socktype == SOCK_DGRAM)
> {
> sock->sd = create_socket_udp(addr, sock->sockflags);
> @@ -2279,7 +2320,11 @@ link_socket_init_phase2(struct link_socket *sock,
> }
>
> /* If socket has not already been created create it now */
> - if (sock->sd == SOCKET_UNDEFINED)
> + if (sock->sd == SOCKET_UNDEFINED
> +#ifdef ENABLE_PLUGIN
> + && !sock->indirect
> +#endif
> + )
> {
> /* If we have no --remote and have still not figured out the
> * protocol family to use we will use the first of the bind */
> @@ -2300,7 +2345,11 @@ link_socket_init_phase2(struct link_socket *sock,
> }
>
> /* Socket still undefined, give a warning and abort connection */
> - if (sock->sd == SOCKET_UNDEFINED)
> + if (sock->sd == SOCKET_UNDEFINED
> +#ifdef ENABLE_PLUGIN
> + && !sock->indirect
> +#endif
> + )
Better use the inline funtcion proto_is_indirect (or similar
sock_is_indirect) that always returns false here and ifdef in header
than to add the ifdefs inline in the code.
> bool
> @@ -3167,6 +3236,10 @@ proto_is_net(int proto)
> bool
> proto_is_dgram(int proto)
> {
> + if (proto_is_indirect(proto))
> + {
> + return true;
> + }
> return proto_is_udp(proto);
> }
I think here need to be a good explaination why indirect is dgram but
not udp and also proto_is_dgram and proto_is_udp need to get some
comment to explain their difference in usage as this is now different.
>
> @@ -3301,6 +3374,18 @@ proto_remote(int proto, bool remote)
> return "TCPv4_CLIENT";
> }
>
> +#ifdef ENABLE_PLUGIN
> + if (proto == PROTO_INDIRECT)
> + {
> + /* FIXME: the string reported here should match the actual transport
> + * protocol being used, however in this function we have no
> knowledge of
> + * what protocol is exactly being used by the transport-plugin.
> + * Therefore we simply return INDIRECT for now.
> + */
> + return "INDIRECT";
> + }
> +#endif
From reading the code this function is only used in creating the string
in options_string which needs to match between peers. As the plugin
emulates UDP/DGRAM behaviour I think we should instead return here the
same as a real UDP protocol.
>
> +#ifdef ENABLE_PLUGIN
> +
> +int link_socket_read_indirect(struct link_socket *sock,
> + struct buffer *buf,
> + struct link_socket_actual *from);
> +
> +int link_socket_write_indirect(struct link_socket *sock,
> + struct buffer *buf,
> + struct link_socket_actual *from);
> +
> +bool proto_is_indirect(int proto);
> +
This prototype definition looks a bit weird here. I see not reason why
the real proto_is_indirect cannot be here
> +#else /* ifdef ENABLE_PLUGIN */
> +
> +static int
> +link_socket_read_indirect(struct link_socket *sock,
> +
> +static int
> +link_socket_write_indirect(struct link_socket *sock,
> +static bool
> +proto_is_indirect(int proto)
I think functions implemented in the header should have the inline
keyword. At least the rest of the header files do it that way.
Arne
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
