Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

firewall ruleset with sequential port list results in blocked port #9009

Closed
Zempashi opened this issue Jul 12, 2024 · 2 comments · Fixed by #9010
Closed

firewall ruleset with sequential port list results in blocked port #9009

Zempashi opened this issue Jul 12, 2024 · 2 comments · Fixed by #9010
Assignees

Comments

@Zempashi
Copy link

Bug Report

Compilation of ruleset is not happening strictly identical as nft tools does it, resulting in broken programmed rules in the kernel.

Description

In our evaluation setup we use a bastion to carry all talosctl command which is in the subnet of the controle-plane nodes. We end up with a firewall rule like this:

apiVersion: v1alpha1
kind: NetworkDefaultActionConfig
ingress: block
---     
apiVersion: v1alpha1
kind: NetworkRuleConfig         
name: node
portSelector:
  ports:
    - 50000 
    - 50001
    - 10250
    - 4240 # cilium                                             
  protocol: tcp                   
ingress:  
  - subnet: <controlplane_subnet>

Applying this ruleset results in blocking the port 50001 (but port 50000, 4240 and 10250 and other firewall rules are applied correctly and working correctly)

Logs

There nothing suspicious looking at the generated ruleset :

table inet talos {                                                                                                                    
        chain ingress {                                                                                                               
                [...]                   
                ip saddr <control_plane_subnet> tcp dport { 4240, 10250, 50000, 50001 } counter packets 25 bytes 1500 accept                  
                [...]
        }                                                                                                                             
}                                                                                                                                     

After calling for help the nftables community (thanks to them btw), looks like the problem comes from rule compilation

$ nft -d netlink list ruleset
[...]
inet talos @__set5
    element 000052c3  : 1 [end]
    element 000051c3  : 0 [end]
    element 000051c3  : 1 [end]
    element 000050c3  : 0 [end]
    element 00000b28  : 1 [end]
    element 00000a28  : 0 [end]
    element 00009110  : 1 [end]
    element 00009010  : 0 [end]
[...]

if we apply ruleset via nft tools

nft list ruleset > rule.set
nft flush ruleset
nft -f rule.set

Now the port 50001 is opened correctly, we can diff the compiled set of ports

inet talos @__set5
    element 000052c3  : 1 [end]
    element 000050c3  : 0 [end]
    element 00000b28  : 1 [end]
    element 00000a28  : 0 [end]
    element 00009110  : 1 [end]
    element 00009010  : 0 [end]

So nft create a config that look like one generated by a range 50000-50001 (and this is the workaround)
even with a port list ruleset

table inet talos {                                                                                                                    
        chain ingress {                                                                                                               
                [...]                   
                ip saddr <control_plane_subnet> tcp dport { 4240, 10250, 50000, 50001 } counter packets 38 bytes 2280 accept                  
                [...]
        }                                                                                                                             
}                                                                                                                                     

This looks like a similar reproduction of:

You may find interesting the discussion linked at the end of the issue:

Environment

  • Talos version: [talosctl version --nodes <problematic nodes>]
Client:                             
        Tag:         v1.7.1         
        SHA:         undefined      
        Built:                      
        Go version:  go1.22.3       
        OS/Arch:     linux/amd64    
Server:                             
        NODE:        10.19.220.1    
        Tag:         v1.7.4         
        SHA:         cb3a8308       
        Built:                      
        Go version:  go1.22.3       
        OS/Arch:     linux/amd64    
        Enabled:     RBAC           
  • Kubernetes version: [kubectl version --short]
Client Version: v1.30.1                                
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.30.1                                
  • Platform:

AMD64 baremetal (metal iso)

@Zempashi Zempashi changed the title firewall ruleset with sequential port list result in blocked port firewall ruleset with sequential port list results in blocked port Jul 12, 2024
@smira
Copy link
Member

smira commented Jul 12, 2024

Thank you for reporting this.

@smira
Copy link
Member

smira commented Jul 12, 2024

I believe the key problem here is the adjacent ports not being merged correctly when building an interval set, which leads to the match failure.

@smira smira self-assigned this Jul 12, 2024
smira added a commit to smira/talos that referenced this issue Aug 5, 2024
Fixes siderolabs#9009

When building a port interval set, sort the ports and merge adjacent
ranges to prevent mismatch on the nftables side.

With address sets, this was already the case due to the way IPRange
builder works, but ports need a manual implementation.

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
(cherry picked from commit f14c479)
smira added a commit to smira/talos that referenced this issue Aug 6, 2024
Fixes siderolabs#9009

When building a port interval set, sort the ports and merge adjacent
ranges to prevent mismatch on the nftables side.

With address sets, this was already the case due to the way IPRange
builder works, but ports need a manual implementation.

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
(cherry picked from commit f14c479)
smira added a commit to smira/talos that referenced this issue Aug 6, 2024
Fixes siderolabs#9009

When building a port interval set, sort the ports and merge adjacent
ranges to prevent mismatch on the nftables side.

With address sets, this was already the case due to the way IPRange
builder works, but ports need a manual implementation.

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
(cherry picked from commit f14c479)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants