From 268107a1660d2d8e18e115956e704e60dbf6a8a7 Mon Sep 17 00:00:00 2001 From: Max Ma Date: Fri, 14 Jun 2024 15:50:01 +0200 Subject: [PATCH 1/3] add egress range check with netmaker network address --- controllers/node.go | 6 ++++++ logic/nodes.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/controllers/node.go b/controllers/node.go index ed104b354..5d836d76f 100644 --- a/controllers/node.go +++ b/controllers/node.go @@ -415,6 +415,12 @@ func createEgressGateway(w http.ResponseWriter, r *http.Request) { } gateway.NetID = params["network"] gateway.NodeID = params["nodeid"] + err = logic.ValidateEgressRange(gateway) + if err != nil { + logger.Log(0, r.Header.Get("user"), "error validating egress range: ", err.Error()) + logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest")) + return + } node, err = logic.CreateEgressGateway(gateway) if err != nil { logger.Log(0, r.Header.Get("user"), diff --git a/logic/nodes.go b/logic/nodes.go index 72f07836d..ff3805065 100644 --- a/logic/nodes.go +++ b/logic/nodes.go @@ -626,6 +626,42 @@ func ValidateParams(nodeid, netid string) (models.Node, error) { return node, nil } +func ValidateEgressRange(gateway models.EgressGatewayRequest) error { + network, err := GetNetworkSettings(gateway.NetID) + if err != nil { + slog.Error("error getting network with netid", "error", gateway.NetID, err.Error) + return errors.New("error getting network with netid: " + gateway.NetID + " " + err.Error()) + } + _, ipv4Net, _ := net.ParseCIDR(network.AddressRange) + _, ipv6Net, _ := net.ParseCIDR(network.AddressRange6) + + for _, v := range gateway.Ranges { + + _, cidr, _ := net.ParseCIDR(v) + if ipv4Net != nil { + + if ContainsCIDR(ipv4Net, cidr) || ContainsCIDR(cidr, ipv4Net) { + slog.Error("egress range should not be the same as or contained in the netmaker network address", "error", cidr.String(), ipv4Net.String()) + return errors.New("egress range should not be the same as or contained in the netmaker network address" + cidr.String() + " " + ipv4Net.String()) + } + } + if ipv6Net != nil { + if ContainsCIDR(ipv6Net, cidr) || ContainsCIDR(cidr, ipv6Net) { + slog.Error("egress range should not be the same as or contained in the netmaker network address", "error", cidr.String(), ipv6Net.String()) + return errors.New("egress range should not be the same as or contained in the netmaker network address" + cidr.String() + " " + ipv6Net.String()) + } + } + } + + return nil +} + +func ContainsCIDR(net1, net2 *net.IPNet) bool { + net1Size, _ := net1.Mask.Size() + net2Size, _ := net2.Mask.Size() + return net1Size <= net2Size && net1.Contains(net2.IP) +} + // GetAllFailOvers - gets all the nodes that are failovers func GetAllFailOvers() ([]models.Node, error) { nodes, err := GetAllNodes() From efa79155d918b673797f0ea7f9d837bfa9c68e5d Mon Sep 17 00:00:00 2001 From: Max Ma Date: Mon, 24 Jun 2024 18:19:51 +0200 Subject: [PATCH 2/3] add egerssrange check for delete egressGW and extClientGW --- controllers/ext_client.go | 22 ++++++++++++++++++++++ controllers/node.go | 10 +++++++++- go.mod | 2 ++ go.sum | 4 ++++ logic/nodes.go | 32 +++++++++++++++----------------- logic/nodes_test.go | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 85 insertions(+), 18 deletions(-) create mode 100644 logic/nodes_test.go diff --git a/controllers/ext_client.go b/controllers/ext_client.go index eb5308bba..1b1ec3dc7 100644 --- a/controllers/ext_client.go +++ b/controllers/ext_client.go @@ -386,6 +386,17 @@ func createExtClient(w http.ResponseWriter, r *http.Request) { logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest")) return } + + var gateway models.EgressGatewayRequest + gateway.NetID = params["network"] + gateway.Ranges = customExtClient.ExtraAllowedIPs + err := logic.ValidateEgressRange(gateway) + if err != nil { + logger.Log(0, r.Header.Get("user"), "error validating egress range: ", err.Error()) + logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest")) + return + } + node, err := logic.GetNodeByID(nodeid) if err != nil { logger.Log(0, r.Header.Get("user"), @@ -530,6 +541,17 @@ func updateExtClient(w http.ResponseWriter, r *http.Request) { return } } + + var gateway models.EgressGatewayRequest + gateway.NetID = params["network"] + gateway.Ranges = update.ExtraAllowedIPs + err = logic.ValidateEgressRange(gateway) + if err != nil { + logger.Log(0, r.Header.Get("user"), "error validating egress range: ", err.Error()) + logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest")) + return + } + var changedID = update.ClientID != oldExtClient.ClientID if !reflect.DeepEqual(update.DeniedACLs, oldExtClient.DeniedACLs) { diff --git a/controllers/node.go b/controllers/node.go index 5d836d76f..bce4563ac 100644 --- a/controllers/node.go +++ b/controllers/node.go @@ -414,7 +414,6 @@ func createEgressGateway(w http.ResponseWriter, r *http.Request) { return } gateway.NetID = params["network"] - gateway.NodeID = params["nodeid"] err = logic.ValidateEgressRange(gateway) if err != nil { logger.Log(0, r.Header.Get("user"), "error validating egress range: ", err.Error()) @@ -464,6 +463,15 @@ func deleteEgressGateway(w http.ResponseWriter, r *http.Request) { logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest")) return } + var gateway models.EgressGatewayRequest + gateway.NetID = params["network"] + gateway.Ranges = node.EgressGatewayRanges + err = logic.ValidateEgressRange(gateway) + if err != nil { + logger.Log(0, r.Header.Get("user"), "error validating egress range: ", err.Error()) + logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest")) + return + } node, err = logic.DeleteEgressGateway(netid, nodeid) if err != nil { logger.Log(0, r.Header.Get("user"), diff --git a/go.mod b/go.mod index 6587dcacb..a5496f3fb 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/lib/pq v1.10.9 github.com/mattn/go-sqlite3 v1.14.22 github.com/rqlite/gorqlite v0.0.0-20240122221808-a8a425b1a6aa + github.com/seancfoley/ipaddress-go v1.6.0 github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e github.com/stretchr/testify v1.9.0 github.com/txn2/txeh v1.5.5 @@ -49,6 +50,7 @@ require ( github.com/gabriel-vasile/mimetype v1.4.3 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/rivo/uniseg v0.2.0 // indirect + github.com/seancfoley/bintree v1.3.1 // indirect github.com/spf13/pflag v1.0.5 // indirect ) diff --git a/go.sum b/go.sum index 1ec3a8eb1..be74666e8 100644 --- a/go.sum +++ b/go.sum @@ -70,6 +70,10 @@ github.com/rqlite/gorqlite v0.0.0-20240122221808-a8a425b1a6aa h1:hxMLFbj+F444JAS github.com/rqlite/gorqlite v0.0.0-20240122221808-a8a425b1a6aa/go.mod h1:xF/KoXmrRyahPfo5L7Szb5cAAUl53dMWBh9cMruGEZg= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/seancfoley/bintree v1.3.1 h1:cqmmQK7Jm4aw8gna0bP+huu5leVOgHGSJBEpUx3EXGI= +github.com/seancfoley/bintree v1.3.1/go.mod h1:hIUabL8OFYyFVTQ6azeajbopogQc2l5C/hiXMcemWNU= +github.com/seancfoley/ipaddress-go v1.6.0 h1:9z7yGmOnV4P2ML/dlR/kCJiv5tp8iHOOetJvxJh/R5w= +github.com/seancfoley/ipaddress-go v1.6.0/go.mod h1:TQRZgv+9jdvzHmKoPGBMxyiaVmoI0rYpfEk8Q/sL/Iw= github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e h1:MRM5ITcdelLK2j1vwZ3Je0FKVCfqOLp5zO6trqMLYs0= github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e/go.mod h1:XV66xRDqSt+GTGFMVlhk3ULuV0y9ZmzeVGR4mloJI3M= diff --git a/logic/nodes.go b/logic/nodes.go index ff3805065..62f49557c 100644 --- a/logic/nodes.go +++ b/logic/nodes.go @@ -19,6 +19,7 @@ import ( "github.com/gravitl/netmaker/models" "github.com/gravitl/netmaker/servercfg" "github.com/gravitl/netmaker/validation" + "github.com/seancfoley/ipaddress-go/ipaddr" "golang.org/x/exp/slog" ) @@ -632,23 +633,20 @@ func ValidateEgressRange(gateway models.EgressGatewayRequest) error { slog.Error("error getting network with netid", "error", gateway.NetID, err.Error) return errors.New("error getting network with netid: " + gateway.NetID + " " + err.Error()) } - _, ipv4Net, _ := net.ParseCIDR(network.AddressRange) - _, ipv6Net, _ := net.ParseCIDR(network.AddressRange6) + ipv4Net := network.AddressRange + ipv6Net := network.AddressRange6 for _, v := range gateway.Ranges { - - _, cidr, _ := net.ParseCIDR(v) - if ipv4Net != nil { - - if ContainsCIDR(ipv4Net, cidr) || ContainsCIDR(cidr, ipv4Net) { - slog.Error("egress range should not be the same as or contained in the netmaker network address", "error", cidr.String(), ipv4Net.String()) - return errors.New("egress range should not be the same as or contained in the netmaker network address" + cidr.String() + " " + ipv4Net.String()) + if ipv4Net != "" { + if ContainsCIDR(ipv4Net, v) { + slog.Error("egress range should not be the same as or contained in the netmaker network address", "error", v, ipv4Net) + return errors.New("egress range should not be the same as or contained in the netmaker network address" + v + " " + ipv4Net) } } - if ipv6Net != nil { - if ContainsCIDR(ipv6Net, cidr) || ContainsCIDR(cidr, ipv6Net) { - slog.Error("egress range should not be the same as or contained in the netmaker network address", "error", cidr.String(), ipv6Net.String()) - return errors.New("egress range should not be the same as or contained in the netmaker network address" + cidr.String() + " " + ipv6Net.String()) + if ipv6Net != "" { + if ContainsCIDR(ipv6Net, v) { + slog.Error("egress range should not be the same as or contained in the netmaker network address", "error", v, ipv6Net) + return errors.New("egress range should not be the same as or contained in the netmaker network address" + v + " " + ipv6Net) } } } @@ -656,10 +654,10 @@ func ValidateEgressRange(gateway models.EgressGatewayRequest) error { return nil } -func ContainsCIDR(net1, net2 *net.IPNet) bool { - net1Size, _ := net1.Mask.Size() - net2Size, _ := net2.Mask.Size() - return net1Size <= net2Size && net1.Contains(net2.IP) +func ContainsCIDR(net1, net2 string) bool { + one, two := ipaddr.NewIPAddressString(net1), + ipaddr.NewIPAddressString(net2) + return one.Contains(two) || two.Contains(one) } // GetAllFailOvers - gets all the nodes that are failovers diff --git a/logic/nodes_test.go b/logic/nodes_test.go new file mode 100644 index 000000000..e3331a6fd --- /dev/null +++ b/logic/nodes_test.go @@ -0,0 +1,33 @@ +package logic + +import ( + "testing" +) + +func TestContainsCIDR(t *testing.T) { + + b := ContainsCIDR("10.1.1.2/32", "10.1.1.0/24") + if !b { + t.Errorf("expected true, returned %v", b) + } + + b = ContainsCIDR("10.1.1.2/32", "10.5.1.0/24") + if b { + t.Errorf("expected false, returned %v", b) + } + + b = ContainsCIDR("fd52:65f5:d685:d11d::1/64", "fd52:65f5:d685:d11d::/64") + if !b { + t.Errorf("expected true, returned %v", b) + } + + b1 := ContainsCIDR("fd10:10::/64", "fd10::/16") + if !b1 { + t.Errorf("expected true, returned %v", b1) + } + + b1 = ContainsCIDR("fd10:10::/64", "fd10::/64") + if b1 { + t.Errorf("expected false, returned %v", b1) + } +} From fc78d652b76646a5573241457fc34fe403ce50e7 Mon Sep 17 00:00:00 2001 From: Max Ma Date: Mon, 24 Jun 2024 18:50:52 +0200 Subject: [PATCH 3/3] remove egress range check for delete --- controllers/node.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/controllers/node.go b/controllers/node.go index bce4563ac..adc631057 100644 --- a/controllers/node.go +++ b/controllers/node.go @@ -463,15 +463,6 @@ func deleteEgressGateway(w http.ResponseWriter, r *http.Request) { logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest")) return } - var gateway models.EgressGatewayRequest - gateway.NetID = params["network"] - gateway.Ranges = node.EgressGatewayRanges - err = logic.ValidateEgressRange(gateway) - if err != nil { - logger.Log(0, r.Header.Get("user"), "error validating egress range: ", err.Error()) - logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest")) - return - } node, err = logic.DeleteEgressGateway(netid, nodeid) if err != nil { logger.Log(0, r.Header.Get("user"),