From 3a7d2998d1249f16c563165da0bb84e53cc49e3f Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Fri, 25 Sep 2026 22:52:43 +0000 Subject: [PATCH] Add Comment to Table and Chain Tables and chains carry userdata since Linux 5.10 (NFTA_TABLE_USERDATA, NFTA_CHAIN_USERDATA), which nft uses for the comment shown by "nft list ruleset". Sets and set elements already expose theirs as Comment, add the same field to Table and Chain, encoded with the libnftnl TLV types NFTNL_UDATA_TABLE_COMMENT and NFTNL_UDATA_CHAIN_COMMENT so nft displays it, and decode it in the list functions. The kernel does not update the userdata of an existing table or chain, so the field documents that changing the comment requires deleting the object. Tests cover the wire format, the round trip through the kernel including the unchanged comment on re-add, and the nft compatibility in both directions: the ruleset nft lists is the same whether created by nft or by the library, and the comments nft writes, which come with more userdata TLVs than the comment, are read back. --- chain.go | 19 +++ integration/nft_test.go | 74 +++++++++++ integration/testdata/add_comments.nft | 10 ++ nftables_test.go | 177 ++++++++++++++++++++++++++ table.go | 16 +++ userdata/userdata.go | 12 ++ 6 files changed, 308 insertions(+) create mode 100644 integration/testdata/add_comments.nft diff --git a/chain.go b/chain.go index a0d18fdf..d3c07aa9 100644 --- a/chain.go +++ b/chain.go @@ -20,10 +20,14 @@ import ( "math" "github.com/google/nftables/binaryutil" + "github.com/google/nftables/userdata" "github.com/mdlayher/netlink" "golang.org/x/sys/unix" ) +// https://github.com/torvalds/linux/blob/cbd2257dc96e3e46217540fcb095a757ffa20d96/include/uapi/linux/netfilter/nf_tables.h#L253 +const nftaChainUserdata = 0x0c + // ChainHook specifies at which step in packet processing the Chain should be // executed. See also // https://wiki.nftables.org/wiki-nftables/index.php/Configuring_chains#Base_chain_hooks @@ -103,6 +107,11 @@ type Chain struct { Type ChainType Policy *ChainPolicy Device string + // Comment is stored in NFTA_CHAIN_USERDATA in the format used by nft, + // so "nft list ruleset" shows it. It requires Linux 5.10 and is ignored + // by older kernels. The kernel does not update the comment of an + // existing chain, changing it requires deleting the chain. + Comment string } // AddChain adds the specified Chain. See also @@ -140,6 +149,12 @@ func (cc *Conn) AddChain(c *Chain) *Chain { {Type: unix.NFTA_CHAIN_TYPE, Data: []byte(c.Type + "\x00")}, })...) } + if c.Comment != "" { + udata := userdata.AppendString(nil, userdata.NFTNL_UDATA_CHAIN_COMMENT, c.Comment) + data = append(data, cc.marshalAttr([]netlink.Attribute{ + {Type: nftaChainUserdata, Data: udata}, + })...) + } cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: nftMsgNewChain.HeaderType(), @@ -314,6 +329,10 @@ func chainFromMsg(msg netlink.Message) (*Chain, error) { c.Hooknum, c.Priority, err = hookFromMsg(b) return err }) + case nftaChainUserdata: + if comment, ok := userdata.GetString(ad.Bytes(), userdata.NFTNL_UDATA_CHAIN_COMMENT); ok { + c.Comment = comment + } } } diff --git a/integration/nft_test.go b/integration/nft_test.go index 14c7d437..76874a97 100644 --- a/integration/nft_test.go +++ b/integration/nft_test.go @@ -85,6 +85,39 @@ func TestNFTables(t *testing.T) { } }, }, + { + name: "AddComments", + scriptPath: "testdata/add_comments.nft", + goCommands: func(t *testing.T, c *nftables.Conn) { + c.FlushRuleset() + + table := c.AddTable(&nftables.Table{ + Name: "test-table", + Family: nftables.TableFamilyINet, + Comment: "table comment", + }) + + c.AddChain(&nftables.Chain{ + Name: "base-chain", + Table: table, + Hooknum: nftables.ChainHookInput, + Priority: nftables.ChainPriorityFilter, + Type: nftables.ChainTypeFilter, + Comment: "base chain comment", + }) + + c.AddChain(&nftables.Chain{ + Name: "regular-chain", + Table: table, + Comment: "regular chain comment", + }) + + err := c.Flush() + if err != nil { + t.Fatalf("Error creating table: %v", err) + } + }, + }, { name: "AddFlowtables", scriptPath: "testdata/add_flowtables.nft", @@ -228,6 +261,47 @@ func TestNFTables(t *testing.T) { } } +// TestListCommentsFromNFT checks that the comments nft stores on tables and +// chains are read back by the Go library. nft adds more userdata than the +// comment, which the library has to skip. +func TestListCommentsFromNFT(t *testing.T) { + c, newNS := nftest.OpenSystemConn(t, *enableSysTests) + defer nftest.CleanupSystemConn(t, newNS) + + if out, err := applyNFTRuleset("testdata/add_comments.nft"); err != nil { + t.Fatalf("Failed to apply nftables script: %v\noutput:%s", err, out) + } + defer func() { + if err := flushNFTRuleset(); err != nil { + t.Fatalf("Failed to flush nftables ruleset: %v", err) + } + }() + + table, err := c.ListTableOfFamily("test-table", nftables.TableFamilyINet) + if err != nil { + t.Fatalf("ListTableOfFamily() failed: %v", err) + } + if got, want := table.Comment, "table comment"; got != want { + t.Errorf("table comment = %q, want %q", got, want) + } + + chains, err := c.ListChainsOfTableFamily(nftables.TableFamilyINet) + if err != nil { + t.Fatalf("ListChainsOfTableFamily() failed: %v", err) + } + got := map[string]string{} + for _, chain := range chains { + got[chain.Name] = chain.Comment + } + want := map[string]string{ + "base-chain": "base chain comment", + "regular-chain": "regular chain comment", + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("chain comments mismatch (-want +got):\n%s", diff) + } +} + func applyNFTRuleset(scriptPath string) (string, error) { cmd := exec.Command("nft", "--debug=all", "-f", scriptPath) out, err := cmd.CombinedOutput() diff --git a/integration/testdata/add_comments.nft b/integration/testdata/add_comments.nft new file mode 100644 index 00000000..0c343fca --- /dev/null +++ b/integration/testdata/add_comments.nft @@ -0,0 +1,10 @@ +table inet test-table { + comment "table comment" + chain base-chain { + comment "base chain comment" + type filter hook input priority filter; policy accept; + } + chain regular-chain { + comment "regular chain comment" + } +} diff --git a/nftables_test.go b/nftables_test.go index 4e915a03..2cbeaee4 100644 --- a/nftables_test.go +++ b/nftables_test.go @@ -5852,6 +5852,183 @@ func TestSetComment(t *testing.T) { } } +func TestTableComment(t *testing.T) { + want := [][]byte{ + // batch begin + []byte("\x00\x00\x00\x0a"), + // nft add table inet filter { comment "test comment" \; } + []byte("\x01\x00\x00\x00\x0b\x00\x01\x00\x66\x69\x6c\x74\x65\x72\x00\x00\x08\x00\x02\x00\x00\x00\x00\x00\x13\x00\x06\x00\x00\x0d\x74\x65\x73\x74\x20\x63\x6f\x6d\x6d\x65\x6e\x74\x00\x00"), + // batch end + []byte("\x00\x00\x00\x0a"), + } + + c, err := nftables.New(expectMessages(t, want)) + if err != nil { + t.Fatal(err) + } + + c.AddTable(&nftables.Table{ + Family: nftables.TableFamilyINet, + Name: "filter", + Comment: "test comment", + }) + + if err := c.Flush(); err != nil { + t.Fatal(err) + } +} + +func TestChainComment(t *testing.T) { + want := [][]byte{ + // batch begin + []byte("\x00\x00\x00\x0a"), + // nft add table inet filter + []byte("\x01\x00\x00\x00\x0b\x00\x01\x00\x66\x69\x6c\x74\x65\x72\x00\x00\x08\x00\x02\x00\x00\x00\x00\x00"), + // nft add chain inet filter base-chain { type filter hook prerouting priority 0 \; comment "test comment" \; } + []byte("\x01\x00\x00\x00\x0b\x00\x01\x00\x66\x69\x6c\x74\x65\x72\x00\x00\x0f\x00\x03\x00\x62\x61\x73\x65\x2d\x63\x68\x61\x69\x6e\x00\x00\x14\x00\x04\x80\x08\x00\x01\x00\x00\x00\x00\x00\x08\x00\x02\x00\x00\x00\x00\x00\x0b\x00\x07\x00\x66\x69\x6c\x74\x65\x72\x00\x00\x13\x00\x0c\x00\x00\x0d\x74\x65\x73\x74\x20\x63\x6f\x6d\x6d\x65\x6e\x74\x00\x00"), + // batch end + []byte("\x00\x00\x00\x0a"), + } + + c, err := nftables.New(expectMessages(t, want)) + if err != nil { + t.Fatal(err) + } + + filter := c.AddTable(&nftables.Table{ + Family: nftables.TableFamilyINet, + Name: "filter", + }) + c.AddChain(&nftables.Chain{ + Name: "base-chain", + Table: filter, + Hooknum: nftables.ChainHookPrerouting, + Priority: nftables.ChainPriorityRef(0), + Type: nftables.ChainTypeFilter, + Comment: "test comment", + }) + + if err := c.Flush(); err != nil { + t.Fatal(err) + } +} + +// TestTableAndChainCommentRoundTrip checks that the comments of tables and +// chains are stored by the kernel and read back by the list functions, and +// that objects without a comment list an empty one. +func TestTableAndChainCommentRoundTrip(t *testing.T) { + conn, newNS := nftest.OpenSystemConn(t, *enableSysTests) + defer nftest.CleanupSystemConn(t, newNS) + conn.FlushRuleset() + defer conn.FlushRuleset() + + commented := conn.AddTable(&nftables.Table{ + Name: "commented", + Family: nftables.TableFamilyINet, + Comment: "table comment", + }) + plain := conn.AddTable(&nftables.Table{ + Name: "plain", + Family: nftables.TableFamilyINet, + }) + conn.AddChain(&nftables.Chain{ + Name: "base", + Table: commented, + Hooknum: nftables.ChainHookInput, + Priority: nftables.ChainPriorityFilter, + Type: nftables.ChainTypeFilter, + Comment: "base chain comment", + }) + conn.AddChain(&nftables.Chain{ + Name: "regular", + Table: commented, + Comment: "regular chain comment", + }) + conn.AddChain(&nftables.Chain{ + Name: "plain", + Table: plain, + }) + if err := conn.Flush(); err != nil { + t.Fatalf("conn.Flush() failed: %v", err) + } + + tables, err := conn.ListTablesOfFamily(nftables.TableFamilyINet) + if err != nil { + t.Fatalf("conn.ListTablesOfFamily() failed: %v", err) + } + gotTables := map[string]string{} + for _, tb := range tables { + gotTables[tb.Name] = tb.Comment + } + wantTables := map[string]string{"commented": "table comment", "plain": ""} + if !reflect.DeepEqual(wantTables, gotTables) { + t.Errorf("table comments = %v, want %v", gotTables, wantTables) + } + + tb, err := conn.ListTableOfFamily("commented", nftables.TableFamilyINet) + if err != nil { + t.Fatalf("conn.ListTableOfFamily() failed: %v", err) + } + if got, want := tb.Comment, "table comment"; got != want { + t.Errorf("ListTableOfFamily() comment = %q, want %q", got, want) + } + + chains, err := conn.ListChainsOfTableFamily(nftables.TableFamilyINet) + if err != nil { + t.Fatalf("conn.ListChainsOfTableFamily() failed: %v", err) + } + gotChains := map[string]string{} + for _, ch := range chains { + gotChains[ch.Table.Name+"/"+ch.Name] = ch.Comment + } + wantChains := map[string]string{ + "commented/base": "base chain comment", + "commented/regular": "regular chain comment", + "plain/plain": "", + } + if !reflect.DeepEqual(wantChains, gotChains) { + t.Errorf("chain comments = %v, want %v", gotChains, wantChains) + } + + ch, err := conn.ListChain(commented, "base") + if err != nil { + t.Fatalf("conn.ListChain() failed: %v", err) + } + if got, want := ch.Comment, "base chain comment"; got != want { + t.Errorf("ListChain() comment = %q, want %q", got, want) + } + + // Adding an existing table or chain again with another comment succeeds + // but the kernel keeps the original comment. + conn.AddTable(&nftables.Table{ + Name: "commented", + Family: nftables.TableFamilyINet, + Comment: "changed", + }) + conn.AddChain(&nftables.Chain{ + Name: "regular", + Table: commented, + Comment: "changed", + }) + if err := conn.Flush(); err != nil { + t.Fatalf("conn.Flush() failed: %v", err) + } + tb, err = conn.ListTableOfFamily("commented", nftables.TableFamilyINet) + if err != nil { + t.Fatalf("conn.ListTableOfFamily() failed: %v", err) + } + if got, want := tb.Comment, "table comment"; got != want { + t.Errorf("table comment after re-adding = %q, want %q", got, want) + } + ch, err = conn.ListChain(commented, "regular") + if err != nil { + t.Fatalf("conn.ListChain() failed: %v", err) + } + if got, want := ch.Comment, "regular chain comment"; got != want { + t.Errorf("chain comment after re-adding = %q, want %q", got, want) + } +} + func TestMasq(t *testing.T) { tests := []struct { name string diff --git a/table.go b/table.go index 3ad89d13..a4fb33c7 100644 --- a/table.go +++ b/table.go @@ -19,6 +19,7 @@ import ( "fmt" "github.com/google/nftables/binaryutil" + "github.com/google/nftables/userdata" "github.com/mdlayher/netlink" "golang.org/x/sys/unix" ) @@ -44,6 +45,8 @@ const ( ) const ( + // https://github.com/torvalds/linux/blob/cbd2257dc96e3e46217540fcb095a757ffa20d96/include/uapi/linux/netfilter/nf_tables.h#L210 + nftaTableUserdata = 0x06 // https://github.com/torvalds/linux/blob/cbd2257dc96e3e46217540fcb095a757ffa20d96/include/uapi/linux/netfilter/nf_tables.h#L211 nftaTableOwner = 0x07 @@ -79,6 +82,11 @@ type Table struct { Flags TableFlags // NFTA_TABLE_FLAGS Owner uint32 // NFTA_TABLE_OWNER Family TableFamily + // Comment is stored in NFTA_TABLE_USERDATA in the format used by nft, + // so "nft list ruleset" shows it. It requires Linux 5.10 and is ignored + // by older kernels. The kernel does not update the comment of an + // existing table, changing it requires deleting the table. + Comment string } // DelTable deletes a specific table, along with all chains/rules it contains. @@ -124,6 +132,10 @@ func (cc *Conn) addTable(t *Table, flag netlink.HeaderFlags) *Table { } attrs = append(attrs, netlink.Attribute{Type: unix.NFTA_TABLE_FLAGS, Data: binaryutil.BigEndian.PutUint32(uint32(t.Flags))}) + if t.Comment != "" { + udata := userdata.AppendString(nil, userdata.NFTNL_UDATA_TABLE_COMMENT, t.Comment) + attrs = append(attrs, netlink.Attribute{Type: nftaTableUserdata, Data: udata}) + } data := cc.marshalAttr(attrs) cc.messages = append(cc.messages, netlinkMessage{ @@ -262,6 +274,10 @@ func tableFromMsg(msg netlink.Message) (*Table, error) { t.Flags = TableFlags(ad.Uint32()) case nftaTableOwner: t.Owner = ad.Uint32() + case nftaTableUserdata: + if comment, ok := userdata.GetString(ad.Bytes(), userdata.NFTNL_UDATA_TABLE_COMMENT); ok { + t.Comment = comment + } } } diff --git a/userdata/userdata.go b/userdata/userdata.go index fbdb0645..3d15f896 100644 --- a/userdata/userdata.go +++ b/userdata/userdata.go @@ -58,6 +58,18 @@ const ( NFTNL_UDATA_SET_ELEM_F_INTERVAL_OPEN uint32 = 0x01 ) +// Table userdata types +// https://git.netfilter.org/libnftnl/tree/include/libnftnl/udata.h +const ( + NFTNL_UDATA_TABLE_COMMENT Type = iota +) + +// Chain userdata types +// https://git.netfilter.org/libnftnl/tree/include/libnftnl/udata.h +const ( + NFTNL_UDATA_CHAIN_COMMENT Type = iota +) + func Append(udata []byte, typ Type, data []byte) []byte { udata = append(udata, byte(typ), byte(len(data))) udata = append(udata, data...)