feat: add User/Pass Authentication to Sock5 Dialer with Tests

go.mod

@@ -6,9 +6,10 @@ require (
 	github.com/eycorsican/go-tun2socks v1.16.11
 	github.com/google/gopacket v1.1.19
 	github.com/shadowsocks/go-shadowsocks2 v0.1.5
-	github.com/stretchr/testify v1.8.2
-	golang.org/x/crypto v0.17.0
-	golang.org/x/net v0.19.0
+	github.com/stretchr/testify v1.8.4
+	github.com/things-go/go-socks5 v0.0.5
+	golang.org/x/crypto v0.18.0
+	golang.org/x/net v0.20.0
 )
 
 require (
@@ -17,7 +18,7 @@ require (
 	github.com/kr/text v0.2.0 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/riobard/go-bloom v0.0.0-20200614022211-cdc8013cb5b3 // indirect
-	golang.org/x/sys v0.15.0 // indirect
+	golang.org/x/sys v0.16.0 // indirect
 	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

go.sum

@@ -1,5 +1,4 @@
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
-github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/eycorsican/go-tun2socks v1.16.11 h1:+hJDNgisrYaGEqoSxhdikMgMJ4Ilfwm/IZDrWRrbaH8=
@@ -19,38 +18,34 @@ github.com/riobard/go-bloom v0.0.0-20200614022211-cdc8013cb5b3/go.mod h1:HgjTstv
 github.com/shadowsocks/go-shadowsocks2 v0.1.5 h1:PDSQv9y2S85Fl7VBeOMF9StzeXZyK1HakRm86CUbr28=
 github.com/shadowsocks/go-shadowsocks2 v0.1.5/go.mod h1:AGGpIoek4HRno4xzyFiAtLHkOpcoznZEkAccaI/rplM=
 github.com/songgao/water v0.0.0-20190725173103-fd331bda3f4b/go.mod h1:P5HUIBuIWKbyjl083/loAegFkfbFNx5i2qEP4CNbm7E=
-github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
-github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
-github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
-github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8=
-github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
+github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
+github.com/things-go/go-socks5 v0.0.5 h1:qvKaGcBkfDrUL33SchHN93srAmYGzb4CxSM2DPYufe8=
+github.com/things-go/go-socks5 v0.0.5/go.mod h1:mtzInf8v5xmsBpHZVbIw2YQYhc4K0jRwzfsH64Uh0IQ=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I=
-golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k=
-golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4=
+golang.org/x/crypto v0.18.0 h1:PGVlW0xEltQnzFZ55hkuX5+KLyrMYhHld1YHO4AKcdc=
+golang.org/x/crypto v0.18.0/go.mod h1:R0j02AL6hcrfOiy9T4ZYp/rcWeMxM3L6QYxlOuEG1mg=
 golang.org/x/lint v0.0.0-20200302205851-738671d3881b/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY=
 golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20191021144547-ec77196f6094/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
-golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c=
-golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U=
+golang.org/x/net v0.20.0 h1:aCL9BSgETF1k+blQaYUBx9hJ9LOGP3gAVemcZlf1Kpo=
+golang.org/x/net v0.20.0/go.mod h1:z8BVo6PvndSri0LbOE3hAn0apkU+1YvI6E70E9jsnvY=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc=
-golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU=
+golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

transport/socks5/socks5.go

@@ -37,6 +37,10 @@ const (
 	ErrAddressTypeNotSupported       = ReplyCode(0x08)
 )
 
+const (
+	authVersionMismatch = "unknown authentication version"
+)
+
 var _ error = (ReplyCode)(0)
 
 // Error returns a human-readable description of the error, based on the SOCKS5 RFC.

transport/socks5/stream_dialer.go

@@ -23,23 +23,89 @@ import (
 	"github.com/Jigsaw-Code/outline-sdk/transport"
 )
 
+// bufferSize: The maximum buffer size is
+// 3 (1 socks version + 1 method selection + 1 methods)
+// + 1 (auth version) + 1 (username length) + 255 (username) + 1 (password length) + 255 (password)
+const (
+	maxCredentialLength = 255
+	minCredentialLength = 1
+	socksProtocolVer    = 0x05
+	noAuthMethod        = 0x00
+	userPassAuthMethod  = 0x02
+	numberOfAuthMethods = 0x01
+	authVersion         = 0x01
+	connectCommand      = 0x01
+	rsv                 = 0x00
+	authSuccess         = 0x00
+	connectSuccess      = 0x00
+	addrLengthIPv4      = 4
+	addrLengthIPv6      = 16
+	bufferSize          = (1 + 1 + 1) + (1 + 1 + 255 + 1 + 255) + 256
+)
+
+// https://datatracker.ietf.org/doc/html/rfc1929
+// Credentials can be nil, and that means no authentication.
+type Credentials struct {
+	username []byte
+	password []byte
+}
+
+func NewCredentials(username, password string) (Credentials, error) {
+	var c Credentials
+	if err := c.setUsername(username); err != nil {
+		return c, err
+	}
+	if err := c.setPassword(password); err != nil {
+		return c, err
+	}
+	return c, nil
+}
+
+// SetUsername sets the username field, ensuring it doesn't exceed 255 bytes in length and is at least 1 byte.
+func (c *Credentials) setUsername(username string) error {
+	usernameBytes := []byte(username)
+	if len(usernameBytes) > maxCredentialLength {
+		return errors.New("username exceeds 255 bytes")
+	}
+	if len(usernameBytes) < minCredentialLength {
+		return errors.New("username must be at least 1 byte")
+	}
+	c.username = usernameBytes
+	return nil
+}
+
+// SetPassword sets the password field, ensuring it doesn't exceed 255 bytes in length and is at least 1 byte.
+func (c *Credentials) setPassword(password string) error {
+	passwordBytes := []byte(password)
+	if len(passwordBytes) > maxCredentialLength {
+		return errors.New("password exceeds 255 bytes")
+	}
+	if len(passwordBytes) < minCredentialLength {
+		return errors.New("password must be at least 1 byte")
+	}
+	c.password = passwordBytes
+	return nil
+}
+
 // NewStreamDialer creates a [transport.StreamDialer] that routes connections to a SOCKS5
 // proxy listening at the given [transport.StreamEndpoint].
-func NewStreamDialer(endpoint transport.StreamEndpoint) (transport.StreamDialer, error) {
+func NewStreamDialer(endpoint transport.StreamEndpoint, cred *Credentials) (transport.StreamDialer, error) {
 	if endpoint == nil {
 		return nil, errors.New("argument endpoint must not be nil")
 	}
-	return &streamDialer{proxyEndpoint: endpoint}, nil
+	return &streamDialer{proxyEndpoint: endpoint, credentials: cred}, nil
 }
 
 type streamDialer struct {
 	proxyEndpoint transport.StreamEndpoint
+	credentials   *Credentials
 }
 
 var _ transport.StreamDialer = (*streamDialer)(nil)
 
 // DialStream implements [transport.StreamDialer].DialStream using SOCKS5.
-// It will send the method and the connect requests in one packet, to avoid an unnecessary roundtrip.
+// It will send the auth method, auth credentials (if auth is chosen), and
+// the connect requests in one packet, to avoid an additional roundtrip.
 // The returned [error] will be of type [ReplyCode] if the server sends a SOCKS error reply code, which
 // you can check against the error constants in this package using [errors.Is].
 func (c *streamDialer) DialStream(ctx context.Context, remoteAddr string) (transport.StreamConn, error) {
@@ -55,80 +121,151 @@ func (c *streamDialer) DialStream(ctx context.Context, remoteAddr string) (trans
 	}()
 
 	// For protocol details, see https://datatracker.ietf.org/doc/html/rfc1928#section-3
+	// Creating a single buffer for method selection, authentication, and connection request
+	// Buffer large enough for method, auth, and connect requests with a domain name address.
+	var buffer [bufferSize]byte
+	var b []byte
 
-	// Buffer large enough for method and connect requests with a domain name address.
-	header := [3 + 4 + 256 + 2]byte{}
+	if c.credentials == nil {
+		// Method selection part: VER = 5, NMETHODS = 1, METHODS = 0 (no auth)
+		// +----+----------+----------+
+		// |VER | NMETHODS | METHODS  |
+		// +----+----------+----------+
+		// | 1  |    1     | 1 to 255 |
+		// +----+----------+----------+
+		b = append(buffer[:0], socksProtocolVer, numberOfAuthMethods, noAuthMethod)
+	} else {
+		// https://datatracker.ietf.org/doc/html/rfc1929
+		// Method selection part: VER = 5, NMETHODS = 1, METHODS = 2 (username/password)
+		b = append(buffer[:0], socksProtocolVer, numberOfAuthMethods, userPassAuthMethod)
 
-	// Method request:
-	// VER = 5, NMETHODS = 1, METHODS = 0 (no auth)
-	b := append(header[:0], 5, 1, 0)
+		// Authentication part: VER = 1, ULEN = 1, UNAME = 1~255, PLEN = 1, PASSWD = 1~255
+		// +----+------+----------+------+----------+
+		// |VER | ULEN |  UNAME   | PLEN |  PASSWD  |
+		// +----+------+----------+------+----------+
+		// | 1  |  1   | 1 to 255 |  1   | 1 to 255 |
+		// +----+------+----------+------+----------+
+		b = append(b, authVersion)
+		b = append(b, byte(len(c.credentials.username)))
+		b = append(b, c.credentials.username...)
+		b = append(b, byte(len(c.credentials.password)))
+		b = append(b, c.credentials.password...)
+	}
 
 	// Connect request:
-	// VER = 5, CMD = 1 (connect), RSV = 0
-	b = append(b, 5, 1, 0)
-	// Destination address Address (ATYP, DST.ADDR, DST.PORT)
+	// VER = 5, CMD = 1 (connect), RSV = 0, DST.ADDR, DST.PORT
+	// +----+-----+-------+------+----------+----------+
+	// |VER | CMD |  RSV  | ATYP | DST.ADDR | DST.PORT |
+	// +----+-----+-------+------+----------+----------+
+	// | 1  |  1  | X'00' |  1   | Variable |    2     |
+	// +----+-----+-------+------+----------+----------+
+	b = append(b, socksProtocolVer, connectCommand, rsv)
+	// TODO: Probably more memory efficient if remoteAddr is added to the buffer directly.
 	b, err = appendSOCKS5Address(b, remoteAddr)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create SOCKS5 address: %w", err)
 	}
 
-	// We merge the method and connect requests because we send a single authentication
-	// method, so there's no point in waiting for the response. This eliminates a roundtrip.
+	// We merge the method and connect requests and only perform one write
+	// because we send a single authentication method, so there's no point
+	// in waiting for the response. This eliminates a roundtrip.
 	_, err = proxyConn.Write(b)
 	if err != nil {
-		return nil, fmt.Errorf("failed to write SOCKS5 request: %w", err)
+		return nil, fmt.Errorf("failed to write combined SOCKS5 request: %w", err)
 	}
 
-	// Read method response (VER, METHOD).
-	if _, err = io.ReadFull(proxyConn, header[:2]); err != nil {
+	// Reading the response:
+	// 1. Read method response (VER, METHOD).
+	// +----+--------+
+	// |VER | METHOD |
+	// +----+--------+
+	// | 1  |   1    |
+	// +----+--------+
+	// buffer[0]: VER, buffer[1]: METHOD
+	// Reuse buffer for better performance.
+	if _, err = io.ReadFull(proxyConn, buffer[:2]); err != nil {
 		return nil, fmt.Errorf("failed to read method server response")
 	}
-	if header[0] != 5 {
-		return nil, fmt.Errorf("invalid protocol version %v. Expected 5", header[0])
+	if buffer[0] != socksProtocolVer {
+		return nil, fmt.Errorf("invalid protocol version %v. Expected 5", buffer[0])
 	}
-	if header[1] != 0 {
-		return nil, fmt.Errorf("unsupported SOCKS authentication method %v. Expected 0 (no auth)", header[1])
+
+	switch buffer[1] {
+	case noAuthMethod:
+		// No authentication required.
+	case userPassAuthMethod:
+		// 2. Read authentication version and status
+		// VER = 1, STATUS = 0
+		// +----+--------+
+		// |VER | STATUS |
+		// +----+--------+
+		// | 1  |   1    |
+		// +----+--------+
+		// VER = 1 means the server should be expecting username/password authentication.
+		// buffer[2]: VER, buffer[3]: STATUS
+		if _, err = io.ReadFull(proxyConn, buffer[2:4]); err != nil {
+			return nil, fmt.Errorf("failed to read authentication version and status: %w", err)
+		}
+		if buffer[2] != authVersion {
+			return nil, fmt.Errorf("invalid authentication version %v. Expected 1", buffer[2])
+		}
+		if buffer[3] != authSuccess {
+			return nil, fmt.Errorf("authentication failed: %v", buffer[3])
+		}
+	default:
+		return nil, fmt.Errorf("unsupported SOCKS authentication method %v. Expected 2", buffer[1])
 	}
 
-	// Read connect response (VER, REP, RSV, ATYP, BND.ADDR, BND.PORT).
+	// 3. Read connect response (VER, REP, RSV, ATYP, BND.ADDR, BND.PORT).
 	// See https://datatracker.ietf.org/doc/html/rfc1928#section-6.
-	if _, err = io.ReadFull(proxyConn, header[:4]); err != nil {
-		return nil, fmt.Errorf("failed to read connect server response")
+	// +----+-----+-------+------+----------+----------+
+	// |VER | REP |  RSV  | ATYP | BND.ADDR | BND.PORT |
+	// +----+-----+-------+------+----------+----------+
+	// | 1  |  1  | X'00' |  1   | Variable |    2     |
+	// +----+-----+-------+------+----------+----------+
+	// buffer[0]: VER
+	// buffer[1]: REP
+	// buffer[2]: RSV
+	// buffer[3]: ATYP
+	if _, err = io.ReadFull(proxyConn, buffer[:4]); err != nil {
+		fmt.Printf("failed to read connect server response: %v", err)
+		return nil, fmt.Errorf("failed to read connect server response: %w", err)
 	}
-	if header[0] != 5 {
-		return nil, fmt.Errorf("invalid protocol version %v. Expected 5", header[0])
+
+	if buffer[0] != socksProtocolVer {
+		return nil, fmt.Errorf("invalid protocol version %v. Expected 5", buffer[0])
 	}
 
-	// Check reply code (REP)
-	if header[1] != 0 {
-		return nil, ReplyCode(header[1])
+	if buffer[1] != connectSuccess {
+		return nil, ReplyCode(buffer[1])
 	}
 
-	toRead := 0
-	switch header[3] {
+	// 4. Read address and length
+	var bndAddrLen int
+	switch buffer[3] {
 	case addrTypeIPv4:
-		toRead = 4
+		bndAddrLen = addrLengthIPv4
 	case addrTypeIPv6:
-		toRead = 16
+		bndAddrLen = addrLengthIPv6
 	case addrTypeDomainName:
-		_, err := io.ReadFull(proxyConn, header[:1])
+		// buffer[8]: length of the domain name
+		_, err := io.ReadFull(proxyConn, buffer[:1])
 		if err != nil {
 			return nil, fmt.Errorf("failed to read address length in connect response: %w", err)
 		}
-		toRead = int(header[0])
+		bndAddrLen = int(buffer[0])
+	default:
+		return nil, fmt.Errorf("invalid address type %v", buffer[3])
 	}
-	// Reads the bound address and port, but we currently ignore them.
+	// 5. Reads the bound address and port, but we currently ignore them.
 	// TODO(fortuna): Should we expose the remote bound address as the net.Conn.LocalAddr()?
-	_, err = io.ReadFull(proxyConn, header[:toRead])
-	if err != nil {
-		return nil, fmt.Errorf("failed to read address in connect response: %w", err)
+	if _, err := io.ReadFull(proxyConn, buffer[:bndAddrLen]); err != nil {
+		return nil, fmt.Errorf("failed to read bound address: %w", err)
 	}
-	// We also ignore the remote bound port number.
-	_, err = io.ReadFull(proxyConn, header[:2])
-	if err != nil {
-		return nil, fmt.Errorf("failed to read port number in connect response: %w", err)
+	// We read but ignore the remote bound port number: BND.PORT
+	if _, err = io.ReadFull(proxyConn, buffer[:2]); err != nil {
+		return nil, fmt.Errorf("failed to read bound port: %w", err)
 	}
-
 	dialSuccess = true
 	return proxyConn, nil
 }