From 295e413abc6f5a9fb8d9b4b2e556cbd9f71bcede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Sch=C3=A4r?= Date: Mon, 24 Mar 2025 10:25:46 +0100 Subject: [PATCH 1/2] Receive replies in Flush Commit 0d9bfa4d18da added code to handle "overrun", but the commit is very misleading. NLMSG_OVERRUN is in fact not a flag, but a complete message type, so the (re&netlink.Overrun) masking makes no sense. Even better, NLMSG_OVERRUN is never actually used by Linux. The actual bug which the commit was attempting to fix is that Flush was not receiving replies which the kernel sent for messages with the echo flag. This change reverts that commit and instead adds code in Flush to receive the replies. I updated tests which simulate the kernel to generate replies. --- conn.go | 66 ++- internal/nftest/nftest.go | 17 +- nftables_test.go | 908 +++----------------------------------- rule.go | 10 +- 4 files changed, 133 insertions(+), 868 deletions(-) diff --git a/conn.go b/conn.go index d4759b1..99baa2d 100644 --- a/conn.go +++ b/conn.go @@ -171,24 +171,6 @@ func receiveAckAware(nlconn *netlink.Conn, sentMsgFlags netlink.HeaderFlags) ([] return reply, nil } - if len(reply) != 0 { - last := reply[len(reply)-1] - for re := last.Header.Type; (re&netlink.Overrun) == netlink.Overrun && (re&netlink.Done) != netlink.Done; re = last.Header.Type { - // we are not finished, the message is overrun - r, err := nlconn.Receive() - if err != nil { - return nil, err - } - reply = append(reply, r...) - last = reply[len(reply)-1] - } - - if last.Header.Type == netlink.Error && binaryutil.BigEndian.Uint32(last.Data[:4]) == 0 { - // we have already collected an ack - return reply, nil - } - } - // Now we expect an ack ack, err := nlconn.Receive() if err != nil { @@ -196,8 +178,7 @@ func receiveAckAware(nlconn *netlink.Conn, sentMsgFlags netlink.HeaderFlags) ([] } if len(ack) == 0 { - // received an empty ack? - return reply, nil + return nil, errors.New("received an empty ack") } msg := ack[0] @@ -263,15 +244,49 @@ func (cc *Conn) Flush() error { } defer func() { _ = closer() }() - if _, err := conn.SendMessages(batch(cc.messages)); err != nil { + messages, err := conn.SendMessages(batch(cc.messages)) + if err != nil { return fmt.Errorf("SendMessages: %w", err) } var errs error + + // Fetch replies. Each message with the Echo flag triggers a reply of the same + // type. Additionally, if the first message of the batch has the Echo flag, we + // get a reply of type NFT_MSG_NEWGEN, which we ignore. + replyIndex := 0 + for replyIndex < len(cc.messages) && cc.messages[replyIndex].Header.Flags&netlink.Echo == 0 { + replyIndex++ + } + replies, err := conn.Receive() + for err == nil && len(replies) != 0 { + reply := replies[0] + if reply.Header.Type == netlink.Error && reply.Header.Sequence == messages[1].Header.Sequence { + // The next message is the acknowledgement for the first message in the + // batch; stop looking for replies. + break + } else if replyIndex < len(cc.messages) { + msg := messages[replyIndex+1] + if msg.Header.Sequence == reply.Header.Sequence && msg.Header.Type == reply.Header.Type { + replyIndex++ + for replyIndex < len(cc.messages) && cc.messages[replyIndex].Header.Flags&netlink.Echo == 0 { + replyIndex++ + } + } + } + replies = replies[1:] + if len(replies) == 0 { + replies, err = conn.Receive() + } + } + // Fetch the requested acknowledgement for each message we sent. - for _, msg := range cc.messages { - if _, err := receiveAckAware(conn, msg.Header.Flags); err != nil { - if errors.Is(err, os.ErrPermission) || errors.Is(err, syscall.ENOBUFS) { + for i := range cc.messages { + if i != 0 { + _, err = conn.Receive() + } + if err != nil { + if errors.Is(err, os.ErrPermission) || errors.Is(err, syscall.ENOBUFS) || errors.Is(err, syscall.ENOMEM) { // Kernel will only send one error to user space. return err } @@ -282,6 +297,9 @@ func (cc *Conn) Flush() error { if errs != nil { return fmt.Errorf("conn.Receive: %w", errs) } + if replyIndex < len(cc.messages) { + return fmt.Errorf("missing reply for message %d in batch", replyIndex) + } return nil } diff --git a/internal/nftest/nftest.go b/internal/nftest/nftest.go index 2709fa7..8d5b496 100644 --- a/internal/nftest/nftest.go +++ b/internal/nftest/nftest.go @@ -25,10 +25,21 @@ func (r *Recorder) Conn() (*nftables.Conn, error) { func(req []netlink.Message) ([]netlink.Message, error) { r.requests = append(r.requests, req...) - acks := make([]netlink.Message, 0, len(req)) + replies := make([]netlink.Message, 0, len(req)) + // Generate replies. + for _, msg := range req { + if msg.Header.Flags&netlink.Echo != 0 { + data := append([]byte{}, msg.Data...) + replies = append(replies, netlink.Message{ + Header: msg.Header, + Data: data, + }) + } + } + // Generate acknowledgements. for _, msg := range req { if msg.Header.Flags&netlink.Acknowledge != 0 { - acks = append(acks, netlink.Message{ + replies = append(replies, netlink.Message{ Header: netlink.Header{ Length: 4, Type: netlink.Error, @@ -39,7 +50,7 @@ func (r *Recorder) Conn() (*nftables.Conn, error) { }) } } - return acks, nil + return replies, nil })) } diff --git a/nftables_test.go b/nftables_test.go index 69855b6..6e9ecfe 100644 --- a/nftables_test.go +++ b/nftables_test.go @@ -79,6 +79,40 @@ func linediff(a, b string) string { return buf.String() } +func expectMessages(t *testing.T, want [][]byte) nftables.ConnOption { + return nftables.WithTestDial(func(req []netlink.Message) ([]netlink.Message, error) { + var replies []netlink.Message + for idx, msg := range req { + b, err := msg.MarshalBinary() + if err != nil { + t.Fatal(err) + } + if len(b) < 16 { + continue + } + b = b[16:] + if len(want) == 0 { + t.Errorf("no want entry for message %d: %x", idx, b) + continue + } + if got, want := b, want[0]; !bytes.Equal(got, want) { + t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) + } + want = want[1:] + + // Generate replies. + if msg.Header.Flags&netlink.Echo != 0 { + data := append([]byte{}, msg.Data...) + replies = append(replies, netlink.Message{ + Header: msg.Header, + Data: data, + }) + } + } + return replies, nil + }) +} + func ifname(n string) []byte { b := make([]byte, 16) copy(b, []byte(n+"\x00")) @@ -370,28 +404,7 @@ func TestConfigureNAT(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -638,28 +651,7 @@ func TestConfigureNATSourceAddress(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -1106,28 +1098,7 @@ func TestAddCounter(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -1172,28 +1143,7 @@ func TestDeleteCounter(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -1225,28 +1175,7 @@ func TestDelRule(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -1272,28 +1201,7 @@ func TestLog(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -1324,28 +1232,7 @@ func TestTProxy(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -1389,28 +1276,7 @@ func TestTProxyWithAddrField(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -1457,28 +1323,7 @@ func TestCt(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -1518,28 +1363,7 @@ func TestSecMarkMarshaling(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - conn, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + conn, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -1922,28 +1746,7 @@ func TestCtSet(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -1982,28 +1785,7 @@ func TestCtStat(t *testing.T) { // batch end []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -2042,28 +1824,7 @@ func TestAddRuleWithPosition(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -2472,28 +2233,7 @@ func TestAddChain(t *testing.T) { } for _, tt := range tests { - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(tt.want[idx]) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - got := b - if !bytes.Equal(got, tt.want[idx]) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(tt.want[idx]))) - } - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, tt.want)) if err != nil { t.Fatal(err) } @@ -2551,28 +2291,7 @@ func TestDelChain(t *testing.T) { } for _, tt := range tests { - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(tt.want[idx]) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - got := b - if !bytes.Equal(got, tt.want[idx]) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(tt.want[idx]))) - } - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, tt.want)) if err != nil { t.Fatal(err) } @@ -3284,28 +3003,7 @@ func TestConfigureClamping(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -3419,28 +3117,7 @@ func TestMatchPacketHeader(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -3549,28 +3226,7 @@ func TestDropVerdict(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -3651,28 +3307,7 @@ func TestCreateUseAnonymousSet(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -5431,28 +5066,7 @@ func TestConfigureNATRedirect(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -5538,28 +5152,7 @@ func TestConfigureJumpVerdict(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -5646,28 +5239,7 @@ func TestConfigureReturnVerdict(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -5733,28 +5305,7 @@ func TestConfigureRangePort(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -5833,28 +5384,7 @@ func TestConfigureRangeIPv4(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -5925,28 +5455,7 @@ func TestConfigureRangeIPv6(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -6039,28 +5548,7 @@ func TestSet4(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -6144,28 +5632,7 @@ func TestSetComment(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -6289,28 +5756,7 @@ func TestMasq(t *testing.T) { } for _, tt := range tests { - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(tt.want[idx]) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - got := b - if !bytes.Equal(got, tt.want[idx]) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(tt.want[idx]))) - } - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, tt.want)) if err != nil { t.Fatal(err) } @@ -6422,28 +5868,7 @@ func TestReject(t *testing.T) { } for _, tt := range tests { - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(tt.want[idx]) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - got := b - if !bytes.Equal(got, tt.want[idx]) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(tt.want[idx]))) - } - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, tt.want)) if err != nil { t.Fatal(err) } @@ -6552,28 +5977,7 @@ func TestFib(t *testing.T) { } for _, tt := range tests { - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(tt.want[idx]) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - got := b - if !bytes.Equal(got, tt.want[idx]) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(tt.want[idx]))) - } - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, tt.want)) if err != nil { t.Fatal(err) } @@ -6713,28 +6117,7 @@ func TestNumgen(t *testing.T) { } for _, tt := range tests { - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(tt.want[idx]) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - got := b - if !bytes.Equal(got, tt.want[idx]) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(tt.want[idx]))) - } - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, tt.want)) if err != nil { t.Fatal(err) } @@ -6800,28 +6183,7 @@ func TestMap(t *testing.T) { } for _, tt := range tests { - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(tt.want[idx]) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - got := b - if !bytes.Equal(got, tt.want[idx]) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(tt.want[idx]))) - } - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, tt.want)) if err != nil { t.Fatal(err) } @@ -6920,28 +6282,7 @@ func TestVmap(t *testing.T) { } for _, tt := range tests { - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(tt.want[idx]) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - got := b - if !bytes.Equal(got, tt.want[idx]) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(tt.want[idx]))) - } - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, tt.want)) if err != nil { t.Fatal(err) } @@ -6982,28 +6323,7 @@ func TestJHash(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -7084,28 +6404,7 @@ func TestDup(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -7184,28 +6483,7 @@ func TestDupWoDev(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -7311,28 +6589,7 @@ func TestQuota(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want[idx]) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - got := b - if !bytes.Equal(got, want[idx]) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want[idx]))) - } - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } @@ -7388,28 +6645,7 @@ func TestStatelessNAT(t *testing.T) { []byte("\x00\x00\x00\x0a"), } - c, err := nftables.New(nftables.WithTestDial( - func(req []netlink.Message) ([]netlink.Message, error) { - for idx, msg := range req { - b, err := msg.MarshalBinary() - if err != nil { - t.Fatal(err) - } - if len(b) < 16 { - continue - } - b = b[16:] - if len(want) == 0 { - t.Errorf("no want entry for message %d: %x", idx, b) - continue - } - if got, want := b, want[0]; !bytes.Equal(got, want) { - t.Errorf("message %d: %s", idx, linediff(nfdump(got), nfdump(want))) - } - want = want[1:] - } - return req, nil - })) + c, err := nftables.New(expectMessages(t, want)) if err != nil { t.Fatal(err) } diff --git a/rule.go b/rule.go index 54146c1..68da81f 100644 --- a/rule.go +++ b/rule.go @@ -94,7 +94,7 @@ func (cc *Conn) GetRules(t *Table, c *Chain) ([]*Rule, error) { message := netlink.Message{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_GETRULE), - Flags: netlink.Request | netlink.Acknowledge | netlink.Dump | unix.NLM_F_ECHO, + Flags: netlink.Request | netlink.Acknowledge | netlink.Dump, }, Data: append(extraHeader(uint8(t.Family), 0), data...), } @@ -164,20 +164,20 @@ func (cc *Conn) newRule(r *Rule, op ruleOperation) *Rule { msgData := []byte{} msgData = append(msgData, data...) - var flags netlink.HeaderFlags if r.UserData != nil { msgData = append(msgData, cc.marshalAttr([]netlink.Attribute{ {Type: unix.NFTA_RULE_USERDATA, Data: r.UserData}, })...) } + var flags netlink.HeaderFlags switch op { case operationAdd: - flags = netlink.Request | netlink.Acknowledge | netlink.Create | unix.NLM_F_ECHO | unix.NLM_F_APPEND + flags = netlink.Request | netlink.Acknowledge | netlink.Create | netlink.Echo | netlink.Append case operationInsert: - flags = netlink.Request | netlink.Acknowledge | netlink.Create | unix.NLM_F_ECHO + flags = netlink.Request | netlink.Acknowledge | netlink.Create | netlink.Echo case operationReplace: - flags = netlink.Request | netlink.Acknowledge | netlink.Replace | unix.NLM_F_ECHO | unix.NLM_F_REPLACE + flags = netlink.Request | netlink.Acknowledge | netlink.Replace } if r.Position != 0 || (r.Flags&(1< Date: Mon, 24 Mar 2025 10:31:04 +0100 Subject: [PATCH 2/2] Set rule handle during flush This change makes it possible to delete rules after inserting them, without needing to query the rules first. Additionally, this allows positioning a new rule next to an existing rule. There are two ways to refer to a rule: Either by ID or by handle. The ID is assigned by userspace, and is only valid within a transaction, so it can only be used before the flush. The handle is assigned by the kernel when the transaction is committed, and can thus only be used after the flush. We thus need to set an ID on each newly created rule, and retrieve the handle of the rule during the flush. I extended the message struct with a pointer to the Rule which the message creates. This allows calling the reply handler callback which sets the handle. I updated tests to add a handle to generated replies for the NFT_MSG_NEWRULE messages. --- chain.go | 6 +++--- conn.go | 41 ++++++++++++++++++++++++++------------- flowtable.go | 4 ++-- internal/nftest/nftest.go | 11 +++++++++++ nftables_test.go | 11 ++++++++++- obj.go | 4 ++-- rule.go | 37 ++++++++++++++++++++++++++++++++--- set.go | 8 ++++---- set_test.go | 5 ++++- table.go | 6 +++--- 10 files changed, 101 insertions(+), 32 deletions(-) diff --git a/chain.go b/chain.go index 4f4c0a5..f1853cf 100644 --- a/chain.go +++ b/chain.go @@ -140,7 +140,7 @@ func (cc *Conn) AddChain(c *Chain) *Chain { {Type: unix.NFTA_CHAIN_TYPE, Data: []byte(c.Type + "\x00")}, })...) } - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_NEWCHAIN), Flags: netlink.Request | netlink.Acknowledge | netlink.Create, @@ -161,7 +161,7 @@ func (cc *Conn) DelChain(c *Chain) { {Type: unix.NFTA_CHAIN_NAME, Data: []byte(c.Name + "\x00")}, }) - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_DELCHAIN), Flags: netlink.Request | netlink.Acknowledge, @@ -179,7 +179,7 @@ func (cc *Conn) FlushChain(c *Chain) { {Type: unix.NFTA_RULE_TABLE, Data: []byte(c.Table.Name + "\x00")}, {Type: unix.NFTA_RULE_CHAIN, Data: []byte(c.Name + "\x00")}, }) - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_DELRULE), Flags: netlink.Request | netlink.Acknowledge, diff --git a/conn.go b/conn.go index 99baa2d..d974b80 100644 --- a/conn.go +++ b/conn.go @@ -41,7 +41,7 @@ type Conn struct { lasting bool // establish a lasting connection to be used across multiple netlink operations. mu sync.Mutex // protects the following state - messages []netlink.Message + messages []netlinkMessage err error nlconn *netlink.Conn // netlink socket using NETLINK_NETFILTER protocol. sockOptions []SockOption @@ -49,6 +49,12 @@ type Conn struct { allocatedIDs uint32 } +type netlinkMessage struct { + Header netlink.Header + Data []byte + rule *Rule +} + // ConnOption is an option to change the behavior of the nftables Conn returned by Open. type ConnOption func(*Conn) @@ -268,6 +274,11 @@ func (cc *Conn) Flush() error { } else if replyIndex < len(cc.messages) { msg := messages[replyIndex+1] if msg.Header.Sequence == reply.Header.Sequence && msg.Header.Type == reply.Header.Type { + // The only messages which set the echo flag are rule create messages. + err := cc.messages[replyIndex].rule.handleCreateReply(reply) + if err != nil { + errs = errors.Join(errs, err) + } replyIndex++ for replyIndex < len(cc.messages) && cc.messages[replyIndex].Header.Flags&netlink.Echo == 0 { replyIndex++ @@ -309,7 +320,7 @@ func (cc *Conn) Flush() error { func (cc *Conn) FlushRuleset() { cc.mu.Lock() defer cc.mu.Unlock() - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_DELTABLE), Flags: netlink.Request | netlink.Acknowledge | netlink.Create, @@ -368,26 +379,30 @@ func (cc *Conn) marshalExpr(fam byte, e expr.Any) []byte { return b } -func batch(messages []netlink.Message) []netlink.Message { - batch := []netlink.Message{ - { - Header: netlink.Header{ - Type: netlink.HeaderType(unix.NFNL_MSG_BATCH_BEGIN), - Flags: netlink.Request, - }, - Data: extraHeader(0, unix.NFNL_SUBSYS_NFTABLES), +func batch(messages []netlinkMessage) []netlink.Message { + batch := make([]netlink.Message, len(messages)+2) + batch[0] = netlink.Message{ + Header: netlink.Header{ + Type: netlink.HeaderType(unix.NFNL_MSG_BATCH_BEGIN), + Flags: netlink.Request, }, + Data: extraHeader(0, unix.NFNL_SUBSYS_NFTABLES), } - batch = append(batch, messages...) + for i, msg := range messages { + batch[i+1] = netlink.Message{ + Header: msg.Header, + Data: msg.Data, + } + } - batch = append(batch, netlink.Message{ + batch[len(messages)+1] = netlink.Message{ Header: netlink.Header{ Type: netlink.HeaderType(unix.NFNL_MSG_BATCH_END), Flags: netlink.Request, }, Data: extraHeader(0, unix.NFNL_SUBSYS_NFTABLES), - }) + } return batch } diff --git a/flowtable.go b/flowtable.go index 93dbcb5..a35712f 100644 --- a/flowtable.go +++ b/flowtable.go @@ -142,7 +142,7 @@ func (cc *Conn) AddFlowtable(f *Flowtable) *Flowtable { {Type: unix.NLA_F_NESTED | NFTA_FLOWTABLE_HOOK, Data: cc.marshalAttr(hookAttr)}, })...) - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_NEWFLOWTABLE), Flags: netlink.Request | netlink.Acknowledge | netlink.Create, @@ -162,7 +162,7 @@ func (cc *Conn) DelFlowtable(f *Flowtable) { {Type: NFTA_FLOWTABLE_NAME, Data: []byte(f.Name)}, }) - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | NFT_MSG_DELFLOWTABLE), Flags: netlink.Request | netlink.Acknowledge, diff --git a/internal/nftest/nftest.go b/internal/nftest/nftest.go index 8d5b496..509bac3 100644 --- a/internal/nftest/nftest.go +++ b/internal/nftest/nftest.go @@ -8,7 +8,9 @@ import ( "testing" "github.com/google/nftables" + "github.com/google/nftables/binaryutil" "github.com/mdlayher/netlink" + "golang.org/x/sys/unix" ) // Recorder provides an nftables connection that does not send to the Linux @@ -21,6 +23,7 @@ type Recorder struct { // Conn opens an nftables connection that records netlink messages into the // Recorder. func (r *Recorder) Conn() (*nftables.Conn, error) { + nextHandle := uint64(1) return nftables.New(nftables.WithTestDial( func(req []netlink.Message) ([]netlink.Message, error) { r.requests = append(r.requests, req...) @@ -30,6 +33,14 @@ func (r *Recorder) Conn() (*nftables.Conn, error) { for _, msg := range req { if msg.Header.Flags&netlink.Echo != 0 { data := append([]byte{}, msg.Data...) + switch msg.Header.Type { + case netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_NEWRULE): + attrs, _ := netlink.MarshalAttributes([]netlink.Attribute{ + {Type: unix.NFTA_RULE_HANDLE, Data: binaryutil.BigEndian.PutUint64(nextHandle)}, + }) + nextHandle++ + data = append(data, attrs...) + } replies = append(replies, netlink.Message{ Header: msg.Header, Data: data, diff --git a/nftables_test.go b/nftables_test.go index 6e9ecfe..fd0fa1a 100644 --- a/nftables_test.go +++ b/nftables_test.go @@ -80,6 +80,7 @@ func linediff(a, b string) string { } func expectMessages(t *testing.T, want [][]byte) nftables.ConnOption { + nextHandle := uint64(1) return nftables.WithTestDial(func(req []netlink.Message) ([]netlink.Message, error) { var replies []netlink.Message for idx, msg := range req { @@ -103,6 +104,14 @@ func expectMessages(t *testing.T, want [][]byte) nftables.ConnOption { // Generate replies. if msg.Header.Flags&netlink.Echo != 0 { data := append([]byte{}, msg.Data...) + switch msg.Header.Type { + case netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_NEWRULE): + attrs, _ := netlink.MarshalAttributes([]netlink.Attribute{ + {Type: unix.NFTA_RULE_HANDLE, Data: binaryutil.BigEndian.PutUint64(nextHandle)}, + }) + nextHandle++ + data = append(data, attrs...) + } replies = append(replies, netlink.Message{ Header: msg.Header, Data: data, @@ -316,7 +325,7 @@ func TestRuleHandle(t *testing.T) { } for _, tt := range tests { - for _, afterFlush := range []bool{false} { + for _, afterFlush := range []bool{false, true} { flushName := map[bool]string{ false: "-before-flush", true: "-after-flush", diff --git a/obj.go b/obj.go index 634931b..65c4402 100644 --- a/obj.go +++ b/obj.go @@ -124,7 +124,7 @@ func (cc *Conn) AddObj(o Obj) Obj { attrs = append(attrs, netlink.Attribute{Type: unix.NLA_F_NESTED | unix.NFTA_OBJ_DATA, Data: data}) } - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_NEWOBJ), Flags: netlink.Request | netlink.Acknowledge | netlink.Create, @@ -146,7 +146,7 @@ func (cc *Conn) DeleteObject(o Obj) { data := cc.marshalAttr(attrs) data = append(data, cc.marshalAttr([]netlink.Attribute{{Type: unix.NLA_F_NESTED | unix.NFTA_OBJ_DATA}})...) - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_DELOBJ), Flags: netlink.Request | netlink.Acknowledge, diff --git a/rule.go b/rule.go index 68da81f..65c6562 100644 --- a/rule.go +++ b/rule.go @@ -48,10 +48,13 @@ const ( type Rule struct { Table *Table Chain *Chain - // Handle identifies an existing Rule. + // Handle identifies an existing Rule. For a new Rule, this field is set + // during the Flush() in which the rule is committed. Make sure to not access + // this field concurrently with this Flush() to avoid data races. Handle uint64 // ID is an identifier for a new Rule, which is assigned by // AddRule/InsertRule, and only valid before the rule is committed by Flush(). + // The field is set to 0 during Flush(). ID uint32 // Position can be set to the Handle of another Rule to insert the new Rule // before (InsertRule) or after (AddRule) the existing rule. @@ -171,11 +174,14 @@ func (cc *Conn) newRule(r *Rule, op ruleOperation) *Rule { } var flags netlink.HeaderFlags + var ruleRef *Rule switch op { case operationAdd: flags = netlink.Request | netlink.Acknowledge | netlink.Create | netlink.Echo | netlink.Append + ruleRef = r case operationInsert: flags = netlink.Request | netlink.Acknowledge | netlink.Create | netlink.Echo + ruleRef = r case operationReplace: flags = netlink.Request | netlink.Acknowledge | netlink.Replace } @@ -190,17 +196,42 @@ func (cc *Conn) newRule(r *Rule, op ruleOperation) *Rule { })...) } - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: newRuleHeaderType, Flags: flags, }, Data: append(extraHeader(uint8(r.Table.Family), 0), msgData...), + rule: ruleRef, }) return r } +func (r *Rule) handleCreateReply(reply netlink.Message) error { + ad, err := netlink.NewAttributeDecoder(reply.Data[4:]) + if err != nil { + return err + } + ad.ByteOrder = binary.BigEndian + var handle uint64 + for ad.Next() { + switch ad.Type() { + case unix.NFTA_RULE_HANDLE: + handle = ad.Uint64() + } + } + if ad.Err() != nil { + return ad.Err() + } + if handle == 0 { + return fmt.Errorf("missing rule handle in create reply") + } + r.Handle = handle + r.ID = 0 + return nil +} + func (cc *Conn) ReplaceRule(r *Rule) *Rule { return cc.newRule(r, operationReplace) } @@ -247,7 +278,7 @@ func (cc *Conn) DelRule(r *Rule) error { } flags := netlink.Request | netlink.Acknowledge - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: delRuleHeaderType, Flags: flags, diff --git a/set.go b/set.go index 431191e..5818ca7 100644 --- a/set.go +++ b/set.go @@ -506,7 +506,7 @@ func (cc *Conn) appendElemList(s *Set, vals []SetElement, hdrType uint16) error {Type: unix.NFTA_SET_ELEM_LIST_ELEMENTS | unix.NLA_F_NESTED, Data: encodedElem}, } - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | hdrType), Flags: netlink.Request | netlink.Acknowledge | netlink.Create, @@ -680,7 +680,7 @@ func (cc *Conn) AddSet(s *Set, vals []SetElement) error { tableInfo = append(tableInfo, netlink.Attribute{Type: unix.NLA_F_NESTED | NFTA_SET_ELEM_EXPRESSIONS, Data: data}) } - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_NEWSET), Flags: netlink.Request | netlink.Acknowledge | netlink.Create, @@ -700,7 +700,7 @@ func (cc *Conn) DelSet(s *Set) { {Type: unix.NFTA_SET_TABLE, Data: []byte(s.Table.Name + "\x00")}, {Type: unix.NFTA_SET_NAME, Data: []byte(s.Name + "\x00")}, }) - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_DELSET), Flags: netlink.Request | netlink.Acknowledge, @@ -717,7 +717,7 @@ func (cc *Conn) FlushSet(s *Set) { {Type: unix.NFTA_SET_TABLE, Data: []byte(s.Table.Name + "\x00")}, {Type: unix.NFTA_SET_NAME, Data: []byte(s.Name + "\x00")}, }) - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_DELSETELEM), Flags: netlink.Request | netlink.Acknowledge, diff --git a/set_test.go b/set_test.go index 65a8e00..dd30f45 100644 --- a/set_test.go +++ b/set_test.go @@ -254,7 +254,10 @@ func TestMarshalSet(t *testing.T) { } msg := c.messages[connMsgSetIdx] - nset, err := setsFromMsg(msg) + nset, err := setsFromMsg(netlink.Message{ + Header: msg.Header, + Data: msg.Data, + }) if err != nil { t.Fatalf("setsFromMsg() error: %+v", err) } diff --git a/table.go b/table.go index 79e486b..3686b7a 100644 --- a/table.go +++ b/table.go @@ -57,7 +57,7 @@ func (cc *Conn) DelTable(t *Table) { {Type: unix.NFTA_TABLE_NAME, Data: []byte(t.Name + "\x00")}, {Type: unix.NFTA_TABLE_FLAGS, Data: []byte{0, 0, 0, 0}}, }) - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_DELTABLE), Flags: netlink.Request | netlink.Acknowledge, @@ -73,7 +73,7 @@ func (cc *Conn) addTable(t *Table, flag netlink.HeaderFlags) *Table { {Type: unix.NFTA_TABLE_NAME, Data: []byte(t.Name + "\x00")}, {Type: unix.NFTA_TABLE_FLAGS, Data: []byte{0, 0, 0, 0}}, }) - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_NEWTABLE), Flags: netlink.Request | netlink.Acknowledge | flag, @@ -103,7 +103,7 @@ func (cc *Conn) FlushTable(t *Table) { data := cc.marshalAttr([]netlink.Attribute{ {Type: unix.NFTA_RULE_TABLE, Data: []byte(t.Name + "\x00")}, }) - cc.messages = append(cc.messages, netlink.Message{ + cc.messages = append(cc.messages, netlinkMessage{ Header: netlink.Header{ Type: netlink.HeaderType((unix.NFNL_SUBSYS_NFTABLES << 8) | unix.NFT_MSG_DELRULE), Flags: netlink.Request | netlink.Acknowledge,