From 4ce116dade32c22c76b03fbc2e593c57f84b261b Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Sat, 7 Dec 2019 11:16:29 +0100 Subject: [PATCH 1/9] attribute - unmarshal using netlink.AttributeDecoder --- attribute.go | 90 ++++++++++++++++++++++++++++------------------- attribute_test.go | 8 +++++ 2 files changed, 62 insertions(+), 36 deletions(-) diff --git a/attribute.go b/attribute.go index 66d936e..0942ee9 100644 --- a/attribute.go +++ b/attribute.go @@ -6,7 +6,6 @@ import ( "github.com/mdlayher/netlink" "github.com/pkg/errors" - "golang.org/x/sys/unix" ) // An Attribute is a copy of a netlink.Attribute that can be nested. @@ -138,6 +137,40 @@ func Uint64Bytes(u uint64) []byte { return d } +// decode fills the Attribute's Children field with Attributes +// obtained by exhausting ad. +func (a *Attribute) decode(ad *netlink.AttributeDecoder) error { + + // Wrap all netlink.Attributes into netfilter.Attributes to support nesting. + for ad.Next() { + + // Copy the netlink attribute's fields into the netfilter attribute. + nfa := Attribute{ + // Only consider the rightmost 14 bits for Type. + // ad.Type implicitly masks the Nested and NetByteOrder bits. + Type: ad.Type(), + Data: ad.Bytes(), + } + + // Boolean flags extracted from the two leftmost bits of Type. + nfa.Nested = (ad.TypeFlags() & netlink.Nested) != 0 + nfa.NetByteOrder = (ad.TypeFlags() & netlink.NetByteOrder) != 0 + + if nfa.NetByteOrder && nfa.Nested { + return errInvalidAttributeFlags + } + + // Unmarshal recursively if the netlink Nested flag is set. + if nfa.Nested { + ad.Nested(nfa.decode) + } + + a.Children = append(a.Children, nfa) + } + + return nil +} + // unmarshalAttributes returns an array of netfilter.Attributes decoded from // a byte array. This byte array should be taken from the netlink.Message's // Data payload after the nfHeaderLen offset. @@ -145,47 +178,31 @@ func unmarshalAttributes(b []byte) ([]Attribute, error) { // Obtain a list of parsed netlink attributes possibly holding // nested Netfilter attributes in their binary Data field. - attrs, err := netlink.UnmarshalAttributes(b) + ad, err := netlink.NewAttributeDecoder(b) if err != nil { return nil, errors.Wrap(err, errWrapNetlinkUnmarshalAttrs) } - var ra []Attribute + // Use the Children element of the Attribute to decode into. + // Attribute already has nested decoding implemented on the type. + var a Attribute - // Only allocate backing array when there are netlink attributes to decode. - if len(attrs) != 0 { - ra = make([]Attribute, 0, len(attrs)) + // Pre-allocate backing array when there are netlink attributes to decode. + if ad.Len() != 0 { + a.Children = make([]Attribute, 0, ad.Len()) } - // Wrap all netlink.Attributes into netfilter.Attributes to support nesting - for _, nla := range attrs { - - // Copy the netlink attribute's fields into the netfilter attribute. - nfa := Attribute{ - // Only consider the rightmost 14 bits for Type - Type: nla.Type & ^(uint16(unix.NLA_F_NESTED) | uint16(unix.NLA_F_NET_BYTEORDER)), - Data: nla.Data, - } - - // Boolean flags extracted from the two leftmost bits of Type - nfa.Nested = (nla.Type & uint16(unix.NLA_F_NESTED)) != 0 - nfa.NetByteOrder = (nla.Type & uint16(unix.NLA_F_NET_BYTEORDER)) != 0 - - if nfa.NetByteOrder && nfa.Nested { - return nil, errInvalidAttributeFlags - } - - // Unmarshal recursively if the netlink Nested flag is set - if nfa.Nested { - if nfa.Children, err = unmarshalAttributes(nla.Data); err != nil { - return nil, err - } - } + // Catch any errors encountered parsing netfilter structures. + if err := a.decode(ad); err != nil { + return nil, err + } - ra = append(ra, nfa) + // Catch any errors encountered by the netlink decoder. + if err := ad.Err(); err != nil { + return nil, err } - return ra, nil + return a.Children, nil } // marshalAttributes marshals a nested attribute structure into a byte slice. @@ -193,7 +210,7 @@ func unmarshalAttributes(b []byte) ([]Attribute, error) { // the nfHeaderLen offset. func marshalAttributes(attrs []Attribute) ([]byte, error) { - // netlink.Attribute to use as scratch buffer, requires a single allocation + // netlink.Attribute to use as scratch buffer minimize allocations. nla := netlink.Attribute{} // Output array, initialized to the length of the input array @@ -209,14 +226,15 @@ func marshalAttributes(attrs []Attribute) ([]byte, error) { // Type field to include it in the marshaling operation nla.Type = nfa.Type + // Embed the Nested and NetByteOrder flags into the netlink Type. switch { case nfa.Nested: - nla.Type = nla.Type | unix.NLA_F_NESTED + nla.Type |= netlink.Nested case nfa.NetByteOrder: - nla.Type = nla.Type | unix.NLA_F_NET_BYTEORDER + nla.Type |= netlink.NetByteOrder } - // Recursively marshal the attribute's children + // Recursively marshal the attribute's children. if nfa.Nested { nfnab, err := marshalAttributes(nfa.Children) if err != nil { diff --git a/attribute_test.go b/attribute_test.go index bd2b35b..c97644e 100644 --- a/attribute_test.go +++ b/attribute_test.go @@ -221,6 +221,14 @@ func TestAttributeUnmarshalErrors(t *testing.T) { b: []byte{1}, errWrap: errWrapNetlinkUnmarshalAttrs, }, + { + name: "invalid attribute flags on top-level attribute", + b: []byte{ + 8, 0, 0, 192, // 192 = nested + netByteOrder + 0, 0, 0, 0, + }, + err: errInvalidAttributeFlags, + }, { name: "invalid attribute flags on nested attribute", b: []byte{ From 4b72800c1c25c006bf7b675cdce7cb2cb84407f2 Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Sat, 7 Dec 2019 12:05:17 +0100 Subject: [PATCH 2/9] attribute.go - (un)marshalAttributes: switch to netlink.Attribute{En,De}coder --- attribute.go | 75 ++++++++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/attribute.go b/attribute.go index 0942ee9..0b8420f 100644 --- a/attribute.go +++ b/attribute.go @@ -171,6 +171,35 @@ func (a *Attribute) decode(ad *netlink.AttributeDecoder) error { return nil } +// encode returns a function that takes an AttributeEncoder and returns error. +// This function can be passed to AttributeEncoder.Nested for recursively +// encoding Attributes. +func (a *Attribute) encode(attrs []Attribute) func(*netlink.AttributeEncoder) error { + + return func(ae *netlink.AttributeEncoder) error { + + for _, nfa := range attrs { + + if nfa.NetByteOrder && nfa.Nested { + return errInvalidAttributeFlags + } + + if nfa.NetByteOrder { + nfa.Type |= netlink.NetByteOrder + } + + if nfa.Nested { + ae.Nested(nfa.Type, nfa.encode(nfa.Children)) + continue + } + + ae.Bytes(nfa.Type, nfa.Data) + } + + return nil + } +} + // unmarshalAttributes returns an array of netfilter.Attributes decoded from // a byte array. This byte array should be taken from the netlink.Message's // Data payload after the nfHeaderLen offset. @@ -210,45 +239,17 @@ func unmarshalAttributes(b []byte) ([]Attribute, error) { // the nfHeaderLen offset. func marshalAttributes(attrs []Attribute) ([]byte, error) { - // netlink.Attribute to use as scratch buffer minimize allocations. - nla := netlink.Attribute{} - - // Output array, initialized to the length of the input array - ra := make([]netlink.Attribute, 0, len(attrs)) + ae := netlink.NewAttributeEncoder() - for _, nfa := range attrs { - - if nfa.NetByteOrder && nfa.Nested { - return nil, errInvalidAttributeFlags - } - - // Save nested or byte order flags back to the netlink.Attribute's - // Type field to include it in the marshaling operation - nla.Type = nfa.Type - - // Embed the Nested and NetByteOrder flags into the netlink Type. - switch { - case nfa.Nested: - nla.Type |= netlink.Nested - case nfa.NetByteOrder: - nla.Type |= netlink.NetByteOrder - } - - // Recursively marshal the attribute's children. - if nfa.Nested { - nfnab, err := marshalAttributes(nfa.Children) - if err != nil { - return nil, err - } - - nla.Data = nfnab - } else { - nla.Data = nfa.Data - } + attr := Attribute{} + if err := attr.encode(attrs)(ae); err != nil { + return nil, err + } - ra = append(ra, nla) + b, err := ae.Encode() + if err != nil { + return nil, err } - // Marshal all Netfilter attributes into binary representation of Netlink attributes - return netlink.MarshalAttributes(ra) + return b, nil } From fb52531c087fe5c85581feefb8e2291400ff9c77 Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Sat, 7 Dec 2019 14:55:52 +0100 Subject: [PATCH 3/9] Makefile - always generate cover.out --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 5d92ce3..c0eaac5 100644 --- a/Makefile +++ b/Makefile @@ -45,8 +45,8 @@ bench: bench-integration: go test -bench=. -tags=integration -exec sudo ./... -cover: cover.out -cover.out: $(SOURCES) +.PHONY: cover +cover: go test -coverprofile=cover.out -covermode=atomic ./... # Remove coverage output from files generated by Stringer. sed -i '/_string.go/d' cover.out From de0c75542c8831e199a4ae7508df9e97dca2e7db Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Sat, 7 Dec 2019 14:59:27 +0100 Subject: [PATCH 4/9] Return decoder error from Attribute.decode() --- attribute.go | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/attribute.go b/attribute.go index 0b8420f..febd1aa 100644 --- a/attribute.go +++ b/attribute.go @@ -141,7 +141,6 @@ func Uint64Bytes(u uint64) []byte { // obtained by exhausting ad. func (a *Attribute) decode(ad *netlink.AttributeDecoder) error { - // Wrap all netlink.Attributes into netfilter.Attributes to support nesting. for ad.Next() { // Copy the netlink attribute's fields into the netfilter attribute. @@ -153,8 +152,8 @@ func (a *Attribute) decode(ad *netlink.AttributeDecoder) error { } // Boolean flags extracted from the two leftmost bits of Type. - nfa.Nested = (ad.TypeFlags() & netlink.Nested) != 0 - nfa.NetByteOrder = (ad.TypeFlags() & netlink.NetByteOrder) != 0 + nfa.Nested = ad.TypeFlags()&netlink.Nested != 0 + nfa.NetByteOrder = ad.TypeFlags()&netlink.NetByteOrder != 0 if nfa.NetByteOrder && nfa.Nested { return errInvalidAttributeFlags @@ -168,7 +167,7 @@ func (a *Attribute) decode(ad *netlink.AttributeDecoder) error { a.Children = append(a.Children, nfa) } - return nil + return ad.Err() } // encode returns a function that takes an AttributeEncoder and returns error. @@ -184,15 +183,15 @@ func (a *Attribute) encode(attrs []Attribute) func(*netlink.AttributeEncoder) er return errInvalidAttributeFlags } - if nfa.NetByteOrder { - nfa.Type |= netlink.NetByteOrder - } - if nfa.Nested { ae.Nested(nfa.Type, nfa.encode(nfa.Children)) continue } + // Manually set the NetByteOrder flag, since ae.Bytes() can't. + if nfa.NetByteOrder { + nfa.Type |= netlink.NetByteOrder + } ae.Bytes(nfa.Type, nfa.Data) } @@ -226,11 +225,6 @@ func unmarshalAttributes(b []byte) ([]Attribute, error) { return nil, err } - // Catch any errors encountered by the netlink decoder. - if err := ad.Err(); err != nil { - return nil, err - } - return a.Children, nil } From 336af6111f58a342300bd8cb1ac794e1cbe21fbd Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Thu, 12 Dec 2019 00:35:20 +0100 Subject: [PATCH 5/9] Update to mdlayher/netlink 1.0.1 --- go.mod | 5 +++-- go.sum | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index a4fbc27..14c1f91 100644 --- a/go.mod +++ b/go.mod @@ -4,8 +4,9 @@ go 1.12 require ( github.com/google/go-cmp v0.3.1 - github.com/mdlayher/netlink v1.0.0 + github.com/mdlayher/netlink v1.0.1-0.20191210152442-a1644773bc99 github.com/pkg/errors v0.8.1 github.com/stretchr/testify v1.3.0 - golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456 + golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553 // indirect + golang.org/x/sys v0.0.0-20191210023423-ac6580df4449 ) diff --git a/go.sum b/go.sum index 2e6070c..6f29df0 100644 --- a/go.sum +++ b/go.sum @@ -7,8 +7,8 @@ github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw github.com/jsimonetti/rtnetlink v0.0.0-20190606172950-9527aa82566a h1:84IpUNXj4mCR9CuCEvSiCArMbzr/TMbuPIadKDwypkI= github.com/jsimonetti/rtnetlink v0.0.0-20190606172950-9527aa82566a/go.mod h1:Oz+70psSo5OFh8DBl0Zv2ACw7Esh6pPUphlvZG9x7uw= github.com/mdlayher/netlink v0.0.0-20190409211403-11939a169225/go.mod h1:eQB3mZE4aiYnlUsyGGCOpPETfdQq4Jhsgf1fk3cwQaA= -github.com/mdlayher/netlink v1.0.0 h1:vySPY5Oxnn/8lxAPn2cK6kAzcZzYJl3KriSLO46OT18= -github.com/mdlayher/netlink v1.0.0/go.mod h1:KxeJAFOFLG6AjpyDkQ/iIhxygIUKD+vcwqcnu43w/+M= +github.com/mdlayher/netlink v1.0.1-0.20191210152442-a1644773bc99 h1:j14xqbiblLsxSSBc6uvABovvqAIr8mHnwaXCKRAtlkk= +github.com/mdlayher/netlink v1.0.1-0.20191210152442-a1644773bc99/go.mod h1:KxeJAFOFLG6AjpyDkQ/iIhxygIUKD+vcwqcnu43w/+M= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -21,9 +21,13 @@ golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJV golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297 h1:k7pJ2yAPLPgbskkFdhRCsA77k2fySZ1zf2zCjvQCiIM= golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553 h1:efeOvDhwQ29Dj3SdAV/MJf8oukgn+8D8WgaCaRMchF8= +golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190411185658-b44545bcd369/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456 h1:ng0gs1AKnRRuEMZoTLLlbOd+C17zUDepwGQBb/n+JVg= golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20191210023423-ac6580df4449 h1:gSbV7h1NRL2G1xTg/owz62CST1oJBmxy4QpMMregXVQ= +golang.org/x/sys v0.0.0-20191210023423-ac6580df4449/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= From 1581f8a94d7a3543d539efd54f272933ae579613 Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Fri, 13 Dec 2019 22:30:48 +0100 Subject: [PATCH 6/9] message.go - add DecodeNetlink with Header/nlAD --- message.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/message.go b/message.go index 93e970a..a4a6be4 100644 --- a/message.go +++ b/message.go @@ -20,6 +20,25 @@ func UnmarshalNetlink(msg netlink.Message) (Header, []Attribute, error) { return h, attrs, nil } +// DecodeNetlink returns msg's Netfilter header and an AttributeDecoder that can be used +// to iteratively decode all Netlink attributes contained in the message. +func DecodeNetlink(msg netlink.Message) (Header, *netlink.AttributeDecoder, error) { + + var h Header + + err := h.unmarshal(msg) + if err != nil { + return Header{}, nil, err + } + + ad, err := netlink.NewAttributeDecoder(msg.Data[nfHeaderLen:]) + if err != nil { + return Header{}, nil, err + } + + return h, ad, nil +} + // MarshalNetlink takes a Netfilter Header and Attributes and returns a netlink.Message. func MarshalNetlink(h Header, attrs []Attribute) (netlink.Message, error) { @@ -32,7 +51,7 @@ func MarshalNetlink(h Header, attrs []Attribute) (netlink.Message, error) { nlm := netlink.Message{Data: make([]byte, 4)} // marshal error ignored, safe to do if msg Data is initialized - h.marshal(&nlm) + _ = h.marshal(&nlm) nlm.Data = append(nlm.Data, ba...) From 09d2eaa08aa4e0752817fa659a01114e66c9e555 Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Fri, 13 Dec 2019 22:31:10 +0100 Subject: [PATCH 7/9] conn_test - ignore mock conn.Send retval --- conn_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conn_test.go b/conn_test.go index 576f28c..6f2c1f1 100644 --- a/conn_test.go +++ b/conn_test.go @@ -96,7 +96,7 @@ func TestConnQueryMulticast(t *testing.T) { func TestConnReceive(t *testing.T) { // Inject a message directly into the nltest connection - connEcho.conn.Send(nlMsgReqAck) + _, _ = connEcho.conn.Send(nlMsgReqAck) // Drain the socket _, err := connEcho.Receive() From f805464aa3faa559aa7f9713c57b1feae5360d97 Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Thu, 19 Dec 2019 09:04:06 +0100 Subject: [PATCH 8/9] Introduce NewAttributeEncoder/Decoder for big-endian codecs --- attribute.go | 41 +++++++++++++++++++++++++++++------------ attribute_test.go | 24 ++++++++++++++---------- errors.go | 4 ---- message.go | 20 +++++++++++--------- message_test.go | 32 +++++++++++++++++++++++++------- 5 files changed, 79 insertions(+), 42 deletions(-) diff --git a/attribute.go b/attribute.go index febd1aa..91b6569 100644 --- a/attribute.go +++ b/attribute.go @@ -5,9 +5,33 @@ import ( "fmt" "github.com/mdlayher/netlink" - "github.com/pkg/errors" ) +// NewAttributeDecoder instantiates a new netlink.AttributeDecoder +// configured with a Big Endian byte order. +func NewAttributeDecoder(b []byte) (*netlink.AttributeDecoder, error) { + ad, err := netlink.NewAttributeDecoder(b) + if err != nil { + return nil, err + } + + // All Netfilter attribute payloads are big-endian. (network byte order) + ad.ByteOrder = binary.BigEndian + + return ad, nil +} + +// NewAttributeDecoder instantiates a new netlink.AttributeEncoder +// configured with a Big Endian byte order. +func NewAttributeEncoder() *netlink.AttributeEncoder { + ae := netlink.NewAttributeEncoder() + + // All Netfilter attribute payloads are big-endian. (network byte order) + ae.ByteOrder = binary.BigEndian + + return ae +} + // An Attribute is a copy of a netlink.Attribute that can be nested. type Attribute struct { @@ -202,14 +226,7 @@ func (a *Attribute) encode(attrs []Attribute) func(*netlink.AttributeEncoder) er // unmarshalAttributes returns an array of netfilter.Attributes decoded from // a byte array. This byte array should be taken from the netlink.Message's // Data payload after the nfHeaderLen offset. -func unmarshalAttributes(b []byte) ([]Attribute, error) { - - // Obtain a list of parsed netlink attributes possibly holding - // nested Netfilter attributes in their binary Data field. - ad, err := netlink.NewAttributeDecoder(b) - if err != nil { - return nil, errors.Wrap(err, errWrapNetlinkUnmarshalAttrs) - } +func unmarshalAttributes(ad *netlink.AttributeDecoder) ([]Attribute, error) { // Use the Children element of the Attribute to decode into. // Attribute already has nested decoding implemented on the type. @@ -228,12 +245,12 @@ func unmarshalAttributes(b []byte) ([]Attribute, error) { return a.Children, nil } -// marshalAttributes marshals a nested attribute structure into a byte slice. +// MarshalAttributes marshals a nested attribute structure into a byte slice. // This byte slice can then be copied into a netlink.Message's Data field after // the nfHeaderLen offset. -func marshalAttributes(attrs []Attribute) ([]byte, error) { +func MarshalAttributes(attrs []Attribute) ([]byte, error) { - ae := netlink.NewAttributeEncoder() + ae := NewAttributeEncoder() attr := Attribute{} if err := attr.encode(attrs)(ae); err != nil { diff --git a/attribute_test.go b/attribute_test.go index c97644e..b81505a 100644 --- a/attribute_test.go +++ b/attribute_test.go @@ -141,7 +141,7 @@ func TestAttributeMarshalAttributes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - b, err := marshalAttributes(tt.attrs) + b, err := MarshalAttributes(tt.attrs) if err != nil { t.Fatalf("unexpected marshal error: %v", err) } @@ -194,7 +194,7 @@ func TestAttributeMarshalErrors(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := marshalAttributes(tt.attrs) + _, err := MarshalAttributes(tt.attrs) require.Error(t, err, "marshal must error") if tt.err != nil { @@ -216,11 +216,6 @@ func TestAttributeUnmarshalErrors(t *testing.T) { err error errWrap string }{ - { - name: "netlink unmarshal error", - b: []byte{1}, - errWrap: errWrapNetlinkUnmarshalAttrs, - }, { name: "invalid attribute flags on top-level attribute", b: []byte{ @@ -242,8 +237,12 @@ func TestAttributeUnmarshalErrors(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := unmarshalAttributes(tt.b) + ad, err := NewAttributeDecoder(tt.b) + if err != nil { + t.Fatal("unexpected error creating AttributeDecoder:", err) + } + _, err = unmarshalAttributes(ad) if err == nil { t.Fatal("unmarshal did not error") } @@ -397,8 +396,13 @@ func TestAttributeMarshalTwoWay(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ad, err := NewAttributeDecoder(tt.b) + if err != nil { + t.Fatal("unexpected error creating AttributeDecoder:", err) + } + // Unmarshal binary content into nested structures - attrs, err := unmarshalAttributes(tt.b) + attrs, err := unmarshalAttributes(ad) require.NoError(t, err) assert.Empty(t, cmp.Diff(tt.attrs, attrs)) @@ -406,7 +410,7 @@ func TestAttributeMarshalTwoWay(t *testing.T) { var b []byte // Attempt re-marshal into binary form - b, err = marshalAttributes(tt.attrs) + b, err = MarshalAttributes(tt.attrs) require.NoError(t, err) assert.Empty(t, cmp.Diff(tt.b, b)) diff --git a/errors.go b/errors.go index 00bd14f..8d0f550 100644 --- a/errors.go +++ b/errors.go @@ -4,10 +4,6 @@ import ( "errors" ) -const ( - errWrapNetlinkUnmarshalAttrs = "error unmarshaling netlink attributes" -) - var ( // errInvalidAttributeFlags specifies if an Attribute's flag configuration is invalid. // From a comment in Linux/include/uapi/linux/netlink.h, Nested and NetByteOrder are mutually exclusive. diff --git a/message.go b/message.go index a4a6be4..2fbb7ac 100644 --- a/message.go +++ b/message.go @@ -1,18 +1,20 @@ package netfilter -import "github.com/mdlayher/netlink" +import ( + "github.com/pkg/errors" + + "github.com/mdlayher/netlink" +) // UnmarshalNetlink unmarshals a netlink.Message into a Netfilter Header and Attributes. func UnmarshalNetlink(msg netlink.Message) (Header, []Attribute, error) { - var h Header - - err := h.unmarshal(msg) + h, ad, err := DecodeNetlink(msg) if err != nil { return Header{}, nil, err } - attrs, err := unmarshalAttributes(msg.Data[nfHeaderLen:]) + attrs, err := unmarshalAttributes(ad) if err != nil { return Header{}, nil, err } @@ -28,12 +30,12 @@ func DecodeNetlink(msg netlink.Message) (Header, *netlink.AttributeDecoder, erro err := h.unmarshal(msg) if err != nil { - return Header{}, nil, err + return Header{}, nil, errors.Wrap(err, "unmarshaling netfilter header") } - ad, err := netlink.NewAttributeDecoder(msg.Data[nfHeaderLen:]) + ad, err := NewAttributeDecoder(msg.Data[nfHeaderLen:]) if err != nil { - return Header{}, nil, err + return Header{}, nil, errors.Wrap(err, "creating attribute decoder") } return h, ad, nil @@ -42,7 +44,7 @@ func DecodeNetlink(msg netlink.Message) (Header, *netlink.AttributeDecoder, erro // MarshalNetlink takes a Netfilter Header and Attributes and returns a netlink.Message. func MarshalNetlink(h Header, attrs []Attribute) (netlink.Message, error) { - ba, err := marshalAttributes(attrs) + ba, err := MarshalAttributes(attrs) if err != nil { return netlink.Message{}, err } diff --git a/message_test.go b/message_test.go index 5ba8d72..37077e8 100644 --- a/message_test.go +++ b/message_test.go @@ -1,13 +1,16 @@ package netfilter import ( - "errors" "testing" + "github.com/pkg/errors" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mdlayher/netlink" + "github.com/mdlayher/netlink/nlenc" ) func TestMessageUnmarshalNetlink(t *testing.T) { @@ -24,7 +27,7 @@ func TestMessageUnmarshalNetlink(t *testing.T) { msg: netlink.Message{ Data: make([]byte, nfHeaderLen-1), }, - err: errMessageLen, + err: errors.Wrap(errMessageLen, "unmarshaling netfilter header"), }, { name: "simple attribute", @@ -45,21 +48,36 @@ func TestMessageUnmarshalNetlink(t *testing.T) { msg: netlink.Message{ Data: make([]byte, nfHeaderLen+1), }, - err: errors.New("error unmarshaling netlink attributes: invalid attribute; length too short or too large"), + err: errors.New("creating attribute decoder: invalid attribute; length too short or too large"), + }, + { + name: "nested and byte order flags set", + msg: netlink.Message{ + Data: append( + []byte{ + 0, 0, 0, 0, + 4, 0}, + nlenc.Uint16Bytes(netlink.Nested|netlink.NetByteOrder)..., + ), + }, + err: errInvalidAttributeFlags, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Extract and parse Netfilter attributes from a Netlink message + // Extract and parse Netfilter attributes from a Netlink message. h, attrs, err := UnmarshalNetlink(tt.msg) - if err != nil { - assert.EqualError(t, err, tt.err.Error()) - // Don't test payload when expecting errors + + if tt.err != nil { + require.Error(t, err) + require.EqualError(t, err, tt.err.Error()) return } + require.NoError(t, err) + if diff := cmp.Diff(tt.attrs, attrs); diff != "" { t.Fatalf("unexpected attributes (-want, +got):\n%s", diff) } From 5ebce80cf9ff69e6b6891d6fbe34d49cc05b3d24 Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Thu, 19 Dec 2019 10:44:33 +0100 Subject: [PATCH 9/9] Introduce UnmarshalAttributes --- attribute.go | 29 +++++++++++++++++++++++++---- attribute_test.go | 39 ++++++++++++++++----------------------- errors.go | 2 ++ message.go | 29 ++++++++++++++++++++++------- message_test.go | 12 ++++++++++++ 5 files changed, 77 insertions(+), 34 deletions(-) diff --git a/attribute.go b/attribute.go index 91b6569..6e7a5a8 100644 --- a/attribute.go +++ b/attribute.go @@ -223,10 +223,10 @@ func (a *Attribute) encode(attrs []Attribute) func(*netlink.AttributeEncoder) er } } -// unmarshalAttributes returns an array of netfilter.Attributes decoded from +// decodeAttributes returns an array of netfilter.Attributes decoded from // a byte array. This byte array should be taken from the netlink.Message's // Data payload after the nfHeaderLen offset. -func unmarshalAttributes(ad *netlink.AttributeDecoder) ([]Attribute, error) { +func decodeAttributes(ad *netlink.AttributeDecoder) ([]Attribute, error) { // Use the Children element of the Attribute to decode into. // Attribute already has nested decoding implemented on the type. @@ -245,6 +245,17 @@ func unmarshalAttributes(ad *netlink.AttributeDecoder) ([]Attribute, error) { return a.Children, nil } +// encodeAttributes encodes a list of Attributes into the given netlink.AttributeEncoder. +func encodeAttributes(ae *netlink.AttributeEncoder, attrs []Attribute) error { + + if ae == nil { + return errNilAttributeEncoder + } + + attr := Attribute{} + return attr.encode(attrs)(ae) +} + // MarshalAttributes marshals a nested attribute structure into a byte slice. // This byte slice can then be copied into a netlink.Message's Data field after // the nfHeaderLen offset. @@ -252,8 +263,7 @@ func MarshalAttributes(attrs []Attribute) ([]byte, error) { ae := NewAttributeEncoder() - attr := Attribute{} - if err := attr.encode(attrs)(ae); err != nil { + if err := encodeAttributes(ae, attrs); err != nil { return nil, err } @@ -264,3 +274,14 @@ func MarshalAttributes(attrs []Attribute) ([]byte, error) { return b, nil } + +// UnmarshalAttributes unmarshals a byte slice into a list of Attributes. +func UnmarshalAttributes(b []byte) ([]Attribute, error) { + + ad, err := NewAttributeDecoder(b) + if err != nil { + return nil, err + } + + return decodeAttributes(ad) +} diff --git a/attribute_test.go b/attribute_test.go index b81505a..23f3f2d 100644 --- a/attribute_test.go +++ b/attribute_test.go @@ -1,6 +1,7 @@ package netfilter import ( + "errors" "strings" "testing" @@ -209,7 +210,7 @@ func TestAttributeMarshalErrors(t *testing.T) { } } -func TestAttributeUnmarshalErrors(t *testing.T) { +func TestAttributeDecoderErrors(t *testing.T) { tests := []struct { name string b []byte @@ -233,31 +234,19 @@ func TestAttributeUnmarshalErrors(t *testing.T) { }, err: errInvalidAttributeFlags, }, + { + name: "decoding invalid attribute", + b: []byte{4, 0, 0}, + err: errors.New("invalid attribute; length too short or too large"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ad, err := NewAttributeDecoder(tt.b) - if err != nil { - t.Fatal("unexpected error creating AttributeDecoder:", err) - } - - _, err = unmarshalAttributes(ad) - if err == nil { - t.Fatal("unmarshal did not error") - } - - if tt.err != nil { - if want, got := tt.err, err; want != got { - t.Fatalf("unexpected error:\n- want: %v\n- got: %v", - want, got.Error()) - } - } else if tt.errWrap != "" { - if !strings.HasPrefix(err.Error(), tt.errWrap+":") { - t.Fatalf("unexpected wrapped error:\n- expected prefix: %v\n- error string: %v", - tt.errWrap, err) - } - } + _, err := UnmarshalAttributes(tt.b) + require.Error(t, err) + require.Error(t, tt.err) + require.EqualError(t, err, tt.err.Error()) }) } } @@ -402,7 +391,7 @@ func TestAttributeMarshalTwoWay(t *testing.T) { } // Unmarshal binary content into nested structures - attrs, err := unmarshalAttributes(ad) + attrs, err := decodeAttributes(ad) require.NoError(t, err) assert.Empty(t, cmp.Diff(tt.attrs, attrs)) @@ -417,3 +406,7 @@ func TestAttributeMarshalTwoWay(t *testing.T) { }) } } + +func TestErrors(t *testing.T) { + assert.EqualError(t, encodeAttributes(nil, nil), errNilAttributeEncoder.Error()) +} diff --git a/errors.go b/errors.go index 8d0f550..e11cf19 100644 --- a/errors.go +++ b/errors.go @@ -14,4 +14,6 @@ var ( errConnIsMulticast = errors.New("Conn is attached to one or more multicast groups and can no longer be used for bidirectional traffic") errNoMulticastGroups = errors.New("need one or more multicast groups to join") + + errNilAttributeEncoder = errors.New("given AttributeEncoder is nil") ) diff --git a/message.go b/message.go index 2fbb7ac..bc9f393 100644 --- a/message.go +++ b/message.go @@ -14,7 +14,7 @@ func UnmarshalNetlink(msg netlink.Message) (Header, []Attribute, error) { return Header{}, nil, err } - attrs, err := unmarshalAttributes(ad) + attrs, err := decodeAttributes(ad) if err != nil { return Header{}, nil, err } @@ -44,18 +44,33 @@ func DecodeNetlink(msg netlink.Message) (Header, *netlink.AttributeDecoder, erro // MarshalNetlink takes a Netfilter Header and Attributes and returns a netlink.Message. func MarshalNetlink(h Header, attrs []Attribute) (netlink.Message, error) { - ba, err := MarshalAttributes(attrs) + ae := NewAttributeEncoder() + if err := encodeAttributes(ae, attrs); err != nil { + return netlink.Message{}, err + } + + return EncodeNetlink(h, ae) +} + +// EncodeNetlink generates a netlink.Message based on a given netfilter header h +// and a pre-filled netlink.AttributeEncoder ae. +func EncodeNetlink(h Header, ae *netlink.AttributeEncoder) (netlink.Message, error) { + + if ae == nil { + return netlink.Message{}, errNilAttributeEncoder + } + + // Encode the AE into a byte slice. + b, err := ae.Encode() if err != nil { return netlink.Message{}, err } - // initialize with 4 bytes of Data before unmarshal - nlm := netlink.Message{Data: make([]byte, 4)} + // Allocate space for the marshaled netfilter header. + nlm := netlink.Message{Data: append(make([]byte, nfHeaderLen), b...)} - // marshal error ignored, safe to do if msg Data is initialized + // marshal error ignored, safe to do if msg Data is initialized. _ = h.marshal(&nlm) - nlm.Data = append(nlm.Data, ba...) - return nlm, nil } diff --git a/message_test.go b/message_test.go index 37077e8..1357510 100644 --- a/message_test.go +++ b/message_test.go @@ -155,3 +155,15 @@ func TestAttributeMarshalNetlink(t *testing.T) { }) } } + +func TestEncodeNetlink(t *testing.T) { + + _, err := EncodeNetlink(Header{}, nil) + assert.EqualError(t, err, errNilAttributeEncoder.Error()) + + // Make ae.Encode() throw an error inside EncodeNetlink. + ae := NewAttributeEncoder() + ae.Do(0, func() ([]byte, error) { return []byte{}, errors.New("test error") }) + _, err = EncodeNetlink(Header{}, ae) + assert.EqualError(t, err, "test error") +}