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

Set with key type nftables.TypeIFName not working #177

Open
rdmcguire opened this issue Aug 28, 2022 · 1 comment
Open

Set with key type nftables.TypeIFName not working #177

rdmcguire opened this issue Aug 28, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@rdmcguire
Copy link

Creating a set with Type: nftables.TypeIFName seems to work, but the set acts strangely.

Sample code for set:

        conn.AddSet(&nftables.Set{
                Table:   table,
                Name:    "test_set",
                KeyType: nftables.TypeIFName,
        }, nil)

I also created a set called "manual_set" manually, and they both list:

nftables pkg set and nft cli set

% nft 'add set test_table manual_set { type ifname; }'
% nft list table test_table
table ip test_table {
        set test_set {
                type ifname
        }

        set manual_set {
                type ifname
        }
}

I then added an element into each set using nft cli and they both seem to succeed, however the set created by this package shows empty elements.

Added Elements

% nft 'add element test_table test_set { "wg0" }'
% nft 'add element test_table manual_set { "wg0" }'
% nft list table test_table                                                                      ☸ kubernetes-admin@50w_k8s:argocd 
table ip test_table {
        set test_set {
                type ifname
                elements = { "" }
        }

        set manual_set {
                type ifname
                elements = { "wg0" }
        }
}

Additionally, trying to add an element through this package fails for either table, though I do wonder if I've missed something in my code:

Attempt to add set element to both tables

        // github.com/google/nftables set and manual set
        testSet, _ := c.GetSetByName(table, "test_set")
        manualSet, _ := c.GetSetByName(table, "manual_set")

        // Second Element
        elements := []nftables.SetElement{{Key: []byte("wg1")}}

        c.SetAddElements(manualSet, elements)
        c.SetAddElements(testSet, elements)

        if err := c.Flush(); err != nil {
                log.Panicf("Error: %s", err)
        }

Running returns error: "conn.Receive: netlink receive: invalid argument"

I've tried a few different key types and have only had success with TypeFamilyIPv4.

@turekt
Copy link
Contributor

turekt commented Aug 30, 2022

Hi @rdmcguire,

I have checked your issue and it seems that there is a bug in the set marshaling logic in the nftables go lib. If we observe the messages sent by the nft cmdline tool:

[
  // NFTA_SET_TABLE, "filter\x00"
  [{nla_len=11, nla_type=0x1}, "\x66\x69\x6c\x74\x65\x72\x00"], 
  // NFTA_SET_NAME, "test_set\x00"
  [{nla_len=13, nla_type=0x2}, "\x74\x65\x73\x74\x5f\x73\x65\x74\x00"], 
  // NFTA_SET_FLAGS, 0
  [{nla_len=8, nla_type=0x3}, "\x00\x00\x00\x00"], 
  // NFTA_SET_KEY_TYPE, IFName (0x29)
  [{nla_len=8, nla_type=0x4}, "\x00\x00\x00\x29"], 
  // NFTA_SET_KEY_LEN, 16
  [{nla_len=8, nla_type=0x5}, "\x00\x00\x00\x10"], 
  // NFTA_SET_ID, 1
  [{nla_len=8, nla_type=0xa}, "\x00\x00\x00\x01"], 
  // NFTA_SET_USERDATA, "\x00\x04\x01\x00\x00\x00"
  [{nla_len=10, nla_type=0xd}, "\x00\x04\x01\x00\x00\x00"]
]

and compare it with the conn.AddSet call that you've written (the data part):

[
  {Header:{Length:0 Type:unknown(2569) Flags:request|acknowledge|0x400 Sequence:0 PID:0} 
  Data:[
    2 0 0 0 
    // NFTA_SET_TABLE, "filter\x00"
    11 0 1 0 102 105 108 116 101 114 0 0 
    // NFTA_SET_NAME, "test_set\x00"
    13 0 2 0 116 101 115 116 95 115 101 116 0 0 0 0 
    // NFTA_SET_FLAGS, 0
    8 0 3 0 0 0 0 0 
    // NFTA_SET_KEY_TYPE, IFName (0x29 = 41)
    8 0 4 0 0 0 0 41 
    // NFTA_SET_KEY_LEN, 16
    8 0 5 0 0 0 0 16 
    // NFTA_SET_ID, 1
    8 0 10 0 0 0 0 1
    // NFTA_SET_USERDATA is not here
  ]}
...

You immediately see that the NFTA_SET_USER_DATA is missing from the message list.

Digging into nftables source code shows that user data is always loaded with at least one TLV structure: https://git.netfilter.org/nftables/tree/src/mnl.c?id=187c6d01d35722618c2711bbc49262c286472c8f#n1165. The one that is loaded by default contains NFTNL_UDATA_SET_KEYBYTEORDER (definition is at https://git.netfilter.org/libnftnl/tree/include/libnftnl/udata.h?id=212479ad2c9200fa858a37de14a2e5e996f10105#n40) and the structure is described here: https://git.netfilter.org/libnftnl/tree/include/udata.h?id=212479ad2c9200fa858a37de14a2e5e996f10105

The description matches the udata structure we see in the message sent by the nft cmdline tool:

// NFTA_SET_USERDATA, {type: NFTNL_UDATA_SET_KEYBYTEORDER (0x00), len: 4, value: 1}
[{nla_len=10, nla_type=0xd}, "\x00\x04\x01\x00\x00\x00"]

Currently, in nftables go lib, setting set user data is done only in certain cases:

nftables/set.go

Line 542 in ec1e802

if s.Anonymous || s.Constant || s.Interval {

I managed to work around your first issue by adding an else to the existing if that loads user data (#180):

else {
	tableInfo = append(tableInfo,
		netlink.Attribute{Type: unix.NFTA_SET_USERDATA, Data: []byte("\x00\x04\x01\x00\x00\x00")})
}

This fixes the first part of the problem. The second part of your issue started working for me when I have aligned the "wg1" string in the code to a 16 byte value:

elements := []nftables.SetElement{{Key: []byte("wg1\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00")}}

My guess is that this is because the IFName type has the key length type set to 16 when marshaling data to nftables. Remember this one from above?

// NFTA_SET_KEY_LEN, 16
8 0 5 0 0 0 0 16

@stapelberg stapelberg added the bug Something isn't working label Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants