[MIR] nftables

[Balint Reczey]

[Availability]

* The package is already in universe and has been supported
by Ubuntu kernels since at least Ubuntu 18.04 LTS. It
builds and is supported on all Ubuntu architectures.

[Rationale]

* nftables is the future CLI and backend for firewalling
which should be available on Ubuntu by default, and is
the preferred tool by the upstream kernel community.

* iptables will be switching to nftables backend, but
iptables availability and usage will probably continue for
forseeable future. It is expected that newer software will
be adopting nftables directly, rather than via iptables
compat tools.

[Security]

* There is no history of of vulnerabilities in the nftables
user space tools (CVE-2015-1573 is in the kernel portion
of nftables).

* The nftables binary package contains the binary
`/usr/bin/nft` which is neither setuid nor setgid. This
binary is the utility that interacts with and configures
the nftables subsystem in the Linux kernel.

* The package also includes a oneshot systemd service
used during boot to load the nftables configuration in
/etc/nftables.conf. As packaged in Debian, this service
is disabled by default.

* It interacts with and configures the network filtering
as performed by the Linux kernel.

[Quality Assurance - function/usage]

* The package works as installed; it does require enabling
the systemd oneshot service to automatically reload defined
rules on boot.

[Quality assurance - maintenance]

LP bugs: https://bugs.launchpad.net/ubuntu/+source/nftables/+bugs
Debian: https://bugs.debian.org/cgi-bin/pkgreport.cgi?repeatmerged=no&src=nftables
Upstream: https://bugzilla.netfilter.org/buglist.cgi?bug_status=__open__&content=&no_redirect=1&order=Importance&product=nftables&query_format=specific

* Ubuntu and Debian bugs are reasonably under
control. Upstream has a larger set of bugs that are
mostly about parsing errors (flex/yacc are complex) and
documentation or feature requests.

[Quality Assurance - testing]

* Tests are not run at build time; there are many tests
run during autopkgtests across all architectures, but the
more extensive ones have been marked as flaky. Example
autopkgtest log:
https://autopkgtest.ubuntu.com/results/autopkgtest-jammy/jammy/amd64/n/nftables/20220117_122101_70524@/log.gz

[Quality Assurance - packaging]

* A debian/watch file is present and works. Lintian reports
nothing substantial, just minor standards version lag as
well as debian/control missing the Rules-Requires-Root:
field (silent-on-rules-requiring-root). It does not depend
on obsolete or about to be demoted packages. There are no
debconf settings or questions.

[UI Standards]

* It is primarily a command line system tool that is
sysadmin facing, that does not contain translations.

[Dependencies]

* Documentation tools used during the build are in
universe; all runtime dependencies are in main. It uses
libjannson for JSON handling, not sure if there's a
preferred JSON library in main.

[Standards compliance]

* This package correctly follows FHS and Debian Policy

[Maintenance/Owner]

* The ubuntu-security team is subscribed to bugs for
nftables. There are no static builds. There are some very
minor embedded code copies that are either disabled at
build time (system gmp is used over embedded mini-gmp)
or are fairly small (David Woodhouse's rbtree). It is
relatively mature software with active upstream commits
(http://git.netfilter.org/nftables/log/) as well as
reasonably active maintenance in Debian.

[Background information]

* The package description explains the package
well. The upstream project is part of the
larger netfilter project, and is documented at
https://netfilter.org/projects/nftables/index.html

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Seth Arnold (seth-arnold)]

(subscribing ubuntu-mir even though this isn't done yet, just in case that was overlooked :)

[Christian Ehrhardt  (paelzer)]

Thanks Seth, but since it is yet incomplete let us set the state to it.
That way we will see it in the incomplete list but know that we can't action yet.

@RBalint - what is the schedule on this 21.04?

[Balint Reczey (rbalint)]

@paelzer This is not planned for 20.10 because in the 20.10 cycle only the iptables backend has been changed to nft.
I can't comment on the timing of this MIR because I think the MIR preparations will be done by the Security Team, who will own nftables itself, too.

[Christian Ehrhardt  (paelzer)]

Assigned to sarnold for security then, re-open when you think it is ready and the team actually has a chance to focus on it.

[Launchpad Janitor (janitor)]

[Expired for nftables (Ubuntu) because there has been no activity for 60 days.]

[Balint Reczey (rbalint)]

keep this MIR alive

[Christian Ehrhardt  (paelzer)]

Ok rbalint, since it is incomplete we need to reflect that this is waiting on someone.
Re-reading the discussion so far that someone is sarnold whom I assigning to this bug for now.

[Dimitri John Ledkov (xnox)]

In trello, there is no asignee to perform the security review.

Thus removing assignee.

@ Security Team, when and who can do security review of nftables? we are overdue to seed nftables by default.

[Christian Ehrhardt  (paelzer)]

Hi Xnox,
I think you misinterpreted the sarnold assignment as waiting for security review.
It wasn't that far.

This was waiting for #4:
quoting
"... I think the MIR preparations will be done by the Security Team, who will own nftables itself, too."

Only once that is done and fully opened it will go
1. MIR Team review
2. (likely) security team review

From the MIR team the info that this might not be as fast as you'd like:
[16:50] <cpaelzer> to quote "... I think the MIR preparations will be done by the Security Team, who will own nftables itself, too."
[16:51] <sarnold> cpaelzer: I can re-raise it at our next team meeting, but given $everything I can't imagine it'll be a priority for the team to push on this
[16:52] <cpaelzer> ok sarnold, I'll update the bug accordingly and re-assign it to you for doing that
[16:52] <sarnold> cpaelzer: cool, thanks

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Andreas Hasenack (ahasenack)]

Could we get this in time for the next LTS? Even the bionic kernel supports nftables, and we missed this in focal too.

[Dimitri John Ledkov (xnox)]

https://media.giphy.com/media/FoH28ucxZFJZu/giphy.gif

[Lukas Märdian (slyon)]

Review for Package: src:nftables
Author: @joalif
Reviewed-by: @slyon

[Summary]
nftables is the future CLI for firewalling which should be available on Ubuntu.
iptables CLI switched to using a nftables backend, but will probably still
exist for a while.
The package is looking good from a MIR perspective, except for missing
symbols tracking of libnftables1 and an explicit agreement about the
maintenance of the embedded sources.

MIR team ACK under the constraint to resolve the below listed required
TODOs and as much as possible having a look at the recommended TODOs.

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main:
libnftables-dev_1.0.2-1_amd64.deb
libnftables1_1.0.2-1_amd64.deb
nftables_1.0.2-1_amd64.deb
python3-nftables_1.0.2-1_amd64.deb

Specific binary packages built, but NOT to be promoted to main: <None>

Notes:
- The package is owned by ubuntu-security, so they might already have checked
it. I'm still assigning it to ubuntu-security.
- The team bug subscriber is already set to ubuntu-security.

Required TODOs:
#1 embedded source present (as stated in bug description: src/rbtree.c), as the
   the security team will be the maintainer of this package, we do not need
   security-team's agreement, but please explicitly state your willingness to
   also maintain those embedded sources in a comment.
#2 symbols tracking is not in place (we see a lintian warning about it), please
   create a .symbols file for libnftables1

Recommended TODOs:
#3 some vague security concerns have been raised, but as the security team
   will be the maintainer of this package I think it will be in good hands
#4 does not have a test suite that runs at build time, please try to enable
   some tests at build time. We're good for now, though, as we have automated
   autopkgtests
#5 The distutils package is deprecated (to be removed in in Python 3.12), try
   to fix this build-time warning by switching to setuptools

[Duplication]
This package replaces iptables. (but iptables will still be around for a while)

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  - checked with check-mir
  - not listed in seeded-in-ubuntu
  - none of the (potentially auto-generated) dependencies (Depends
    and Recommends) that are present after build are not in main
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems: None

[Embedded sources and static linking]
OK:
- no static linking
- does not have odd Built-Using entries
- not a go package, no extra constraints to consider in that regard

Problems:
- embedded source present (as stated in bug description: src/rbtree.c)

[Security]
OK:
- history of CVEs does not look concerning
- does not use webkit1,2
- does not use lib*v8 directly
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)

Problems:
- does parse data formats
- can be used t...

[Christian Ehrhardt  (paelzer)]

Marking as incomplete to reflect that there were TODOs identified.
This is on security twice now:
- security review
- driving the case overall

[Steve Beattie (sbeattie)]

For the required todos:

1) yes, the Ubuntu Security team is willing to maintain the embedded code copies.

2) debian symbols tracking: https://bugs.launchpad.net/ubuntu/+source/nftables/+bug/1965464

For the recommended todos, we will try to make progress on those.

Thanks!

[Christian Ehrhardt  (paelzer)]

Thanks for the info Steve, glad to see progress on that.

If I might ask - what about the security review? I assume you have kind of done that already before trying to suggest to promote it, but formally security should state somewhere here that you have done your usual checks.

Oh and finally this hasn't a milestone yet, do you expect to make the change for 22.04 still or is this for later?

[Alex Murray (alexmurray)]

I reviewed nftables 1.0.2-1ubuntu1 as checked into jammy. This shouldn't
be considered a full audit but rather a quick gauge of maintainability.

nftables is a replacement for iptables etc - it provides userspace tooling
to control the Netfilter packet classification system within the Linux
kernel and can be used to implemenent firewall, advanced packet routing,
traffic control and other use-cases.

- No CVE History
- Security relevant Build-Depends:
  - libjansson-dev for JSON parsing
  - libmnl-dev for netlink message handling
- pre/post inst/rm scripts
  - nftables binary package has autogenerated (by dh_installsystemd)
    scripts to setup systemd for nftables daemon service
  - python3-nftables binary package has autogenerated (by dh_python3)
    scripts to compile python files on install
- No init scripts
- systemd units for the nft daemon
  - Loads / unloads nft rules on startup / shutdown
  - Confines the daemon by using both ProtectSystem=full and
    ProtectHome=true so that it cannot write to /usr, /boot, /efi and /etc
    and that /home, /root and /run/user are inaccessible
- No dbus services
- No setuid binaries
- 1 binary in PATH
  - -rwxr-xr-x root/root 26856 2022-03-18 11:45 ./usr/sbin/nft
- No sudo fragments
- No polkit files
- No udev rules
- No unit tests run during build
- Autopkgtests
  - Runs the high level 'shell' based internal test suite
  - Runs internal nft monitor testsuite to ensure output of 'nft monitor'
    is as expected
  - Runs test of systemd service to ensure rules get loaded / unloaded
    appropriately by the systemd unit
  - Contains a reference to running the internal python-based regression
    testsuite of nft but this is commented out - I thought it might be easy
    to get this running (see LP: #1966017) but turns out there are still
    issues there so perhaps that is best left for a future task
- No cron jobs
- Clean build logs

- No processes spawned
- Lots of dynamic memory management (since is written in C) but appears to
  be careful / defensive - exit's with an error if fails to allocate memory
  which is fine as this is a command-line tool and appears to check buffer
  sizes etc as needed
- File IO
  - Paths are specified in input files / rules etc as input
  - Files are not written to, only read from
- Logging appears careful and defensive
- Environment variable usage
  - HOME is used to store a history file for cli interface to store past
    commands etc
- No apparent use of privileged functions
- No use of cryptography / random number sources etc
- No apparent use of temp files
- No direct use of networking
  - Uses netlink for communication with kernel but whilst this is socket
    based it does not allow remote access or any other such similar attack
    surface nor does it handle untrusted input
- No use of WebKit
- No use of PolicyKit

- No significant cppcheck results
- Lots of Coverity results but none look super critical - given nftables is
  expected to handle only trusted input I can't see how they could be used
  to cross a security boundary etc
- Lots of shellcheck results generated by upstream 'shell' and 'monitor'
  test suites but since these come from upstream and ar...

[Steve Beattie (sbeattie)]

python distutils deprecation has been filed as a bug upstream at https://bugzilla.netfilter.org/show_bug.cgi?id=1594

For the security review, while I did do some review while preparing the MIR request, I supsect it is preferable for the submitter to not also be the one to do the security review. Alex gracefully agreed to perform it, as seen above.

Yes, we would like to land this for 22.04 LTS, if possible.

Thanks!

[Christian Ehrhardt  (paelzer)]

Ok, summarizing the change
Required:
#1 embedded source
 => Done: security said that is ok for them
#2 symbols tracking
 => Done: resolved and improved via bug 1965464

Recommended:
#3 some vague security concerns
 => Done: did not come up in the security review
#4 does not have a test suite that runs at build time, please try to enable
   some tests at build time. We're good for now, though, as we have automated
   autopkgtests
 => Open: but not blocking
#5 The distutils package is deprecated
 => Thanks seth for the pointer, that is ok then as it will be resolved and should be ok a future merge.

Overall, we are good to go now.
There is nothing in seeds or dependency yet, that would be up to you (?Steve B.?) to update.
Once done it would show up in component mismatches and we can promote it.
Assigning it back to you to be clear.

P.S. are there any follow on actions in packages using iptables to switch over, if so could they be filed as bugs?

[Andreas Hasenack (ahasenack)]

The seed change[1] is approved, we are just waiting for the jammy beta block to be lifted to merge it.

1. https://code.launchpad.net/~alexmurray/ubuntu-seeds/+git/ubuntu-seeds/+merge/417621

[Christian Ehrhardt  (paelzer)]

This shows in component mismatches (seed change landed)

nftables: libnftables-dev libnftables1 nftables
MIR: #1887187 (Fix Committed)
[Reverse-Depends: Rescued from nftables (Uploader: paelzer) (Uploader: paelzer), Ubuntu.Jammy standard seed, nftables (Uploader: paelzer)]

It is only in jammy (not in proposed).
 nftables | 1.0.2-1ubuntu2 | jammy/universe | source, amd64, arm64, armhf, ppc64el, riscv64, s390x

Dependencies right now pull in only source + libnftables1 nftables - that is what I'll promote to avoid demoting the others later.
If you want more e.g. -dev you'll need to seed/depend on it.

Override component to main
libnftables1 1.0.2-1ubuntu2 in jammy amd64: universe/libs/optional/100% -> main
libnftables1 1.0.2-1ubuntu2 in jammy arm64: universe/libs/optional/100% -> main
libnftables1 1.0.2-1ubuntu2 in jammy armhf: universe/libs/optional/100% -> main
libnftables1 1.0.2-1ubuntu2 in jammy ppc64el: universe/libs/optional/100% -> main
libnftables1 1.0.2-1ubuntu2 in jammy riscv64: universe/libs/optional/100% -> main
libnftables1 1.0.2-1ubuntu2 in jammy s390x: universe/libs/optional/100% -> main
nftables 1.0.2-1ubuntu2 in jammy amd64: universe/net/extra/100% -> main
nftables 1.0.2-1ubuntu2 in jammy arm64: universe/net/extra/100% -> main
nftables 1.0.2-1ubuntu2 in jammy armhf: universe/net/extra/100% -> main
nftables 1.0.2-1ubuntu2 in jammy ppc64el: universe/net/extra/100% -> main
nftables 1.0.2-1ubuntu2 in jammy riscv64: universe/net/extra/100% -> main
nftables 1.0.2-1ubuntu2 in jammy s390x: universe/net/extra/100% -> main
Override [y|N]? y
12 publications overridden.

$ ./change-override -c main -s jammy nftables --source-only
Override component to main
nftables 1.0.2-1ubuntu2 in jammy: universe/misc -> main
Override [y|N]? y
1 publication overridden.