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 <[email protected]> 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.
