On Wed, Jan 24, 2018 at 11:27:51PM +0000, Tom Smyth wrote: > Hello, Martin, Remi, All > Im very excited about this feature, Thanks Martin, > Please see Comments inline below > > On 23 January 2018 at 18:06, Remi Locherer <remi.loche...@relo.ch> wrote: > > On Mon, Jan 22, 2018 at 04:23:59PM +0100, Martin Pieuchot wrote: > >> Diff below adds a new feature to bridge(4), similar to Cisco's Protected > >> Port but with more possibilities. > >> > >> The idea is to prevent traffic to flow between some members of a bridge(4). > >> For example: > >> - you want to prevent some of your servers to talk to each others, or > >> - you want employees/students/clients to have access to internet and a > >> printer but not to each other, or > >> - you want VMs to have access to an uplink but not to each other > > > > I think the last example is really useful and makes sense. > Agreed > > > > For the other cases there would most likely be a switch between the other > > host and the OpenBSD box. Then you need to enforce the policy on that > > switch. > Other examples where it would be useful if the OpenBSD box is terminating > alot of Tunnels and you want to limit broadcast domains. > > > >> With the diff below it is now possible to defined "protected groups". > >> Interface that are part of such groups cannot send/receive traffic > >> to/from any other member of the group. > > > > Why several protected groups and not just a single one? > Having multiple protected groups can be useful in the following circumstances: > Where you want to isolate clients / vms from each other, > and where you have 2 redundant up-link ports in the bridge that could other > wise > create a loop. > (where spanning tree cannot be deployed (due to incompatible or > limited switches)
Wouldn't this duplicate BUM traffic? In this situation I would try a setup with trunk(4) for the uplinks. At least the failover mode should work if your switches have a limited feature set. > e.g > a) loop prevention in 2 up link ports on a bridge > so if the 2 up-link ports are added to the same protected group they cannot > form a loop as the 2 up-links are isolated from each other > Uplink ports would be em1 and em2 and we would add them to protected group1 > em1 and em2 cannot communicate with each other and cant create a loop > b)preventing vms from talking to each other by adding them to another > protected > group > e.g. > tap1 and tap2 would be added to protected group 2 > so tap1 and tap2 would be isolated from each other, and because they are > members > of a different protected group to em1 and em2 in the same bridge they > can communicate > freely with either em1 or em2 > > I have used this technique on certain Layer 2 networks where STP was > not possible or > desirable,, and it would give the administrator granular control > of traffic flow in the bridge > Other Added benefits > (control broadcast domains etc) > increase security for access networks (prevent rogue dhcp Servers) > > I hope this helps and Im really excited with this diff :) > Thanks Guys > > > > > >> > >> In the example below, em1 and em2 can only send/receive traffic through > >> em0 because they are both in the protected group 2. > >> > >> bridge0: flags=41<UP,RUNNING> > >> index 9 llprio 3 > >> groups: bridge > >> priority 32768 hellotime 2 fwddelay 15 maxage 20 holdcnt 6 proto > >> rstp > >> designated: id 00:00:00:00:00:00 priority 0 > >> em0 flags=3<LEARNING,DISCOVER> > >> port 6 ifpriority 0 ifcost 0 > >> em1 flags=3<LEARNING,DISCOVER> > >> port 7 ifpriority 0 ifcost 0 protected 2 > >> em2 flags=3<LEARNING,DISCOVER> > >> port 8 ifpriority 0 ifcost 0 protected 2 > >> > >> Adding an interface to a protected group is as simple as: > >> > >> # ifconfig bridge0 protected em1 2 > >> > >> Removing it: > >> > >> # ifconfig bridge0 -protected em1 > >> > >> Comments? Oks? > >> > >> Index: sys/net/if_bridge.c > >> =================================================================== > >> RCS file: /cvs/src/sys/net/if_bridge.c,v > >> retrieving revision 1.301 > >> diff -u -p -r1.301 if_bridge.c > >> --- sys/net/if_bridge.c 10 Jan 2018 23:50:39 -0000 1.301 > >> +++ sys/net/if_bridge.c 22 Jan 2018 14:27:41 -0000 > >> @@ -409,6 +409,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c > >> } > >> req->ifbr_ifsflags = p->bif_flags; > >> req->ifbr_portno = p->ifp->if_index & 0xfff; > >> + req->ifbr_protected = p->bif_protected; > >> if (p->bif_flags & IFBIF_STP) { > >> bp = p->bif_stp; > >> req->ifbr_state = bstp_getstate(bs, bp); > >> @@ -496,6 +497,19 @@ bridge_ioctl(struct ifnet *ifp, u_long c > >> brop->ifbop_last_tc_time.tv_sec = bs->bs_last_tc_time.tv_sec; > >> brop->ifbop_last_tc_time.tv_usec = > >> bs->bs_last_tc_time.tv_usec; > >> break; > >> + case SIOCBRDGSIFPROT: > >> + ifs = ifunit(req->ifbr_ifsname); > >> + if (ifs == NULL) { > >> + error = ENOENT; > >> + break; > >> + } > >> + p = (struct bridge_iflist *)ifs->if_bridgeport; > >> + if (p == NULL || p->bridge_sc != sc) { > >> + error = ESRCH; > >> + break; > >> + } > >> + p->bif_protected = req->ifbr_protected; > >> + break; > >> case SIOCBRDGRTS: > >> case SIOCBRDGGCACHE: > >> case SIOCBRDGGPRI: > >> @@ -594,6 +608,7 @@ bridge_bifconf(struct bridge_softc *sc, > >> strlcpy(breq->ifbr_ifsname, p->ifp->if_xname, IFNAMSIZ); > >> breq->ifbr_ifsflags = p->bif_flags; > >> breq->ifbr_portno = p->ifp->if_index & 0xfff; > >> + breq->ifbr_protected = p->bif_protected; > >> if (p->bif_flags & IFBIF_STP) { > >> bp = p->bif_stp; > >> breq->ifbr_state = bstp_getstate(sc->sc_stp, bp); > >> @@ -848,6 +863,7 @@ bridgeintr_frame(struct bridge_softc *sc > >> struct bridge_rtnode *dst_p; > >> struct ether_addr *dst, *src; > >> struct ether_header eh; > >> + u_int32_t protected; > >> int len; > >> > >> > >> @@ -960,6 +976,7 @@ bridgeintr_frame(struct bridge_softc *sc > >> bridge_broadcast(sc, src_if, &eh, m); > >> return; > >> } > >> + protected = ifl->bif_protected; > >> > >> /* > >> * At this point, we're dealing with a unicast frame going to a > >> @@ -975,6 +992,14 @@ bridgeintr_frame(struct bridge_softc *sc > >> m_freem(m); > >> return; > >> } > >> + /* > >> + * Do not transmit if both ports are part of the same protected > >> + * group. > >> + */ > >> + if (protected != 0 && protected == ifl->bif_protected) { > >> + m_freem(m); > >> + return; > >> + } > >> if (bridge_filterrule(&ifl->bif_brlout, &eh, m) == BRL_ACTION_BLOCK) > >> { > >> m_freem(m); > >> return; > >> @@ -1159,6 +1184,10 @@ bridge_broadcast(struct bridge_softc *sc > >> struct mbuf *mc; > >> struct ifnet *dst_if; > >> int len, used = 0; > >> + u_int32_t protected; > >> + > >> + p = (struct bridge_iflist *)ifp->if_bridgeport; > >> + protected = p->bif_protected; > >> > >> TAILQ_FOREACH(p, &sc->sc_iflist, next) { > >> dst_if = p->ifp; > >> @@ -1177,6 +1206,13 @@ bridge_broadcast(struct bridge_softc *sc > >> /* Drop non-IP frames if the appropriate flag is set. */ > >> if (p->bif_flags & IFBIF_BLOCKNONIP && > >> bridge_blocknonip(eh, m)) > >> + continue; > >> + > >> + /* > >> + * Do not transmit if both ports are part of the same > >> + * protected group. > >> + */ > >> + if (protected != 0 && protected == p->bif_protected) > >> continue; > >> > >> if (bridge_filterrule(&p->bif_brlout, eh, m) == > >> BRL_ACTION_BLOCK) > >> Index: sys/net/if_bridge.h > >> =================================================================== > >> RCS file: /cvs/src/sys/net/if_bridge.h,v > >> retrieving revision 1.55 > >> diff -u -p -r1.55 if_bridge.h > >> --- sys/net/if_bridge.h 20 Jan 2017 05:03:48 -0000 1.55 > >> +++ sys/net/if_bridge.h 22 Jan 2018 15:13:07 -0000 > >> @@ -46,6 +46,7 @@ struct ifbreq { > >> char ifbr_ifsname[IFNAMSIZ]; /* member ifs name */ > >> u_int32_t ifbr_ifsflags; /* member ifs flags */ > >> u_int32_t ifbr_portno; /* member port number */ > >> + u_int32_t ifbr_protected; /* protected group number */ > >> > >> u_int8_t ifbr_state; /* member stp state */ > >> u_int8_t ifbr_priority; /* member stp priority */ > >> @@ -398,6 +399,7 @@ struct bridge_iflist { > >> struct brl_head bif_brlout; /* output rules */ > >> struct ifnet *ifp; /* member interface > >> */ > >> u_int32_t bif_flags; /* member flags */ > >> + u_int32_t bif_protected; /* protected group */ > >> void *bif_dhcookie; > >> }; > >> #define bif_state bif_stp->bp_state > >> Index: sys/sys/sockio.h > >> =================================================================== > >> RCS file: /cvs/src/sys/sys/sockio.h,v > >> retrieving revision 1.72 > >> diff -u -p -r1.72 sockio.h > >> --- sys/sys/sockio.h 24 Oct 2017 09:36:13 -0000 1.72 > >> +++ sys/sys/sockio.h 22 Jan 2018 14:09:09 -0000 > >> @@ -89,6 +89,7 @@ > >> #define SIOCBRDGDADDR _IOW('i', 71, struct ifbareq) /* delete > >> addr */ > >> #define SIOCBRDGFLUSH _IOW('i', 72, struct ifbreq) /* flush > >> addr cache */ > >> #define SIOCBRDGADDL _IOW('i', 73, struct ifbreq) /* add local > >> port */ > >> +#define SIOCBRDGSIFPROT _IOW('i', 74, struct ifbreq) /* set > >> protected grp */ > >> > >> #define SIOCBRDGARL _IOW('i', 77, struct ifbrlreq) /* add bridge rule */ > >> #define SIOCBRDGFRL _IOW('i', 78, struct ifbrlreq) /* flush brdg rules > >> */ > >> Index: sbin/ifconfig/brconfig.c > >> =================================================================== > >> RCS file: /cvs/src/sbin/ifconfig/brconfig.c,v > >> retrieving revision 1.16 > >> diff -u -p -r1.16 brconfig.c > >> --- sbin/ifconfig/brconfig.c 31 Jul 2017 02:32:11 -0000 1.16 > >> +++ sbin/ifconfig/brconfig.c 22 Jan 2018 14:30:38 -0000 > >> @@ -336,6 +336,8 @@ bridge_list(char *delim) > >> printf("port %u ifpriority %u ifcost %u", > >> reqp->ifbr_portno, reqp->ifbr_priority, > >> reqp->ifbr_path_cost); > >> + if (reqp->ifbr_protected) > >> + printf(" protected %u", reqp->ifbr_protected); > >> if (reqp->ifbr_ifsflags & IFBIF_STP) > >> printf(" %s role %s", > >> stpstates[reqp->ifbr_state], > >> @@ -452,6 +454,41 @@ bridge_priority(const char *arg, int d) > >> bp.ifbrp_prio = v; > >> if (ioctl(s, SIOCBRDGSPRI, (caddr_t)&bp) < 0) > >> err(1, "%s", name); > >> +} > >> + > >> +void > >> +bridge_protect(const char *ifname, const char *val) > >> +{ > >> + struct ifbreq breq; > >> + unsigned long v; > >> + char *endptr; > >> + > >> + strlcpy(breq.ifbr_name, name, sizeof(breq.ifbr_name)); > >> + strlcpy(breq.ifbr_ifsname, ifname, sizeof(breq.ifbr_ifsname)); > >> + > >> + errno = 0; > >> + v = strtoul(val, &endptr, 0); > >> + if (val[0] == '\0' || endptr[0] != '\0' || v == 0 || v > UINT32_MAX > >> || > >> + (errno == ERANGE && v == ULONG_MAX)) > >> + err(1, "invalid arg for protected group: %s", val); > >> + breq.ifbr_protected = v; > >> + > >> + if (ioctl(s, SIOCBRDGSIFPROT, (caddr_t)&breq) < 0) > >> + err(1, "%s: %s", name, val); > >> +} > >> + > >> +void > >> +bridge_unprotect(const char *ifname, int d) > >> +{ > >> + struct ifbreq breq; > >> + > >> + strlcpy(breq.ifbr_name, name, sizeof(breq.ifbr_name)); > >> + strlcpy(breq.ifbr_ifsname, ifname, sizeof(breq.ifbr_ifsname)); > >> + > >> + breq.ifbr_protected = 0; > >> + > >> + if (ioctl(s, SIOCBRDGSIFPROT, (caddr_t)&breq) < 0) > >> + err(1, "%s: %d", name, 0); > >> } > >> > >> void > >> Index: sbin/ifconfig/brconfig.h > >> =================================================================== > >> RCS file: /cvs/src/sbin/ifconfig/brconfig.h,v > >> retrieving revision 1.12 > >> diff -u -p -r1.12 brconfig.h > >> --- sbin/ifconfig/brconfig.h 16 Jan 2018 10:33:55 -0000 1.12 > >> +++ sbin/ifconfig/brconfig.h 22 Jan 2018 14:12:39 -0000 > >> @@ -52,6 +52,8 @@ void bridge_addrs(const char *, int); > >> void bridge_hellotime(const char *, int); > >> void bridge_fwddelay(const char *, int); > >> void bridge_maxage(const char *, int); > >> +void bridge_protect(const char *, const char *); > >> +void bridge_unprotect(const char *, int); > >> void bridge_proto(const char *, int); > >> void bridge_ifprio(const char *, const char *); > >> void bridge_ifcost(const char *, const char *); > >> Index: sbin/ifconfig/ifconfig.8 > >> =================================================================== > >> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v > >> retrieving revision 1.292 > >> diff -u -p -r1.292 ifconfig.8 > >> --- sbin/ifconfig/ifconfig.8 16 Jan 2018 10:33:55 -0000 1.292 > >> +++ sbin/ifconfig/ifconfig.8 22 Jan 2018 15:11:32 -0000 > >> @@ -662,6 +662,13 @@ The default is 100 entries. > >> .It Cm maxage Ar time > >> Set the time (in seconds) that a spanning tree protocol configuration is > >> valid. > >> Defaults to 20 seconds, minimum of 6, maximum of 40. > >> +.It Cm protected Ar interface Ar id > >> +Put > >> +.Ar interface > >> +in protected mode. > >> +A protected interface does not forward traffic to any other protected > >> interface > >> +having the same > >> +.Ar id . > >> .It Cm proto Ar value > >> Force the spanning tree protocol version. > >> The available values are > >> Index: sbin/ifconfig/ifconfig.c > >> =================================================================== > >> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v > >> retrieving revision 1.353 > >> diff -u -p -r1.353 ifconfig.c > >> --- sbin/ifconfig/ifconfig.c 16 Jan 2018 10:33:55 -0000 1.353 > >> +++ sbin/ifconfig/ifconfig.c 22 Jan 2018 14:17:17 -0000 > >> @@ -472,6 +472,8 @@ const struct cmd { > >> { "-edge", NEXTARG, 0, unsetedge }, > >> { "autoedge", NEXTARG, 0, setautoedge }, > >> { "-autoedge", NEXTARG, 0, unsetautoedge }, > >> + { "protected", NEXTARG2, 0, NULL, bridge_protect > >> }, > >> + { "-protected", NEXTARG, 0, bridge_unprotect }, > >> { "ptp", NEXTARG, 0, setptp }, > >> { "-ptp", NEXTARG, 0, unsetptp }, > >> { "autoptp", NEXTARG, 0, setautoptp }, > > > > > > -- > Kindest regards, > Tom Smyth > > Mobile: +353 87 6193172 > The information contained in this E-mail is intended only for the > confidential use of the named recipient. If the reader of this message > is not the intended recipient or the person responsible for > delivering it to the recipient, you are hereby notified that you have > received this communication in error and that any review, > dissemination or copying of this communication is strictly prohibited. > If you have received this in error, please notify the sender > immediately by telephone at the number above and erase the message > You are requested to carry out your own virus check before > opening any attachment.