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

NFTA_RULE_HANDLE vs. NFTA_RULE_POSITION attribute type for "rule add" and "rule insert" operations #127

Open
constcrist opened this issue Sep 1, 2021 · 2 comments

Comments

@constcrist
Copy link

this issue involves some reverse engineering done on my system:

cco@firiel:~$ uname -a
Linux firiel 5.8.0-63-generic #71-Ubuntu SMP Tue Jul 13 15:59:12 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
cco@firiel:~$ lsb_release -a
LSB Version:	core-11.1.0ubuntu2-noarch:printing-11.1.0ubuntu2-noarch:security-11.1.0ubuntu2-noarch
Distributor ID:	Ubuntu
Description:	Ubuntu 20.10
Release:	20.10
Codename:	groovy

I ran into some problems when trying use the nftables.Conn.AddRule() and nftables.Conn.InsertRule() for adding or inserting new rules in a specified position.

I have checked with strace which are the systemcalls to which the nft tool translates "rule add/insert with handle" operations; like this for example:

strace -o /tmp/nft_dump nft insert rule ip filter MONITORING handle 382 ip saddr @whitelist counter

I have seen in the output that only the attribute NFTA_RULE_POSITION (0x6) is used in netlink messages for these operations; it should be filled in with the actual handle where the rule should be added or inserted. here is the relevant sendmsg() sycall from the strace output:

sendmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, 
    msg_iov=[
        {iov_base=
            [
            {
                {len=20, type=NFNL_MSG_BATCH_BEGIN, flags=NLM_F_REQUEST, seq=0, pid=0}, 
                {nfgen_family=AF_UNSPEC, version=NFNETLINK_V0, res_id=htons(2560)}, 
                {
                    {len=188, type=NFNL_SUBSYS_NFTABLES<<8|NFT_MSG_NEWRULE, flags=NLM_F_REQUEST|NLM_F_CREATE, seq=1, pid=0}, 
                    {
                        nfgen_family=AF_INET, version=NFNETLINK_V0, res_id=htons(0), 
                        [
                            {{nla_len=11, nla_type=NFNETLINK_V1}, "\x66\x69\x6c\x74\x65\x72\x00"}, 
                            {{nla_len=15, nla_type=0x2}, "\x4d\x4f\x4e\x49\x54\x4f\x52\x49\x4e\x47\x00"}, 
                            {{nla_len=12, nla_type=0x6}, "\x00\x00\x00\x00\x00\x00\x01\x84"},
                            {{nla_len=128, nla_type=NLA_F_NESTED|0x4}, "\x34\x00\x01\x80\x0c\x00\x01\x00\x70\x61\x79\x6c\x6f\x61\x64\x00\x24\x00\x02\x80\x08\x00\x01\x00\x00\x00\x00\x01\x08\x00\x02\x00"...}
                        ]
                    }, 
                    {
                        {len=20, type=NFNL_MSG_BATCH_END, flags=NLM_F_REQUEST, seq=2, pid=0}, 
                        {nfgen_family=AF_UNSPEC, version=NFNETLINK_V0, res_id=htons(2560)}
                    }
            ], iov_len=228}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 228

as far as I can tell from my reverse engineering sessions, NFTA_RULE_HANDLE is NOT used when adding or inserting rules with specified handle through the nft command line.

now, one can still use the APIs nftables.Conn.AddRule, nftables.Conn.InsertRule by just setting the Rule.Handle to 0 and the actual handle in Rule.Position

maybe:

  1. this behaviour should be documented somehow in nftables;
  2. encoding of Rule.Handle should be removed from the insert/add APIs
@stapelberg
Copy link
Collaborator

cc @alexispires who added insert/replace support

@alexispires
Copy link
Contributor

alexispires commented Oct 26, 2021

The only reason why NFTA_RULE_HANDLE is passed in netlink messages is because we tried to prevents a breaking change. Before my changes AddRule allow to make replacement operations (more at #86 (comment)). Therefore an AddRule or InsertRule with an handle has the same behavior as ReplaceRule. Indeed, if handle is zeroed it won't be passed, but it's not a problem since zero is the default value of an uint64 field. Imo it's not a concern, do you have an example of where it could be an issue ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants