Skip to content

Commit

Permalink
lib/connections: Try TCP punchthrough (fixes #4259) (#5753)
Browse files Browse the repository at this point in the history
  • Loading branch information
@AudriusButkevicius
AudriusButkevicius committed Jun 16, 2020
1 parent d09c8f0 commit f619a7f
Show file tree
Hide file tree
Showing 14 changed files with 287 additions and 149 deletions.
66 changes: 40 additions & 26 deletions cmd/stdiscosrv/apisrv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,26 +132,33 @@ func (s *apiSrv) handler(w http.ResponseWriter, req *http.Request) {
log.Println(reqID, req.Method, req.URL)
}

var remoteIP net.IP
remoteAddr := &net.TCPAddr{
IP: nil,
Port: -1,
}

if s.useHTTP {
remoteIP = net.ParseIP(req.Header.Get("X-Forwarded-For"))
remoteAddr.IP = net.ParseIP(req.Header.Get("X-Forwarded-For"))
if parsedPort, err := strconv.ParseInt(req.Header.Get("X-Forwarded-Port"), 10, 0); err == nil {
remoteAddr.Port = int(parsedPort)
}
} else {
addr, err := net.ResolveTCPAddr("tcp", req.RemoteAddr)
var err error
remoteAddr, err = net.ResolveTCPAddr("tcp", req.RemoteAddr)
if err != nil {
log.Println("remoteAddr:", err)
lw.Header().Set("Retry-After", errorRetryAfterString())
http.Error(lw, "Internal Server Error", http.StatusInternalServerError)
apiRequestsTotal.WithLabelValues("no_remote_addr").Inc()
return
}
remoteIP = addr.IP
}

switch req.Method {
case "GET":
s.handleGET(ctx, lw, req)
case "POST":
s.handlePOST(ctx, remoteIP, lw, req)
s.handlePOST(ctx, remoteAddr, lw, req)
default:
http.Error(lw, "Method Not Allowed", http.StatusMethodNotAllowed)
}
Expand Down Expand Up @@ -217,7 +224,7 @@ func (s *apiSrv) handleGET(ctx context.Context, w http.ResponseWriter, req *http
w.Write(bs)
}

func (s *apiSrv) handlePOST(ctx context.Context, remoteIP net.IP, w http.ResponseWriter, req *http.Request) {
func (s *apiSrv) handlePOST(ctx context.Context, remoteAddr *net.TCPAddr, w http.ResponseWriter, req *http.Request) {
reqID := ctx.Value(idKey).(requestID)

rawCert := certificateBytes(req)
Expand All @@ -244,15 +251,15 @@ func (s *apiSrv) handlePOST(ctx context.Context, remoteIP net.IP, w http.Respons

deviceID := protocol.NewDeviceID(rawCert)

addresses := fixupAddresses(remoteIP, ann.Addresses)
addresses := fixupAddresses(remoteAddr, ann.Addresses)
if len(addresses) == 0 {
announceRequestsTotal.WithLabelValues("bad_request").Inc()
w.Header().Set("Retry-After", errorRetryAfterString())
http.Error(w, "Bad Request", http.StatusBadRequest)
return
}

if err := s.handleAnnounce(remoteIP, deviceID, addresses); err != nil {
if err := s.handleAnnounce(deviceID, addresses); err != nil {
announceRequestsTotal.WithLabelValues("internal_error").Inc()
w.Header().Set("Retry-After", errorRetryAfterString())
http.Error(w, "Internal Server Error", http.StatusInternalServerError)
Expand All @@ -269,7 +276,7 @@ func (s *apiSrv) Stop() {
s.listener.Close()
}

func (s *apiSrv) handleAnnounce(remote net.IP, deviceID protocol.DeviceID, addresses []string) error {
func (s *apiSrv) handleAnnounce(deviceID protocol.DeviceID, addresses []string) error {
key := deviceID.String()
now := time.Now()
expire := now.Add(addressExpiryTime).UnixNano()
Expand Down Expand Up @@ -364,7 +371,7 @@ func certificateBytes(req *http.Request) []byte {

// fixupAddresses checks the list of addresses, removing invalid ones and
// replacing unspecified IPs with the given remote IP.
func fixupAddresses(remote net.IP, addresses []string) []string {
func fixupAddresses(remote *net.TCPAddr, addresses []string) []string {
fixed := make([]string, 0, len(addresses))
for _, annAddr := range addresses {
uri, err := url.Parse(annAddr)
Expand All @@ -384,27 +391,34 @@ func fixupAddresses(remote net.IP, addresses []string) []string {
continue
}

if host == "" || ip.IsUnspecified() {
// Replace the unspecified IP with the request source.
if remote != nil {
if host == "" || ip.IsUnspecified() {
// Replace the unspecified IP with the request source.

// ... unless the request source is the loopback address or
// multicast/unspecified (can't happen, really).
if remote.IsLoopback() || remote.IsMulticast() || remote.IsUnspecified() {
continue
}
// ... unless the request source is the loopback address or
// multicast/unspecified (can't happen, really).
if remote.IP.IsLoopback() || remote.IP.IsMulticast() || remote.IP.IsUnspecified() {
continue
}

// Do not use IPv6 remote address if requested scheme is ...4
// (i.e., tcp4, etc.)
if strings.HasSuffix(uri.Scheme, "4") && remote.To4() == nil {
continue
}
// Do not use IPv6 remote address if requested scheme is ...4
// (i.e., tcp4, etc.)
if strings.HasSuffix(uri.Scheme, "4") && remote.IP.To4() == nil {
continue
}

// Do not use IPv4 remote address if requested scheme is ...6
if strings.HasSuffix(uri.Scheme, "6") && remote.To4() != nil {
continue
// Do not use IPv4 remote address if requested scheme is ...6
if strings.HasSuffix(uri.Scheme, "6") && remote.IP.To4() != nil {
continue
}

host = remote.IP.String()
}

host = remote.String()
// If zero port was specified, use remote port.
if port == "0" && remote.Port > 0 {
port = fmt.Sprintf("%d", remote.Port)
}
}

uri.Host = net.JoinHostPort(host, port)
Expand Down
41 changes: 32 additions & 9 deletions cmd/stdiscosrv/apisrv_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,45 +14,61 @@ import (

func TestFixupAddresses(t *testing.T) {
cases := []struct {
remote net.IP
remote *net.TCPAddr
in []string
out []string
}{
{ // verbatim passthrough
in: []string{"tcp://1.2.3.4:22000"},
out: []string{"tcp://1.2.3.4:22000"},
}, { // unspecified replaced by remote
remote: net.ParseIP("1.2.3.4"),
remote: addr("1.2.3.4", 22000),
in: []string{"tcp://:22000", "tcp://192.0.2.42:22000"},
out: []string{"tcp://1.2.3.4:22000", "tcp://192.0.2.42:22000"},
}, { // unspecified not used as replacement
remote: net.ParseIP("0.0.0.0"),
remote: addr("0.0.0.0", 22000),
in: []string{"tcp://:22000", "tcp://192.0.2.42:22000"},
out: []string{"tcp://192.0.2.42:22000"},
}, { // unspecified not used as replacement
remote: net.ParseIP("::"),
remote: addr("::", 22000),
in: []string{"tcp://:22000", "tcp://192.0.2.42:22000"},
out: []string{"tcp://192.0.2.42:22000"},
}, { // localhost not used as replacement
remote: net.ParseIP("127.0.0.1"),
remote: addr("127.0.0.1", 22000),
in: []string{"tcp://:22000", "tcp://192.0.2.42:22000"},
out: []string{"tcp://192.0.2.42:22000"},
}, { // localhost not used as replacement
remote: net.ParseIP("::1"),
remote: addr("::1", 22000),
in: []string{"tcp://:22000", "tcp://192.0.2.42:22000"},
out: []string{"tcp://192.0.2.42:22000"},
}, { // multicast not used as replacement
remote: net.ParseIP("224.0.0.1"),
remote: addr("224.0.0.1", 22000),
in: []string{"tcp://:22000", "tcp://192.0.2.42:22000"},
out: []string{"tcp://192.0.2.42:22000"},
}, { // multicast not used as replacement
remote: net.ParseIP("ff80::42"),
remote: addr("ff80::42", 22000),
in: []string{"tcp://:22000", "tcp://192.0.2.42:22000"},
out: []string{"tcp://192.0.2.42:22000"},
}, { // explicitly announced weirdness is also filtered
remote: net.ParseIP("192.0.2.42"),
remote: addr("192.0.2.42", 22000),
in: []string{"tcp://:22000", "tcp://127.1.2.3:22000", "tcp://[::1]:22000", "tcp://[ff80::42]:22000"},
out: []string{"tcp://192.0.2.42:22000"},
}, { // port remapping
remote: addr("123.123.123.123", 9000),
in: []string{"tcp://0.0.0.0:0"},
out: []string{"tcp://123.123.123.123:9000"},
}, { // unspecified port remapping
remote: addr("123.123.123.123", 9000),
in: []string{"tcp://:0"},
out: []string{"tcp://123.123.123.123:9000"},
}, { // empty remapping
remote: addr("123.123.123.123", 9000),
in: []string{"tcp://"},
out: []string{},
}, { // port only remapping
remote: addr("123.123.123.123", 9000),
in: []string{"tcp://44.44.44.44:0"},
out: []string{"tcp://44.44.44.44:9000"},
},
}

Expand All @@ -63,3 +79,10 @@ func TestFixupAddresses(t *testing.T) {
}
}
}

func addr(host string, port int) *net.TCPAddr {
return &net.TCPAddr{
IP: net.ParseIP(host),
Port: port,
}
}
19 changes: 2 additions & 17 deletions lib/connections/quic_misc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"net"

"github.com/lucas-clemente/quic-go"
"github.com/syncthing/syncthing/lib/util"
)

var (
Expand Down Expand Up @@ -65,21 +66,5 @@ func (q *quicTlsConn) ConnectionState() tls.ConnectionState {

// Sort available packet connections by ip address, preferring unspecified local address.
func packetConnLess(i interface{}, j interface{}) bool {
iIsUnspecified := false
jIsUnspecified := false
iLocalAddr := i.(net.PacketConn).LocalAddr()
jLocalAddr := j.(net.PacketConn).LocalAddr()

if host, _, err := net.SplitHostPort(iLocalAddr.String()); err == nil {
iIsUnspecified = host == "" || net.ParseIP(host).IsUnspecified()
}
if host, _, err := net.SplitHostPort(jLocalAddr.String()); err == nil {
jIsUnspecified = host == "" || net.ParseIP(host).IsUnspecified()
}

if jIsUnspecified == iIsUnspecified {
return len(iLocalAddr.Network()) < len(jLocalAddr.Network())
}

return iIsUnspecified
return util.AddressUnspecifiedLess(i.(net.PacketConn).LocalAddr(), j.(net.PacketConn).LocalAddr())
}
92 changes: 0 additions & 92 deletions lib/connections/quic_misc_test.go
View file

This file was deleted.

2 changes: 1 addition & 1 deletion lib/connections/tcp_dial.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (d *tcpDialer) Dial(ctx context.Context, _ protocol.DeviceID, uri *url.URL)

timeoutCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()
conn, err := dialer.DialContext(timeoutCtx, uri.Scheme, uri.Host)
conn, err := dialer.DialContextReusePort(timeoutCtx, uri.Scheme, uri.Host)
if err != nil {
return internalConn{}, err
}
Expand Down

0 comments on commit f619a7f

Please sign in to comment.