Allow when user has only a subnet route (#1734)

* Add test because of issue 1604

* Add peer for routes

* Revert previous change to try different way to add peer

* Add traces

* Remove traces

* Make sure tests have IPPrefix comparator

* Get allowedIps before loop

* Remove comment

* Add composite literals :)
This commit is contained in:
DeveloperDragon 2024-02-12 11:44:37 +01:00 committed by GitHub
parent 47405931c6
commit e3553aae50
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 79 additions and 2 deletions

View File

@ -2794,7 +2794,75 @@ func Test_getFilteredByACLPeers(t *testing.T) {
}, },
}, },
}, },
{
name: "subnet-router-with-only-route",
args: args{
nodes: []*types.Node{
{
ID: 1,
IPAddresses: []netip.Addr{netip.MustParseAddr("100.64.0.1")},
Hostname: "user1",
User: types.User{Name: "user1"},
},
{
ID: 2,
IPAddresses: []netip.Addr{netip.MustParseAddr("100.64.0.2")},
Hostname: "router",
User: types.User{Name: "router"},
Routes: types.Routes{
types.Route{
NodeID: 2,
Prefix: types.IPPrefix(netip.MustParsePrefix("10.33.0.0/16")),
IsPrimary: true,
Enabled: true,
},
},
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{
"100.64.0.1/32",
},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.33.0.0/16", Ports: tailcfg.PortRangeAny},
},
},
},
node: &types.Node{
ID: 1,
IPAddresses: []netip.Addr{netip.MustParseAddr("100.64.0.1")},
Hostname: "user1",
User: types.User{Name: "user1"},
},
},
want: []*types.Node{
{
ID: 2,
IPAddresses: []netip.Addr{netip.MustParseAddr("100.64.0.2")},
Hostname: "router",
User: types.User{Name: "router"},
Routes: types.Routes{
types.Route{
NodeID: 2,
Prefix: types.IPPrefix(netip.MustParsePrefix("10.33.0.0/16")),
IsPrimary: true,
Enabled: true,
},
},
},
},
},
} }
// TODO(kradalby): Remove when we have gotten rid of IPPrefix type
prefixComparer := cmp.Comparer(func(x, y types.IPPrefix) bool {
return x == y
})
comparers := append([]cmp.Option{}, util.Comparers...)
comparers = append(comparers, prefixComparer)
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
got := FilterNodesByACL( got := FilterNodesByACL(
@ -2802,7 +2870,7 @@ func Test_getFilteredByACLPeers(t *testing.T) {
tt.args.nodes, tt.args.nodes,
tt.args.rules, tt.args.rules,
) )
if diff := cmp.Diff(tt.want, got, util.Comparers...); diff != "" { if diff := cmp.Diff(tt.want, got, comparers...); diff != "" {
t.Errorf("FilterNodesByACL() unexpected result (-want +got):\n%s", diff) t.Errorf("FilterNodesByACL() unexpected result (-want +got):\n%s", diff)
} }
}) })

View File

@ -208,6 +208,15 @@ func (node *Node) IsEphemeral() bool {
} }
func (node *Node) CanAccess(filter []tailcfg.FilterRule, node2 *Node) bool { func (node *Node) CanAccess(filter []tailcfg.FilterRule, node2 *Node) bool {
allowedIPs := append([]netip.Addr{}, node2.IPAddresses...)
for _, route := range node2.Routes {
if route.Enabled {
allowedIPs = append(allowedIPs, netip.Prefix(route.Prefix).Addr())
}
}
for _, rule := range filter { for _, rule := range filter {
// TODO(kradalby): Cache or pregen this // TODO(kradalby): Cache or pregen this
matcher := matcher.MatchFromFilterRule(rule) matcher := matcher.MatchFromFilterRule(rule)
@ -216,7 +225,7 @@ func (node *Node) CanAccess(filter []tailcfg.FilterRule, node2 *Node) bool {
continue continue
} }
if matcher.DestsContainsIP([]netip.Addr(node2.IPAddresses)) { if matcher.DestsContainsIP(allowedIPs) {
return true return true
} }
} }