From 211824995dcb277a4a8d86f605c3d11c21bc72f5 Mon Sep 17 00:00:00 2001
From: turekt <32360115+turekt@users.noreply.github.com>
Date: Mon, 21 Feb 2022 21:42:39 +0000
Subject: [PATCH] Log expression refactor (#147)

Fixes https://github.com/google/nftables/issues/113

Log expression implementation changed to better support different log options
Added uint16 support to the binaryutil package
Changed old log expression tests that were failing after change
Added a new test to check the implementation for multiple log options
---
 binaryutil/binaryutil.go |   9 +++
 expr/log.go              | 128 ++++++++++++++++++++++++--------
 nftables_test.go         | 156 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 258 insertions(+), 35 deletions(-)

diff --git a/binaryutil/binaryutil.go b/binaryutil/binaryutil.go
index 11b5dac..27755f5 100644
--- a/binaryutil/binaryutil.go
+++ b/binaryutil/binaryutil.go
@@ -26,6 +26,7 @@ type ByteOrder interface {
 	PutUint16(v uint16) []byte
 	PutUint32(v uint32) []byte
 	PutUint64(v uint64) []byte
+	Uint16(b []byte) uint16
 	Uint32(b []byte) uint32
 	Uint64(b []byte) uint64
 }
@@ -54,6 +55,10 @@ func (nativeEndian) PutUint64(v uint64) []byte {
 	return buf
 }
 
+func (nativeEndian) Uint16(b []byte) uint16 {
+	return *(*uint16)(unsafe.Pointer(&b[0]))
+}
+
 func (nativeEndian) Uint32(b []byte) uint32 {
 	return *(*uint32)(unsafe.Pointer(&b[0]))
 }
@@ -86,6 +91,10 @@ func (bigEndian) PutUint64(v uint64) []byte {
 	return buf
 }
 
+func (bigEndian) Uint16(b []byte) uint16 {
+	return binary.BigEndian.Uint16(b)
+}
+
 func (bigEndian) Uint32(b []byte) uint32 {
 	return binary.BigEndian.Uint32(b)
 }
diff --git a/expr/log.go b/expr/log.go
index 27ec0e1..7730a92 100644
--- a/expr/log.go
+++ b/expr/log.go
@@ -17,43 +17,99 @@ package expr
 import (
 	"encoding/binary"
 
+	"github.com/google/nftables/binaryutil"
 	"github.com/mdlayher/netlink"
 	"golang.org/x/sys/unix"
 )
 
+type LogLevel uint32
+
+const (
+	// See https://git.netfilter.org/nftables/tree/include/linux/netfilter/nf_tables.h?id=5b364657a35f4e4cd5d220ba2a45303d729c8eca#n1226
+	LogLevelEmerg LogLevel = iota
+	LogLevelAlert
+	LogLevelCrit
+	LogLevelErr
+	LogLevelWarning
+	LogLevelNotice
+	LogLevelInfo
+	LogLevelDebug
+	LogLevelAudit
+)
+
+type LogFlags uint32
+
+const (
+	// See https://git.netfilter.org/nftables/tree/include/linux/netfilter/nf_log.h?id=5b364657a35f4e4cd5d220ba2a45303d729c8eca
+	LogFlagsTCPSeq LogFlags = 0x01 << iota
+	LogFlagsTCPOpt
+	LogFlagsIPOpt
+	LogFlagsUID
+	LogFlagsNFLog
+	LogFlagsMACDecode
+	LogFlagsMask LogFlags = 0x2f
+)
+
 // Log defines type for NFT logging
+// See https://git.netfilter.org/libnftnl/tree/src/expr/log.c?id=09456c720e9c00eecc08e41ac6b7c291b3821ee5#n25
 type Log struct {
-	Key  uint32
+	Level LogLevel
+	// Refers to log flags (flags all, flags ip options, ...)
+	Flags LogFlags
+	// Equivalent to expression flags.
+	// Indicates that an option is set by setting a bit
+	// on index referred by the NFTA_LOG_* value.
+	// See https://cs.opensource.google/go/x/sys/+/3681064d:unix/ztypes_linux.go;l=2126;drc=3681064d51587c1db0324b3d5c23c2ddbcff6e8f
+	Key        uint32
+	Snaplen    uint32
+	Group      uint16
+	QThreshold uint16
+	// Log prefix string content
 	Data []byte
 }
 
 func (e *Log) marshal() ([]byte, error) {
-	var data []byte
-	var err error
-	switch e.Key {
-	case unix.NFTA_LOG_GROUP:
-		data, err = netlink.MarshalAttributes([]netlink.Attribute{
-			{Type: unix.NFTA_LOG_GROUP, Data: e.Data},
-		})
-	case unix.NFTA_LOG_PREFIX:
-		prefix := append(e.Data, '\x00')
-		data, err = netlink.MarshalAttributes([]netlink.Attribute{
-			{Type: unix.NFTA_LOG_PREFIX, Data: prefix},
-		})
-	case unix.NFTA_LOG_SNAPLEN:
-		data, err = netlink.MarshalAttributes([]netlink.Attribute{
-			{Type: unix.NFTA_LOG_SNAPLEN, Data: e.Data},
-		})
-	case unix.NFTA_LOG_QTHRESHOLD:
-		data, err = netlink.MarshalAttributes([]netlink.Attribute{
-			{Type: unix.NFTA_LOG_QTHRESHOLD, Data: e.Data},
-		})
-	case unix.NFTA_LOG_LEVEL:
-		level := append(e.Data, '\x00')
-		data, err = netlink.MarshalAttributes([]netlink.Attribute{
-			{Type: unix.NFTA_LOG_LEVEL, Data: level},
+	// Per https://git.netfilter.org/libnftnl/tree/src/expr/log.c?id=09456c720e9c00eecc08e41ac6b7c291b3821ee5#n129
+	attrs := make([]netlink.Attribute, 0)
+	if e.Key&(1<<unix.NFTA_LOG_GROUP) != 0 {
+		attrs = append(attrs, netlink.Attribute{
+			Type: unix.NFTA_LOG_GROUP,
+			Data: binaryutil.BigEndian.PutUint16(e.Group),
 		})
 	}
+	if e.Key&(1<<unix.NFTA_LOG_PREFIX) != 0 {
+		prefix := append(e.Data, '\x00')
+		attrs = append(attrs, netlink.Attribute{
+			Type: unix.NFTA_LOG_PREFIX,
+			Data: prefix,
+		})
+	}
+	if e.Key&(1<<unix.NFTA_LOG_SNAPLEN) != 0 {
+		attrs = append(attrs, netlink.Attribute{
+			Type: unix.NFTA_LOG_SNAPLEN,
+			Data: binaryutil.BigEndian.PutUint32(e.Snaplen),
+		})
+	}
+	if e.Key&(1<<unix.NFTA_LOG_QTHRESHOLD) != 0 {
+		attrs = append(attrs, netlink.Attribute{
+			Type: unix.NFTA_LOG_QTHRESHOLD,
+			Data: binaryutil.BigEndian.PutUint16(e.QThreshold),
+		})
+	}
+	if e.Key&(1<<unix.NFTA_LOG_LEVEL) != 0 {
+		attrs = append(attrs, netlink.Attribute{
+			Type: unix.NFTA_LOG_LEVEL,
+			Data: binaryutil.BigEndian.PutUint32(uint32(e.Level)),
+		})
+	}
+	if e.Key&(1<<unix.NFTA_LOG_FLAGS) != 0 {
+		attrs = append(attrs, netlink.Attribute{
+			Type: unix.NFTA_LOG_FLAGS,
+			Data: binaryutil.BigEndian.PutUint32(uint32(e.Flags)),
+		})
+	}
+
+	data, err := netlink.MarshalAttributes(attrs)
 	if err != nil {
 		return nil, err
 	}
@@ -71,15 +127,23 @@ func (e *Log) unmarshal(data []byte) error {
 	}
 
 	ad.ByteOrder = binary.BigEndian
-	if ad.Next() {
-		e.Key = uint32(ad.Type())
-		e.Data = ad.Bytes()
-		switch e.Key {
+	for ad.Next() {
+		e.Key |= 1 << uint32(ad.Type())
+		data := ad.Bytes()
+		switch ad.Type() {
+		case unix.NFTA_LOG_GROUP:
+			e.Group = binaryutil.BigEndian.Uint16(data)
 		case unix.NFTA_LOG_PREFIX:
-			fallthrough
-		case unix.NFTA_LOG_LEVEL:
 			// Getting rid of \x00 at the end of string
-			e.Data = e.Data[:len(e.Data)-1]
+			e.Data = data[:len(data)-1]
+		case unix.NFTA_LOG_SNAPLEN:
+			e.Snaplen = binaryutil.BigEndian.Uint32(data)
+		case unix.NFTA_LOG_QTHRESHOLD:
+			e.QThreshold = binaryutil.BigEndian.Uint16(data)
+		case unix.NFTA_LOG_LEVEL:
+			e.Level = LogLevel(binaryutil.BigEndian.Uint32(data))
+		case unix.NFTA_LOG_FLAGS:
+			e.Flags = LogFlags(binaryutil.BigEndian.Uint32(data))
 		}
 	}
 	return ad.Err()
diff --git a/nftables_test.go b/nftables_test.go
index cd166ce..87d009f 100644
--- a/nftables_test.go
+++ b/nftables_test.go
@@ -569,6 +569,151 @@ func TestConfigureNATSourceAddress(t *testing.T) {
 	}
 }
 
+func TestExprLogOptions(t *testing.T) {
+	c, newNS := openSystemNFTConn(t)
+	defer cleanupSystemNFTConn(t, newNS)
+
+	c.FlushRuleset()
+	defer c.FlushRuleset()
+
+	filter := c.AddTable(&nftables.Table{
+		Family: nftables.TableFamilyIPv4,
+		Name:   "filter",
+	})
+	input := c.AddChain(&nftables.Chain{
+		Name:     "input",
+		Table:    filter,
+		Type:     nftables.ChainTypeFilter,
+		Hooknum:  nftables.ChainHookInput,
+		Priority: nftables.ChainPriorityFilter,
+	})
+	forward := c.AddChain(&nftables.Chain{
+		Name:     "forward",
+		Table:    filter,
+		Type:     nftables.ChainTypeFilter,
+		Hooknum:  nftables.ChainHookInput,
+		Priority: nftables.ChainPriorityFilter,
+	})
+
+	keyGQ := uint32((1 << unix.NFTA_LOG_GROUP) | (1 << unix.NFTA_LOG_QTHRESHOLD) | (1 << unix.NFTA_LOG_SNAPLEN))
+	c.AddRule(&nftables.Rule{
+		Table: filter,
+		Chain: input,
+		Exprs: []expr.Any{
+			&expr.Log{
+				Key:        keyGQ,
+				QThreshold: uint16(20),
+				Group:      uint16(1),
+				Snaplen:    uint32(132),
+			},
+		},
+	})
+
+	keyPL := uint32((1 << unix.NFTA_LOG_PREFIX) | (1 << unix.NFTA_LOG_LEVEL) | (1 << unix.NFTA_LOG_FLAGS))
+	c.AddRule(&nftables.Rule{
+		Table: filter,
+		Chain: forward,
+		Exprs: []expr.Any{
+			&expr.Log{
+				Key:   keyPL,
+				Data:  []byte("LOG FORWARD"),
+				Level: expr.LogLevelDebug,
+				Flags: expr.LogFlagsTCPOpt | expr.LogFlagsIPOpt,
+			},
+		},
+	})
+
+	if err := c.Flush(); err != nil {
+		t.Errorf("c.Flush() failed: %v", err)
+	}
+
+	rules, err := c.GetRule(
+		&nftables.Table{
+			Family: nftables.TableFamilyIPv4,
+			Name:   "filter",
+		},
+		&nftables.Chain{
+			Name: "input",
+		},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if got, want := len(rules), 1; got != want {
+		t.Fatalf("unexpected number of rules: got %d, want %d", got, want)
+	}
+
+	rule := rules[0]
+	if got, want := len(rule.Exprs), 1; got != want {
+		t.Fatalf("unexpected number of exprs: got %d, want %d", got, want)
+	}
+
+	le, ok := rule.Exprs[0].(*expr.Log)
+	if !ok {
+		t.Fatalf("unexpected expression type: got %T, want *expr.Log", rule.Exprs[0])
+	}
+
+	if got, want := le.Key, keyGQ; got != want {
+		t.Fatalf("unexpected log key: got %d, want %d", got, want)
+	}
+
+	if got, want := le.Group, uint16(1); got != want {
+		t.Fatalf("unexpected group: got %d, want %d", got, want)
+	}
+
+	if got, want := le.QThreshold, uint16(20); got != want {
+		t.Fatalf("unexpected queue-threshold: got %d, want %d", got, want)
+	}
+
+	if got, want := le.Snaplen, uint32(132); got != want {
+		t.Fatalf("unexpected snaplen: got %d, want %d", got, want)
+	}
+
+	rules, err = c.GetRule(
+		&nftables.Table{
+			Family: nftables.TableFamilyIPv4,
+			Name:   "filter",
+		},
+		&nftables.Chain{
+			Name: "forward",
+		},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if got, want := len(rules), 1; got != want {
+		t.Fatalf("unexpected number of rules: got %d, want %d", got, want)
+	}
+
+	rule = rules[0]
+	if got, want := len(rule.Exprs), 1; got != want {
+		t.Fatalf("unexpected number of exprs: got %d, want %d", got, want)
+	}
+
+	le, ok = rule.Exprs[0].(*expr.Log)
+	if !ok {
+		t.Fatalf("unexpected expression type: got %T, want *expr.Log", rule.Exprs[0])
+	}
+
+	if got, want := le.Key, keyPL; got != want {
+		t.Fatalf("unexpected log key: got %d, want %d", got, want)
+	}
+
+	if got, want := string(le.Data), "LOG FORWARD"; got != want {
+		t.Fatalf("unexpected prefix data: got %s, want %s", got, want)
+	}
+
+	if got, want := le.Level, expr.LogLevelDebug; got != want {
+		t.Fatalf("unexpected log level: got %d, want %d", got, want)
+	}
+
+	if got, want := le.Flags, expr.LogFlagsTCPOpt|expr.LogFlagsIPOpt; got != want {
+		t.Fatalf("unexpected log flags: got %d, want %d", got, want)
+	}
+}
+
 func TestExprLogPrefix(t *testing.T) {
 	c, newNS := openSystemNFTConn(t)
 	defer cleanupSystemNFTConn(t, newNS)
@@ -593,7 +738,7 @@ func TestExprLogPrefix(t *testing.T) {
 		Chain: input,
 		Exprs: []expr.Any{
 			&expr.Log{
-				Key:  unix.NFTA_LOG_PREFIX,
+				Key:  1 << unix.NFTA_LOG_PREFIX,
 				Data: []byte("LOG INPUT"),
 			},
 		},
@@ -628,12 +773,17 @@ func TestExprLogPrefix(t *testing.T) {
 		t.Fatalf("Exprs[0] is type %T, want *expr.Log", rules[0].Exprs[0])
 	}
 
-	if got, want := logExpr.Key, uint32(unix.NFTA_LOG_PREFIX); got != want {
+	// nftables defaults to warn log level when no level is specified and group is not defined
+	// see https://wiki.nftables.org/wiki-nftables/index.php/Logging_traffic
+	if got, want := logExpr.Key, uint32((1<<unix.NFTA_LOG_PREFIX)|(1<<unix.NFTA_LOG_LEVEL)); got != want {
 		t.Fatalf("unexpected *expr.Log key: got %d, want %d", got, want)
 	}
 	if got, want := string(logExpr.Data), "LOG INPUT"; got != want {
 		t.Fatalf("unexpected *expr.Log data: got %s, want %s", got, want)
 	}
+	if got, want := logExpr.Level, expr.LogLevelWarning; got != want {
+		t.Fatalf("unexpected *expr.Log level: got %d, want %d", got, want)
+	}
 }
 
 func TestGetRule(t *testing.T) {
@@ -920,7 +1070,7 @@ func TestLog(t *testing.T) {
 		Chain: &nftables.Chain{Name: "ipv4chain-1", Type: nftables.ChainTypeFilter},
 		Exprs: []expr.Any{
 			&expr.Log{
-				Key:  unix.NFTA_LOG_PREFIX,
+				Key:  1 << unix.NFTA_LOG_PREFIX,
 				Data: []byte("nftables"),
 			},
 		},