From ecd62fb785e2839b2a7f4496cb1c918c4cddf76a Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 26 Apr 2023 17:37:40 +0200 Subject: [PATCH] remove terrible filter code Signed-off-by: Kristoffer Dalby --- acls.go | 72 -------------------------- acls_test.go | 135 ------------------------------------------------- app.go | 8 ++- machine.go | 139 --------------------------------------------------- 4 files changed, 3 insertions(+), 351 deletions(-) diff --git a/acls.go b/acls.go index 3e146cde..71ad628e 100644 --- a/acls.go +++ b/acls.go @@ -136,14 +136,6 @@ func (h *Headscale) UpdateACLRules() error { log.Trace().Interface("ACL", rules).Msg("ACL rules generated") h.aclRules = rules - // Precompute a map of which sources can reach each destination, this is - // to provide quicker lookup when we calculate the peerlist for the map - // response to nodes. - // aclPeerCacheMap := generateACLPeerCacheMap(rules) - // h.aclPeerCacheMapRW.Lock() - // h.aclPeerCacheMap = aclPeerCacheMap - // h.aclPeerCacheMapRW.Unlock() - if featureEnableSSH() { sshRules, err := h.generateSSHRules() if err != nil { @@ -161,70 +153,6 @@ func (h *Headscale) UpdateACLRules() error { return nil } -// // generateACLPeerCacheMap takes a list of Tailscale filter rules and generates a map -// // of which Sources ("*" and IPs) can access destinations. This is to speed up the -// // process of generating MapResponses when deciding which Peers to inform nodes about. -// func generateACLPeerCacheMap(rules []tailcfg.FilterRule) map[string][]string { -// aclCachePeerMap := make(map[string][]string) -// for _, rule := range rules { -// for _, srcIP := range rule.SrcIPs { -// for _, ip := range expandACLPeerAddr(srcIP) { -// if data, ok := aclCachePeerMap[ip]; ok { -// for _, dstPort := range rule.DstPorts { -// data = append(data, dstPort.IP) -// } -// aclCachePeerMap[ip] = data -// } else { -// dstPortsMap := make([]string, 0) -// for _, dstPort := range rule.DstPorts { -// dstPortsMap = append(dstPortsMap, dstPort.IP) -// } -// aclCachePeerMap[ip] = dstPortsMap -// } -// } -// } -// } -// -// log.Trace().Interface("ACL Cache Map", aclCachePeerMap).Msg("ACL Peer Cache Map generated") -// -// return aclCachePeerMap -// } -// -// // expandACLPeerAddr takes a "tailcfg.FilterRule" "IP" and expands it into -// // something our cache logic can look up, which is "*" or single IP addresses. -// // This is probably quite inefficient, but it is a result of -// // "make it work, then make it fast", and a lot of the ACL stuff does not -// // work, but people have tried to make it fast. -// func expandACLPeerAddr(srcIP string) []string { -// if ip, err := netip.ParseAddr(srcIP); err == nil { -// return []string{ip.String()} -// } -// -// if cidr, err := netip.ParsePrefix(srcIP); err == nil { -// addrs := []string{} -// -// ipRange := netipx.RangeOfPrefix(cidr) -// -// from := ipRange.From() -// too := ipRange.To() -// -// if from == too { -// return []string{from.String()} -// } -// -// for from != too && from.Less(too) { -// addrs = append(addrs, from.String()) -// from = from.Next() -// } -// addrs = append(addrs, too.String()) // Add the last IP address in the range -// -// return addrs -// } -// -// // probably "*" or other string based "IP" -// return []string{srcIP} -// } - // generateFilterRules takes a set of machines and an ACLPolicy and generates a // set of Tailscale compatible FilterRules used to allow traffic on clients. func (pol *ACLPolicy) generateFilterRules( diff --git a/acls_test.go b/acls_test.go index fef92e42..30dfbe31 100644 --- a/acls_test.go +++ b/acls_test.go @@ -1661,141 +1661,6 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { } } -// func Test_expandACLPeerAddr(t *testing.T) { -// type args struct { -// srcIP string -// } -// tests := []struct { -// name string -// args args -// want []string -// }{ -// { -// name: "asterix", -// args: args{ -// srcIP: "*", -// }, -// want: []string{"*"}, -// }, -// { -// name: "ip", -// args: args{ -// srcIP: "10.0.0.1", -// }, -// want: []string{"10.0.0.1"}, -// }, -// { -// name: "ip/32", -// args: args{ -// srcIP: "10.0.0.1/32", -// }, -// want: []string{"10.0.0.1"}, -// }, -// { -// name: "ip/30", -// args: args{ -// srcIP: "10.0.0.1/30", -// }, -// want: []string{ -// "10.0.0.0", -// "10.0.0.1", -// "10.0.0.2", -// "10.0.0.3", -// }, -// }, -// { -// name: "ip/28", -// args: args{ -// srcIP: "192.168.0.128/28", -// }, -// want: []string{ -// "192.168.0.128", "192.168.0.129", "192.168.0.130", -// "192.168.0.131", "192.168.0.132", "192.168.0.133", -// "192.168.0.134", "192.168.0.135", "192.168.0.136", -// "192.168.0.137", "192.168.0.138", "192.168.0.139", -// "192.168.0.140", "192.168.0.141", "192.168.0.142", -// "192.168.0.143", -// }, -// }, -// } -// for _, tt := range tests { -// t.Run(tt.name, func(t *testing.T) { -// if got := expandACLPeerAddr(tt.args.srcIP); !reflect.DeepEqual(got, tt.want) { -// t.Errorf("expandACLPeerAddr() = %v, want %v", got, tt.want) -// } -// }) -// } -// } - -// func Test_expandACLPeerAddrV6(t *testing.T) { -// type args struct { -// srcIP string -// } -// tests := []struct { -// name string -// args args -// want []string -// }{ -// { -// name: "asterix", -// args: args{ -// srcIP: "*", -// }, -// want: []string{"*"}, -// }, -// { -// name: "ipfull", -// args: args{ -// srcIP: "fd7a:115c:a1e0:ab12:4943:cd96:624c:3166", -// }, -// want: []string{"fd7a:115c:a1e0:ab12:4943:cd96:624c:3166"}, -// }, -// { -// name: "ipzerocompression", -// args: args{ -// srcIP: "fd7a:115c:a1e0:ab12:4943:cd96:624c::", -// }, -// want: []string{"fd7a:115c:a1e0:ab12:4943:cd96:624c:0"}, -// }, -// { -// name: "ip/128", -// args: args{ -// srcIP: "fd7a:115c:a1e0:ab12:4943:cd96:624c:3166/128", -// }, -// want: []string{"fd7a:115c:a1e0:ab12:4943:cd96:624c:3166"}, -// }, -// { -// name: "ip/127", -// args: args{ -// srcIP: "fd7a:115c:a1e0:ab12:4943:cd96:624c:0000/127", -// }, -// want: []string{ -// "fd7a:115c:a1e0:ab12:4943:cd96:624c:0", -// "fd7a:115c:a1e0:ab12:4943:cd96:624c:1", -// }, -// }, -// { -// name: "ip/126", -// args: args{ -// srcIP: "fd7a:115c:a1e0:ab12:4943:cd96:624c:0000/126", -// }, -// want: []string{ -// "fd7a:115c:a1e0:ab12:4943:cd96:624c:0", -// "fd7a:115c:a1e0:ab12:4943:cd96:624c:1", -// "fd7a:115c:a1e0:ab12:4943:cd96:624c:2", -// "fd7a:115c:a1e0:ab12:4943:cd96:624c:3", -// }, -// }, -// } -// for _, tt := range tests { -// t.Run(tt.name, func(t *testing.T) { -// if got := expandACLPeerAddr(tt.args.srcIP); !reflect.DeepEqual(got, tt.want) { -// t.Errorf("expandACLPeerAddr() = %v, want %v", got, tt.want) -// } -// }) -// } -// } - func TestACLPolicy_generateFilterRules(t *testing.T) { type field struct { pol ACLPolicy diff --git a/app.go b/app.go index b3066916..79784ba1 100644 --- a/app.go +++ b/app.go @@ -84,11 +84,9 @@ type Headscale struct { DERPMap *tailcfg.DERPMap DERPServer *DERPServer - aclPolicy *ACLPolicy - aclRules []tailcfg.FilterRule - aclPeerCacheMapRW sync.RWMutex - aclPeerCacheMap map[string][]string - sshPolicy *tailcfg.SSHPolicy + aclPolicy *ACLPolicy + aclRules []tailcfg.FilterRule + sshPolicy *tailcfg.SSHPolicy lastStateChange *xsync.MapOf[string, time.Time] diff --git a/machine.go b/machine.go index 3da41809..982ed78a 100644 --- a/machine.go +++ b/machine.go @@ -204,145 +204,6 @@ func filterMachinesByACL( return result } -// filterMachinesByACL returns the list of peers authorized to be accessed from a given machine. -// func filterMachinesByACL( -// machine *Machine, -// machines Machines, -// lock *sync.RWMutex, -// aclPeerCacheMap map[string][]string, -// ) Machines { -// log.Trace(). -// Caller(). -// Str("self", machine.Hostname). -// Str("input", machines.String()). -// Msg("Finding peers filtered by ACLs") -// -// peers := make(map[uint64]Machine) -// // Aclfilter peers here. We are itering through machines in all users and search through the computed aclRules -// // for match between rule SrcIPs and DstPorts. If the rule is a match we allow the machine to be viewable. -// machineIPs := machine.IPAddresses.ToStringSlice() -// -// // TODO(kradalby): Remove this lock, I suspect its not a good idea, and might not be necessary, -// // we only set this at startup atm (reading ACLs) and it might become a bottleneck. -// lock.RLock() -// -// for _, peer := range machines { -// if peer.ID == machine.ID { -// continue -// } -// peerIPs := peer.IPAddresses.ToStringSlice() -// -// if dstMap, ok := aclPeerCacheMap["*"]; ok { -// // match source and all destination -// -// for _, dst := range dstMap { -// if dst == "*" { -// peers[peer.ID] = peer -// -// continue -// } -// } -// -// // match source and all destination -// for _, peerIP := range peerIPs { -// for _, dst := range dstMap { -// _, cdr, _ := net.ParseCIDR(dst) -// ip := net.ParseIP(peerIP) -// if dst == peerIP || (cdr != nil && ip != nil && cdr.Contains(ip)) { -// peers[peer.ID] = peer -// -// continue -// } -// } -// } -// -// // match all sources and source -// for _, machineIP := range machineIPs { -// for _, dst := range dstMap { -// _, cdr, _ := net.ParseCIDR(dst) -// ip := net.ParseIP(machineIP) -// if dst == machineIP || (cdr != nil && ip != nil && cdr.Contains(ip)) { -// peers[peer.ID] = peer -// -// continue -// } -// } -// } -// } -// -// for _, machineIP := range machineIPs { -// if dstMap, ok := aclPeerCacheMap[machineIP]; ok { -// // match source and all destination -// for _, dst := range dstMap { -// if dst == "*" { -// peers[peer.ID] = peer -// -// continue -// } -// } -// -// // match source and destination -// for _, peerIP := range peerIPs { -// for _, dst := range dstMap { -// _, cdr, _ := net.ParseCIDR(dst) -// ip := net.ParseIP(peerIP) -// if dst == peerIP || (cdr != nil && ip != nil && cdr.Contains(ip)) { -// peers[peer.ID] = peer -// -// continue -// } -// } -// } -// } -// } -// -// for _, peerIP := range peerIPs { -// if dstMap, ok := aclPeerCacheMap[peerIP]; ok { -// // match source and all destination -// for _, dst := range dstMap { -// if dst == "*" { -// peers[peer.ID] = peer -// -// continue -// } -// } -// -// // match return path -// for _, machineIP := range machineIPs { -// for _, dst := range dstMap { -// _, cdr, _ := net.ParseCIDR(dst) -// ip := net.ParseIP(machineIP) -// if dst == machineIP || (cdr != nil && ip != nil && cdr.Contains(ip)) { -// peers[peer.ID] = peer -// -// continue -// } -// } -// } -// } -// } -// } -// -// lock.RUnlock() -// -// authorizedPeers := make(Machines, 0, len(peers)) -// for _, m := range peers { -// authorizedPeers = append(authorizedPeers, m) -// } -// sort.Slice( -// authorizedPeers, -// func(i, j int) bool { return authorizedPeers[i].ID < authorizedPeers[j].ID }, -// ) -// -// log.Trace(). -// Caller(). -// Str("self", machine.Hostname). -// Str("peers", authorizedPeers.String()). -// Msg("Authorized peers") -// -// return authorizedPeers -// } - func (h *Headscale) ListPeers(machine *Machine) (Machines, error) { log.Trace(). Caller().