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

net: resolver add unexpected bytes to udp packet when wrapping the net.Conn #49750

Open
ChenTanyi opened this issue Nov 23, 2021 · 3 comments
Open

Comments

@ChenTanyi
Copy link

@ChenTanyi ChenTanyi commented Nov 23, 2021

What version of Go are you using (go version)?

$ go version
go version go1.17.3 linux/amd64

Does this issue reproduce with the latest release?

Yes, I tested in latest golang docker.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.3"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/root/test/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2826491904=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.17.3 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.17.3
uname -sr: Linux 5.4.0-1062-azure
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Debian GLIBC 2.31-13+deb11u2) stable release version 2.31.

What did you do?

1. Start a simple udp echo server in localhost
#!/usr/bin/env python3
import socket

s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
s.bind(('', 8080))

while True:
    msg, addr = s.recvfrom(1024)
    print(msg)
    print(msg.hex(' '))
2. Run a dns resolver client, but redirect the dns udp packets to the echo server
package main

import (
	"context"
	"net"
	"time"
)

func main() {
	c := &net.Dialer{
		Timeout: 10 * time.Second,
	}
	r := &net.Resolver{
		PreferGo: true,
		Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
			return c.Dial("udp", "127.0.0.1:8080")
		},
	}
	ip, err := r.LookupHost(context.Background(), "www.google.com")
	if err != nil {
		panic(err)
	}
	println(ip[0])
}
3. Wrap the net.Conn and run again.
package main

import (
	"context"
	"net"
	"time"
)

type duplicateConn struct {
	conn net.Conn
}

func (d *duplicateConn) Read(b []byte) (n int, err error) {
	n, err = d.conn.Read(b)
	// if n > 0 {
	// 	fmt.Printf("Read %d: %x\n", n, b[:n])
	// }
	// if err != nil {
	// 	fmt.Printf("Read Error: %v\n", err)
	// }
	return
}

func (d *duplicateConn) Write(b []byte) (n int, err error) {
	n, err = d.conn.Write(b)
	// if n > 0 {
	// 	fmt.Printf("Write %d: %x\n", n, b[:n])
	// }
	// if err != nil {
	// 	fmt.Printf("Write Error: %v\n", err)
	// }
	return
}

func (d *duplicateConn) Close() error {
	return d.conn.Close()
}

func (d *duplicateConn) LocalAddr() net.Addr {
	return d.conn.LocalAddr()
}

func (d *duplicateConn) RemoteAddr() net.Addr {
	return d.conn.RemoteAddr()
}

func (d *duplicateConn) SetDeadline(t time.Time) error {
	return d.conn.SetDeadline(t)
}

func (d *duplicateConn) SetReadDeadline(t time.Time) error {
	return d.conn.SetReadDeadline(t)
}

func (d *duplicateConn) SetWriteDeadline(t time.Time) error {
	return d.conn.SetWriteDeadline(t)
}

func main() {
	c := &net.Dialer{
		Timeout: 10 * time.Second,
	}
	r := &net.Resolver{
		PreferGo: true,
		Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
			conn, err := c.Dial("udp", "127.0.0.1:8080")
			return &duplicateConn{conn: conn}, err
		},
	}
	ip, err := r.LookupHost(context.Background(), "www.google.com")
	if err != nil {
		panic(err)
	}
	println(ip[0])
}
4. Test wrapping net.Conn only, which means the problem is not come from the struct wrapping.
package main

import (
	"net"
	"time"
)

type duplicateConn struct {
	conn net.Conn
}

func (d *duplicateConn) Read(b []byte) (n int, err error) {
	n, err = d.conn.Read(b)
	// if n > 0 {
	// 	fmt.Printf("Read %d: %x\n", n, b[:n])
	// }
	// if err != nil {
	// 	fmt.Printf("Read Error: %v\n", err)
	// }
	return
}

func (d *duplicateConn) Write(b []byte) (n int, err error) {
	n, err = d.conn.Write(b)
	// if n > 0 {
	// 	fmt.Printf("Write %d: %s\n", n, b[:n])
	// }
	// if err != nil {
	// 	fmt.Printf("Write Error: %v\n", err)
	// }
	return
}

func (d *duplicateConn) Close() error {
	return d.conn.Close()
}

func (d *duplicateConn) LocalAddr() net.Addr {
	return d.conn.LocalAddr()
}

func (d *duplicateConn) RemoteAddr() net.Addr {
	return d.conn.RemoteAddr()
}

func (d *duplicateConn) SetDeadline(t time.Time) error {
	return d.conn.SetDeadline(t)
}

func (d *duplicateConn) SetReadDeadline(t time.Time) error {
	return d.conn.SetReadDeadline(t)
}

func (d *duplicateConn) SetWriteDeadline(t time.Time) error {
	return d.conn.SetWriteDeadline(t)
}

func main() {
	c := &net.Dialer{
		Timeout: 10 * time.Second,
	}

	conn, err := c.Dial("udp", "127.0.0.1:8080")
	if err != nil {
		panic(err)
	}
	d := &duplicateConn{conn: conn}
	d.Write([]byte("test"))
	d.Write([]byte{1, 240, 192})
}

What did you expect to see?

Both 2 and 3 send valid dns packet to echo server or dns server.

For 4, echo server receives same data as I sent, which is expected.

What did you see instead?

The data sent by 2 is a valid dns packet. If I change destination to 8.8.8.8:53, I could get the dns result.
The data sent by 3 is not a valid dns packet. If I change destination to 8.8.8.8:53, I can't get the dns result.

The following is the data received from echo server, I found the invalid data would always have two more bytes in the front of the packet. These two bytes seems to be the length of the following bytes whenever I change the hostname.
b'\x8ax\x01\x00\x00\x01\x00\x00\x00\x00\x00\x00\x03www\x06google\x03com\x07default\x03svc\x07cluster\x05local\x00\x00\x1c\x00\x01'
8a 78 01 00 00 01 00 00 00 00 00 00 03 77 77 77 06 67 6f 6f 67 6c 65 03 63 6f 6d 07 64 65 66 61 75 6c 74 03 73 76 63 07 63 6c 75 73 74 65 72 05 6c 6f 63 61 6c 00 00 1c 00 01
b'\\y\x01\x00\x00\x01\x00\x00\x00\x00\x00\x00\x03www\x06google\x03com\x07default\x03svc\x07cluster\x05local\x00\x00\x01\x00\x01'
5c 79 01 00 00 01 00 00 00 00 00 00 03 77 77 77 06 67 6f 6f 67 6c 65 03 63 6f 6d 07 64 65 66 61 75 6c 74 03 73 76 63 07 63 6c 75 73 74 65 72 05 6c 6f 63 61 6c 00 00 01 00 01
b'\x00:\x86\xf9\x01\x00\x00\x01\x00\x00\x00\x00\x00\x00\x03www\x06google\x03com\x07default\x03svc\x07cluster\x05local\x00\x00\x1c\x00\x01'
00 3a 86 f9 01 00 00 01 00 00 00 00 00 00 03 77 77 77 06 67 6f 6f 67 6c 65 03 63 6f 6d 07 64 65 66 61 75 6c 74 03 73 76 63 07 63 6c 75 73 74 65 72 05 6c 6f 63 61 6c 00 00 1c 00 01
b'\x00:K_\x01\x00\x00\x01\x00\x00\x00\x00\x00\x00\x03www\x06google\x03com\x07default\x03svc\x07cluster\x05local\x00\x00\x01\x00\x01'
00 3a 4b 5f 01 00 00 01 00 00 00 00 00 00 03 77 77 77 06 67 6f 6f 67 6c 65 03 63 6f 6d 07 64 65 66 61 75 6c 74 03 73 76 63 07 63 6c 75 73 74 65 72 05 6c 6f 63 61 6c 00 00 01 00 01

The first 4 lines is valid dns packet, length 58.
The last 4 line is invalid dns packet, length 60, while 003a is 58 in OCT.

@heschi
Copy link
Contributor

@heschi heschi commented Nov 24, 2021

Loading

@heschi heschi added this to the Backlog milestone Nov 24, 2021
@davecheney
Copy link
Contributor

@davecheney davecheney commented Nov 24, 2021

In 3 the dialer will always use udp regardless of the network requested. This may cause a problem if the dns resolver was trying to to use tcp because the udp response was truncated

Loading

@ChenTanyi
Copy link
Author

@ChenTanyi ChenTanyi commented Nov 25, 2021

@davecheney Thanks for advice! I would use network directly in dailer for further usage.
As the test in this issue, I've printed the network and address before c.Dail. They are always "udp", so I simplify it.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants