[Openvpn-devel,4/9] VLAN: filter multicast and client-to-client unicast traffic

Message ID 20191009143422.9419-5-a@unstable.cc
State Accepted
Headers show
Series support VLANs in TAP mode | expand

Commit Message

Antonio Quartulli Oct. 9, 2019, 3:34 a.m. UTC
With this change, client-to-client communications are possible only if
clients were configured with the same PVID.

At the same time also broadcast packets are now forwarded only to hosts
belonging to the originator VLAN.

Signed-off-by: Fabian Knittel <fabian.knittel@lettink.de>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
 src/openvpn/multi.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)


Gert Doering Nov. 7, 2019, 9 a.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Your patch has been applied to the master branch.

Stared at the code, did quite a bit of testing, found interesting effects.

What this patch does is "client-to-client isolation according to pvid"
(so if you have clients with "vlan-pvid 200" in their ccd/ file, and
other clients with "vlan-pvid 207", only those with the same ID can 
talk to each other).  This is as desired.

What it also does is completely break TAP-to-client communication if
"--vlan-tagging" is enabled - broadcasts ("...incoming_tun()") are
broadcasted everywhere, but unicast packets are never delivered as
they are looked up with a dst PVID of "0" while the "...incoming_link()"
part has learned then with the correct per-client pvid (defaulting 
to "@1").  The necessary adjustments for this are coming in a later
patch in the series, but it makes testing individual bits a bit
more complex (I hacked multi.c to use a non-0 server pvid and that
made tap<->client work again, so the basics are sound).

If --vlan-tagging is disabled, all tests pass.  So this is not breaking
existing functionality, just not adding all required new bits yet.

(And it's not touching any non-TAP code paths anyway)

commit 1c57ea76a256330314d53999bce3e09644b420f9
Author: Antonio Quartulli
Date:   Wed Oct 9 16:34:17 2019 +0200

     VLAN: filter multicast and client-to-client unicast traffic

     Signed-off-by: Fabian Knittel <fabian.knittel@lettink.de>
     Signed-off-by: Antonio Quartulli <a@unstable.cc>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20191009143422.9419-5-a@unstable.cc>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18922.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>

kind regards,

Gert Doering


diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 95b33e7a..e733ca9a 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2643,10 +2643,12 @@  multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst
             else if (TUNNEL_TYPE(m->top.c1.tuntap) == DEV_TYPE_TAP)
+                uint16_t vid = 0;
 #ifdef ENABLE_PF
                 struct mroute_addr edest;
                 if (m->top.options.vlan_tagging)
                     if (vlan_is_tagged(&c->c2.to_tun))
@@ -2655,6 +2657,10 @@  multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst
                         msg(D_VLAN_DEBUG, "dropping incoming VLAN-tagged frame");
                         c->c2.to_tun.len = 0;
+                    else
+                    {
+                        vid = c->options.vlan_pvid;
+                    }
                 /* extract packet source and dest addresses */
                 mroute_flags = mroute_extract_addr_from_packet(&src,
@@ -2665,7 +2671,7 @@  multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst
-                                                               0,
+                                                               vid,
@@ -2678,7 +2684,8 @@  multi_process_incoming_link(struct multi_context *m, struct multi_instance *inst
                             if (mroute_flags & (MROUTE_EXTRACT_BCAST|MROUTE_EXTRACT_MCAST))
-                                multi_bcast(m, &c->c2.to_tun, m->pending, NULL, 0);
+                                multi_bcast(m, &c->c2.to_tun, m->pending, NULL,
+                                            vid);
                             else /* try client-to-client routing */