Change the maximm handshake length to 8192 bytes.

 * handhake_ntor_test now is considerably more comprehensive.
 * The padding related constants in the spec were clarified.

This breaks wireprotocol compatibility.

5 files changed, 246 insertions, 48 deletions

diff --git a/doc/obfs4-spec.txt b/doc/obfs4-spec.txt
index e4092ed..1ac2f30 100644
--- a/doc/obfs4-spec.txt
+++ b/doc/obfs4-spec.txt
@@ -58,7 +58,7 @@
    obfs4 provides integrity and confidentiality of the underlying traffic,
    and authentication of the server.
 
-3. Notation, Constants and Terminology
+3. Notation and Terminology
 
    All Curve25519 keys and Elligator 2 representatives are transmitted in the
    Little Endian representation, for ease of integration with current
@@ -81,6 +81,71 @@
    The server distributes the public component of the identity key (B) and
    NODEID to the client via an out-of-band mechanism.
 
+   Data sent as part of the handshake are padded to random lengths to attempt to
+   obfuscate the initial flow signature.  The constants used are as follows:
+
+     MaximumHandshakeLength = 8192
+
+       Maximum size of a handshake request or response, including padding.
+
+     MarkLength = 16
+
+       Length of M_C/M_S (A HMAC-SHA256-128 digest).
+
+     MACLength = 16
+
+       Length of MAC_C/MAC_S (A HMAC-SHA256-128 digest).
+
+     RepresentativeLength = 32
+
+       Length of a Elligator 2 representative of a Curve25519 public key.
+
+     AuthLength = 32
+
+       Length of the ntor AUTH tag (A HMAC-SHA256 digest).
+
+     InlineSeedFrameLength = 53
+
+       Length of a unpadded TYPE_PRNG_SEED frame.
+
+     ServerHandshakeLength = 96
+
+       The length of the non-padding data in a handshake response.
+
+       RepresentativeLength + AuthLength + MarkLength + MACLength
+
+     ServerMaxPadLength = 8096
+
+       The maximum amount of padding in a handshake response.
+
+       MaximumHandshakeLength - ServerHandshakeLength
+
+     ServerMinPadLength = InlineSeedFrameLength
+
+       The minimum amount of padding in a handshake response.
+
+     ClientHandshakeLength = 64
+
+       The length of the non-padding data in a handshake request.
+
+       RepresentativeLength + MarkLength + MACLength
+
+     ClientMinPadLength = 85
+
+       The minimum amount of padding in a handshake request.
+
+       (ServerHandshakeLength + ServerMinPadLength) - ClientHandshakeLength
+
+     ClientMaxPadLength = 8128
+
+       The maximum amount of padding in a handshake request.
+
+       MaximumHandshakeLength - ClientHandshakeLength
+
+   The amount of padding is chosen such that the smallest possible request and
+   response (requests and responses with the minimum amount of padding) are
+   equal in size.  For details on the InlineSeedFrameLength, see section 7.
+
    The client handshake process is as follows.
 
     1. The client generates an ephemeral Curve25519 keypair X,x and an
@@ -89,7 +154,7 @@
     2. The client sends a handshake request to the server where:
 
            X' = Elligator 2 representative of X (32 bytes)
-           P_C = Random padding [85, 1384] bytes long
+           P_C = Random padding [ClientMinPadLength, ClientMaxPadLength] bytes
            M_C = HMAC-SHA256-128(B | NODEID, X')
            E = String representation of the number of hours since the UNIX
                epoch
@@ -145,7 +210,7 @@
 
            Y' = Elligator 2 Representative of Y (32 bytes)
            AUTH = The ntor authentication tag (32 bytes)
-           P_S = Random padding [0, 1352] bytes long
+           P_S = Random padding [ServerMinPadLength, ServerMaxPadLength] bytes
            M_S = HMAC-SHA256-128(B | NODEID, Y')
            E' = E from the client request
            MAC_S = HMAC-SHA256-128(B | NODEID, Y' | AUTH | P_S | M_S | E')
@@ -228,7 +293,9 @@
    part of the serverResponse if it always sends the frame immediately
    following the serverResponse body.  If implementations chose to do this,
    the TYPE_PRNG_SEED frame MUST have 0 bytes of padding, and P_S MUST
-   consist of [0,1299] bytes of random padding.
+   be generated with a ServerMinPadLength of 0 (P_S consists of [0,8096]
+   bytes of random data).  The calculation of ClientMinPadLength however is
+   unchanged (P_C still consists of [85,8128] bytes of random data).
  
 7. References
 
diff --git a/handshake_ntor.go b/handshake_ntor.go
index 3a8b36e..8baa382 100644
--- a/handshake_ntor.go
+++ b/handshake_ntor.go
@@ -44,18 +44,18 @@ import (
 )
 
 const (
+	maxHandshakeLength = 8192
+
 	clientMinPadLength = (serverMinHandshakeLength + inlineSeedFrameLength) -
 		clientMinHandshakeLength
-	clientMaxPadLength       = framing.MaximumSegmentLength - clientMinHandshakeLength
+	clientMaxPadLength       = maxHandshakeLength - clientMinHandshakeLength
 	clientMinHandshakeLength = ntor.RepresentativeLength + markLength + macLength
-	clientMaxHandshakeLength = framing.MaximumSegmentLength
 
 	serverMinPadLength = 0
-	serverMaxPadLength = framing.MaximumSegmentLength - (serverMinHandshakeLength +
+	serverMaxPadLength = maxHandshakeLength - (serverMinHandshakeLength +
 		inlineSeedFrameLength)
 	serverMinHandshakeLength = ntor.RepresentativeLength + ntor.AuthLength +
 		markLength + macLength
-	serverMaxHandshakeLength = framing.MaximumSegmentLength
 
 	markLength = sha256.Size / 2
 	macLength  = sha256.Size / 2
@@ -113,7 +113,8 @@ type clientHandshake struct {
 	serverIdentity *ntor.PublicKey
 	epochHour      []byte
 
-	mac hash.Hash
+	padLen int
+	mac    hash.Hash
 
 	serverRepresentative *ntor.Representative
 	serverAuth           *ntor.Auth
@@ -130,6 +131,7 @@ func newClientHandshake(nodeID *ntor.NodeID, serverIdentity *ntor.PublicKey) (*c
 	}
 	hs.nodeID = nodeID
 	hs.serverIdentity = serverIdentity
+	hs.padLen = randRange(clientMinPadLength, clientMaxPadLength)
 	hs.mac = hmac.New(sha256.New, append(hs.serverIdentity.Bytes()[:], hs.nodeID.Bytes()[:]...))
 
 	return hs, nil
@@ -151,7 +153,7 @@ func (hs *clientHandshake) generateHandshake() ([]byte, error) {
 	//    epoch.
 
 	// Generate the padding
-	pad, err := makePad(clientMinPadLength, clientMaxPadLength)
+	pad, err := makePad(hs.padLen)
 	if err != nil {
 		return nil, err
 	}
@@ -193,9 +195,9 @@ func (hs *clientHandshake) parseServerHandshake(resp []byte) (int, []byte, error
 
 	// Attempt to find the mark + MAC.
 	pos := findMarkMac(hs.serverMark, resp, ntor.RepresentativeLength+ntor.AuthLength+serverMinPadLength,
-		serverMaxHandshakeLength, false)
+		maxHandshakeLength, false)
 	if pos == -1 {
-		if len(resp) >= serverMaxHandshakeLength {
+		if len(resp) >= maxHandshakeLength {
 			return 0, nil, ErrInvalidHandshake
 		}
 		return 0, nil, ErrMarkNotFoundYet
@@ -232,7 +234,8 @@ type serverHandshake struct {
 	epochHour      []byte
 	serverAuth     *ntor.Auth
 
-	mac hash.Hash
+	padLen int
+	mac    hash.Hash
 
 	clientRepresentative *ntor.Representative
 	clientMark           []byte
@@ -242,6 +245,7 @@ func newServerHandshake(nodeID *ntor.NodeID, serverIdentity *ntor.Keypair) *serv
 	hs := new(serverHandshake)
 	hs.nodeID = nodeID
 	hs.serverIdentity = serverIdentity
+	hs.padLen = randRange(serverMinPadLength, serverMaxPadLength)
 	hs.mac = hmac.New(sha256.New, append(hs.serverIdentity.Public().Bytes()[:], hs.nodeID.Bytes()[:]...))
 
 	return hs
@@ -267,9 +271,9 @@ func (hs *serverHandshake) parseClientHandshake(filter *replayFilter, resp []byt
 
 	// Attempt to find the mark + MAC.
 	pos := findMarkMac(hs.clientMark, resp, ntor.RepresentativeLength+clientMinPadLength,
-		clientMaxHandshakeLength, true)
+		maxHandshakeLength, true)
 	if pos == -1 {
-		if len(resp) >= clientMaxHandshakeLength {
+		if len(resp) >= maxHandshakeLength {
 			return nil, ErrInvalidHandshake
 		}
 		return nil, ErrMarkNotFoundYet
@@ -349,7 +353,7 @@ func (hs *serverHandshake) generateHandshake() ([]byte, error) {
 	//    epoch.
 
 	// Generate the padding
-	pad, err := makePad(serverMinPadLength, serverMaxPadLength)
+	pad, err := makePad(hs.padLen)
 	if err != nil {
 		return nil, err
 	}
@@ -422,8 +426,7 @@ func findMarkMac(mark, buf []byte, startPos, maxPos int, fromTail bool) (pos int
 	return
 }
 
-func makePad(min, max int) ([]byte, error) {
-	padLen := randRange(min, max)
+func makePad(padLen int) ([]byte, error) {
 	pad := make([]byte, padLen)
 	_, err := rand.Read(pad)
 	if err != nil {
diff --git a/handshake_ntor_test.go b/handshake_ntor_test.go
index 73b43bf..b3e0a4d 100644
--- a/handshake_ntor_test.go
+++ b/handshake_ntor_test.go
@@ -38,44 +38,166 @@ func TestHandshakeNtor(t *testing.T) {
 	// Generate the server node id and id keypair.
 	nodeID, _ := ntor.NewNodeID([]byte("\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10\x11\x12\x13"))
 	idKeypair, _ := ntor.NewKeypair(false)
+	serverFilter, _ := newReplayFilter()
+
+	// Test client handshake padding.
+	for l := clientMinPadLength; l <= clientMaxPadLength; l++ {
+		// Generate the client state and override the pad length.
+		clientHs, err := newClientHandshake(nodeID, idKeypair.Public())
+		if err != nil {
+			t.Fatalf("[%d:0] newClientHandshake failed:", l, err)
+		}
+		clientHs.padLen = l
+
+		// Generate what the client will send to the server.
+		clientBlob, err := clientHs.generateHandshake()
+		if err != nil {
+			t.Fatalf("[%d:0] clientHandshake.generateHandshake() failed: %s", l, err)
+		}
+		if len(clientBlob) > maxHandshakeLength {
+			t.Fatalf("[%d:0] Generated client body is oversized: %d", l, len(clientBlob))
+		}
+		if len(clientBlob) < clientMinHandshakeLength {
+			t.Fatalf("[%d:0] Generated client body is undersized: %d", l, len(clientBlob))
+		}
+		if len(clientBlob) != clientMinHandshakeLength+l {
+			t.Fatalf("[%d:0] Generated client body incorrect size: %d", l, len(clientBlob))
+		}
+
+		// Generate the server state and override the pad length.
+		serverHs := newServerHandshake(nodeID, idKeypair)
+		serverHs.padLen = serverMinPadLength
+
+		// Parse the client handshake message.
+		serverSeed, err := serverHs.parseClientHandshake(serverFilter, clientBlob)
+		if err != nil {
+			t.Fatalf("[%d:0] serverHandshake.parseClientHandshake() failed: %s", l, err)
+		}
+
+		// Genrate what the server will send to the client.
+		serverBlob, err := serverHs.generateHandshake()
+		if err != nil {
+			t.Fatal("[%d:0]: serverHandshake.generateHandshake() failed: %s", l, err)
+		}
+
+		// Parse the server handshake message.
+		clientHs.serverRepresentative = nil
+		n, clientSeed, err := clientHs.parseServerHandshake(serverBlob)
+		if err != nil {
+			t.Fatalf("[%d:0] clientHandshake.parseServerHandshake() failed: %s", l, err)
+		}
+		if n != len(serverBlob) {
+			t.Fatalf("[%d:0] clientHandshake.parseServerHandshake() has bytes remaining: %d", l, n)
+		}
+
+		// Ensure the derived shared secret is the same.
+		if 0 != bytes.Compare(clientSeed, serverSeed) {
+			t.Fatalf("[%d:0] client/server seed mismatch", l)
+		}
+	}
+
+	// Test server handshake padding.
+	for l := serverMinPadLength; l <= serverMaxPadLength+inlineSeedFrameLength; l++ {
+		// Generate the client state and override the pad length.
+		clientHs, err := newClientHandshake(nodeID, idKeypair.Public())
+		if err != nil {
+			t.Fatalf("[%d:0] newClientHandshake failed:", l, err)
+		}
+		clientHs.padLen = clientMinPadLength
+
+		// Generate what the client will send to the server.
+		clientBlob, err := clientHs.generateHandshake()
+		if err != nil {
+			t.Fatalf("[%d:1] clientHandshake.generateHandshake() failed: %s", l, err)
+		}
+		if len(clientBlob) > maxHandshakeLength {
+			t.Fatalf("[%d:1] Generated client body is oversized: %d", l, len(clientBlob))
+		}
+
+		// Generate the server state and override the pad length.
+		serverHs := newServerHandshake(nodeID, idKeypair)
+		serverHs.padLen = l
+
+		// Parse the client handshake message.
+		serverSeed, err := serverHs.parseClientHandshake(serverFilter, clientBlob)
+		if err != nil {
+			t.Fatalf("[%d:1] serverHandshake.parseClientHandshake() failed: %s", l, err)
+		}
 
-	// Intialize the client and server handshake states
+		// Genrate what the server will send to the client.
+		serverBlob, err := serverHs.generateHandshake()
+		if err != nil {
+			t.Fatal("[%d:1]: serverHandshake.generateHandshake() failed: %s", l, err)
+		}
+
+		// Parse the server handshake message.
+		n, clientSeed, err := clientHs.parseServerHandshake(serverBlob)
+		if err != nil {
+			t.Fatalf("[%d:1] clientHandshake.parseServerHandshake() failed: %s", l, err)
+		}
+		if n != len(serverBlob) {
+			t.Fatalf("[%d:1] clientHandshake.parseServerHandshake() has bytes remaining: %d", l, n)
+		}
+
+		// Ensure the derived shared secret is the same.
+		if 0 != bytes.Compare(clientSeed, serverSeed) {
+			t.Fatalf("[%d:1] client/server seed mismatch", l)
+		}
+	}
+
+	// Test oversized client padding.
 	clientHs, err := newClientHandshake(nodeID, idKeypair.Public())
 	if err != nil {
-		t.Fatal("newClientHandshake failed:", err)
+		t.Fatalf("newClientHandshake failed:", err)
 	}
-	serverHs := newServerHandshake(nodeID, idKeypair)
-	serverFilter, _ := newReplayFilter()
 
-	// Generate what the client will send to the server.
-	cToS, err := clientHs.generateHandshake()
+	clientHs.padLen = clientMaxPadLength + 1
+	clientBlob, err := clientHs.generateHandshake()
 	if err != nil {
-		t.Fatal("clientHandshake.generateHandshake() failed", err)
+		t.Fatalf("clientHandshake.generateHandshake() (forced oversize) failed: %s", err)
+	}
+	serverHs := newServerHandshake(nodeID, idKeypair)
+	_, err = serverHs.parseClientHandshake(serverFilter, clientBlob)
+	if err == nil {
+		t.Fatalf("serverHandshake.parseClientHandshake() succeded (oversized)")
 	}
 
-	// Parse the client handshake message.
-	serverSeed, err := serverHs.parseClientHandshake(serverFilter, cToS)
+	// Test undersized client padding.
+	clientHs.padLen = clientMinPadLength - 1
+	clientBlob, err = clientHs.generateHandshake()
 	if err != nil {
-		t.Fatal("serverHandshake.parseClientHandshake() failed", err)
+		t.Fatalf("clientHandshake.generateHandshake() (forced undersize) failed: %s", err)
+	}
+	serverHs = newServerHandshake(nodeID, idKeypair)
+	_, err = serverHs.parseClientHandshake(serverFilter, clientBlob)
+	if err == nil {
+		t.Fatalf("serverHandshake.parseClientHandshake() succeded (undersized)")
 	}
 
-	// Genrate what the server will send to the client.
-	sToC, err := serverHs.generateHandshake()
+	// Test oversized server padding.
+	//
+	// NB: serverMaxPadLength isn't the real maxPadLength that triggers client
+	// rejection, because the implementation is written with the asusmption
+	// that/ the PRNG_SEED is also inlined with the response.  Thus the client
+	// actually accepts longer padding.  The server handshake test and this
+	// test adjust around that.
+	clientHs.padLen = clientMinPadLength
+	clientBlob, err = clientHs.generateHandshake()
 	if err != nil {
-		t.Fatal("serverHandshake.generateHandshake() failed", err)
+		t.Fatalf("clientHandshake.generateHandshake() failed: %s", err)
 	}
-
-	// Parse the server handshake message.
-	n, clientSeed, err := clientHs.parseServerHandshake(sToC)
+	serverHs = newServerHandshake(nodeID, idKeypair)
+	serverHs.padLen = serverMaxPadLength + inlineSeedFrameLength + 1
+	_, err = serverHs.parseClientHandshake(serverFilter, clientBlob)
 	if err != nil {
-		t.Fatal("clientHandshake.parseServerHandshake() failed", err)
+		t.Fatalf("serverHandshake.parseClientHandshake() failed: %s", err)
 	}
-	if n != len(sToC) {
-		t.Fatalf("clientHandshake.parseServerHandshake() has bytes remaining: %d", n)
+	serverBlob, err := serverHs.generateHandshake()
+	if err != nil {
+		t.Fatal("serverHandshake.generateHandshake() (forced oversize) failed: %s", err)
 	}
-
-	// Ensure the derived shared secret is the same.
-	if 0 != bytes.Compare(clientSeed, serverSeed) {
-		t.Fatalf("client/server seed mismatch")
+	_, _, err = clientHs.parseServerHandshake(serverBlob)
+	if err == nil {
+		t.Fatalf("clientHandshake.parseServerHandshake() succeded (oversized)")
 	}
 }
diff --git a/obfs4.go b/obfs4.go
index e9ba2e8..f32c222 100644
--- a/obfs4.go
+++ b/obfs4.go
@@ -48,7 +48,7 @@ const (
 	headerLength      = framing.FrameOverhead + packetOverhead
 	connectionTimeout = time.Duration(30) * time.Second
 
-	maxCloseDelayBytes = framing.MaximumSegmentLength * 5
+	maxCloseDelayBytes = maxHandshakeLength
 	maxCloseDelay      = 60
 )
 
@@ -181,7 +181,7 @@ func (c *Obfs4Conn) clientHandshake(nodeID *ntor.NodeID, publicKey *ntor.PublicK
 	}
 
 	// Consume the server handshake.
-	var hsBuf [serverMaxHandshakeLength]byte
+	var hsBuf [maxHandshakeLength]byte
 	for {
 		var n int
 		n, err = c.conn.Read(hsBuf[:])
@@ -235,7 +235,7 @@ func (c *Obfs4Conn) serverHandshake(nodeID *ntor.NodeID, keypair *ntor.Keypair)
 	}
 
 	// Consume the client handshake.
-	var hsBuf [clientMaxHandshakeLength]byte
+	var hsBuf [maxHandshakeLength]byte
 	for {
 		var n int
 		n, err = c.conn.Read(hsBuf[:])
diff --git a/replay_filter.go b/replay_filter.go
index f5c64f8..5e98b89 100644
--- a/replay_filter.go
+++ b/replay_filter.go
@@ -78,7 +78,7 @@ func newReplayFilter() (filter *replayFilter, err error) {
 }
 
 // testAndSet queries the filter for buf, adds it if it was not present and
-// returns if it has added the entry or not.
+// returns if it has added the entry or not.  This method is threadsafe.
 func (f *replayFilter) testAndSet(now int64, buf []byte) bool {
 	hash := siphash.Hash(f.key[0], f.key[1], buf)
 
@@ -102,7 +102,8 @@ func (f *replayFilter) testAndSet(now int64, buf []byte) bool {
 }
 
 // compactFilter purges entries that are too old to be relevant.  If the filter
-// is filled to maxFilterCapacity, it will force purge a single entry.
+// is filled to maxFilterCapacity, it will force purge a single entry.  This
+// method is NOT threadsafe.
 func (f *replayFilter) compactFilter(now int64) {
 	e := f.fifo.Front()
 	for e != nil {
@@ -116,8 +117,7 @@ func (f *replayFilter) compactFilter(now int64) {
 				// a lot.  This will eventually self-correct, but "eventually"
 				// could be a long time.  As much as this sucks, jettison the
 				// entire filter.
-				f.filter = make(map[uint64]*filterEntry)
-				f.fifo = list.New()
+				f.reset()
 				return
 			}
 			if deltaT < 3600*2 {
@@ -135,4 +135,10 @@ func (f *replayFilter) compactFilter(now int64) {
 	}
 }
 
+// reset purges the entire filter.  This methoid is NOT threadsafe.
+func (f *replayFilter) reset() {
+	f.filter = make(map[uint64]*filterEntry)
+	f.fifo = list.New()
+}
+
 /* vim :set ts=4 sw=4 sts=4 noet : */