Skip to content

Commit 957abbe

Browse files
kerumetogvisor-bot
authored andcommitted
Updated nftables package to return syserr.Error (POSIX) errors
Needed to update nftables package to return POSIX errors for better integration with netfilter sockets. This allows for more precise error handling and propagation. PiperOrigin-RevId: 776255695
1 parent 1b63d8b commit 957abbe

File tree

18 files changed

+297
-226
lines changed

18 files changed

+297
-226
lines changed

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

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
package netfilter
1717

1818
import (
19+
"fmt"
20+
1921
"gvisor.dev/gvisor/pkg/abi/linux"
2022
"gvisor.dev/gvisor/pkg/context"
2123
"gvisor.dev/gvisor/pkg/log"
@@ -91,23 +93,30 @@ func (p *Protocol) ProcessMessage(ctx context.Context, s *netlink.Socket, msg *n
9193
// TODO: b/421437663 - Match the message type and call the appropriate Nftables function.
9294
switch msgType {
9395
case linux.NFT_MSG_NEWTABLE:
94-
return p.newTable(nft, attrs, family, hdr.Flags)
96+
if err := p.newTable(nft, attrs, family, hdr.Flags); err != nil {
97+
log.Debugf("Nftables new table error: %s", err)
98+
return err.GetError()
99+
}
100+
return nil
95101
case linux.NFT_MSG_GETTABLE:
96-
return p.getTable(nft, attrs, family, hdr.Flags, ms)
102+
if err := p.getTable(nft, attrs, family, hdr.Flags, ms); err != nil {
103+
log.Debugf("Nftables get table error: %s", err)
104+
return err.GetError()
105+
}
106+
return nil
97107
default:
98108
log.Debugf("Unsupported message type: %d", msgType)
99-
return syserr.ErrInvalidArgument
109+
return syserr.ErrNotSupported
100110
}
101111
}
102112

103113
// newTable creates a new table for the given family.
104-
func (p *Protocol) newTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, flags uint16) *syserr.Error {
114+
func (p *Protocol) newTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, flags uint16) *syserr.AnnotatedError {
105115
// TODO: b/421437663 - Handle the case where the table name is set to empty string.
106116
// The table name is required.
107117
tabNameBytes, ok := attrs[linux.NFTA_TABLE_NAME]
108118
if !ok {
109-
log.Debugf("Nftables: Table name attribute is malformed or not found")
110-
return syserr.ErrInvalidArgument
119+
return syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Table name attribute is malformed or not found"))
111120
}
112121

113122
var dormant bool
@@ -117,47 +126,42 @@ func (p *Protocol) newTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.Bytes
117126
}
118127

119128
tab, err := nft.GetTable(family, tabNameBytes.String())
129+
if err != nil && err.GetError() != syserr.ErrNoFileOrDir {
130+
return err
131+
}
120132

121133
// If a table already exists, only update its dormant flags if NLM_F_EXCL and NLM_F_REPLACE
122134
// are not set. From net/netfilter/nf_tables_api.c:nf_tables_newtable:nf_tables_updtable
123-
if tab != nil && err == nil {
135+
if tab != nil {
124136
if flags&linux.NLM_F_EXCL == linux.NLM_F_EXCL {
125-
log.Debugf("Nftables: Table with name: %s already exists", tabNameBytes.String())
126-
return syserr.ErrExists
137+
return syserr.NewAnnotatedError(syserr.ErrExists, fmt.Sprintf("Nftables: Table with name: %s already exists", tabNameBytes.String()))
127138
}
128139

129140
if flags&linux.NLM_F_REPLACE == linux.NLM_F_REPLACE {
130-
log.Debugf("Nftables: Table with name: %s already exists and NLM_F_REPLACE is not supported", tabNameBytes.String())
131-
return syserr.ErrNotSupported
141+
return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("Nftables: Table with name: %s already exists and NLM_F_REPLACE is not supported", tabNameBytes.String()))
132142
}
133143
} else {
134-
// There does not seem to be a way to add comments to a table using the nft binary.
135144
tab, err = nft.CreateTable(family, tabNameBytes.String())
136145
if err != nil {
137-
log.Debugf("Nftables: Failed to create table with name: %s. Error: %s", tabNameBytes.String(), err.Error())
138-
// If there is an error, it is not a duplicate error (checked above).
139-
return syserr.ErrInvalidArgument
146+
return err
140147
}
141148
}
142149

143150
tab.SetDormant(dormant)
144151
return nil
145152
}
146153

147-
// getTable returns a table for the given family. Returns nil on success and
148-
// a sys.error on failure.
149-
func (p *Protocol) getTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, flags uint16, ms *nlmsg.MessageSet) *syserr.Error {
154+
// getTable returns a table for the given family.
155+
func (p *Protocol) getTable(nft *nftables.NFTables, attrs map[uint16]nlmsg.BytesView, family stack.AddressFamily, flags uint16, ms *nlmsg.MessageSet) *syserr.AnnotatedError {
150156
// The table name is required.
151157
tabNameBytes, ok := attrs[linux.NFTA_TABLE_NAME]
152158
if !ok {
153-
log.Debugf("Nftables: Table name attribute is malformed or not found")
154-
return syserr.ErrInvalidArgument
159+
return syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("Nftables: Table name attribute is malformed or not found"))
155160
}
156161

157162
tab, err := nft.GetTable(family, tabNameBytes.String())
158163
if err != nil {
159-
log.Debugf("Nftables: ENOENT for table with name: %s", tabNameBytes.String())
160-
return syserr.ErrNoFileOrDir
164+
return err
161165
}
162166

163167
tabName := tab.GetName()

pkg/syserr/syserr.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,27 @@ func (e *Error) ToLinux() errno.Errno {
125125
return e.errno
126126
}
127127

128+
// AnnotatedError represents an error with an additional message.
129+
type AnnotatedError struct {
130+
error *Error
131+
message string
132+
}
133+
134+
// Error implements Error() for the error interface
135+
func (e *AnnotatedError) Error() string {
136+
return fmt.Sprintf("%s: %s", e.error.String(), e.message)
137+
}
138+
139+
// NewAnnotatedError creates a new AnnotatedError with the given error and message.
140+
func NewAnnotatedError(error *Error, message string) *AnnotatedError {
141+
return &AnnotatedError{error: error, message: message}
142+
}
143+
144+
// GetError returns the underlying error.
145+
func (e *AnnotatedError) GetError() *Error {
146+
return e.error
147+
}
148+
128149
// TODO(b/34162363): Remove or replace most of these errors.
129150
//
130151
// Some of the errors should be replaced with package specific errors and

pkg/tcpip/nftables/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ go_library(
3131
"//pkg/abi/linux",
3232
"//pkg/atomicbitops",
3333
"//pkg/rand",
34+
"//pkg/syserr",
3435
"//pkg/tcpip",
3536
"//pkg/tcpip/checksum",
3637
"//pkg/tcpip/header",

pkg/tcpip/nftables/nft_bitwise.go

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"fmt"
2020

2121
"gvisor.dev/gvisor/pkg/abi/linux"
22+
"gvisor.dev/gvisor/pkg/syserr"
2223
"gvisor.dev/gvisor/pkg/tcpip/stack"
2324
)
2425

@@ -65,30 +66,30 @@ type bitwise struct {
6566
}
6667

6768
// newBitwiseBool creates a new bitwise boolean operation.
68-
func newBitwiseBool(sreg, dreg uint8, mask, xor []byte) (*bitwise, error) {
69+
func newBitwiseBool(sreg, dreg uint8, mask, xor []byte) (*bitwise, *syserr.AnnotatedError) {
6970
if isVerdictRegister(sreg) || isVerdictRegister(dreg) {
70-
return nil, fmt.Errorf("bitwise operation cannot use verdict register as source or destination")
71+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("bitwise operation does not support verdict register as source or destination register"))
7172
}
7273
blen := len(mask)
7374
if blen != len(xor) {
74-
return nil, fmt.Errorf("bitwise boolean operation mask and xor must be the same length")
75+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("bitwise boolean operation mask and xor data lengths must be the same"))
7576
}
7677
if blen > linux.NFT_REG_SIZE || (blen > linux.NFT_REG32_SIZE && (is4ByteRegister(sreg) || is4ByteRegister(dreg))) {
77-
return nil, fmt.Errorf("bitwise operation length %d is too long for source register %d, destination register %d", blen, sreg, dreg)
78+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("bitwise boolean operation cannot use more than %d bytes", linux.NFT_REG_SIZE))
7879
}
7980
return &bitwise{sreg: sreg, dreg: dreg, bop: linux.NFT_BITWISE_BOOL, blen: uint8(blen), mask: newBytesData(mask), xor: newBytesData(xor)}, nil
8081
}
8182

8283
// newBitwiseShift creates a new bitwise shift operation.
83-
func newBitwiseShift(sreg, dreg, blen uint8, shift uint32, right bool) (*bitwise, error) {
84+
func newBitwiseShift(sreg, dreg, blen uint8, shift uint32, right bool) (*bitwise, *syserr.AnnotatedError) {
8485
if isVerdictRegister(sreg) || isVerdictRegister(dreg) {
85-
return nil, fmt.Errorf("bitwise operation cannot use verdict register as source or destination")
86+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("bitwise shift operation does not support verdict register as source or destination register"))
8687
}
8788
if blen > linux.NFT_REG_SIZE || (blen > linux.NFT_REG32_SIZE && (is4ByteRegister(sreg) || is4ByteRegister(dreg))) {
88-
return nil, fmt.Errorf("bitwise operation length %d is too long for source register %d, destination register %d", blen, sreg, dreg)
89+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("bitwise operation length %d is too long for source register %d, destination register %d", blen, sreg, dreg))
8990
}
9091
if shift >= bitshiftLimit {
91-
return nil, fmt.Errorf("bitwise operation shift %d must be less than %d", shift, bitshiftLimit)
92+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("bitwise operation shift %d must be less than %d", shift, bitshiftLimit))
9293
}
9394
bop := bitwiseOp(linux.NFT_BITWISE_LSHIFT)
9495
if right {
@@ -180,11 +181,12 @@ func (op bitwise) evaluate(regs *registerSet, pkt *stack.PacketBuffer, rule *Rul
180181
if op.bop == linux.NFT_BITWISE_BOOL {
181182
evaluateBitwiseBool(sregBuf, dregBuf, op.mask.data, op.xor.data)
182183
return
184+
}
185+
186+
if op.bop == linux.NFT_BITWISE_LSHIFT {
187+
evaluateBitwiseLshift(sregBuf, dregBuf, op.shift)
183188
} else {
184-
if op.bop == linux.NFT_BITWISE_LSHIFT {
185-
evaluateBitwiseLshift(sregBuf, dregBuf, op.shift)
186-
} else {
187-
evaluateBitwiseRshift(sregBuf, dregBuf, op.shift)
188-
}
189+
evaluateBitwiseRshift(sregBuf, dregBuf, op.shift)
189190
}
191+
190192
}

pkg/tcpip/nftables/nft_byteorder.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"fmt"
2020

2121
"gvisor.dev/gvisor/pkg/abi/linux"
22+
"gvisor.dev/gvisor/pkg/syserr"
2223
"gvisor.dev/gvisor/pkg/tcpip/stack"
2324
)
2425

@@ -54,35 +55,35 @@ func (bop byteorderOp) String() string {
5455
}
5556

5657
// validateByteorderOp ensures the byteorder operator is valid.
57-
func validateByteorderOp(bop byteorderOp) error {
58+
func validateByteorderOp(bop byteorderOp) *syserr.AnnotatedError {
5859
switch bop {
5960
// Supported operators.
6061
case linux.NFT_BYTEORDER_NTOH, linux.NFT_BYTEORDER_HTON:
6162
return nil
6263
default:
63-
return fmt.Errorf("invalid byteorder operator: %d", int(bop))
64+
return syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("invalid byteorder operator: %d", int(bop)))
6465
}
6566
}
6667

6768
// newByteorder creates a new byteorder operation.
68-
func newByteorder(sreg, dreg uint8, bop byteorderOp, blen, size uint8) (*byteorder, error) {
69+
func newByteorder(sreg, dreg uint8, bop byteorderOp, blen, size uint8) (*byteorder, *syserr.AnnotatedError) {
6970
if isVerdictRegister(sreg) || isVerdictRegister(dreg) {
70-
return nil, fmt.Errorf("byteorder operation cannot use verdict register")
71+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("byteorder operation does not support verdict register as source or destination register"))
7172
}
7273
if err := validateByteorderOp(bop); err != nil {
7374
return nil, err
7475
}
7576
if blen > linux.NFT_REG_SIZE {
76-
return nil, fmt.Errorf("byteorder operation cannot have length greater than the max register size of %d bytes", linux.NFT_REG_SIZE)
77+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("byteorder operation cannot use more than %d bytes", linux.NFT_REG_SIZE))
7778
}
7879
if (is4ByteRegister(sreg) || is4ByteRegister(dreg)) && blen > linux.NFT_REG32_SIZE {
79-
return nil, fmt.Errorf("byteorder operation cannot have length greater than the max register size of %d bytes", linux.NFT_REG32_SIZE)
80+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("byteorder operation cannot use more than %d bytes", linux.NFT_REG32_SIZE))
8081
}
8182
if size > blen {
82-
return nil, fmt.Errorf("byteorder operation cannot have size greater than length")
83+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("byteorder operation cannot use more than %d bytes", blen))
8384
}
8485
if size != 2 && size != 4 && size != 8 {
85-
return nil, fmt.Errorf("byteorder operation size must be 2, 4, or 8 bytes")
86+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("byteorder operation size %d is not supported", size))
8687
}
8788
return &byteorder{sreg: sreg, dreg: dreg, bop: bop, blen: blen, size: size}, nil
8889
}

pkg/tcpip/nftables/nft_comparison.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"fmt"
2020

2121
"gvisor.dev/gvisor/pkg/abi/linux"
22+
"gvisor.dev/gvisor/pkg/syserr"
2223
"gvisor.dev/gvisor/pkg/tcpip/stack"
2324
)
2425

@@ -56,19 +57,19 @@ func (cop cmpOp) String() string {
5657
}
5758

5859
// validateComparisonOp ensures the comparison operator is valid.
59-
func validateComparisonOp(cop cmpOp) error {
60+
func validateComparisonOp(cop cmpOp) *syserr.AnnotatedError {
6061
switch cop {
6162
case linux.NFT_CMP_EQ, linux.NFT_CMP_NEQ, linux.NFT_CMP_LT, linux.NFT_CMP_LTE, linux.NFT_CMP_GT, linux.NFT_CMP_GTE:
6263
return nil
6364
default:
64-
return fmt.Errorf("invalid comparison operator: %d", int(cop))
65+
return syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("invalid comparison operator: %d", int(cop)))
6566
}
6667
}
6768

6869
// newComparison creates a new comparison operation.
69-
func newComparison(sreg uint8, op int, data []byte) (*comparison, error) {
70+
func newComparison(sreg uint8, op int, data []byte) (*comparison, *syserr.AnnotatedError) {
7071
if isVerdictRegister(sreg) {
71-
return nil, fmt.Errorf("comparison operation cannot use verdict register as source")
72+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("comparison operation does not support verdict register as source register"))
7273
}
7374
bytesData := newBytesData(data)
7475
if err := bytesData.validateRegister(sreg); err != nil {

pkg/tcpip/nftables/nft_immediate.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package nftables
1616

1717
import (
18+
"gvisor.dev/gvisor/pkg/syserr"
1819
"gvisor.dev/gvisor/pkg/tcpip/stack"
1920
)
2021

@@ -25,7 +26,7 @@ type immediate struct {
2526
}
2627

2728
// newImmediate creates a new immediate operation.
28-
func newImmediate(dreg uint8, data registerData) (*immediate, error) {
29+
func newImmediate(dreg uint8, data registerData) (*immediate, *syserr.AnnotatedError) {
2930
if err := data.validateRegister(dreg); err != nil {
3031
return nil, err
3132
}

pkg/tcpip/nftables/nft_metaload.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"fmt"
2020

2121
"gvisor.dev/gvisor/pkg/abi/linux"
22+
"gvisor.dev/gvisor/pkg/syserr"
2223
"gvisor.dev/gvisor/pkg/tcpip/header"
2324
"gvisor.dev/gvisor/pkg/tcpip/stack"
2425
)
@@ -111,7 +112,7 @@ var metaDataLengths = map[metaKey]int{
111112
}
112113

113114
// validateMetaKey ensures the meta key is valid.
114-
func validateMetaKey(key metaKey) error {
115+
func validateMetaKey(key metaKey) *syserr.AnnotatedError {
115116
switch key {
116117
case linux.NFT_META_LEN, linux.NFT_META_PROTOCOL, linux.NFT_META_NFPROTO,
117118
linux.NFT_META_L4PROTO, linux.NFT_META_SKUID, linux.NFT_META_SKGID,
@@ -120,20 +121,20 @@ func validateMetaKey(key metaKey) error {
120121

121122
return nil
122123
default:
123-
return fmt.Errorf("invalid meta key: %d", int(key))
124+
return syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("meta key %v is not supported", key))
124125
}
125126
}
126127

127128
// newMetaLoad creates a new metaLoad operation.
128-
func newMetaLoad(key metaKey, dreg uint8) (*metaLoad, error) {
129+
func newMetaLoad(key metaKey, dreg uint8) (*metaLoad, *syserr.AnnotatedError) {
129130
if isVerdictRegister(dreg) {
130-
return nil, fmt.Errorf("meta load operation cannot use verdict register as destination")
131+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("meta load operation does not support verdict register as destination register"))
131132
}
132133
if err := validateMetaKey(key); err != nil {
133134
return nil, err
134135
}
135136
if metaDataLengths[key] > 4 && !is16ByteRegister(dreg) {
136-
return nil, fmt.Errorf("meta load operation cannot use 4-byte register as destination for key %s", key)
137+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("meta load operation cannot use 4-byte register as destination for key %v", key))
137138
}
138139

139140
return &metaLoad{key: key, dreg: dreg}, nil

pkg/tcpip/nftables/nft_metaset.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"fmt"
1919

2020
"gvisor.dev/gvisor/pkg/abi/linux"
21+
"gvisor.dev/gvisor/pkg/syserr"
2122
"gvisor.dev/gvisor/pkg/tcpip"
2223
"gvisor.dev/gvisor/pkg/tcpip/stack"
2324
)
@@ -32,7 +33,7 @@ type metaSet struct {
3233
}
3334

3435
// checkMetaKeySetCompatible checks that the meta key is valid for meta set.
35-
func checkMetaKeySetCompatible(key metaKey) error {
36+
func checkMetaKeySetCompatible(key metaKey) *syserr.AnnotatedError {
3637
switch key {
3738
// Supported meta keys.
3839
case linux.NFT_META_PKTTYPE:
@@ -41,17 +42,17 @@ func checkMetaKeySetCompatible(key metaKey) error {
4142
case linux.NFT_META_MARK, linux.NFT_META_PRIORITY,
4243
linux.NFT_META_NFTRACE, linux.NFT_META_SECMARK:
4344

44-
return fmt.Errorf("meta key %v is not supported for meta set", key)
45+
return syserr.NewAnnotatedError(syserr.ErrNotSupported, fmt.Sprintf("meta key %v is not supported", key))
4546
// All other keys cannot be used with meta set (strictly for loading).
4647
default:
47-
return fmt.Errorf("meta key %v is not compatible with meta set", key)
48+
return syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("meta key %v is not supported for meta set", key))
4849
}
4950
}
5051

5152
// newMetaSet creates a new metaSet operation.
52-
func newMetaSet(key metaKey, sreg uint8) (*metaSet, error) {
53+
func newMetaSet(key metaKey, sreg uint8) (*metaSet, *syserr.AnnotatedError) {
5354
if isVerdictRegister(sreg) {
54-
return nil, fmt.Errorf("meta set operation cannot use verdict register as destination")
55+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("meta set operation does not support verdict register as source register"))
5556
}
5657
if err := validateMetaKey(key); err != nil {
5758
return nil, err
@@ -60,7 +61,7 @@ func newMetaSet(key metaKey, sreg uint8) (*metaSet, error) {
6061
return nil, err
6162
}
6263
if metaDataLengths[key] > 4 && !is16ByteRegister(sreg) {
63-
return nil, fmt.Errorf("meta load operation cannot use 4-byte register as destination for key %s", key)
64+
return nil, syserr.NewAnnotatedError(syserr.ErrInvalidArgument, fmt.Sprintf("meta load operation cannot use 4-byte register as destination for key %s", key))
6465
}
6566

6667
return &metaSet{key: key, sreg: sreg}, nil

0 commit comments

Comments
 (0)