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

Require nft v1.0.1 or later #4

Merged
merged 2 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,21 @@ that is quite different from all documented examples of nftables usage
because there is no easy way to convert the "standard" representation
of nftables rules into the netlink form.

(Actually, that's not quite true: the `nft` CLI is just a thin wrapper
around `libnftables`, and it would be possible for knftables to use
cgo to invoke that library instead of using an external binary.
However, this would be harder to build and ship, so I'm not bothering
with that for now. But this could be done in the future without
needing to change knftables's API.)
(Actually, it's not quite true that there's no other usable API: the
`nft` CLI is just a thin wrapper around `libnftables`, and it would be
possible for knftables to use cgo to invoke that library instead of
using an external binary. However, this would be harder to build and
ship, so I'm not bothering with that for now. But this could be done
in the future without needing to change knftables's API.)

knftables requires nft version 1.0.1 or later, because earlier
versions would download and process the entire ruleset regardless of
what you were doing, which, besides being pointlessly inefficient,
means that in some cases, other people using new features in _their_
tables could prevent you from modifying _your_ table. (In particular,
a change in how some rules are generated starting in nft 1.0.3
triggers a crash in nft 0.9.9 and earlier, _even if you aren't looking
at the table containing that rule_.)

## Usage

Expand Down
20 changes: 17 additions & 3 deletions nftables.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"encoding/json"
"fmt"
"os/exec"
"strings"
)

// Interface is an interface for running nftables commands against a given family and table.
Expand Down Expand Up @@ -73,7 +74,8 @@ type realNFTables struct {
path string
}

// for unit tests
// newInternal creates a new nftables.Interface for interacting with the given table; this
// is split out from New() so it can be used from unit tests with a fakeExec.
func newInternal(family Family, table string, execer execer) (Interface, error) {
var err error

Expand All @@ -91,17 +93,29 @@ func newInternal(family Family, table string, execer execer) (Interface, error)
return nil, fmt.Errorf("could not find nftables binary: %w", err)
}

cmd := exec.Command(nft.path, "--check", "add", "table", string(nft.family), nft.table)
_, err = nft.exec.Run(cmd)
cmd := exec.Command(nft.path, "--version")
danwinship marked this conversation as resolved.
Show resolved Hide resolved
out, err := nft.exec.Run(cmd)
if err != nil {
return nil, fmt.Errorf("could not run nftables command: %w", err)
}
if strings.HasPrefix(out, "nftables v0.") || strings.HasPrefix(out, "nftables v1.0.0 ") {
aojea marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf("nft version must be v1.0.1 or later (got %s)", strings.TrimSpace(out))
}

// Check that (a) nft works, (b) we have permission, (c) the kernel is new enough
// to support object comments.
cmd = exec.Command(nft.path, "--check", "add", "table", string(nft.family), nft.table,
"{", "comment", `"test"`, "}",
)
_, err = nft.exec.Run(cmd)
if err != nil {
// Try again, checking just that (a) nft works, (b) we have permission.
cmd := exec.Command(nft.path, "--check", "add", "table", string(nft.family), nft.table)
_, err = nft.exec.Run(cmd)
if err != nil {
return nil, fmt.Errorf("could not run nftables command: %w", err)
}

nft.noObjectComments = true
}

Expand Down
52 changes: 39 additions & 13 deletions nftables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ func newTestInterface(t *testing.T, family Family, tableName string) (Interface,
}
fexec.expected = append(fexec.expected,
expectedCmd{
args: []string{"/nft", "--check", "add", "table", ip, tableName},
args: []string{"/nft", "--version"},
stdout: "nftables v1.0.7 (Old Doc Yak)\n",
},
expectedCmd{
args: []string{"/nft", "--check", "add", "table", ip, tableName,
Expand Down Expand Up @@ -420,16 +421,28 @@ func TestFeatures(t *testing.T) {
for _, tc := range []struct {
name string
commands []expectedCmd
result nftContext
result *nftContext
}{
{
name: "old nftables",
commands: []expectedCmd{
{
args: []string{
"/nft", "--version",
},
stdout: "nftables v0.9.3 (Topsy)\n",
},
},
result: nil,
},
{
name: "all features",
commands: []expectedCmd{
{
args: []string{
"/nft", "--check",
"add", "table", "ip", "testing",
"/nft", "--version",
},
stdout: "nftables v1.0.7 (Old Doc Yak)\n",
},
{
args: []string{
Expand All @@ -439,7 +452,7 @@ func TestFeatures(t *testing.T) {
},
},
},
result: nftContext{
result: &nftContext{
family: IPv4Family,
table: "testing",
},
Expand All @@ -449,9 +462,9 @@ func TestFeatures(t *testing.T) {
commands: []expectedCmd{
{
args: []string{
"/nft", "--check",
"add", "table", "ip", "testing",
"/nft", "--version",
},
stdout: "nftables v1.0.7 (Old Doc Yak)\n",
},
{
args: []string{
Expand All @@ -461,8 +474,14 @@ func TestFeatures(t *testing.T) {
},
err: fmt.Errorf("Error: syntax error, unexpected comment"),
},
{
args: []string{
"/nft", "--check",
"add", "table", "ip", "testing",
},
},
},
result: nftContext{
result: &nftContext{
family: IPv4Family,
table: "testing",

Expand All @@ -475,11 +494,18 @@ func TestFeatures(t *testing.T) {
fexec.expected = tc.commands
nft, err := newInternal(IPv4Family, "testing", fexec)
if err != nil {
t.Fatalf("Unexpected error creating Interface: %v", err)
}
result := nft.(*realNFTables).nftContext
if !reflect.DeepEqual(tc.result, result) {
t.Errorf("Expected %#v, got %#v", tc.result, result)
if tc.result != nil {
t.Fatalf("Unexpected error creating Interface: %v", err)
}
} else {
result := nft.(*realNFTables).nftContext
if tc.result != nil {
if !reflect.DeepEqual(*tc.result, result) {
t.Errorf("Expected %#v, got %#v", *tc.result, result)
}
} else {
t.Fatalf("Expected failure, got %#v", result)
}
}
})
}
Expand Down
Loading