Rework header obfuscation and add tests for fallbacks

client.go

@@ -650,21 +650,19 @@ func (cl *Client) establishOutgoingConn(t *Torrent, addr IpPort) (c *connection,
 		return t.dialTimeout()
 	}())
 	defer cancel()
-	obfuscatedHeaderFirst := !cl.config.DisableEncryption && !cl.config.PreferNoEncryption
+	obfuscatedHeaderFirst := cl.config.HeaderObfuscationPolicy.Preferred
 	c, err = cl.establishOutgoingConnEx(t, addr, ctx, obfuscatedHeaderFirst)
 	if err != nil {
+		//cl.logger.Printf("error establish connection to %s (obfuscatedHeader=%t): %v", addr, obfuscatedHeaderFirst, err)
 		return
 	}
 	if c != nil {
 		torrent.Add("initiated conn with preferred header obfuscation", 1)
 		return
 	}
-	if cl.config.ForceEncryption {
-		// We should have just tried with an obfuscated header. A plaintext
-		// header can't result in an encrypted connection, so we're done.
-		if !obfuscatedHeaderFirst {
-			panic(cl.config.EncryptionPolicy)
-		}
+	if cl.config.HeaderObfuscationPolicy.RequirePreferred {
+		// We should have just tried with the preferred header obfuscation. If it was required,
+		// there's nothing else to try.
 		return
 	}
 	// Try again with encryption if we didn't earlier, or without if we did.
@@ -715,16 +713,7 @@ func (cl *Client) initiateHandshakes(c *connection, t *Torrent) (ok bool, err er
 			}{c.r, c.w},
 			t.infoHash[:],
 			nil,
-			func() mse.CryptoMethod {
-				switch {
-				case cl.config.ForceEncryption:
-					return mse.CryptoMethodRC4
-				case cl.config.DisableEncryption:
-					return mse.CryptoMethodPlaintext
-				default:
-					return mse.AllSupportedCrypto
-				}
-			}(),
+			cl.config.CryptoProvides,
 		)
 		c.setRW(rw)
 		if err != nil {
@@ -766,7 +755,7 @@ func (cl *Client) forSkeys(f func([]byte) bool) {
 func (cl *Client) receiveHandshakes(c *connection) (t *Torrent, err error) {
 	defer perf.ScopeTimerErr(&err)()
 	var rw io.ReadWriter
-	rw, c.headerEncrypted, c.cryptoMethod, err = handleEncryption(c.rw(), cl.forSkeys, cl.config.EncryptionPolicy)
+	rw, c.headerEncrypted, c.cryptoMethod, err = handleEncryption(c.rw(), cl.forSkeys, cl.config.HeaderObfuscationPolicy, cl.config.CryptoSelector)
 	c.setRW(rw)
 	if err == nil || err == mse.ErrNoSecretKeyMatch {
 		if c.headerEncrypted {
@@ -783,8 +772,8 @@ func (cl *Client) receiveHandshakes(c *connection) (t *Torrent, err error) {
 		}
 		return
 	}
-	if cl.config.ForceEncryption && !c.headerEncrypted {
-		err = errors.New("connection not encrypted")
+	if cl.config.HeaderObfuscationPolicy.RequirePreferred && c.headerEncrypted != cl.config.HeaderObfuscationPolicy.Preferred {
+		err = errors.New("connection not have required header obfuscation")
 		return
 	}
 	ih, ok, err := cl.connBTHandshake(c, nil)
@@ -895,7 +884,7 @@ func (cl *Client) sendInitialMessages(conn *connection, torrent *Torrent) {
 					V:            cl.config.ExtendedHandshakeClientVersion,
 					Reqq:         64, // TODO: Really?
 					YourIp:       pp.CompactIp(conn.remoteAddr.IP),
-					Encryption:   !cl.config.DisableEncryption,
+					Encryption:   cl.config.HeaderObfuscationPolicy.Preferred || !cl.config.HeaderObfuscationPolicy.RequirePreferred,
 					Port:         cl.incomingPeerPort(),
 					MetadataSize: torrent.metadataSize(),
 					// TODO: We can figured these out specific to the socket

client_test.go

@@ -12,6 +12,11 @@ import (
 	"testing"
 	"time"
 
+	"github.com/bradfitz/iter"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/time/rate"
+
 	"github.com/anacrolix/dht"
 	_ "github.com/anacrolix/envpprof"
 	"github.com/anacrolix/missinggo"
@@ -21,10 +26,6 @@ import (
 	"github.com/anacrolix/torrent/iplist"
 	"github.com/anacrolix/torrent/metainfo"
 	"github.com/anacrolix/torrent/storage"
-	"github.com/bradfitz/iter"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"golang.org/x/time/rate"
 )
 
 func TestingConfig() *ClientConfig {
@@ -968,22 +969,40 @@ func makeMagnet(t *testing.T, cl *Client, dir string, name string) string {
 
 // https://github.com/anacrolix/torrent/issues/114
 func TestMultipleTorrentsWithEncryption(t *testing.T) {
+	testSeederLeecherPair(
+		t,
+		func(cfg *ClientConfig) {
+			cfg.HeaderObfuscationPolicy.Preferred = true
+			cfg.HeaderObfuscationPolicy.RequirePreferred = true
+		},
+		func(cfg *ClientConfig) {
+			cfg.HeaderObfuscationPolicy.RequirePreferred = false
+		},
+	)
+}
+
+// Test that the leecher can download a torrent in its entirety from the seeder. Note that the
+// seeder config is done first.
+func testSeederLeecherPair(t *testing.T, seeder func(*ClientConfig), leecher func(*ClientConfig)) {
 	cfg := TestingConfig()
-	cfg.DisableUTP = true
 	cfg.Seed = true
 	cfg.DataDir = filepath.Join(cfg.DataDir, "server")
-	cfg.ForceEncryption = true
 	os.Mkdir(cfg.DataDir, 0755)
+	seeder(cfg)
 	server, err := NewClient(cfg)
 	require.NoError(t, err)
 	defer server.Close()
 	defer testutil.ExportStatusWriter(server, "s")()
 	magnet1 := makeMagnet(t, server, cfg.DataDir, "test1")
+	// Extra torrents are added to test the seeder having to match incoming obfuscated headers
+	// against more than one torrent. See issue #114
 	makeMagnet(t, server, cfg.DataDir, "test2")
+	for i := 0; i < 100; i++ {
+		makeMagnet(t, server, cfg.DataDir, fmt.Sprintf("test%d", i+2))
+	}
 	cfg = TestingConfig()
-	cfg.DisableUTP = true
 	cfg.DataDir = filepath.Join(cfg.DataDir, "client")
-	cfg.ForceEncryption = true
+	leecher(cfg)
 	client, err := NewClient(cfg)
 	require.NoError(t, err)
 	defer client.Close()
@@ -996,6 +1015,37 @@ func TestMultipleTorrentsWithEncryption(t *testing.T) {
 	client.WaitAll()
 }
 
+// This appears to be the situation with the S3 BitTorrent client.
+func TestObfuscatedHeaderFallbackSeederDisallowsLeecherPrefers(t *testing.T) {
+	// Leecher prefers obfuscation, but the seeder does not allow it.
+	testSeederLeecherPair(
+		t,
+		func(cfg *ClientConfig) {
+			cfg.HeaderObfuscationPolicy.Preferred = false
+			cfg.HeaderObfuscationPolicy.RequirePreferred = true
+		},
+		func(cfg *ClientConfig) {
+			cfg.HeaderObfuscationPolicy.Preferred = true
+			cfg.HeaderObfuscationPolicy.RequirePreferred = false
+		},
+	)
+}
+
+func TestObfuscatedHeaderFallbackSeederRequiresLeecherPrefersNot(t *testing.T) {
+	// Leecher prefers no obfuscation, but the seeder enforces it.
+	testSeederLeecherPair(
+		t,
+		func(cfg *ClientConfig) {
+			cfg.HeaderObfuscationPolicy.Preferred = true
+			cfg.HeaderObfuscationPolicy.RequirePreferred = true
+		},
+		func(cfg *ClientConfig) {
+			cfg.HeaderObfuscationPolicy.Preferred = false
+			cfg.HeaderObfuscationPolicy.RequirePreferred = false
+		},
+	)
+}
+
 func TestClientAddressInUse(t *testing.T) {
 	s, _ := NewUtpSocket("udp", ":50007", nil)
 	if s != nil {

config.go

@@ -12,6 +12,7 @@ import (
 	"github.com/anacrolix/missinggo/conntrack"
 	"github.com/anacrolix/missinggo/expect"
 	"github.com/anacrolix/torrent/iplist"
+	"github.com/anacrolix/torrent/mse"
 	"github.com/anacrolix/torrent/storage"
 	"golang.org/x/time/rate"
 )
@@ -67,7 +68,9 @@ type ClientConfig struct {
 	// used.
 	DefaultStorage storage.ClientImpl
 
-	EncryptionPolicy
+	HeaderObfuscationPolicy HeaderObfuscationPolicy
+	CryptoProvides          mse.CryptoMethod
+	CryptoSelector          mse.CryptoSelector
 
 	// Sets usage of Socks5 Proxy. Authentication should be included in the url if needed.
 	// Examples: socks5://demo:demo@192.168.99.100:1080
@@ -155,14 +158,19 @@ func NewDefaultClientConfig() *ClientConfig {
 		UploadRateLimiter:              unlimited,
 		DownloadRateLimiter:            unlimited,
 		ConnTracker:                    conntrack.NewInstance(),
+		HeaderObfuscationPolicy: HeaderObfuscationPolicy{
+			Preferred:        true,
+			RequirePreferred: false,
+		},
+		CryptoSelector: mse.DefaultCryptoSelector,
+		CryptoProvides: mse.AllSupportedCrypto,
 	}
 	cc.ConnTracker.SetNoMaxEntries()
 	cc.ConnTracker.Timeout = func(conntrack.Entry) time.Duration { return 0 }
 	return cc
 }
 
-type EncryptionPolicy struct {
-	DisableEncryption  bool
-	ForceEncryption    bool // Don't allow unobfuscated connections.
-	PreferNoEncryption bool
+type HeaderObfuscationPolicy struct {
+	RequirePreferred bool // Whether the value of Preferred is a strict requirement
+	Preferred        bool // Whether header obfuscation is preferred
 }

handshake.go

@@ -30,14 +30,15 @@ func (r deadlineReader) Read(b []byte) (int, error) {
 func handleEncryption(
 	rw io.ReadWriter,
 	skeys mse.SecretKeyIter,
-	policy EncryptionPolicy,
+	policy HeaderObfuscationPolicy,
+	selector mse.CryptoSelector,
 ) (
 	ret io.ReadWriter,
 	headerEncrypted bool,
 	cryptoMethod mse.CryptoMethod,
 	err error,
 ) {
-	if !policy.ForceEncryption {
+	if !policy.RequirePreferred || !policy.Preferred {
 		var protocol [len(pp.Protocol)]byte
 		_, err = io.ReadFull(rw, protocol[:])
 		if err != nil {
@@ -54,20 +55,13 @@ func handleEncryption(
 			ret = rw
 			return
 		}
+		if policy.RequirePreferred {
+			err = fmt.Errorf("unexpected protocol string %q and header obfuscation disabled", protocol)
+			return
+		}
 	}
 	headerEncrypted = true
-	ret, cryptoMethod, err = mse.ReceiveHandshake(rw, skeys, func(provides mse.CryptoMethod) mse.CryptoMethod {
-		switch {
-		case policy.ForceEncryption:
-			return mse.CryptoMethodRC4
-		case policy.DisableEncryption:
-			return mse.CryptoMethodPlaintext
-		case policy.PreferNoEncryption && provides&mse.CryptoMethodPlaintext != 0:
-			return mse.CryptoMethodPlaintext
-		default:
-			return mse.DefaultCryptoSelector(provides)
-		}
-	})
+	ret, cryptoMethod, err = mse.ReceiveHandshake(rw, skeys, selector)
 	return
 }