Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/net/route: ParseRIB fails with errMessageTooShort on an AF_ROUTE message from Darwin #44740

Open
bradfitz opened this issue Mar 2, 2021 · 0 comments
Open

x/net/route: ParseRIB fails with errMessageTooShort on an AF_ROUTE message from Darwin #44740

bradfitz opened this issue Mar 2, 2021 · 0 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin
Milestone
Backlog

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Mar 2, 2021

I have a macOS program which listens to route changes from the kernel.

It opens an AF_ROUTE socket:

        fd, err := unix.Socket(unix.AF_ROUTE, unix.SOCK_RAW, 0)

Then reads messages in a loop:

        n, err := unix.Read(m.fd, m.buf[:])
        if err != nil {
                return nil, err
        }
        msgs, err := route.ParseRIB(route.RIBTypeRoute, m.buf[:n])

It works in general, until it hit this 50 byte message from the kernel:

func TestIssue1416RIB(t *testing.T) {
        const ribHex = `32 00 05 10 30 00 00 00 00 00 00 00 04 00 00 00 14 12 04 00 06 03 06 00 65 6e 30 ac 87 a3 19 7f 82 00 00 00 0e 12 00 00 00 00 06 00 91 e0 f0 01 00 00`
        rtmMsg, err := hex.DecodeString(strings.ReplaceAll(ribHex, " ", ""))
        if err != nil {
                t.Fatal(err)
        }
        msgs, err := route.ParseRIB(route.RIBTypeRoute, rtmMsg)
        if err != nil {
                t.Fatal(err)
        }
        t.Logf("Got: %#v", msgs)
}

Adding some logging and a panic where it was returning the error:

=== RUN   TestIssue1416RIB
2021/03/02 09:32:32 ifmam len=50
2021/03/02 09:32:32 Got: &{Version:5 Type:16 Flags:0 Index:4 Addrs:[] raw:[50 0 5 16 48 0 0 0 0 0 0 0 4 0 0 0 20 18 4 0 6 3 6 0 101 110 48 172 135 163 25 127 130 0 0 0 14 18 0 0 0 0 6 0 145 224 240 1 0 0]}
2021/03/02 09:32:32 numAttrs = 48, bodyOff = 16, len(b) = 50, len remain = 34
2021/03/02 09:32:32 link[4] = &route.LinkAddr{Index:4, Name:"en0", Addr:[]uint8{0xac, 0x87, 0xa3, 0x19, 0x7f, 0x82}}
2021/03/02 09:32:32 link[5] = &route.LinkAddr{Index:0, Name:"", Addr:[]uint8{0x91, 0xe0, 0xf0, 0x1, 0x0, 0x0}}
--- FAIL: TestIssue1416RIB (0.00s)
panic: foo: len(b) 14 < roundup(14) of 16 [recovered]
        panic: foo: len(b) 14 < roundup(14) of 16

goroutine 6 [running]:
testing.tRunner.func1.2(0x41c2020, 0xc0000510b0)
        /Users/bradfitz/go/src/testing/testing.go:1144 +0x332
testing.tRunner.func1(0xc000001e00)
        /Users/bradfitz/go/src/testing/testing.go:1147 +0x4b6
panic(0x41c2020, 0xc0000510b0)
        /Users/bradfitz/go/src/runtime/panic.go:965 +0x1b9
golang.org/x/net/route.parseAddrs(0x30, 0x42193b0, 0xc0000640f0, 0x22, 0x60, 0x4056c34, 0x430a400, 0x432cfe0, 0x40dcd44, 0x0)
        /Users/bradfitz/src/golang.org/x/net/route/address.go:389 +0x725
golang.org/x/net/route.(*wireFormat).parseInterfaceMulticastAddrMessage(0xc00000e210, 0x1, 0xc0000640e0, 0x32, 0x70, 0x40142c2, 0xc00003c648, 0x5fce0ba0, 0xb4929af58f107e09)
        /Users/bradfitz/src/golang.org/x/net/route/interface_multicast.go:36 +0x4d3
golang.org/x/net/route.ParseRIB(0x1, 0xc0000640e0, 0x32, 0x70, 0x64, 0x70, 0x32, 0x0, 0x0)
        /Users/bradfitz/src/golang.org/x/net/route/message.go:59 +0x23b
tailscale.com/wgengine/monitor.TestIssue1416RIB(0xc000001e00)
        /Users/bradfitz/src/tailscale.com/wgengine/monitor/monitor_darwin_test.go:21 +0x15f
testing.tRunner(0xc000001e00, 0x4219d90)
        /Users/bradfitz/go/src/testing/testing.go:1194 +0xef
created by testing.(*T).Run
        /Users/bradfitz/go/src/testing/testing.go:1239 +0x2b3
exit status 2
FAIL    tailscale.com/wgengine/monitor  0.083s

Notably, that LinkAddr (0x91, 0xe0, 0xf0, 0x1, 0x0, 0x0) in the final 6 bytes of the 50 byte message.

But parseAddrs is expecting some padding at the end? The errMessageTooShort case it's hitting:

func parseAddrs(attrs uint, fn func(int, []byte) (int, Addr, error), b []byte) ([]Addr, error) {
        var as [sysRTAX_MAX]Addr
        af := int(sysAF_UNSPEC)
        for i := uint(0); i < sysRTAX_MAX && len(b) >= roundup(0); i++ {
                if attrs&(1<<i) == 0 {
                        continue
                }
                if i <= sysRTAX_BRD {
                        switch b[1] {
                        case sysAF_LINK:
                                a, err := parseLinkAddr(b)
                                if err != nil {
                                        return nil, err
                                }  
                                as[i] = a
                                l := roundup(int(b[0]))
                                if len(b) < l {
                                        return nil, errMessageTooShort
                                }  
                                b = b[l:]

Maybe this package was designed purely for getting the RIBs via sysctl (with https://pkg.go.dev/golang.org/x/net/route#FetchRIB) where they differ from the format sent over AF_ROUTE sockets? Maybe?

/cc @ianlancetaylor @mikioh @tklauser @danderson @DentonGentry
@bradfitz bradfitz mentioned this issue Mar 2, 2021
Closed
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 2, 2021
@dmitshur dmitshur added this to the Backlog milestone Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
2 participants
@bradfitz @dmitshur