From 8cc1c35bbdc5c9bd6b3965311497885ce72317a8 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Thu, 12 Dec 2019 15:48:24 -0800 Subject: Write simple ACCEPT rules to the filter table. This gets us closer to passing the iptables tests and opens up iptables so it can be worked on by multiple people. A few restrictions are enforced for security (i.e. we don't want to let users write a bunch of iptables rules and then just not enforce them): - Only the filter table is writable. - Only ACCEPT rules with no matching criteria can be added. --- pkg/abi/linux/netfilter.go | 82 +++++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 33 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/netfilter.go b/pkg/abi/linux/netfilter.go index 269ba5567..0bcb232de 100644 --- a/pkg/abi/linux/netfilter.go +++ b/pkg/abi/linux/netfilter.go @@ -42,6 +42,13 @@ const ( NF_RETURN = -NF_REPEAT - 1 ) +var VerdictStrings = map[int32]string{ + -NF_DROP - 1: "DROP", + -NF_ACCEPT - 1: "ACCEPT", + -NF_QUEUE - 1: "QUEUE", + NF_RETURN: "RETURN", +} + // Socket options. These correspond to values in // include/uapi/linux/netfilter_ipv4/ip_tables.h. const ( @@ -179,7 +186,7 @@ const SizeOfXTCounters = 16 // the user data. type XTEntryMatch struct { MatchSize uint16 - Name [XT_EXTENSION_MAXNAMELEN]byte + Name ExtensionName Revision uint8 // Data is omitted here because it would cause XTEntryMatch to be an // extra byte larger (see http://www.catb.org/esr/structure-packing/). @@ -199,7 +206,7 @@ const SizeOfXTEntryMatch = 32 // the user data. type XTEntryTarget struct { TargetSize uint16 - Name [XT_EXTENSION_MAXNAMELEN]byte + Name ExtensionName Revision uint8 // Data is omitted here because it would cause XTEntryTarget to be an // extra byte larger (see http://www.catb.org/esr/structure-packing/). @@ -226,9 +233,9 @@ const SizeOfXTStandardTarget = 40 // ErrorName. It corresponds to struct xt_error_target in // include/uapi/linux/netfilter/x_tables.h. type XTErrorTarget struct { - Target XTEntryTarget - ErrorName [XT_FUNCTION_MAXNAMELEN]byte - _ [2]byte + Target XTEntryTarget + Name ErrorName + _ [2]byte } // SizeOfXTErrorTarget is the size of an XTErrorTarget. @@ -237,7 +244,7 @@ const SizeOfXTErrorTarget = 64 // IPTGetinfo is the argument for the IPT_SO_GET_INFO sockopt. It corresponds // to struct ipt_getinfo in include/uapi/linux/netfilter_ipv4/ip_tables.h. type IPTGetinfo struct { - Name [XT_TABLE_MAXNAMELEN]byte + Name TableName ValidHooks uint32 HookEntry [NF_INET_NUMHOOKS]uint32 Underflow [NF_INET_NUMHOOKS]uint32 @@ -248,16 +255,11 @@ type IPTGetinfo struct { // SizeOfIPTGetinfo is the size of an IPTGetinfo. const SizeOfIPTGetinfo = 84 -// TableName returns the table name. -func (info *IPTGetinfo) TableName() string { - return tableName(info.Name[:]) -} - // IPTGetEntries is the argument for the IPT_SO_GET_ENTRIES sockopt. It // corresponds to struct ipt_get_entries in // include/uapi/linux/netfilter_ipv4/ip_tables.h. type IPTGetEntries struct { - Name [XT_TABLE_MAXNAMELEN]byte + Name TableName Size uint32 _ [4]byte // Entrytable is omitted here because it would cause IPTGetEntries to @@ -266,34 +268,22 @@ type IPTGetEntries struct { // Entrytable [0]IPTEntry } -// TableName returns the entries' table name. -func (entries *IPTGetEntries) TableName() string { - return tableName(entries.Name[:]) -} - // SizeOfIPTGetEntries is the size of an IPTGetEntries. const SizeOfIPTGetEntries = 40 -// KernelIPTGetEntries is identical to IPTEntry, but includes the Elems field. -// This struct marshaled via the binary package to write an KernelIPTGetEntries -// to userspace. +// KernelIPTGetEntries is identical to IPTGetEntries, but includes the +// Entrytable field. This struct marshaled via the binary package to write an +// KernelIPTGetEntries to userspace. type KernelIPTGetEntries struct { - Name [XT_TABLE_MAXNAMELEN]byte - Size uint32 - _ [4]byte + IPTGetEntries Entrytable []KernelIPTEntry } -// TableName returns the entries' table name. -func (entries *KernelIPTGetEntries) TableName() string { - return tableName(entries.Name[:]) -} - // IPTReplace is the argument for the IPT_SO_SET_REPLACE sockopt. It // corresponds to struct ipt_replace in // include/uapi/linux/netfilter_ipv4/ip_tables.h. type IPTReplace struct { - Name [XT_TABLE_MAXNAMELEN]byte + Name TableName ValidHooks uint32 NumEntries uint32 Size uint32 @@ -306,14 +296,40 @@ type IPTReplace struct { // Entries [0]IPTEntry } +type KernelIPTReplace struct { + IPTReplace + Entries [0]IPTEntry +} + // SizeOfIPTReplace is the size of an IPTReplace. const SizeOfIPTReplace = 96 -func tableName(name []byte) string { - for i, c := range name { +type ExtensionName [XT_EXTENSION_MAXNAMELEN]byte + +// String implements fmt.Stringer. +func (en ExtensionName) String() string { + return name(en[:]) +} + +type TableName [XT_TABLE_MAXNAMELEN]byte + +// String implements fmt.Stringer. +func (tn TableName) String() string { + return name(tn[:]) +} + +type ErrorName [XT_FUNCTION_MAXNAMELEN]byte + +// String implements fmt.Stringer. +func (fn ErrorName) String() string { + return name(fn[:]) +} + +func name(cstring []byte) string { + for i, c := range cstring { if c == 0 { - return string(name[:i]) + return string(cstring[:i]) } } - return string(name) + return string(cstring) } -- cgit v1.2.3 From 446a250996d9c946d9a5279f7fd081cc1be0bd11 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Wed, 8 Jan 2020 11:20:48 -0800 Subject: Comment cleanup. --- pkg/abi/linux/netfilter.go | 2 ++ pkg/sentry/socket/netfilter/netfilter.go | 2 -- pkg/tcpip/iptables/types.go | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/netfilter.go b/pkg/abi/linux/netfilter.go index 0bcb232de..35d66d622 100644 --- a/pkg/abi/linux/netfilter.go +++ b/pkg/abi/linux/netfilter.go @@ -42,6 +42,8 @@ const ( NF_RETURN = -NF_REPEAT - 1 ) +// VerdictStrings maps int verdicts to the strings they represent. It is used +// for debugging. var VerdictStrings = map[int32]string{ -NF_DROP - 1: "DROP", -NF_ACCEPT - 1: "ACCEPT", diff --git a/pkg/sentry/socket/netfilter/netfilter.go b/pkg/sentry/socket/netfilter/netfilter.go index b7867a576..347342f98 100644 --- a/pkg/sentry/socket/netfilter/netfilter.go +++ b/pkg/sentry/socket/netfilter/netfilter.go @@ -376,8 +376,6 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error { Size: replace.Size, }) ipt.Tables[replace.Name.String()] = table - // TODO: Do we need to worry about locking? We could write rules while - // packets traverse tables. stack.SetIPTables(ipt) return nil diff --git a/pkg/tcpip/iptables/types.go b/pkg/tcpip/iptables/types.go index fe0394a31..540f8c0b4 100644 --- a/pkg/tcpip/iptables/types.go +++ b/pkg/tcpip/iptables/types.go @@ -113,11 +113,11 @@ type Table struct { // Rules holds the rules that make up the table. Rules []Rule - // BuiltinChains maps builtin chains to their entrypoints. + // BuiltinChains maps builtin chains to their entrypoint rule in Rules. BuiltinChains map[Hook]int - // Underflows maps builtin chains to their underflow point (i.e. the - // rule to execute if the chain returns without a verdict). + // Underflows maps builtin chains to their underflow rule in Rules + // (i.e. the rule to execute if the chain returns without a verdict). Underflows map[Hook]int // UserChains holds user-defined chains for the keyed by name. Users -- cgit v1.2.3 From f26a576984052a235b63ec79081a8c4a8c8ffc00 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Wed, 8 Jan 2020 16:35:01 -0800 Subject: Addressed GH comments --- pkg/abi/linux/netfilter.go | 15 ++++-- pkg/sentry/socket/netfilter/netfilter.go | 87 +++++++++++++------------------- pkg/sentry/socket/netstack/netstack.go | 5 +- 3 files changed, 47 insertions(+), 60 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/netfilter.go b/pkg/abi/linux/netfilter.go index 35d66d622..c4f4ea0b1 100644 --- a/pkg/abi/linux/netfilter.go +++ b/pkg/abi/linux/netfilter.go @@ -298,6 +298,7 @@ type IPTReplace struct { // Entries [0]IPTEntry } +// KernelIPTEntry is identical to IPTReplace, but includes the Entries field. type KernelIPTReplace struct { IPTReplace Entries [0]IPTEntry @@ -306,28 +307,32 @@ type KernelIPTReplace struct { // SizeOfIPTReplace is the size of an IPTReplace. const SizeOfIPTReplace = 96 +// ExtensionName holds the name of a netfilter extension. type ExtensionName [XT_EXTENSION_MAXNAMELEN]byte // String implements fmt.Stringer. func (en ExtensionName) String() string { - return name(en[:]) + return goString(en[:]) } +// ExtensionName holds the name of a netfilter table. type TableName [XT_TABLE_MAXNAMELEN]byte // String implements fmt.Stringer. func (tn TableName) String() string { - return name(tn[:]) + return goString(tn[:]) } +// ExtensionName holds the name of a netfilter error. These can also hold +// user-defined chains. type ErrorName [XT_FUNCTION_MAXNAMELEN]byte // String implements fmt.Stringer. -func (fn ErrorName) String() string { - return name(fn[:]) +func (en ErrorName) String() string { + return goString(en[:]) } -func name(cstring []byte) string { +func goString(cstring []byte) string { for i, c := range cstring { if c == 0 { return string(cstring[:i]) diff --git a/pkg/sentry/socket/netfilter/netfilter.go b/pkg/sentry/socket/netfilter/netfilter.go index 347342f98..799865b03 100644 --- a/pkg/sentry/socket/netfilter/netfilter.go +++ b/pkg/sentry/socket/netfilter/netfilter.go @@ -53,7 +53,7 @@ func GetInfo(t *kernel.Task, ep tcpip.Endpoint, outPtr usermem.Addr) (linux.IPTG } // Find the appropriate table. - table, err := findTable(ep, info.Name.String()) + table, err := findTable(ep, info.Name) if err != nil { return linux.IPTGetinfo{}, err } @@ -84,7 +84,7 @@ func GetEntries(t *kernel.Task, ep tcpip.Endpoint, outPtr usermem.Addr, outLen i } // Find the appropriate table. - table, err := findTable(ep, userEntries.Name.String()) + table, err := findTable(ep, userEntries.Name) if err != nil { return linux.KernelIPTGetEntries{}, err } @@ -96,19 +96,19 @@ func GetEntries(t *kernel.Task, ep tcpip.Endpoint, outPtr usermem.Addr, outLen i return linux.KernelIPTGetEntries{}, err } if binary.Size(entries) > uintptr(outLen) { - log.Infof("Insufficient GetEntries output size: %d", uintptr(outLen)) + log.Warningf("Insufficient GetEntries output size: %d", uintptr(outLen)) return linux.KernelIPTGetEntries{}, syserr.ErrInvalidArgument } return entries, nil } -func findTable(ep tcpip.Endpoint, tableName string) (iptables.Table, *syserr.Error) { +func findTable(ep tcpip.Endpoint, tablename linux.TableName) (iptables.Table, *syserr.Error) { ipt, err := ep.IPTables() if err != nil { return iptables.Table{}, syserr.FromError(err) } - table, ok := ipt.Tables[tableName] + table, ok := ipt.Tables[tablename.String()] if !ok { return iptables.Table{}, syserr.ErrInvalidArgument } @@ -138,17 +138,17 @@ func FillDefaultIPTables(stack *stack.Stack) { // format expected by the iptables tool. Linux stores each table as a binary // blob that can only be traversed by parsing a bit, reading some offsets, // jumping to those offsets, parsing again, etc. -func convertNetstackToBinary(name string, table iptables.Table) (linux.KernelIPTGetEntries, metadata, *syserr.Error) { +func convertNetstackToBinary(tablename string, table iptables.Table) (linux.KernelIPTGetEntries, metadata, *syserr.Error) { // Return values. var entries linux.KernelIPTGetEntries var meta metadata // The table name has to fit in the struct. - if linux.XT_TABLE_MAXNAMELEN < len(name) { - log.Infof("Table name too long.") + if linux.XT_TABLE_MAXNAMELEN < len(tablename) { + log.Warningf("Table name %q too long.", tablename) return linux.KernelIPTGetEntries{}, metadata{}, syserr.ErrInvalidArgument } - copy(entries.Name[:], name) + copy(entries.Name[:], tablename) for ruleIdx, rule := range table.Rules { // Is this a chain entry point? @@ -273,11 +273,12 @@ func translateToStandardVerdict(val int32) (iptables.Verdict, *syserr.Error) { case -linux.NF_DROP - 1: return iptables.Drop, nil case -linux.NF_QUEUE - 1: - log.Infof("Unsupported iptables verdict QUEUE.") + log.Warningf("Unsupported iptables verdict QUEUE.") case linux.NF_RETURN: - log.Infof("Unsupported iptables verdict RETURN.") + log.Warningf("Unsupported iptables verdict RETURN.") + default: + log.Warningf("Unknown iptables verdict %d.", val) } - log.Infof("Unknown iptables verdict %d.", val) return iptables.Invalid, syserr.ErrInvalidArgument } @@ -288,7 +289,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error { // Get the basic rules data (struct ipt_replace). if len(optVal) < linux.SizeOfIPTReplace { - log.Infof("netfilter.SetEntries: optVal has insufficient size for replace %d", len(optVal)) + log.Warningf("netfilter.SetEntries: optVal has insufficient size for replace %d", len(optVal)) return syserr.ErrInvalidArgument } var replace linux.IPTReplace @@ -302,7 +303,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error { case iptables.TablenameFilter: table = iptables.EmptyFilterTable() default: - log.Infof(fmt.Sprintf("We don't yet support writing to the %q table (gvisor.dev/issue/170)", replace.Name.String())) + log.Warningf("We don't yet support writing to the %q table (gvisor.dev/issue/170)", replace.Name.String()) return syserr.ErrInvalidArgument } @@ -312,7 +313,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error { for entryIdx := uint32(0); entryIdx < replace.NumEntries; entryIdx++ { // Get the struct ipt_entry. if len(optVal) < linux.SizeOfIPTEntry { - log.Infof("netfilter: optVal has insufficient size for entry %d", len(optVal)) + log.Warningf("netfilter: optVal has insufficient size for entry %d", len(optVal)) return syserr.ErrInvalidArgument } var entry linux.IPTEntry @@ -328,7 +329,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error { // filtering. We reject any nonzero IPTIP values for now. emptyIPTIP := linux.IPTIP{} if entry.IP != emptyIPTIP { - log.Infof("netfilter: non-empty struct iptip found") + log.Warningf("netfilter: non-empty struct iptip found") return syserr.ErrInvalidArgument } @@ -358,11 +359,11 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error { } } if ruleIdx := table.BuiltinChains[hk]; ruleIdx == iptables.HookUnset { - log.Infof("Hook %v is unset.", hk) + log.Warningf("Hook %v is unset.", hk) return syserr.ErrInvalidArgument } if ruleIdx := table.Underflows[hk]; ruleIdx == iptables.HookUnset { - log.Infof("Underflow %v is unset.", hk) + log.Warningf("Underflow %v is unset.", hk) return syserr.ErrInvalidArgument } } @@ -385,7 +386,7 @@ func SetEntries(stack *stack.Stack, optVal []byte) *syserr.Error { // along with the number of bytes it occupies in optVal. func parseTarget(optVal []byte) (iptables.Target, uint32, *syserr.Error) { if len(optVal) < linux.SizeOfXTEntryTarget { - log.Infof("netfilter: optVal has insufficient size for entry target %d", len(optVal)) + log.Warningf("netfilter: optVal has insufficient size for entry target %d", len(optVal)) return nil, 0, syserr.ErrInvalidArgument } var target linux.XTEntryTarget @@ -395,14 +396,14 @@ func parseTarget(optVal []byte) (iptables.Target, uint32, *syserr.Error) { case "": // Standard target. if len(optVal) < linux.SizeOfXTStandardTarget { - log.Infof("netfilter.SetEntries: optVal has insufficient size for standard target %d", len(optVal)) + log.Warningf("netfilter.SetEntries: optVal has insufficient size for standard target %d", len(optVal)) return nil, 0, syserr.ErrInvalidArgument } - var target linux.XTStandardTarget + var standardTarget linux.XTStandardTarget buf = optVal[:linux.SizeOfXTStandardTarget] - binary.Unmarshal(buf, usermem.ByteOrder, &target) + binary.Unmarshal(buf, usermem.ByteOrder, &standardTarget) - verdict, err := translateToStandardVerdict(target.Verdict) + verdict, err := translateToStandardVerdict(standardTarget.Verdict) if err != nil { return nil, 0, err } @@ -424,9 +425,9 @@ func parseTarget(optVal []byte) (iptables.Target, uint32, *syserr.Error) { log.Infof("netfilter.SetEntries: optVal has insufficient size for error target %d", len(optVal)) return nil, 0, syserr.ErrInvalidArgument } - var target linux.XTErrorTarget + var errorTarget linux.XTErrorTarget buf = optVal[:linux.SizeOfXTErrorTarget] - binary.Unmarshal(buf, usermem.ByteOrder, &target) + binary.Unmarshal(buf, usermem.ByteOrder, &errorTarget) // Error targets are used in 2 cases: // * An actual error case. These rules have an error @@ -435,11 +436,11 @@ func parseTarget(optVal []byte) (iptables.Target, uint32, *syserr.Error) { // somehow fall through every rule. // * To mark the start of a user defined chain. These // rules have an error with the name of the chain. - switch target.Name.String() { + switch errorTarget.Name.String() { case errorTargetName: return iptables.PanicTarget{}, linux.SizeOfXTErrorTarget, nil default: - log.Infof("Unknown error target %q doesn't exist or isn't supported yet.", target.Name.String()) + log.Infof("Unknown error target %q doesn't exist or isn't supported yet.", errorTarget.Name.String()) return nil, 0, syserr.ErrInvalidArgument } } @@ -449,22 +450,6 @@ func parseTarget(optVal []byte) (iptables.Target, uint32, *syserr.Error) { return nil, 0, syserr.ErrInvalidArgument } -func chainNameFromHook(hook int) string { - switch hook { - case linux.NF_INET_PRE_ROUTING: - return iptables.ChainNamePrerouting - case linux.NF_INET_LOCAL_IN: - return iptables.ChainNameInput - case linux.NF_INET_FORWARD: - return iptables.ChainNameForward - case linux.NF_INET_LOCAL_OUT: - return iptables.ChainNameOutput - case linux.NF_INET_POST_ROUTING: - return iptables.ChainNamePostrouting - } - panic(fmt.Sprintf("Unknown hook %d does not correspond to a builtin chain")) -} - func hookFromLinux(hook int) iptables.Hook { switch hook { case linux.NF_INET_PRE_ROUTING: @@ -489,7 +474,7 @@ func printReplace(optVal []byte) { replaceBuf := optVal[:linux.SizeOfIPTReplace] optVal = optVal[linux.SizeOfIPTReplace:] binary.Unmarshal(replaceBuf, usermem.ByteOrder, &replace) - log.Infof("kevin: Replacing table %q: %+v", replace.Name.String(), replace) + log.Infof("Replacing table %q: %+v", replace.Name.String(), replace) // Read in the list of entries at the end of replace. var totalOffset uint16 @@ -497,33 +482,33 @@ func printReplace(optVal []byte) { var entry linux.IPTEntry entryBuf := optVal[:linux.SizeOfIPTEntry] binary.Unmarshal(entryBuf, usermem.ByteOrder, &entry) - log.Infof("kevin: Entry %d (total offset %d): %+v", entryIdx, totalOffset, entry) + log.Infof("Entry %d (total offset %d): %+v", entryIdx, totalOffset, entry) totalOffset += entry.NextOffset if entry.TargetOffset == linux.SizeOfIPTEntry { - log.Infof("kevin: Entry has no matches.") + log.Infof("Entry has no matches.") } else { - log.Infof("kevin: Entry has matches.") + log.Infof("Entry has matches.") } var target linux.XTEntryTarget targetBuf := optVal[entry.TargetOffset : entry.TargetOffset+linux.SizeOfXTEntryTarget] binary.Unmarshal(targetBuf, usermem.ByteOrder, &target) - log.Infof("kevin: Target named %q: %+v", target.Name.String(), target) + log.Infof("Target named %q: %+v", target.Name.String(), target) switch target.Name.String() { case "": var standardTarget linux.XTStandardTarget stBuf := optVal[entry.TargetOffset : entry.TargetOffset+linux.SizeOfXTStandardTarget] binary.Unmarshal(stBuf, usermem.ByteOrder, &standardTarget) - log.Infof("kevin: Standard target with verdict %q (%d).", linux.VerdictStrings[standardTarget.Verdict], standardTarget.Verdict) + log.Infof("Standard target with verdict %q (%d).", linux.VerdictStrings[standardTarget.Verdict], standardTarget.Verdict) case errorTargetName: var errorTarget linux.XTErrorTarget etBuf := optVal[entry.TargetOffset : entry.TargetOffset+linux.SizeOfXTErrorTarget] binary.Unmarshal(etBuf, usermem.ByteOrder, &errorTarget) - log.Infof("kevin: Error target with name %q.", errorTarget.Name.String()) + log.Infof("Error target with name %q.", errorTarget.Name.String()) default: - log.Infof("kevin: Unknown target type.") + log.Infof("Unknown target type.") } optVal = optVal[entry.NextOffset:] diff --git a/pkg/sentry/socket/netstack/netstack.go b/pkg/sentry/socket/netstack/netstack.go index 8c07eef4b..cd3dd1a53 100644 --- a/pkg/sentry/socket/netstack/netstack.go +++ b/pkg/sentry/socket/netstack/netstack.go @@ -1368,10 +1368,7 @@ func (s *SocketOperations) SetSockOpt(t *kernel.Task, level int, name int, optVa return syserr.ErrNoDevice } // Stack must be a netstack stack. - if err := netfilter.SetEntries(stack.(*Stack).Stack, optVal); err != nil { - return err - } - return nil + return netfilter.SetEntries(stack.(*Stack).Stack, optVal) case linux.IPT_SO_SET_ADD_COUNTERS: // TODO(gvisor.dev/issue/170): Counter support. -- cgit v1.2.3 From ae060a63d9ad1bfb65b84a2ccbaf2893c5a50b76 Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Wed, 8 Jan 2020 17:30:08 -0800 Subject: More GH comments. --- pkg/abi/linux/netfilter.go | 6 +++--- pkg/sentry/socket/netfilter/netfilter.go | 8 ++++---- pkg/tcpip/iptables/BUILD | 5 ++++- pkg/tcpip/iptables/iptables.go | 6 +++--- pkg/tcpip/iptables/targets.go | 16 +++++++++++----- 5 files changed, 25 insertions(+), 16 deletions(-) (limited to 'pkg/abi') diff --git a/pkg/abi/linux/netfilter.go b/pkg/abi/linux/netfilter.go index c4f4ea0b1..33fcc6c95 100644 --- a/pkg/abi/linux/netfilter.go +++ b/pkg/abi/linux/netfilter.go @@ -298,7 +298,7 @@ type IPTReplace struct { // Entries [0]IPTEntry } -// KernelIPTEntry is identical to IPTReplace, but includes the Entries field. +// KernelIPTReplace is identical to IPTReplace, but includes the Entries field. type KernelIPTReplace struct { IPTReplace Entries [0]IPTEntry @@ -315,7 +315,7 @@ func (en ExtensionName) String() string { return goString(en[:]) } -// ExtensionName holds the name of a netfilter table. +// TableName holds the name of a netfilter table. type TableName [XT_TABLE_MAXNAMELEN]byte // String implements fmt.Stringer. @@ -323,7 +323,7 @@ func (tn TableName) String() string { return goString(tn[:]) } -// ExtensionName holds the name of a netfilter error. These can also hold +// ErrorName holds the name of a netfilter error. These can also hold // user-defined chains. type ErrorName [XT_FUNCTION_MAXNAMELEN]byte diff --git a/pkg/sentry/socket/netfilter/netfilter.go b/pkg/sentry/socket/netfilter/netfilter.go index 799865b03..60bb30a9f 100644 --- a/pkg/sentry/socket/netfilter/netfilter.go +++ b/pkg/sentry/socket/netfilter/netfilter.go @@ -210,8 +210,8 @@ func marshalTarget(target iptables.Target) []byte { return marshalStandardTarget(iptables.Accept) case iptables.UnconditionalDropTarget: return marshalStandardTarget(iptables.Drop) - case iptables.PanicTarget: - return marshalPanicTarget() + case iptables.ErrorTarget: + return marshalErrorTarget() default: panic(fmt.Errorf("unknown target of type %T", target)) } @@ -230,7 +230,7 @@ func marshalStandardTarget(verdict iptables.Verdict) []byte { return binary.Marshal(ret, usermem.ByteOrder, target) } -func marshalPanicTarget() []byte { +func marshalErrorTarget() []byte { // This is an error target named error target := linux.XTErrorTarget{ Target: linux.XTEntryTarget{ @@ -438,7 +438,7 @@ func parseTarget(optVal []byte) (iptables.Target, uint32, *syserr.Error) { // rules have an error with the name of the chain. switch errorTarget.Name.String() { case errorTargetName: - return iptables.PanicTarget{}, linux.SizeOfXTErrorTarget, nil + return iptables.ErrorTarget{}, linux.SizeOfXTErrorTarget, nil default: log.Infof("Unknown error target %q doesn't exist or isn't supported yet.", errorTarget.Name.String()) return nil, 0, syserr.ErrInvalidArgument diff --git a/pkg/tcpip/iptables/BUILD b/pkg/tcpip/iptables/BUILD index cc5f531e2..64769c333 100644 --- a/pkg/tcpip/iptables/BUILD +++ b/pkg/tcpip/iptables/BUILD @@ -11,5 +11,8 @@ go_library( ], importpath = "gvisor.dev/gvisor/pkg/tcpip/iptables", visibility = ["//visibility:public"], - deps = ["//pkg/tcpip/buffer"], + deps = [ + "//pkg/log", + "//pkg/tcpip/buffer", + ], ) diff --git a/pkg/tcpip/iptables/iptables.go b/pkg/tcpip/iptables/iptables.go index 9e7005374..db0450a21 100644 --- a/pkg/tcpip/iptables/iptables.go +++ b/pkg/tcpip/iptables/iptables.go @@ -45,7 +45,7 @@ func DefaultTables() IPTables { Rule{Target: UnconditionalAcceptTarget{}}, Rule{Target: UnconditionalAcceptTarget{}}, Rule{Target: UnconditionalAcceptTarget{}}, - Rule{Target: PanicTarget{}}, + Rule{Target: ErrorTarget{}}, }, BuiltinChains: map[Hook]int{ Prerouting: 0, @@ -65,7 +65,7 @@ func DefaultTables() IPTables { Rules: []Rule{ Rule{Target: UnconditionalAcceptTarget{}}, Rule{Target: UnconditionalAcceptTarget{}}, - Rule{Target: PanicTarget{}}, + Rule{Target: ErrorTarget{}}, }, BuiltinChains: map[Hook]int{ Prerouting: 0, @@ -82,7 +82,7 @@ func DefaultTables() IPTables { Rule{Target: UnconditionalAcceptTarget{}}, Rule{Target: UnconditionalAcceptTarget{}}, Rule{Target: UnconditionalAcceptTarget{}}, - Rule{Target: PanicTarget{}}, + Rule{Target: ErrorTarget{}}, }, BuiltinChains: map[Hook]int{ Input: 0, diff --git a/pkg/tcpip/iptables/targets.go b/pkg/tcpip/iptables/targets.go index 2c3598e3d..d65ed8df5 100644 --- a/pkg/tcpip/iptables/targets.go +++ b/pkg/tcpip/iptables/targets.go @@ -16,7 +16,10 @@ package iptables -import "gvisor.dev/gvisor/pkg/tcpip/buffer" +import ( + "gvisor.dev/gvisor/pkg/log" + "gvisor.dev/gvisor/pkg/tcpip/buffer" +) // UnconditionalAcceptTarget accepts all packets. type UnconditionalAcceptTarget struct{} @@ -34,10 +37,13 @@ func (UnconditionalDropTarget) Action(packet buffer.VectorisedView) (Verdict, st return Drop, "" } -// PanicTarget just panics. It represents a target that should be unreachable. -type PanicTarget struct{} +// ErrorTarget logs an error and drops the packet. It represents a target that +// should be unreachable. +type ErrorTarget struct{} // Actions implements Target.Action. -func (PanicTarget) Action(packet buffer.VectorisedView) (Verdict, string) { - panic("PanicTarget triggered.") +func (ErrorTarget) Action(packet buffer.VectorisedView) (Verdict, string) { + log.Warningf("ErrorTarget triggered.") + return Drop, "" + } -- cgit v1.2.3