Drop unnecessary size checks and corrections for Netlink attributes.

Some Netlink attributes have a fixed size. The map attempted to make adding
attributes safer and easier by having default sizes for certain attributes.
However, attributes are identified via their enums, which often overlap for
different types of requests, making these default sizes not hold true for all
attributes that map to it.

PiperOrigin-RevId: 778632075

Author: kerumeto

pkg/abi/linux/netlink.go

@@ -66,21 +66,35 @@ type NetlinkMessageHeader struct {
 // NetlinkMessageHeaderSize is the size of NetlinkMessageHeader.
 const NetlinkMessageHeaderSize = 16
 
-// Netlink message header flags, from uapi/linux/netlink.h.
+// Netlink message header flag values, from uapi/linux/netlink.h.
 const (
 	NLM_F_REQUEST   = 0x1
 	NLM_F_MULTI     = 0x2
 	NLM_F_ACK       = 0x4
 	NLM_F_ECHO      = 0x8
 	NLM_F_DUMP_INTR = 0x10
-	NLM_F_ROOT      = 0x100
-	NLM_F_MATCH     = 0x200
-	NLM_F_ATOMIC    = 0x400
-	NLM_F_DUMP      = NLM_F_ROOT | NLM_F_MATCH
-	NLM_F_REPLACE   = 0x100
-	NLM_F_EXCL      = 0x200
-	NLM_F_CREATE    = 0x400
-	NLM_F_APPEND    = 0x800
+)
+
+// Netlink message header flags for GET requests, from uapi/linux/netlink.h.
+const (
+	NLM_F_ROOT   = 0x100
+	NLM_F_MATCH  = 0x200
+	NLM_F_ATOMIC = 0x400
+	NLM_F_DUMP   = NLM_F_ROOT | NLM_F_MATCH
+)
+
+// Netlink message header flags for NEW requests, from uapi/linux/netlink.h.
+const (
+	NLM_F_REPLACE = 0x100
+	NLM_F_EXCL    = 0x200
+	NLM_F_CREATE  = 0x400
+	NLM_F_APPEND  = 0x800
+)
+
+// Netlink message header flags for DELETE requests, from uapi/linux/netlink.h.
+const (
+	NLM_F_NONREC = 0x100
+	NLM_F_BULK   = 0x200
 )
 
 // Standard netlink message types, from uapi/linux/netlink.h.

pkg/abi/linux/nf_tables.go

@@ -16,6 +16,18 @@ package linux
 
 // This file contains constants required to support nf_tables.
 
+// Name length constants for nf_table structures. These correspond to values in
+// include/uapi/linux/netfilter/nf_tables.h.
+const (
+	NFT_NAME_MAXLEN      = 256
+	NFT_TABLE_MAXNAMELEN = NFT_NAME_MAXLEN
+	NFT_CHAIN_MAXNAMELEN = NFT_NAME_MAXLEN
+	NFT_SET_MAXNAMELEN   = NFT_NAME_MAXLEN
+	NFT_OBJ_MAXNAMELEN   = NFT_NAME_MAXLEN
+	NFT_USERDATA_MAXLEN  = 256
+	NFT_OSF_MAXGENRELEN  = 16
+)
+
 // 16-byte Registers that can be used to maintain state for rules.
 // These correspond to values in include/uapi/linux/netfilter/nf_tables.h.
 const (
@@ -127,7 +139,10 @@ const (
 // NfTableFlags represents table flags that can be set for a table, namely dormant.
 // These correspond to values in include/uapi/linux/netfilter/nf_tables.h.
 const (
-	NFT_TABLE_F_DORMANT = 0x1
+	NFT_TABLE_F_DORMANT uint32 = 0x1
+	NFT_TABLE_F_OWNER          = 0x2
+	NFT_TABLE_F_PERSIST        = 0x4
+	NFT_TABLE_F_MASK           = NFT_TABLE_F_DORMANT | NFT_TABLE_F_OWNER | NFT_TABLE_F_PERSIST
 )
 
 // NfTableAttributes represents the netfilter table attributes.

pkg/sentry/socket/netlink/netfilter/BUILD

@@ -13,6 +13,7 @@ go_library(
         "//pkg/abi/linux",
         "//pkg/context",
         "//pkg/log",
+        "//pkg/marshal/primitive",
         "//pkg/sentry/inet",
         "//pkg/sentry/kernel",
         "//pkg/sentry/socket/netlink",

pkg/sentry/socket/netlink/netfilter/protocol.go

@@ -21,6 +21,7 @@ import (
 	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/log"
+	"gvisor.dev/gvisor/pkg/marshal/primitive"
 	"gvisor.dev/gvisor/pkg/sentry/inet"
 	"gvisor.dev/gvisor/pkg/sentry/kernel"
 	"gvisor.dev/gvisor/pkg/sentry/socket/netlink"
@@ -89,11 +90,19 @@ func (p *Protocol) ProcessMessage(ctx context.Context, s *netlink.Socket, msg *n
 	}
 
 	// Nftables functions error check the address family value.
-	family := stack.AddressFamily(nfGenMsg.Family)
+	family, err := nftables.AFtoNetlinkAF(nfGenMsg.Family)
 	// TODO: b/421437663 - Match the message type and call the appropriate Nftables function.
 	switch msgType {
 	case linux.NFT_MSG_NEWTABLE:
-		if err := p.newTable(nft, attrs, family, hdr.Flags); err != nil {
+		// We only check the error value in the case of NFT_MSG_NEWTABLE as linux returns
+		// an EOPNOTSUPP error only in that case. Otherwise the other operations will return
+		// errors specific to their function.
+		if err != nil {
+			log.Debugf("Nftables: Unsupported address family: %d", int(nfGenMsg.Family))
+			return err
+		}
+
+		if err := p.newTable(nft, attrs, family, hdr.Flags, ms); err != nil {
 			log.Debugf("Nftables new table error: %s", err)
 			return err.GetError()
 		}
@@ -104,49 +113,106 @@ func (p *Protocol) ProcessMessage(ctx context.Context, s *netlink.Socket, msg *n
 			return err.GetError()
 		}
 		return nil
+	case linux.NFT_MSG_DELTABLE, linux.NFT_MSG_DESTROYTABLE:
+		if err := p.deleteTable(nft, attrs, family, hdr, msgType, ms); err != nil {
+			log.Debugf("Nftables delete table error: %s", err)
+			return err.GetError()
+		}
+		return nil
 	default:
 		log.Debugf("Unsupported message type: %d", msgType)
 		return syserr.ErrNotSupported
 	}
 }
 
 // newTable creates a new table for the given family.
-func (p *Protocol) newTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, flags uint16) *syserr.AnnotatedError {
+func (p *Protocol) newTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, flags uint16, ms *nlmsg.MessageSet) *syserr.AnnotatedError {
 	// TODO: b/421437663 - Handle the case where the table name is set to empty string.
 	// The table name is required.
 	tabNameBytes, ok := attrs[linux.NFTA_TABLE_NAME]
 	if !ok {
 		return syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Table name attribute is malformed or not found"))
 	}
 
-	var dormant bool
-	if dbytes, ok := attrs[linux.NFTA_TABLE_FLAGS]; ok {
-		dflag, _ := dbytes.Uint32()
-		dormant = (dflag & linux.NFT_TABLE_F_DORMANT) == linux.NFT_TABLE_F_DORMANT
-	}
-
-	tab, err := nft.GetTable(family, tabNameBytes.String())
+	tab, err := nft.GetTable(family, tabNameBytes.String(), uint32(ms.PortID))
 	if err != nil && err.GetError() != syserr.ErrNoFileOrDir {
 		return err
 	}
 
 	// If a table already exists, only update its dormant flags if NLM_F_EXCL and NLM_F_REPLACE
 	// are not set. From net/netfilter/nf_tables_api.c:nf_tables_newtable:nf_tables_updtable
 	if tab != nil {
-		if flags&linux.NLM_F_EXCL == linux.NLM_F_EXCL {
-			return syserr.NewAnnotatedError(syserr.ErrExists, fmt.Sprintf("Nftables: Table with name: %s already exists", tabNameBytes.String()))
+		if flags&linux.NLM_F_EXCL != 0 {
+			return syserr.NewAnnotatedError(syserr.ErrExists, fmt.Sprintf("Nftables: Table with name: %s already exists", tab.GetName()))
 		}
 
-		if flags&linux.NLM_F_REPLACE == linux.NLM_F_REPLACE {
-			return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Table with name: %s already exists and NLM_F_REPLACE is not supported", tabNameBytes.String()))
+		if flags&linux.NLM_F_REPLACE != 0 {
+			return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Table with name: %s already exists and NLM_F_REPLACE is not supported", tab.GetName()))
 		}
-	} else {
-		tab, err = nft.CreateTable(family, tabNameBytes.String())
-		if err != nil {
+
+		return p.updateTable(nft, tab, attrs, family, ms)
+	}
+
+	// TODO: b/421437663 - Support additional user-specified table flags.
+	var attrFlags uint32 = 0
+	if uflags, ok := attrs[linux.NFTA_TABLE_FLAGS]; ok {
+		attrFlags, _ = uflags.Uint32()
+		// Flags sent through the NFTA_TABLE_FLAGS attribute are of type uint32
+		// but should only have user flags set. This check needs to be done before table creation.
+		if attrFlags & ^uint32(linux.NFT_TABLE_F_MASK) != 0 {
+			return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Table flags set are not supported"))
+		}
+	}
+
+	tab, err = nft.CreateTable(family, tabNameBytes.String())
+	if err != nil {
+		return err
+	}
+
+	if udata, ok := attrs[linux.NFTA_TABLE_USERDATA]; ok {
+		tab.SetUserData(udata)
+	}
+
+	// Flags should only be assigned after we have successfully created the table.
+	dormant := (attrFlags & uint32(linux.NFT_TABLE_F_DORMANT)) != 0
+	tab.SetDormant(dormant)
+
+	owner := (attrFlags & uint32(linux.NFT_TABLE_F_OWNER)) != 0
+	if owner {
+		if err := tab.SetOwner(uint32(ms.PortID)); err != nil {
 			return err
 		}
 	}
 
+	return nil
+}
+
+// updateTable updates an existing table.
+func (p *Protocol) updateTable(nft *nftables.NFTables, tab *nftables.Table, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, ms *nlmsg.MessageSet) *syserr.AnnotatedError {
+	var attrFlags uint32
+	if uflags, ok := attrs[linux.NFTA_TABLE_FLAGS]; ok {
+		attrFlags, _ = uflags.Uint32()
+		// This check needs to be done before table update.
+		if attrFlags & ^uint32(linux.NFT_TABLE_F_MASK) > 0 {
+			return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Table flags set are not supported"))
+		}
+	}
+
+	// When updating the table, if the table has an owner but the owner flag isn't set,
+	// the table should not be updated.
+	// From net/netfilter/nf_tables_api.c:nf_tables_updtable.
+	if tab.HasOwner() && (attrFlags&uint32(linux.NFT_TABLE_F_OWNER)) == 0 {
+		return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Table with name: %s already has an owner but NFT_TABLE_F_OWNER was not set when updating the table", tab.GetName()))
+	}
+
+	// The owner is only updated if the table has no previous owner.
+	if !tab.HasOwner() && attrFlags&uint32(linux.NFT_TABLE_F_OWNER) != 0 {
+		if err := tab.SetOwner(uint32(ms.PortID)); err != nil {
+			return err
+		}
+	}
+
+	dormant := (attrFlags & uint32(linux.NFT_TABLE_F_DORMANT)) != 0
 	tab.SetDormant(dormant)
 	return nil
 }
@@ -159,12 +225,16 @@ func (p *Protocol) getTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.Bytes
 		return syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Table name attribute is malformed or not found"))
 	}
 
-	tab, err := nft.GetTable(family, tabNameBytes.String())
+	tab, err := nft.GetTable(family, tabNameBytes.String(), uint32(ms.PortID))
 	if err != nil {
 		return err
 	}
 
 	tabName := tab.GetName()
+	userFlags, err := tab.GetLinuxUserFlagSet()
+	if err != nil {
+		return err
+	}
 	m := ms.AddMessage(linux.NetlinkMessageHeader{
 		Type: uint16(linux.NFNL_SUBSYS_NFTABLES)<<8 | uint16(linux.NFT_MSG_GETTABLE),
 	})
@@ -176,13 +246,73 @@ func (p *Protocol) getTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.Bytes
 		ResourceID: uint16(0),
 	})
 	m.PutAttrString(linux.NFTA_TABLE_NAME, tabName)
+	m.PutAttr(linux.NFTA_TABLE_USE, primitive.AllocateUint32(uint32(tab.ChainCount())))
+	m.PutAttr(linux.NFTA_TABLE_HANDLE, primitive.AllocateUint64(tab.GetHandle()))
+	m.PutAttr(linux.NFTA_TABLE_FLAGS, primitive.AllocateUint8(userFlags))
+
+	if tab.HasOwner() {
+		m.PutAttr(linux.NFTA_TABLE_OWNER, primitive.AllocateUint32(tab.GetOwner()))
+	}
+
+	if tab.HasUserData() {
+		m.PutAttr(linux.NFTA_TABLE_USERDATA, primitive.AsByteSlice(tab.GetUserData()))
+	}
+
 	return nil
 }
 
+// deleteTable deletes a table for the given family.
+func (p *Protocol) deleteTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, hdr linux.NetlinkMessageHeader, msgType linux.NfTableMsgType, ms *nlmsg.MessageSet) *syserr.AnnotatedError {
+	if family == stack.Unspec || (!hasAttr(linux.NFTA_TABLE_NAME, attrs) && !hasAttr(linux.NFTA_TABLE_HANDLE, attrs)) {
+		nft.Flush(attrs, uint32(ms.PortID))
+		return nil
+	}
+
+	var tab *nftables.Table
+	var err *syserr.AnnotatedError
+	if tabHandleBytes, ok := attrs[linux.NFTA_TABLE_HANDLE]; ok {
+		tabHandle, ok := tabHandleBytes.Uint64()
+		if !ok {
+			return syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Table handle attribute is malformed or not found"))
+		}
+
+		tab, err = nft.GetTableByHandle(family, uint64(tabHandle), uint32(ms.PortID))
+	} else {
+		tabNameBytes, ok := attrs[linux.NFTA_TABLE_NAME]
+		if !ok {
+			return syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Table name attribute is malformed or not found"))
+		}
+		tab, err = nft.GetTable(family, tabNameBytes.String(), uint32(ms.PortID))
+	}
+
+	if err != nil {
+		// Ignore ENOENT if DESTROY_TABLE is set
+		if err.GetError() == syserr.ErrNoFileOrDir && msgType == linux.NFT_MSG_DESTROYTABLE {
+			return nil
+		}
+		return err
+	}
+
+	// Don't delete the table if it is not empty and NLM_F_NONREC is set.
+	if hdr.Flags&linux.NLM_F_NONREC == linux.NLM_F_NONREC && tab.ChainCount() > 0 {
+		return syserr.NewAnnotatedError(syserr.ErrBusy, fmt.Sprintf("Nftables: Table with family: %d and name: %s already exists", int(family), tab.GetName()))
+	}
+
+	_, err = nft.DeleteTable(family, tab.GetName())
+	return err
+}
+
+// netLinkMessagePayloadSize returns the size of the netlink message payload.
 func netLinkMessagePayloadSize(h *linux.NetlinkMessageHeader) int {
 	return int(h.Length) - linux.NetlinkMessageHeaderSize
 }
 
+// hasAttr returns whether the given attribute key is present in the attribute map.
+func hasAttr(attrName uint16, attrs map[uint16]nlmsg.BytesView) bool {
+	_, ok := attrs[attrName]
+	return ok
+}
+
 // init registers the NETLINK_NETFILTER provider.
 func init() {
 	netlink.RegisterProvider(linux.NETLINK_NETFILTER, NewProtocol)

pkg/sentry/socket/netlink/nlmsg/BUILD

@@ -10,11 +10,15 @@ go_library(
     srcs = [
         "message.go",
     ],
-    visibility = ["//pkg/sentry:internal"],
+    visibility = [
+        "//pkg/sentry:internal",
+        "//pkg/tcpip/nftables:__subpackages__",
+    ],
     deps = [
         "//pkg/abi/linux",
         "//pkg/bits",
         "//pkg/hostarch",
+        "//pkg/log",
         "//pkg/marshal",
         "//pkg/marshal/primitive",
     ],

pkg/sentry/socket/netlink/nlmsg/message.go

@@ -22,6 +22,7 @@ import (
 	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/bits"
 	"gvisor.dev/gvisor/pkg/hostarch"
+	"gvisor.dev/gvisor/pkg/log"
 	"gvisor.dev/gvisor/pkg/marshal"
 	"gvisor.dev/gvisor/pkg/marshal/primitive"
 )
@@ -252,17 +253,20 @@ func (v AttrsView) ParseFirst() (hdr linux.NetlinkAttrHeader, value []byte, rest
 
 	hdrBytes, ok := b.Extract(linux.NetlinkAttrHeaderSize)
 	if !ok {
+		log.Debugf("Failed to parse netlink attributes at header stage")
 		return
 	}
 	hdr.UnmarshalUnsafe(hdrBytes)
 
 	value, ok = b.Extract(int(hdr.Length) - linux.NetlinkAttrHeaderSize)
 	if !ok {
+		log.Debugf("Failed to parse %d bytes after %d header bytes", int(hdr.Length)-linux.NetlinkAttrHeaderSize, linux.NetlinkAttrHeaderSize)
 		return
 	}
 
 	_, ok = b.Extract(alignPad(int(hdr.Length), linux.NLA_ALIGNTO))
 	if !ok {
+		log.Debugf("Failed to parse netlink attributes at aligning stage")
 		return
 	}
 
@@ -323,6 +327,17 @@ func (v *BytesView) Uint32() (uint32, bool) {
 	return uint32(val), true
 }
 
+// Uint64 converts the raw attribute value to uint64.
+func (v *BytesView) Uint64() (uint64, bool) {
+	attr := []byte(*v)
+	val := primitive.Uint64(0)
+	if len(attr) != val.SizeBytes() {
+		return 0, false
+	}
+	val.UnmarshalBytes(attr)
+	return uint64(val), true
+}
+
 // Int32 converts the raw attribute value to int32.
 func (v *BytesView) Int32() (int32, bool) {
 	attr := []byte(*v)

pkg/tcpip/nftables/BUILD

@@ -31,6 +31,7 @@ go_library(
         "//pkg/abi/linux",
         "//pkg/atomicbitops",
         "//pkg/rand",
+        "//pkg/sentry/socket/netlink/nlmsg",
         "//pkg/syserr",
         "//pkg/tcpip",
         "//pkg/tcpip/checksum",