Skip to content

Commit

Permalink
Fixed dual stack bridge
Browse files Browse the repository at this point in the history
+ better error messages for dual stack objects
  • Loading branch information
hknutzen committed Jan 8, 2025
1 parent fdd7edb commit 4d3e739
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 43 deletions.
3 changes: 3 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
{{$NEXT}}

- Fixed dual stack bridge. IPv4 and IPv6 interfaces were accidently
processed together.

6.075 2024-12-19 11:53:37+01:00 Europe/Berlin

- Fix: Attribute 'policy_distribution_point' at dual stack router
Expand Down
43 changes: 23 additions & 20 deletions go/pkg/pass1/check-ip.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package pass1

import (
"maps"
"net/netip"
"slices"
"strings"

"go4.org/netipx"
Expand Down Expand Up @@ -53,28 +51,31 @@ func (c *spoc) checkSubnetOf() {
}

func (c *spoc) checkIPAddressesAndBridges() {
prefix2net := make(map[string][]*network)
type nameV46 struct {
name string
v6 bool
}
prefix2net := make(map[nameV46][]*network)
for _, n := range c.allNetworks {
// Group bridged networks by prefix of name.
// Group bridged networks by IPv4/v6 type and prefix of name.
if n.ipType == bridgedIP {
prefix, _, _ := strings.Cut(n.name, "/")
prefix2net[prefix] = append(prefix2net[prefix], n)
prefixV46 := nameV46{prefix, n.ipV6}
prefix2net[prefixV46] = append(prefix2net[prefixV46], n)
} else if n.ipType == unnumberedIP {
l := n.interfaces
if len(l) > 2 {
c.err(
"Unnumbered %s is connected to more than two interfaces:\n%s",
n.name, l.nameList())
n.vxName(), l.nameList())
}
} else if !(n.ipType == tunnelIP || n.loopback) {
c.checkIPAddr(n)
}
}

// Check address conflicts for collected parts of bridged networks.
// Sort prefix names for deterministic error messages.
for _, prefix := range slices.Sorted(maps.Keys(prefix2net)) {
l := prefix2net[prefix]
for prefixV46, l := range prefix2net {
dummy := new(network)
seen := make(map[*routerIntf]bool)
for _, n := range l {
Expand All @@ -91,7 +92,7 @@ func (c *spoc) checkIPAddressesAndBridges() {
c.checkIPAddr(dummy)

// Check collected parts of bridged networks.
c.checkBridgedNetworks(prefix, l)
c.checkBridgedNetworks(prefixV46.name, l)
}
}

Expand Down Expand Up @@ -126,7 +127,8 @@ func (c *spoc) checkIPAddr(n *network) {
ip := intf.ip
if other, found := ip2name[ip]; found {
if !(intf.redundant && redundant[other]) {
c.err("Duplicate IP address for %s and %s", other, intf)
c.err("Duplicate IP address for %s and %s",
other, intf.vxName())
}
} else {
ip2name[ip] = intf.name
Expand All @@ -139,7 +141,7 @@ func (c *spoc) checkIPAddr(n *network) {
if shortIntf != nil && routeIntf != nil {
c.err("Can't generate static routes for %s"+
" because IP address is unknown for:\n%s",
routeIntf, shortIntf.nameList())
routeIntf.vxName(), shortIntf.nameList())
}

// Optimization: No need to collect .routeInZone at bridge router.
Expand All @@ -156,7 +158,7 @@ func (c *spoc) checkIPAddr(n *network) {
}
rg := h.ipRange
if other, found := range2name[rg]; found {
c.err("Duplicate IP address for %s and %s", other, h)
c.err("Duplicate IP address for %s and %s", other, h.vxName())
} else {
range2name[rg] = h.name
}
Expand All @@ -170,15 +172,15 @@ func (c *spoc) checkIPAddr(n *network) {
}
for ip, other := range ip2name {
if rg.Contains(ip) {
c.err("Duplicate IP address for %s and %s", other, h)
c.err("Duplicate IP address for %s and %s", other, h.vxName())
}
}
}

for _, h := range n.hosts {
if h.ip.IsValid() {
if other, found := ip2name[h.ip]; found {
c.err("Duplicate IP address for %s and %s", other, h)
c.err("Duplicate IP address for %s and %s", other, h.vxName())
} else {
ip2name[h.ip] = h.name
}
Expand All @@ -198,12 +200,12 @@ func (c *spoc) checkBridgedNetworks(prefix string, l netList) {
if n, found := c.symTable.network[prefix[len("network:"):]]; found {
c.err(
"Must not define %s together with bridged networks of same name",
n)
n.vxName())
}
n1 := l[0]
group := l[1:]
if len(group) == 0 {
c.warn("Bridged %s must not be used solitary", n1)
c.warn("Bridged %s must not be used solitary", n1.vxName())
}
seen := make(map[*router]bool)
connected := make(map[*network]bool)
Expand All @@ -214,7 +216,8 @@ func (c *spoc) checkBridgedNetworks(prefix string, l netList) {
n2 := next[0]
next = next[1:]
if n1.ipp != n2.ipp {
c.err("%s and %s must have identical address", n1, n2)
c.err("%s and %s must have identical address",
n1.vxName(), n2.vxName())
}
connected[n2] = true
for _, in := range n2.interfaces {
Expand All @@ -230,7 +233,7 @@ func (c *spoc) checkBridgedNetworks(prefix string, l netList) {
if l3 := in.layer3Intf; l3 != nil {
if !n1.ipp.Contains(l3.ip) {
c.err("%s's IP doesn't match address of bridged networks",
l3)
l3.vxName())
}
}
for _, out := range r.interfaces {
Expand All @@ -246,7 +249,7 @@ func (c *spoc) checkBridgedNetworks(prefix string, l netList) {
}
for _, n2 := range group {
if !connected[n2] {
c.err("%s and %s must be connected by bridge", n2, n1)
c.err("%s and %s must be connected by bridge", n2.vxName(), n1.vxName())
}
}
}
4 changes: 2 additions & 2 deletions go/pkg/pass1/find-subnets.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (c *spoc) findSubnetsInZoneCluster0(z0 *zone) {
// Found two different networks with identical address.
if other := ipMap[ipp.Addr()]; other != nil {
c.err("%s and %s have identical address in %s",
other.name, n.name, z0.name)
other.vxName(), n.vxName(), z0.name)
} else {

// Store original network under its address.
Expand Down Expand Up @@ -422,7 +422,7 @@ func (c *spoc) findSubnetsInNatDomain0(domains []*natDomain, networks netList) {
for _, z := range domain.zones {
if z.cluster[0] == cl {
c.err("%s and %s have identical address in %s",
n, other, z)
n.vxName(), other.vxName(), z)
break
}
}
Expand Down
31 changes: 19 additions & 12 deletions go/pkg/pass1/setup-objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,15 +431,6 @@ func (c *spoc) getAndCheckLayer3(r *router, a *ast.Router) string {
bName = a1.Name
}
}
if l3Name != "" {
// Check existence of layer3 interface.
if !slices.ContainsFunc(a.Interfaces, func(a1 *ast.Attribute) bool {
return a1.Name == l3Name
}) {
c.err("Must define %s at %s for corresponding bridge interfaces",
l3Name, a.Name)
}
}
}
return l3Name
}
Expand Down Expand Up @@ -1820,6 +1811,7 @@ func (c *spoc) setupRouter2(r *router) {
}

isCryptoHub := false
var layer3Intf *routerIntf
for _, intf := range r.interfaces {
if intf.hub != nil || intf.spoke != nil {
if r.model.crypto == "" {
Expand All @@ -1833,9 +1825,24 @@ func (c *spoc) setupRouter2(r *router) {
// Link bridged interfaces with corresponding layer3 device.
// Used in findAutoInterfaces.
if intf.ipType == bridgedIP {
layer3Name := intf.name[len("interface:"):]
layer3Name, _, _ = strings.Cut(layer3Name, "/")
intf.layer3Intf = c.symTable.routerIntf[layer3Name]
if layer3Intf == nil {
i := slices.IndexFunc(r.interfaces, func(intf *routerIntf) bool {
if intf.isLayer3 {
return true
}
return false
})
if i == -1 {
l3Name, _, _ := strings.Cut(intf.vxName(), "/")
c.err("Must define %s for corresponding bridge interfaces",
l3Name)
// Prevent further errors.
layer3Intf = intf
} else {
layer3Intf = r.interfaces[i]
}
}
intf.layer3Intf = layer3Intf
}
}
if r.model.doAuth {
Expand Down
27 changes: 20 additions & 7 deletions go/pkg/pass1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,19 @@ func (x *network) getUp() someObj {
}
func (x *network) intfList() intfList { return x.interfaces }
func (x network) isCombined46() bool { return x.combined46 != nil }
func (x network) vxName() string {
return vxName(x.name, x.ipV6, x.combined46 != nil)
}

func vxName(name string, ipV6, isCombined46 bool) string {
if isCombined46 {
if ipV6 {
return "IPv6 " + name
}
return "IPv4 " + name
}
return name
}

type netList []*network

Expand Down Expand Up @@ -225,6 +238,9 @@ type host struct {
}

func (x host) isCombined46() bool { return x.combined46 != nil }
func (x host) vxName() string {
return vxName(x.name, x.ipV6, x.combined46 != nil)
}

type model struct {
commentChar string
Expand Down Expand Up @@ -388,6 +404,9 @@ func (intf routerIntf) getCrypto() *crypto {
return intf.realIntf.spoke
}
func (x routerIntf) isCombined46() bool { return x.combined46 != nil }
func (x routerIntf) vxName() string {
return vxName(x.name, x.ipV6, x.combined46 != nil)
}

type intfList []*routerIntf

Expand Down Expand Up @@ -536,13 +555,7 @@ type area struct {

func (x area) String() string { return x.name }
func (x area) vxName() string {
if x.combined46 != nil {
if x.ipV6 {
return "IPv6 " + x.name
}
return "IPv4 " + x.name
}
return x.name
return vxName(x.name, x.ipV6, x.combined46 != nil)
}
func (x area) isCombined46() bool { return x.combined46 != nil }

Expand Down
2 changes: 1 addition & 1 deletion go/testdata/bridged.t
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ router:bridge = {
}
network:n1/right = { ip = 10.1.1.0/24; }
=ERROR=
Error: Must define interface:n1 at router:bridge for corresponding bridge interfaces
Error: Must define interface:bridge.n1 for corresponding bridge interfaces
=END=
############################################################
Expand Down
76 changes: 76 additions & 0 deletions go/testdata/ipv46-combined.t
Original file line number Diff line number Diff line change
Expand Up @@ -901,3 +901,79 @@ network:n1 = { ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64; }
Error: Duplicate any:n1-v4 and any:n1-v6 in any:[network:n1-v6]
Error: Duplicate any:n1-v4 and any:n1-v6 in any:[network:n1-v4]
=END=

############################################################
=TITLE=Bridged network
=INPUT=
network:n1/left = {
ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64;
}
router:bridge = {
model = ASA;
managed;
interface:n1 = { ip = 10.1.1.9; ip6 = 2001:db8:1:1::9; hardware = device; }
interface:n1/left = { hardware = left; }
interface:n1/right = { hardware = right; }
}
network:n1/right = {
ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64;
}
=WARNING=NONE

############################################################
=TITLE=Bridge with missing IPv6 address
=INPUT=
network:n1/left = {
ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64;
}
router:bridge = {
model = ASA;
managed;
interface:n1 = { ip = 10.1.1.9; hardware = device; }
interface:n1/left = { hardware = left; }
interface:n1/right = { hardware = right; }
}
network:n1/right = {
ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64;
}
=ERROR=
Error: Must define IPv6 interface:bridge.n1 for corresponding bridge interfaces
=END=

############################################################
=TITLE=IPv6 part of unnumbered network has more than two interfaces
=INPUT=
network:u = { unnumbered; unnumbered6; }
router:r1 = { interface:u = { unnumbered6; } }
router:r2 = { interface:u = { unnumbered; unnumbered6; } }
router:r3 = { interface:u = { unnumbered; unnumbered6; } }
=ERROR=
Error: Unnumbered IPv6 network:u is connected to more than two interfaces:
- interface:r1.u
- interface:r2.u
- interface:r3.u
=END=

############################################################
=TITLE=Duplicate IPv6 address of hosts
=INPUT=
network:n1 = { ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64;
host:h1 = { ip = 10.1.1.1; ip6 = 2001:db8:1:1::1; }
host:h2 = { ip = 10.1.1.2; ip6 = 2001:db8:1:1::1; }
}
=ERROR=
Error: Duplicate IP address for host:h1 and IPv6 host:h2
=END=

############################################################
=TITLE=Duplicate IPv4 address of networks
=INPUT=
network:n1 = { ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64; }
network:n2 = { ip = 10.1.1.0/24; ip6 = 2001:db8:1:2::/64; }
router:r1 = {
interface:n1;
interface:n2;
}
=ERROR=
Error: IPv4 network:n1 and IPv4 network:n2 have identical address in any:[network:n1]
=END=
2 changes: 1 addition & 1 deletion go/testdata/ipv6/bridged_ipv6.t
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ router:bridge = {
}
network:n1/right = { ip = ::a01:100/120; }
=ERROR=
Error: Must define interface:n1 at router:bridge for corresponding bridge interfaces
Error: Must define interface:bridge.n1 for corresponding bridge interfaces
=END=
############################################################
Expand Down

0 comments on commit 4d3e739

Please sign in to comment.