Re: [Openvpn-devel] [PATCH 14/25] dco: implement dco support for p2mp/server code path

2022-07-05 Thread Arne Schwabe


Am 05.07.2022 um 16:53 schrieb Antonio Quartulli:

Hi,

On 05/07/2022 14:31, Heiko Hund wrote:

On Freitag, 24. Juni 2022 10:37:58 CEST Antonio Quartulli wrote:

+    uint8_t *ptr = BPTR(>dco_packet_in);
+    uint8_t op = ptr[0] >> P_OPCODE_SHIFT;
+    if (op == P_DATA_V2 || op == P_DATA_V2)


This looks odd. Seems you wanted to check for a second opcode, or is it
obsolete?


we wanted want to bail out on DATA packets - not sure why this was 
repeated twice. Removing one check.




The first one should be P_DATA_V1


Arne



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


Re: [Openvpn-devel] [PATCH 14/25] dco: implement dco support for p2mp/server code path

2022-07-05 Thread Antonio Quartulli

Hi,

On 05/07/2022 14:31, Heiko Hund wrote:

On Freitag, 24. Juni 2022 10:37:58 CEST Antonio Quartulli wrote:

+uint8_t *ptr = BPTR(>dco_packet_in);
+uint8_t op = ptr[0] >> P_OPCODE_SHIFT;
+if (op == P_DATA_V2 || op == P_DATA_V2)


This looks odd. Seems you wanted to check for a second opcode, or is it
obsolete?


we wanted want to bail out on DATA packets - not sure why this was 
repeated twice. Removing one check.





+const char *reason = "(unknown reason by ovpn-dco)";


Wouldn't "ovpn-dco: unknown reason" be better?


Yeah, I'll do as you suggested.

While looking at this function, I realized that these checks should be 
performed before calling multi_process_incoming_link().


Fixing this in v2...

Thanks!


--
Antonio Quartulli


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


Re: [Openvpn-devel] [PATCH 14/25] dco: implement dco support for p2mp/server code path

2022-07-05 Thread Heiko Hund
On Freitag, 24. Juni 2022 10:37:58 CEST Antonio Quartulli wrote:
> +uint8_t *ptr = BPTR(>dco_packet_in);
> +uint8_t op = ptr[0] >> P_OPCODE_SHIFT;
> +if (op == P_DATA_V2 || op == P_DATA_V2)

This looks odd. Seems you wanted to check for a second opcode, or is it 
obsolete?

> +const char *reason = "(unknown reason by ovpn-dco)";

Wouldn't "ovpn-dco: unknown reason" be better?






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


[Openvpn-devel] [PATCH 14/25] dco: implement dco support for p2mp/server code path

2022-06-24 Thread Antonio Quartulli
This change introduces ovpn-dco support along the p2mp/server code path.
Some code seems to be duplicate of the p2p version, but details are
different, so it couldn't be shared.

Signed-off-by: Antonio Quartulli 
---
 src/openvpn/dco.c   | 203 ++
 src/openvpn/dco.h   |  49 ++
 src/openvpn/mtcp.c  |  59 +---
 src/openvpn/mudp.c  |  13 +++
 src/openvpn/multi.c | 212 +++-
 src/openvpn/multi.h |  14 ++-
 6 files changed, 494 insertions(+), 56 deletions(-)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 2919c46d..48e007ea 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -36,6 +36,7 @@
 #include "crypto.h"
 #include "dco.h"
 #include "errlevel.h"
+#include "multi.h"
 #include "openvpn.h"
 #include "ssl_common.h"
 #include "ssl_ncp.h"
@@ -390,4 +391,206 @@ dco_remove_peer(struct context *c)
 }
 }
 
+static bool
+dco_multi_get_localaddr(struct multi_context *m, struct multi_instance *mi,
+struct sockaddr_storage *local)
+{
+#if ENABLE_IP_PKTINFO
+struct context *c = >context;
+
+if (!(c->options.sockflags & SF_USE_IP_PKTINFO))
+{
+return false;
+}
+
+struct link_socket_actual *actual = >c2.link_socket_info->lsa->actual;
+
+switch (actual->dest.addr.sa.sa_family)
+{
+case AF_INET:
+{
+struct sockaddr_in *sock_in4 = (struct sockaddr_in *)local;
+#if defined(HAVE_IN_PKTINFO) && defined(HAVE_IPI_SPEC_DST)
+sock_in4->sin_addr = actual->pi.in4.ipi_addr;
+#elif defined(IP_RECVDSTADDR)
+sock_in4->sin_addr = actual->pi.in4;
+#else
+/* source IP not available on this platform */
+return false;
+#endif
+sock_in4->sin_family = AF_INET;
+break;
+}
+
+case AF_INET6:
+{
+struct sockaddr_in6 *sock_in6 = (struct sockaddr_in6 *)local;
+sock_in6->sin6_addr = actual->pi.in6.ipi6_addr;
+sock_in6->sin6_family = AF_INET6;
+break;
+}
+
+default:
+ASSERT(false);
+}
+
+return true;
+#else  /* if ENABLE_IP_PKTINFO */
+return false;
+#endif /* if ENABLE_IP_PKTINFO */
+}
+
+int
+dco_multi_add_new_peer(struct multi_context *m, struct multi_instance *mi)
+{
+struct context *c = >context;
+
+int peer_id = mi->context.c2.tls_multi->peer_id;
+struct sockaddr *remoteaddr, *localaddr = NULL;
+struct sockaddr_storage local = { 0 };
+int sd = c->c2.link_socket->sd;
+
+if (c->mode == CM_CHILD_TCP)
+{
+/* the remote address will be inferred from the TCP socket endpoint */
+remoteaddr = NULL;
+}
+else
+{
+ASSERT(c->c2.link_socket_info->connection_established);
+remoteaddr = >c2.link_socket_info->lsa->actual.dest.addr.sa;
+}
+
+struct in_addr remote_ip4 = { 0 };
+struct in6_addr *remote_addr6 = NULL;
+struct in_addr *remote_addr4 = NULL;
+
+/* In server mode we need to fetch the remote addresses from the push 
config */
+if (c->c2.push_ifconfig_defined)
+{
+remote_ip4.s_addr =  htonl(c->c2.push_ifconfig_local);
+remote_addr4 = _ip4;
+}
+if (c->c2.push_ifconfig_ipv6_defined)
+{
+remote_addr6 = >c2.push_ifconfig_ipv6_local;
+}
+
+if (dco_multi_get_localaddr(m, mi, ))
+{
+localaddr = (struct sockaddr *)
+}
+
+int ret = dco_new_peer(>c1.tuntap->dco, peer_id, sd, localaddr,
+   remoteaddr, remote_addr4, remote_addr6);
+if (ret < 0)
+{
+return ret;
+}
+
+c->c2.tls_multi->dco_peer_added = true;
+
+if (c->mode == CM_CHILD_TCP)
+{
+multi_tcp_dereference_instance(m->mtcp, mi);
+if (close(sd))
+{
+msg(D_DCO|M_ERRNO, "error closing TCP socket after DCO handover");
+}
+c->c2.link_socket->info.dco_installed = true;
+c->c2.link_socket->sd = SOCKET_UNDEFINED;
+}
+
+return 0;
+}
+
+void
+dco_install_iroute(struct multi_context *m, struct multi_instance *mi,
+   struct mroute_addr *addr)
+{
+#if defined(TARGET_LINUX)
+if (!dco_enabled(>top.options))
+{
+return;
+}
+
+int addrtype = (addr->type & MR_ADDR_MASK);
+
+/* If we do not have local IP addr to install, skip the route */
+if ((addrtype == MR_ADDR_IPV6 && 
!mi->context.c2.push_ifconfig_ipv6_defined)
+|| (addrtype == MR_ADDR_IPV4 && !mi->context.c2.push_ifconfig_defined))
+{
+return;
+}
+
+struct context *c = >context;
+const char *dev = c->c1.tuntap->actual_name;
+
+if (addrtype == MR_ADDR_IPV6)
+{
+int netbits = 128;
+if (addr->type & MR_WITH_NETBITS)
+{
+netbits = addr->netbits;
+}
+
+net_route_v6_add(>top.net_ctx, >v6.addr, netbits,
+ >context.c2.push_ifconfig_ipv6_local, dev, 0,
+