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.

Sign up for GitHub

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

Jump to bottom

bug: GetRule returns entries for all chains, not the one provided #114

Open
greenpau opened this issue Aug 29, 2020 · 7 comments
Open

bug: GetRule returns entries for all chains, not the one provided #114

greenpau opened this issue Aug 29, 2020 · 7 comments

Comments

@greenpau
Copy link
Contributor

I make GetRule() call in the following way.

	tb := &nftables.Table{
		Name: tableName,
	}
	if v == "4" {
		tb.Family = nftables.TableFamilyIPv4
	} else {
		tb.Family = nftables.TableFamilyIPv6
	}

	ch := &nftables.Chain{
		Name:  chainName,
		Table: tb,
	}

	rules, err := conn.GetRule(tb, ch)
	if err != nil {
		return nil, err
	}

The issue is I am getting back all chains, not the table and change I passed as arguments.
Here is a dump of such response, Please note the Name: (string) (len=10) "prerouting" and Name: (string) (len=11) "postrouting",.

(*utils.chainInfo)(0xc00005d6d0)({
         RuleCount: (int) 9,
         Positions: ([]uint64) (len=9 cap=16) {
          (uint64) 0,
          (uint64) 0,
          (uint64) 0,
          (uint64) 7,
          (uint64) 8,
          (uint64) 0,
          (uint64) 0,
          (uint64) 0,
          (uint64) 2
         },
         Handles: ([]uint64) (len=9 cap=16) {
          (uint64) 6,
          (uint64) 11,
          (uint64) 7,
          (uint64) 8,
          (uint64) 9,
          (uint64) 12,
          (uint64) 2,
          (uint64) 2,
          (uint64) 3
         },
         Rules: ([]*nftables.Rule) (len=9 cap=16) {
          (*nftables.Rule)(0xc00005d770)({
           Table: (*nftables.Table)(0xc0001dc8e0)({
            Name: (string) (len=3) "nat",
            Use: (uint32) 0,
            Flags: (uint32) 0,
            Family: (nftables.TableFamily) 0
           }),
           Chain: (*nftables.Chain)(0xc0001cad80)({
            Name: (string) (len=11) "postrouting",
            Table: (*nftables.Table)(<nil>),
            Hooknum: (nftables.ChainHook) 0,
            Priority: (nftables.ChainPriority) 0,
            Type: (nftables.ChainType) "",
            Policy: (*nftables.ChainPolicy)(<nil>)
           }),
           Position: (uint64) 0,
           Handle: (uint64) 6,
           Exprs: ([]expr.Any) (len=1 cap=1) {
            (*expr.Verdict)(0xc0001dc9e0)({
             Kind: (expr.VerdictKind) -3,
             Chain: (string) (len=31) "cni-npo-163852eb9be80b2d5547968"
            })
           },
           UserData: ([]uint8) <nil>
          }),
          (*nftables.Rule)(0xc00005d7c0)({
           Table: (*nftables.Table)(0xc0001dcaa0)({
            Name: (string) (len=3) "nat",
            Use: (uint32) 0,
            Flags: (uint32) 0,
            Family: (nftables.TableFamily) 0
           }),
           Chain: (*nftables.Chain)(0xc0001cb080)({
            Name: (string) (len=10) "prerouting",
            Table: (*nftables.Table)(<nil>),
            Hooknum: (nftables.ChainHook) 0,
            Priority: (nftables.ChainPriority) 0,
            Type: (nftables.ChainType) "",
            Policy: (*nftables.ChainPolicy)(<nil>)
           }),
           Position: (uint64) 0,
           Handle: (uint64) 11,
           Exprs: ([]expr.Any) (len=1 cap=1) {
            (*expr.Verdict)(0xc0001dcba0)({
             Kind: (expr.VerdictKind) -3,
             Chain: (string) (len=31) "cni-npr-163852eb9be80b2d5547968"
            })
           },
           UserData: ([]uint8) <nil>
          }),
          (*nftables.Rule)(0xc00005d810)({
           Table: (*nftables.Table)(0xc0001dcc60)({
            Name: (string) (len=3) "nat",
            Use: (uint32) 0,
            Flags: (uint32) 0,
            Family: (nftables.TableFamily) 0
           }),
           Chain: (*nftables.Chain)(0xc0001cb380)({
            Name: (string) (len=31) "cni-npo-163852eb9be80b2d5547968",
            Table: (*nftables.Table)(<nil>),
            Hooknum: (nftables.ChainHook) 0,
            Priority: (nftables.ChainPriority) 0,
            Type: (nftables.ChainType) "",
            Policy: (*nftables.ChainPolicy)(<nil>)
           }),
           Position: (uint64) 0,
           Handle: (uint64) 7,
           Exprs: ([]expr.Any) (len=7 cap=8) {
            (*expr.Payload)(0xc00001fda0)({
             OperationType: (expr.PayloadOperationType) 0,
             DestRegister: (uint32) 1,
             SourceRegister: (uint32) 0,
             Base: (expr.PayloadBase) 1,
             Offset: (uint32) 12,
             Len: (uint32) 4,
             CsumType: (expr.PayloadCsumType) 0,
             CsumOffset: (uint32) 0,
             CsumFlags: (uint32) 0
            }),
            (*expr.Cmp)(0xc0001dcd60)({
             Op: (expr.CmpOp) 0,
             Register: (uint32) 1,
             Data: ([]uint8) (len=4 cap=4) {
              00000000  0a 58 00 07                                       |.X..|
             }
            }),
            (*expr.Payload)(0xc00001fdd0)({
             OperationType: (expr.PayloadOperationType) 0,
             DestRegister: (uint32) 1,
             SourceRegister: (uint32) 0,
             Base: (expr.PayloadBase) 1,
             Offset: (uint32) 16,
             Len: (uint32) 4,
             CsumType: (expr.PayloadCsumType) 0,
             CsumOffset: (uint32) 0,
             CsumFlags: (uint32) 0
            }),
            (*expr.Bitwise)(0xc0001cb840)({
             SourceRegister: (uint32) 1,
             DestRegister: (uint32) 1,
             Len: (uint32) 4,
             Mask: ([]uint8) (len=4 cap=4) {
              00000000  ff ff ff 00                                       |....|
             },
             Xor: ([]uint8) (len=4 cap=4) {
              00000000  00 00 00 00                                       |....|
             }
            }),
            (*expr.Cmp)(0xc0001dcf20)({
             Op: (expr.CmpOp) 0,
             Register: (uint32) 1,
             Data: ([]uint8) (len=4 cap=4) {
              00000000  e0 00 00 00                                       |....|
             }
            }),
            (*expr.Counter)(0xc0001c30b0)({
             Bytes: (uint64) 0,
             Packets: (uint64) 0
            }),
            (*expr.Verdict)(0xc0001dd080)({
             Kind: (expr.VerdictKind) -5,
             Chain: (string) ""
            })
           },
           UserData: ([]uint8) <nil>
          }),
          (*nftables.Rule)(0xc00005d860)({
           Table: (*nftables.Table)(0xc0001dd140)({
            Name: (string) (len=3) "nat",
            Use: (uint32) 0,
            Flags: (uint32) 0,
            Family: (nftables.TableFamily) 0
           }),
           Chain: (*nftables.Chain)(0xc0001cbe40)({
            Name: (string) (len=31) "cni-npo-163852eb9be80b2d5547968",
            Table: (*nftables.Table)(<nil>),
            Hooknum: (nftables.ChainHook) 0,
            Priority: (nftables.ChainPriority) 0,
            Type: (nftables.ChainType) "",
            Policy: (*nftables.ChainPolicy)(<nil>)
           }),
           Position: (uint64) 7,
           Handle: (uint64) 8,
           Exprs: ([]expr.Any) (len=6 cap=8) {
            (*expr.Payload)(0xc00001ff50)({
             OperationType: (expr.PayloadOperationType) 0,
             DestRegister: (uint32) 1,
             SourceRegister: (uint32) 0,
             Base: (expr.PayloadBase) 1,
             Offset: (uint32) 12,
             Len: (uint32) 4,
             CsumType: (expr.PayloadCsumType) 0,
             CsumOffset: (uint32) 0,
             CsumFlags: (uint32) 0
            }),
            (*expr.Cmp)(0xc0001dd240)({
             Op: (expr.CmpOp) 0,
             Register: (uint32) 1,
             Data: ([]uint8) (len=4 cap=4) {
              00000000  0a 58 00 07                                       |.X..|
             }
            }),
            (*expr.Payload)(0xc00001ff80)({
             OperationType: (expr.PayloadOperationType) 0,
             DestRegister: (uint32) 1,
             SourceRegister: (uint32) 0,
             Base: (expr.PayloadBase) 1,
             Offset: (uint32) 16,
             Len: (uint32) 4,
             CsumType: (expr.PayloadCsumType) 0,
             CsumOffset: (uint32) 0,
             CsumFlags: (uint32) 0
            }),
            (*expr.Cmp)(0xc0001dd360)({
             Op: (expr.CmpOp) 0,
             Register: (uint32) 1,
             Data: ([]uint8) (len=4 cap=4) {
              00000000  ff ff ff ff                                       |....|
             }
            }),
            (*expr.Counter)(0xc0001c3210)({
             Bytes: (uint64) 0,
             Packets: (uint64) 0
            }),
            (*expr.Verdict)(0xc0001dd4c0)({
             Kind: (expr.VerdictKind) -5,
             Chain: (string) ""
            })
           },
           UserData: ([]uint8) <nil>
          }),
          (*nftables.Rule)(0xc00005d8b0)({
           Table: (*nftables.Table)(0xc0001dd560)({
            Name: (string) (len=3) "nat",
            Use: (uint32) 0,
            Flags: (uint32) 0,
            Family: (nftables.TableFamily) 0
           }),
           Chain: (*nftables.Chain)(0xc0001e4740)({
            Name: (string) (len=31) "cni-npo-163852eb9be80b2d5547968",
            Table: (*nftables.Table)(<nil>),
            Hooknum: (nftables.ChainHook) 0,
            Priority: (nftables.ChainPriority) 0,
            Type: (nftables.ChainType) "",
            Policy: (*nftables.ChainPolicy)(<nil>)
           }),
           Position: (uint64) 8,
           Handle: (uint64) 9,
           Exprs: ([]expr.Any) (len=5 cap=8) {
            (*expr.Payload)(0xc0001e6060)({
             OperationType: (expr.PayloadOperationType) 0,
             DestRegister: (uint32) 1,
             SourceRegister: (uint32) 0,
             Base: (expr.PayloadBase) 1,
             Offset: (uint32) 12,
             Len: (uint32) 4,
             CsumType: (expr.PayloadCsumType) 0,
             CsumOffset: (uint32) 0,
             CsumFlags: (uint32) 0
            }),
            (*expr.Cmp)(0xc0001dd660)({
             Op: (expr.CmpOp) 0,
             Register: (uint32) 1,
             Data: ([]uint8) (len=4 cap=4) {
              00000000  0a 58 00 07                                       |.X..|
             }
            }),
            (*expr.Meta)(0xc0001c3340)({
             Key: (expr.MetaKey) 7,
             SourceRegister: (bool) false,
             Register: (uint32) 1
            }),
            (*expr.Cmp)(0xc0001dd780)({
             Op: (expr.CmpOp) 0,
             Register: (uint32) 1,
             Data: ([]uint8) (len=16 cap=16) {
              00000000  63 6e 69 2d 70 6f 64 6d  61 6e 30 00 00 00 00 00  |cni-podman0.....|
             }
            }),
            (*expr.Counter)(0xc0001c3390)({
             Bytes: (uint64) 0,
             Packets: (uint64) 0
            })
           },
           UserData: ([]uint8) <nil>
          }),
          (*nftables.Rule)(0xc00005d900)({
           Table: (*nftables.Table)(0xc0001dd8c0)({
            Name: (string) (len=3) "nat",
            Use: (uint32) 0,
            Flags: (uint32) 0,
            Family: (nftables.TableFamily) 0
           }),
           Chain: (*nftables.Chain)(0xc0001e4f00)({
            Name: (string) (len=31) "cni-npr-163852eb9be80b2d5547968",
            Table: (*nftables.Table)(<nil>),
            Hooknum: (nftables.ChainHook) 0,
            Priority: (nftables.ChainPriority) 0,
            Type: (nftables.ChainType) "",
            Policy: (*nftables.ChainPolicy)(<nil>)
           }),
           Position: (uint64) 0,
           Handle: (uint64) 12,
           Exprs: ([]expr.Any) (len=7 cap=8) {
            (*expr.Meta)(0xc0001c33e0)({
             Key: (expr.MetaKey) 16,
             SourceRegister: (bool) false,
             Register: (uint32) 1
            }),
            (*expr.Cmp)(0xc0001dd9c0)({
             Op: (expr.CmpOp) 0,
             Register: (uint32) 1,
             Data: ([]uint8) (len=1 cap=1) {
              00000000  06                                                |.|
             }
            }),
            (*expr.Payload)(0xc0001e61b0)({
             OperationType: (expr.PayloadOperationType) 0,
             DestRegister: (uint32) 1,
             SourceRegister: (uint32) 0,
             Base: (expr.PayloadBase) 2,
             Offset: (uint32) 2,
             Len: (uint32) 2,
             CsumType: (expr.PayloadCsumType) 0,
             CsumOffset: (uint32) 0,
             CsumFlags: (uint32) 0
            }),
            (*expr.Cmp)(0xc0001ddae0)({
             Op: (expr.CmpOp) 0,
             Register: (uint32) 1,
             Data: ([]uint8) (len=2 cap=2) {
              00000000  b3 ef                                             |..|
             }
            }),
            (*expr.Immediate)(0xc0001ddb80)({
             Register: (uint32) 1,
             Data: ([]uint8) (len=4 cap=4) {
              00000000  0a 58 00 07                                       |.X..|
             }
            }),
            (*expr.Immediate)(0xc0001ddc20)({
             Register: (uint32) 2,
             Data: ([]uint8) (len=2 cap=2) {
              00000000  00 50                                             |.P|
             }
            }),
            (*expr.NAT)(0xc0001de520)({
             Type: (expr.NATType) 1,
             Family: (uint32) 2,
             RegAddrMin: (uint32) 1,
             RegAddrMax: (uint32) 1,
             RegProtoMin: (uint32) 2,
             RegProtoMax: (uint32) 2,
             Random: (bool) false,
             FullyRandom: (bool) false,
             Persistent: (bool) false
            })
           },
           UserData: ([]uint8) <nil>
          }),
          (*nftables.Rule)(0xc00005d950)({
           Table: (*nftables.Table)(0xc0001ddd20)({
            Name: (string) (len=3) "raw",
            Use: (uint32) 0,
            Flags: (uint32) 0,
            Family: (nftables.TableFamily) 0
           }),
           Chain: (*nftables.Chain)(0xc0001e5880)({
            Name: (string) (len=10) "prerouting",
            Table: (*nftables.Table)(<nil>),
            Hooknum: (nftables.ChainHook) 0,
            Priority: (nftables.ChainPriority) 0,
            Type: (nftables.ChainType) "",
            Policy: (*nftables.ChainPolicy)(<nil>)
           }),
           Position: (uint64) 0,
           Handle: (uint64) 2,
           Exprs: ([]expr.Any) (len=12 cap=16) {
            (*expr.Counter)(0xc0001c3520)({
             Bytes: (uint64) 0,
             Packets: (uint64) 0
            }),
            (*expr.Meta)(0xc0001c3560)({
             Key: (expr.MetaKey) 16,
             SourceRegister: (bool) false,
             Register: (uint32) 1
            }),
            (*expr.Cmp)(0xc0001ddea0)({
             Op: (expr.CmpOp) 0,
             Register: (uint32) 1,
             Data: ([]uint8) (len=1 cap=1) {
              00000000  06                                                |.|
             }
            }),
            (*expr.Payload)(0xc0001e63c0)({
             OperationType: (expr.PayloadOperationType) 0,
             DestRegister: (uint32) 1,
             SourceRegister: (uint32) 0,
             Base: (expr.PayloadBase) 2,
             Offset: (uint32) 2,
             Len: (uint32) 2,
             CsumType: (expr.PayloadCsumType) 0,
             CsumOffset: (uint32) 0,
             CsumFlags: (uint32) 0
            }),
            (*expr.Cmp)(0xc0001ddfa0)({
             Op: (expr.CmpOp) 0,
             Register: (uint32) 1,
             Data: ([]uint8) (len=2 cap=2) {
              00000000  b3 ef                                             |..|
             }
            }),
            (*expr.Immediate)(0xc0001ec040)({
             Register: (uint32) 1,
             Data: ([]uint8) (len=4 cap=4) {
              00000000  0a 58 00 07                                       |.X..|
             }
            }),
            (*expr.Immediate)(0xc0001ec0e0)({
             Register: (uint32) 2,
             Data: ([]uint8) (len=2 cap=2) {
              00000000  00 50                                             |.P|
             }
            }),
            (*expr.Immediate)(0xc0001ec180)({
             Register: (uint32) 1,
             Data: ([]uint8) (len=4 cap=4) {
              00000000  0a 58 00 07                                       |.X..|
             }
            }),
            (*expr.Payload)(0xc0001e6420)({
             OperationType: (expr.PayloadOperationType) 0,
             DestRegister: (uint32) 0,
             SourceRegister: (uint32) 1,
             Base: (expr.PayloadBase) 1,
             Offset: (uint32) 16,
             Len: (uint32) 4,
             CsumType: (expr.PayloadCsumType) 1,
             CsumOffset: (uint32) 10,
             CsumFlags: (uint32) 0
            }),
            (*expr.Immediate)(0xc0001ec280)({
             Register: (uint32) 1,
             Data: ([]uint8) (len=2 cap=2) {
              00000000  00 50                                             |.P|
             }
            }),
            (*expr.Payload)(0xc0001e6480)({
             OperationType: (expr.PayloadOperationType) 0,
             DestRegister: (uint32) 0,
             SourceRegister: (uint32) 1,
             Base: (expr.PayloadBase) 2,
             Offset: (uint32) 2,
             Len: (uint32) 2,
             CsumType: (expr.PayloadCsumType) 1,
             CsumOffset: (uint32) 16,
             CsumFlags: (uint32) 0
            }),
            (*expr.Verdict)(0xc0001ec3e0)({
             Kind: (expr.VerdictKind) -5,
             Chain: (string) ""
            })
           },
           UserData: ([]uint8) <nil>
          }),
          (*nftables.Rule)(0xc00005d9a0)({
           Table: (*nftables.Table)(0xc0001ec480)({
            Name: (string) (len=6) "filter",
            Use: (uint32) 0,
            Flags: (uint32) 0,
            Family: (nftables.TableFamily) 0
           }),
           Chain: (*nftables.Chain)(0xc0001ee880)({
            Name: (string) (len=7) "forward",
            Table: (*nftables.Table)(<nil>),
            Hooknum: (nftables.ChainHook) 0,
            Priority: (nftables.ChainPriority) 0,
            Type: (nftables.ChainType) "",
            Policy: (*nftables.ChainPolicy)(<nil>)
           }),
           Position: (uint64) 0,
           Handle: (uint64) 2,
           Exprs: ([]expr.Any) <nil>,
           UserData: ([]uint8) <nil>
          }),
          (*nftables.Rule)(0xc00005d9f0)({
           Table: (*nftables.Table)(0xc0001ec560)({
            Name: (string) (len=6) "filter",
            Use: (uint32) 0,
            Flags: (uint32) 0,
            Family: (nftables.TableFamily) 0
           }),
           Chain: (*nftables.Chain)(0xc0001eea00)({
            Name: (string) (len=7) "forward",
            Table: (*nftables.Table)(<nil>),
            Hooknum: (nftables.ChainHook) 0,
            Priority: (nftables.ChainPriority) 0,
            Type: (nftables.ChainType) "",
            Policy: (*nftables.ChainPolicy)(<nil>)
           }),
           Position: (uint64) 2,
           Handle: (uint64) 3,
           Exprs: ([]expr.Any) (len=2 cap=2) {
            (*expr.Counter)(0xc0001c3800)({
             Bytes: (uint64) 0,
             Packets: (uint64) 0
            }),
            (*expr.Verdict)(0xc0001ec6c0)({
             Kind: (expr.VerdictKind) 0,
             Chain: (string) ""
            })
           },
           UserData: ([]uint8) <nil>
          })
         }
        })
@greenpau
Copy link
Contributor Author

Here is the relevant function https://github.com/greenpau/cni-plugins/blob/dnat/pkg/utils/get_chain_props.go

@greenpau
Copy link
Contributor Author

There is another aspect for this. There is no regards as to IPv4 vs. IPv6 chains ...

The filtering could happen here ...

nftables/rule.go

Lines 81 to 88 in 7127d9d

var rules []*Rule
for _, msg := range reply {
r, err := ruleFromMsg(msg)
if err != nil {
return nil, err
}
rules = append(rules, r)
}

One other though is that perhaps ruleFromMsg() needs to be looked into....

@greenpau
Copy link
Contributor Author

There is no "default" handling of various "AttributeDecoder" types.

nftables/rule.go

Lines 288 to 306 in 7127d9d

for ad.Next() {
switch ad.Type() {
case unix.NFTA_RULE_TABLE:
r.Table = &Table{Name: ad.String()}
case unix.NFTA_RULE_CHAIN:
r.Chain = &Chain{Name: ad.String()}
case unix.NFTA_RULE_EXPRESSIONS:
ad.Do(func(b []byte) error {
r.Exprs, err = exprsFromMsg(b)
return err
})
case unix.NFTA_RULE_POSITION:
r.Position = ad.Uint64()
case unix.NFTA_RULE_HANDLE:
r.Handle = ad.Uint64()
case unix.NFTA_RULE_USERDATA:
r.UserData = ad.Bytes()
}
}

@stapelberg
Copy link
Collaborator

Maybe a similar commit to fdd795d might be needed?

Haven’t looked at it, and won’t have time to do so either, so I’ll rely on a PR to fix this.

@randolphcyg
Copy link

we can fix this problem by add a judgment at rule.go line 87:

var rules []*Rule
for _, msg := range reply {
    r, err := ruleFromMsg(msg)
    if err != nil {
	    return nil, err
    }

    if r.Table.Name == t.Name && r.Table.Family == t.Family && r.Chain.Name == c.Name {
	    rules = append(rules, r)
    }
}

Apart from this，we need to fix ruleFromMsg method by add a assignment statement at rule.go line 295:

case unix.NFTA_RULE_TABLE:
    r.Table = &Table{Name: ad.String()}
    r.Table.Family = TableFamily(msg.Data[0])

this line is aim to return a rule's father table's net family attr, instead of return all 0 back to user， thus we could make a judgment on the protocol cluster of the target table and the obtained tables, at the end of it, we can successfully get the rules for a specific table and a specific chain.

and the other fix will be push to my fork repo github.com/RandolphCYG/nftables

@stapelberg
Copy link
Collaborator

Can you send your change as a pull request as well? Or is there any reason why it couldn’t be upstreamed? :)

@randolphcyg
Copy link

Can you send your change as a pull request as well? Or is there any reason why it couldn’t be upstreamed? :)

hey, here comes my first pull request, could you help me check it ?

#133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@stapelberg @greenpau @randolphcyg