From 0db717e1b97fc521d6638cdeeeca44f2a0cbf038 Mon Sep 17 00:00:00 2001 From: AbdelrahmanElawady Date: Fri, 4 Aug 2023 19:00:57 +0300 Subject: [PATCH 1/2] Add access control to Varnish --- lib/go-atscfg/ipallowdotyaml.go | 320 +++++++++++++++----------- lib/varnishcfg/access_control.go | 100 ++++++++ lib/varnishcfg/access_control_test.go | 233 +++++++++++++++++++ lib/varnishcfg/backends.go | 9 - lib/varnishcfg/vcl.go | 2 +- lib/varnishcfg/vclbuilder.go | 7 + 6 files changed, 529 insertions(+), 142 deletions(-) create mode 100644 lib/varnishcfg/access_control.go create mode 100644 lib/varnishcfg/access_control_test.go diff --git a/lib/go-atscfg/ipallowdotyaml.go b/lib/go-atscfg/ipallowdotyaml.go index 99c5692c1e..ce19421b9c 100644 --- a/lib/go-atscfg/ipallowdotyaml.go +++ b/lib/go-atscfg/ipallowdotyaml.go @@ -20,6 +20,7 @@ package atscfg */ import ( + "errors" "net" "sort" "strconv" @@ -73,62 +74,20 @@ func MakeIPAllowDotYAML( return Cfg{}, makeErr(warnings, "this server missing HostName") } - params := paramsToMultiMap(filterParams(serverParams, IPAllowConfigFileName, "", "", "")) - ipAllowDat := []ipAllowYAMLData{} // localhost is trusted. ipAllowDat = append([]ipAllowYAMLData{yamlAllowAll(`127.0.0.1`)}, ipAllowDat...) ipAllowDat = append([]ipAllowYAMLData{yamlAllowAll(`::1`)}, ipAllowDat...) - // default for coalesce_ipv4 = 24, 5 and for ipv6 48, 5; override with the parameters in the server profile. - coalesceMaskLenV4 := DefaultCoalesceMaskLenV4 - coalesceNumberV4 := DefaultCoalesceNumberV4 - coalesceMaskLenV6 := DefaultCoalesceMaskLenV6 - coalesceNumberV6 := DefaultCoalesceNumberV6 + ips := GetPurgeIPs(serverParams) - for name, vals := range params { - for _, val := range vals { - switch name { - case ParamPurgeAllowIP: - for _, ip := range strings.Split(val, ",") { - ipAllowDat = append(ipAllowDat, yamlAllowAll(strings.TrimSpace(ip))) - } - case ParamCoalesceMaskLenV4: - if vi, err := strconv.Atoi(val); err != nil { - warnings = append(warnings, "got param '"+name+"' val '"+val+"' not a number, ignoring!") - } else if coalesceMaskLenV4 != DefaultCoalesceMaskLenV4 { - warnings = append(warnings, "got multiple param '"+name+"' - ignoring val '"+val+"'!") - } else { - coalesceMaskLenV4 = vi - } - case ParamCoalesceNumberV4: - if vi, err := strconv.Atoi(val); err != nil { - warnings = append(warnings, "got param '"+name+"' val '"+val+"' not a number, ignoring!") - } else if coalesceNumberV4 != DefaultCoalesceNumberV4 { - warnings = append(warnings, "got multiple param '"+name+"' - ignoring val '"+val+"'!") - } else { - coalesceNumberV4 = vi - } - case ParamCoalesceMaskLenV6: - if vi, err := strconv.Atoi(val); err != nil { - warnings = append(warnings, "got param '"+name+"' val '"+val+"' not a number, ignoring!") - } else if coalesceMaskLenV6 != DefaultCoalesceMaskLenV6 { - warnings = append(warnings, "got multiple param '"+name+"' - ignoring val '"+val+"'!") - } else { - coalesceMaskLenV6 = vi - } - case ParamCoalesceNumberV6: - if vi, err := strconv.Atoi(val); err != nil { - warnings = append(warnings, "got param '"+name+"' val '"+val+"' not a number, ignoring!") - } else if coalesceNumberV6 != DefaultCoalesceNumberV6 { - warnings = append(warnings, "got multiple param '"+name+"' - ignoring val '"+val+"'!") - } else { - coalesceNumberV6 = vi - } - } - } + for _, ip := range ips { + ipAllowDat = append(ipAllowDat, yamlAllowAll(strings.TrimSpace(ip))) } + coalesceMaskLenV4, coalesceNumberV4, coalesceMaskLenV6, coalesceNumberV6, ws := GetCoalesceMaskAndNumber(serverParams) + + warnings = append(warnings, ws...) // for edges deny "PUSH|PURGE|DELETE", allow everything else to everyone. isMid := strings.HasPrefix(server.Type, tc.MidTypePrefix) @@ -137,92 +96,22 @@ func MakeIPAllowDotYAML( ipAllowDat = append(ipAllowDat, yamlAllowAllButPushPurgeDelete(`::/0`)) } else { - ips := []*net.IPNet{} - ip6s := []*net.IPNet{} - - cgMap := map[string]tc.CacheGroupNullable{} - for _, cg := range cacheGroups { - if cg.Name == nil { - return Cfg{}, makeErr(warnings, "got cachegroup with nil name!") - } - cgMap[*cg.Name] = cg - } - - if server.Cachegroup == nil { - return Cfg{}, makeErr(warnings, "server had nil Cachegroup!") - } - - serverCG, ok := cgMap[*server.Cachegroup] - if !ok { - return Cfg{}, makeErr(warnings, "server cachegroup not in cachegroups!") - } - - childCGNames := getTopologyDirectChildren(tc.CacheGroupName(*server.Cachegroup), topologies) - - childCGs := map[string]tc.CacheGroupNullable{} - for cgName, _ := range childCGNames { - childCGs[string(cgName)] = cgMap[string(cgName)] - } - - for cgName, cg := range cgMap { - if (cg.ParentName != nil && *cg.ParentName == *serverCG.Name) || (cg.SecondaryParentName != nil && *cg.SecondaryParentName == *serverCG.Name) { - childCGs[cgName] = cg - } - } - - // sort servers, to guarantee things like IP coalescing are deterministic - sort.Sort(serversSortByName(servers)) - for _, childServer := range servers { - if childServer.Cachegroup == nil { - warnings = append(warnings, "Servers had server with nil Cachegroup, skipping!") - continue - } else if childServer.HostName == nil { - warnings = append(warnings, "Servers had server with nil HostName, skipping!") - continue - } - - // We need to add IPs to the allow of - // - all children of this server - // - all monitors, if this server is a Mid - // - _, isChild := childCGs[*childServer.Cachegroup] - if !isChild && !strings.HasPrefix(server.Type, tc.MidTypePrefix) && string(childServer.Type) != tc.MonitorTypeName { - continue - } - - for _, svInterface := range childServer.Interfaces { - for _, svAddr := range svInterface.IPAddresses { - if ip := net.ParseIP(svAddr.Address); ip != nil { - // got an IP - convert it to a CIDR and add it to the list - if ip4 := ip.To4(); ip4 != nil { - ips = append(ips, util.IPToCIDR(ip4)) - } else { - ip6s = append(ip6s, util.IPToCIDR(ip)) - } - } else { - // not an IP, try a CIDR - if ip, cidr, err := net.ParseCIDR(svAddr.Address); err != nil { - // not a CIDR or IP - error out - warnings = append(warnings, "server '"+*server.HostName+"' IP '"+svAddr.Address+" is not an IP address or CIDR - skipping!") - } else if ip == nil { - // not a CIDR or IP - error out - warnings = append(warnings, "server '"+*server.HostName+"' IP '"+svAddr.Address+" failed to parse as IP or CIDR - skipping!") - } else { - // got a valid CIDR - add it to the list - if ip4 := ip.To4(); ip4 != nil { - ips = append(ips, cidr) - } else { - ip6s = append(ip6s, cidr) - } - } - } - } - } + cidrs, cidr6s, ws, err := GetAllowedCIDRsForMid( + server, + servers, + cacheGroups, + topologies, + coalesceNumberV4, + coalesceMaskLenV4, + coalesceNumberV6, + coalesceMaskLenV6, + ) + warnings = append(warnings, ws...) + + if err != nil { + return Cfg{}, makeErr(warnings, err.Error()) } - cidrs := util.CoalesceCIDRs(ips, coalesceNumberV4, coalesceMaskLenV4) - cidr6s := util.CoalesceCIDRs(ip6s, coalesceNumberV6, coalesceMaskLenV6) - for _, cidr := range cidrs { ipAllowDat = append(ipAllowDat, yamlAllowAllButPushPurge(cidr.String())) } @@ -350,3 +239,170 @@ func yamlDenyAll(rangeStr string) ipAllowYAMLData { Methods: []string{YAMLMethodAll}, } } + +// GetPurgeIPs returns IPs allowed for PURGE requests. +func GetPurgeIPs(serverParams []tc.Parameter) []string { + ips := make([]string, 0) + + params := paramsToMultiMap(filterParams(serverParams, IPAllowConfigFileName, "", "", "")) + + for _, val := range params[ParamPurgeAllowIP] { + ips = append(ips, strings.Split(val, ",")...) + } + return ips +} + +// GetCoalesceMaskAndNumber returns coalesce mask length and number for ipv4 and ipv6. +func GetCoalesceMaskAndNumber(serverParams []tc.Parameter) (int, int, int, int, []string) { + warnings := make([]string, 0) + + // default for coalesce_ipv4 = 24, 5 and for ipv6 48, 5; override with the parameters in the server profile. + coalesceMaskLenV4 := DefaultCoalesceMaskLenV4 + coalesceNumberV4 := DefaultCoalesceNumberV4 + coalesceMaskLenV6 := DefaultCoalesceMaskLenV6 + coalesceNumberV6 := DefaultCoalesceNumberV6 + + params := paramsToMultiMap(filterParams(serverParams, IPAllowConfigFileName, "", "", "")) + + for name, vals := range params { + for _, val := range vals { + switch name { + case ParamCoalesceMaskLenV4: + if vi, err := strconv.Atoi(val); err != nil { + warnings = append(warnings, "got param '"+name+"' val '"+val+"' not a number, ignoring!") + } else if coalesceMaskLenV4 != DefaultCoalesceMaskLenV4 { + warnings = append(warnings, "got multiple param '"+name+"' - ignoring val '"+val+"'!") + } else { + coalesceMaskLenV4 = vi + } + case ParamCoalesceNumberV4: + if vi, err := strconv.Atoi(val); err != nil { + warnings = append(warnings, "got param '"+name+"' val '"+val+"' not a number, ignoring!") + } else if coalesceNumberV4 != DefaultCoalesceNumberV4 { + warnings = append(warnings, "got multiple param '"+name+"' - ignoring val '"+val+"'!") + } else { + coalesceNumberV4 = vi + } + case ParamCoalesceMaskLenV6: + if vi, err := strconv.Atoi(val); err != nil { + warnings = append(warnings, "got param '"+name+"' val '"+val+"' not a number, ignoring!") + } else if coalesceMaskLenV6 != DefaultCoalesceMaskLenV6 { + warnings = append(warnings, "got multiple param '"+name+"' - ignoring val '"+val+"'!") + } else { + coalesceMaskLenV6 = vi + } + case ParamCoalesceNumberV6: + if vi, err := strconv.Atoi(val); err != nil { + warnings = append(warnings, "got param '"+name+"' val '"+val+"' not a number, ignoring!") + } else if coalesceNumberV6 != DefaultCoalesceNumberV6 { + warnings = append(warnings, "got multiple param '"+name+"' - ignoring val '"+val+"'!") + } else { + coalesceNumberV6 = vi + } + } + } + } + return coalesceMaskLenV4, coalesceNumberV4, coalesceMaskLenV6, coalesceNumberV6, nil +} + +// GetAllowedCIDRsForMid returns CIDRs allowed for all methods other than Push and Purge to mid servers. +func GetAllowedCIDRsForMid( + server *Server, + servers []Server, + cacheGroups []tc.CacheGroupNullable, + topologies []tc.Topology, + coalesceNumberV4 int, + coalesceMaskLenV4 int, + coalesceNumberV6 int, + coalesceMaskLenV6 int, +) ([]*net.IPNet, []*net.IPNet, []string, error) { + + ips := []*net.IPNet{} + ip6s := []*net.IPNet{} + warnings := make([]string, 0) + + cgMap := map[string]tc.CacheGroupNullable{} + for _, cg := range cacheGroups { + if cg.Name == nil { + return nil, nil, warnings, errors.New("got cachegroup with nil name!") + } + cgMap[*cg.Name] = cg + } + + if server.Cachegroup == nil { + return nil, nil, warnings, errors.New("server had nil Cachegroup!") + } + + serverCG, ok := cgMap[*server.Cachegroup] + if !ok { + return nil, nil, warnings, errors.New("server cachegroup not in cachegroups!") + } + + childCGNames := getTopologyDirectChildren(tc.CacheGroupName(*server.Cachegroup), topologies) + + childCGs := map[string]tc.CacheGroupNullable{} + for cgName, _ := range childCGNames { + childCGs[string(cgName)] = cgMap[string(cgName)] + } + + for cgName, cg := range cgMap { + if (cg.ParentName != nil && *cg.ParentName == *serverCG.Name) || (cg.SecondaryParentName != nil && *cg.SecondaryParentName == *serverCG.Name) { + childCGs[cgName] = cg + } + } + + // sort servers, to guarantee things like IP coalescing are deterministic + sort.Sort(serversSortByName(servers)) + for _, childServer := range servers { + if childServer.Cachegroup == nil { + warnings = append(warnings, "Servers had server with nil Cachegroup, skipping!") + continue + } else if childServer.HostName == nil { + warnings = append(warnings, "Servers had server with nil HostName, skipping!") + continue + } + + // We need to add IPs to the allow of + // - all children of this server + // - all monitors, if this server is a Mid + // + _, isChild := childCGs[*childServer.Cachegroup] + if !isChild && !strings.HasPrefix(server.Type, tc.MidTypePrefix) && string(childServer.Type) != tc.MonitorTypeName { + continue + } + + for _, svInterface := range childServer.Interfaces { + for _, svAddr := range svInterface.IPAddresses { + if ip := net.ParseIP(svAddr.Address); ip != nil { + // got an IP - convert it to a CIDR and add it to the list + if ip4 := ip.To4(); ip4 != nil { + ips = append(ips, util.IPToCIDR(ip4)) + } else { + ip6s = append(ip6s, util.IPToCIDR(ip)) + } + } else { + // not an IP, try a CIDR + if ip, cidr, err := net.ParseCIDR(svAddr.Address); err != nil { + // not a CIDR or IP - error out + warnings = append(warnings, "server '"+*server.HostName+"' IP '"+svAddr.Address+" is not an IP address or CIDR - skipping!") + } else if ip == nil { + // not a CIDR or IP - error out + warnings = append(warnings, "server '"+*server.HostName+"' IP '"+svAddr.Address+" failed to parse as IP or CIDR - skipping!") + } else { + // got a valid CIDR - add it to the list + if ip4 := ip.To4(); ip4 != nil { + ips = append(ips, cidr) + } else { + ip6s = append(ip6s, cidr) + } + } + } + } + } + } + + cidrs := util.CoalesceCIDRs(ips, coalesceNumberV4, coalesceMaskLenV4) + cidr6s := util.CoalesceCIDRs(ip6s, coalesceNumberV6, coalesceMaskLenV6) + + return cidrs, cidr6s, warnings, nil +} diff --git a/lib/varnishcfg/access_control.go b/lib/varnishcfg/access_control.go new file mode 100644 index 0000000000..7e67252d87 --- /dev/null +++ b/lib/varnishcfg/access_control.go @@ -0,0 +1,100 @@ +package varnishcfg + +import ( + "fmt" + "net" + "strings" + + "github.com/apache/trafficcontrol/lib/go-atscfg" + "github.com/apache/trafficcontrol/lib/go-tc" +) + +func (v VCLBuilder) configureAccessControl(vclFile *vclFile) ([]string, error) { + warnings := make([]string, 0) + allowAllIPs := []string{ + `"127.0.0.1"`, + `"::1"`, + } + + purgeIPs := atscfg.GetPurgeIPs(v.toData.ServerParams) + for _, ip := range purgeIPs { + if parts := strings.Split(ip, `/`); len(parts) == 2 { + allowAllIPs = append(allowAllIPs, fmt.Sprintf(`"%s"/%s`, parts[0], parts[1])) + continue + } + allowAllIPs = append(allowAllIPs, fmt.Sprintf(`"%s"`, ip)) + } + + if v.toData.Server.Type == tc.CacheTypeEdge.String() { + configureAccessControlForEdge(vclFile.acls, vclFile.subroutines, allowAllIPs) + return warnings, nil + } + if v.toData.Server.Type == tc.CacheTypeMid.String() { + + coalesceMaskLenV4, coalesceNumberV4, coalesceMaskLenV6, coalesceNumberV6, ws := atscfg.GetCoalesceMaskAndNumber(v.toData.ServerParams) + + warnings = append(warnings, ws...) + cidrs, cidr6s, ws, err := atscfg.GetAllowedCIDRsForMid( + v.toData.Server, + v.toData.Servers, + v.toData.CacheGroups, + v.toData.Topologies, + coalesceNumberV4, + coalesceMaskLenV4, + coalesceNumberV6, + coalesceMaskLenV6, + ) + warnings = append(warnings, ws...) + if err != nil { + return warnings, err + } + allowAllButPushPurge := cidrsToVarnishCIDRs(append(cidrs, cidr6s...)) + allowAllButPushPurge = append(allowAllButPushPurge, `"10.0.0.0"/8`, `"172.16.0.0"/12`, `"192.168.0.0"/16`) + + configureAccessControlForMid(vclFile.acls, vclFile.subroutines, allowAllIPs, allowAllButPushPurge) + } + return warnings, nil +} + +// cidrsToVarnishCIDRs converts CIDRs from the format IP/mask to "IP"/mask +func cidrsToVarnishCIDRs(cidrs []*net.IPNet) []string { + varnishCIDRs := make([]string, 0) + for _, cidr := range cidrs { + cidrParts := strings.Split(cidr.String(), "/") + varnishCIDR := fmt.Sprintf(`"%s"/%s`, cidrParts[0], cidrParts[1]) + varnishCIDRs = append(varnishCIDRs, varnishCIDR) + } + return varnishCIDRs +} + +func configureAccessControlForEdge(acls, subroutines map[string][]string, allowAllIPs []string) { + acls["allow_all"] = append(acls["allow_all"], allowAllIPs...) + + subroutines["vcl_recv"] = append(subroutines["vcl_recv"], []string{ + `if ((req.method == "PUSH" || req.method == "PURGE" || req.method == "DELETE") && !client.ip ~ allow_all) {`, + ` return (synth(405));`, + `}`, + `if (req.method == "PURGE") {`, + ` return (purge);`, + `}`, + }...) +} + +func configureAccessControlForMid(acls, subroutines map[string][]string, allowAllIPs, allowAllButPushPurge []string) { + acls["allow_all"] = append(acls["allow_all"], allowAllIPs...) + acls["allow_all_but_push_purge"] = append(acls["allow_all_but_push_purge"], allowAllButPushPurge...) + + subroutines["vcl_recv"] = append(subroutines["vcl_recv"], []string{ + // push and purge are not allowed except for allow_all acl + `if ((req.method == "PUSH" || req.method == "PURGE") && client.ip ~ allow_all_but_push_purge) {`, + ` return (synth(405));`, + `}`, + // mid cache only accepts requests from from allowed IPs + `if (!client.ip ~ allow_all_but_push_purge && !client.ip ~ allow_all) {`, + ` return (synth(405));`, + `}`, + `if (req.method == "PURGE") {`, + ` return (purge);`, + `}`, + }...) +} diff --git a/lib/varnishcfg/access_control_test.go b/lib/varnishcfg/access_control_test.go new file mode 100644 index 0000000000..f8a3de3e48 --- /dev/null +++ b/lib/varnishcfg/access_control_test.go @@ -0,0 +1,233 @@ +package varnishcfg + +import ( + "net" + "reflect" + "testing" + + "github.com/apache/trafficcontrol/cache-config/t3cutil" + "github.com/apache/trafficcontrol/lib/go-atscfg" + "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/lib/go-util" +) + +func TestConfigureAccessControl(t *testing.T) { + t.Run("edge server", func(t *testing.T) { + vb := NewVCLBuilder(&t3cutil.ConfigData{ + Server: &atscfg.Server{Type: "EDGE"}, + ServerParams: []tc.Parameter{ + { + ConfigFile: "ip_allow.config", + Name: "purge_allow_ip", + Value: "1.1.1.1,2.2.2.2,3.3.3.3/16", + }, + { + ConfigFile: "other_file", + Name: "not relevant", + }, + }, + }) + vclFile := newVCLFile(defaultVCLVersion) + vb.configureAccessControl(&vclFile) + expectedVCLFile := newVCLFile(defaultVCLVersion) + expectedVCLFile.acls["allow_all"] = []string{ + `"127.0.0.1"`, + `"::1"`, + `"1.1.1.1"`, + `"2.2.2.2"`, + `"3.3.3.3"/16`, + } + expectedVCLFile.subroutines["vcl_recv"] = []string{ + `if ((req.method == "PUSH" || req.method == "PURGE" || req.method == "DELETE") && !client.ip ~ allow_all) {`, + ` return (synth(405));`, + `}`, + `if (req.method == "PURGE") {`, + ` return (purge);`, + `}`, + } + if !reflect.DeepEqual(vclFile, expectedVCLFile) { + t.Errorf("got %v want %v", vclFile, expectedVCLFile) + } + }) + t.Run("mid server", func(t *testing.T) { + vb := NewVCLBuilder(&t3cutil.ConfigData{ + Server: &atscfg.Server{ + Type: "MID", + HostName: util.StrPtr("server0"), + Cachegroup: util.StrPtr("cg0"), + }, + ServerParams: []tc.Parameter{ + { + ConfigFile: "ip_allow.config", + Name: atscfg.ParamPurgeAllowIP, + Value: "1.1.1.1,2.2.2.2", + }, + }, + CacheGroups: []tc.CacheGroupNullable{ + {Name: util.StrPtr("cg0")}, + }, + Servers: []atscfg.Server{ + { + HostName: util.StrPtr("child0"), + Cachegroup: util.StrPtr("childcg"), + Type: tc.MonitorTypeName, + Interfaces: []tc.ServerInterfaceInfoV40{ + { + ServerInterfaceInfo: tc.ServerInterfaceInfo{ + Name: "eth0", + IPAddresses: []tc.ServerIPAddress{{Address: "1.1.1.1"}}, + }, + }, + }, + }, + { + HostName: util.StrPtr("child1"), + Cachegroup: util.StrPtr("childcg"), + Type: tc.MonitorTypeName, + Interfaces: []tc.ServerInterfaceInfoV40{ + { + ServerInterfaceInfo: tc.ServerInterfaceInfo{ + Name: "eth0", + IPAddresses: []tc.ServerIPAddress{{Address: "2001:DB8:2::2/64"}}, + }, + }, + }, + }, + }, + }) + vclFile := newVCLFile(defaultVCLVersion) + warnings, err := vb.configureAccessControl(&vclFile) + if len(warnings) > 0 { + t.Errorf("got warnings %v", warnings) + } + if err != nil { + t.Errorf("got error while configuring acl: %s", err) + } + expectedVCLFile := newVCLFile(defaultVCLVersion) + expectedVCLFile.acls["allow_all"] = []string{ + `"127.0.0.1"`, + `"::1"`, + `"1.1.1.1"`, + `"2.2.2.2"`, + } + expectedVCLFile.acls["allow_all_but_push_purge"] = []string{ + `"1.1.1.1"/32`, + `"2001:db8:2::"/64`, + `"10.0.0.0"/8`, + `"172.16.0.0"/12`, + `"192.168.0.0"/16`, + } + expectedVCLFile.subroutines["vcl_recv"] = []string{ + `if ((req.method == "PUSH" || req.method == "PURGE") && client.ip ~ allow_all_but_push_purge) {`, + ` return (synth(405));`, + `}`, + `if (!client.ip ~ allow_all_but_push_purge && !client.ip ~ allow_all) {`, + ` return (synth(405));`, + `}`, + `if (req.method == "PURGE") {`, + ` return (purge);`, + `}`, + } + if !reflect.DeepEqual(vclFile, expectedVCLFile) { + t.Errorf("got %v want %v", vclFile, expectedVCLFile) + } + }) +} + +func TestCIDRsToVarnishCIDRs(t *testing.T) { + cidrs := make([]*net.IPNet, 0) + cidrsString := []string{ + "1.0.0.0/10", + "192.168.1.0/24", + "2002:0:0:1234::/64", + } + for _, cidrString := range cidrsString { + _, cidr, err := net.ParseCIDR(cidrString) + if err != nil { + t.Errorf("failed to parse cidr %s", cidrString) + } + + cidrs = append(cidrs, cidr) + } + gotCIDRs := cidrsToVarnishCIDRs(cidrs) + expectedCIDRs := []string{ + `"1.0.0.0"/10`, + `"192.168.1.0"/24`, + `"2002:0:0:1234::"/64`, + } + if !reflect.DeepEqual(gotCIDRs, expectedCIDRs) { + t.Errorf("got %v want %v", gotCIDRs, expectedCIDRs) + } +} + +func TestConfigureAccessControlForEdge(t *testing.T) { + acls := make(map[string][]string) + subroutines := make(map[string][]string) + allowAllIPs := []string{ + "1.1.1.1", + "2.2.2.2", + } + configureAccessControlForEdge(acls, subroutines, allowAllIPs) + + expectedACLs := map[string][]string{ + "allow_all": {"1.1.1.1", "2.2.2.2"}, + } + expectedSubroutines := map[string][]string{ + "vcl_recv": { + `if ((req.method == "PUSH" || req.method == "PURGE" || req.method == "DELETE") && !client.ip ~ allow_all) {`, + ` return (synth(405));`, + `}`, + `if (req.method == "PURGE") {`, + ` return (purge);`, + `}`, + }, + } + + if !reflect.DeepEqual(acls, expectedACLs) { + t.Errorf("got %v want %v", acls, expectedACLs) + } + if !reflect.DeepEqual(subroutines, expectedSubroutines) { + t.Errorf("got %v want %v", subroutines, expectedSubroutines) + } + +} +func TestConfigureAccessControlForMid(t *testing.T) { + acls := make(map[string][]string) + subroutines := make(map[string][]string) + allowAllIPs := []string{ + "1.1.1.1", + "2.2.2.2", + } + allowAllButPushPurge := []string{ + "3.3.3.3", + "4.4.4.4", + } + + configureAccessControlForMid(acls, subroutines, allowAllIPs, allowAllButPushPurge) + + expectedACLs := map[string][]string{ + "allow_all": {"1.1.1.1", "2.2.2.2"}, + "allow_all_but_push_purge": {"3.3.3.3", "4.4.4.4"}, + } + expectedSubroutines := map[string][]string{ + "vcl_recv": { + `if ((req.method == "PUSH" || req.method == "PURGE") && client.ip ~ allow_all_but_push_purge) {`, + ` return (synth(405));`, + `}`, + `if (!client.ip ~ allow_all_but_push_purge && !client.ip ~ allow_all) {`, + ` return (synth(405));`, + `}`, + `if (req.method == "PURGE") {`, + ` return (purge);`, + `}`, + }, + } + + if !reflect.DeepEqual(acls, expectedACLs) { + t.Errorf("got %v want %v", acls, expectedACLs) + } + if !reflect.DeepEqual(subroutines, expectedSubroutines) { + t.Errorf("got %v want %v", subroutines, expectedSubroutines) + } + +} diff --git a/lib/varnishcfg/backends.go b/lib/varnishcfg/backends.go index 7ff67796eb..90e760fdc3 100644 --- a/lib/varnishcfg/backends.go +++ b/lib/varnishcfg/backends.go @@ -86,17 +86,11 @@ func assignBackends(subroutines map[string][]string, svc *atscfg.ParentAbstracti lines = append(lines, "}") - if _, ok := subroutines["vcl_recv"]; !ok { - subroutines["vcl_recv"] = make([]string, 0) - } subroutines["vcl_recv"] = append(subroutines["vcl_recv"], lines...) if len(hostHeaderLines) == 0 { return } - if _, ok := subroutines["vcl_backend_fetch"]; !ok { - subroutines["vcl_backend_fetch"] = make([]string, 0) - } subroutines["vcl_backend_fetch"] = append(subroutines["vcl_backend_fetch"], hostHeaderLines...) } @@ -138,9 +132,6 @@ func addDirectors(subroutines map[string][]string, svc *atscfg.ParentAbstraction lines = append(lines, fallbackDirectorLines...) - if _, ok := subroutines["vcl_init"]; !ok { - subroutines["vcl_init"] = make([]string, 0) - } subroutines["vcl_init"] = append(subroutines["vcl_init"], lines...) } diff --git a/lib/varnishcfg/vcl.go b/lib/varnishcfg/vcl.go index 5608b21ff8..7054ad5c3b 100644 --- a/lib/varnishcfg/vcl.go +++ b/lib/varnishcfg/vcl.go @@ -61,7 +61,7 @@ func (v vclFile) String() string { for name, acl := range v.acls { txt += fmt.Sprintf("acl %s {\n", name) for _, entry := range acl { - txt += fmt.Sprintf("\t%s\n", entry) + txt += fmt.Sprintf("\t%s;\n", entry) } txt += fmt.Sprint("}\n") } diff --git a/lib/varnishcfg/vclbuilder.go b/lib/varnishcfg/vclbuilder.go index a40221c87f..70c89de9c7 100644 --- a/lib/varnishcfg/vclbuilder.go +++ b/lib/varnishcfg/vclbuilder.go @@ -47,6 +47,13 @@ func (vb *VCLBuilder) BuildVCLFile() (string, []string, error) { warnings := make([]string, 0) v := newVCLFile(defaultVCLVersion) + // access control should be added first to ensure no request processed if it is not allowed + aclWarnings, err := vb.configureAccessControl(&v) + warnings = append(warnings, aclWarnings...) + if err != nil { + return "", nil, fmt.Errorf("(warnings: %s) %w", strings.Join(warnings, ", "), err) + } + atsMajorVersion := uint(9) parents, dataWarns, err := atscfg.MakeParentDotConfigData( From ec6460895d0ddda0ab16efe64a9c9da4994d9dd4 Mon Sep 17 00:00:00 2001 From: AbdelrahmanElawady Date: Wed, 16 Aug 2023 16:52:46 +0300 Subject: [PATCH 2/2] Add license --- lib/varnishcfg/access_control.go | 19 +++++++++++++++++++ lib/varnishcfg/access_control_test.go | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/lib/varnishcfg/access_control.go b/lib/varnishcfg/access_control.go index 7e67252d87..204a83fce4 100644 --- a/lib/varnishcfg/access_control.go +++ b/lib/varnishcfg/access_control.go @@ -1,5 +1,24 @@ package varnishcfg +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + import ( "fmt" "net" diff --git a/lib/varnishcfg/access_control_test.go b/lib/varnishcfg/access_control_test.go index f8a3de3e48..d0ed33f720 100644 --- a/lib/varnishcfg/access_control_test.go +++ b/lib/varnishcfg/access_control_test.go @@ -1,5 +1,24 @@ package varnishcfg +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + import ( "net" "reflect"