From 9008f91097f0e614f9d589f5e4ceea4ace0e54d0 Mon Sep 17 00:00:00 2001
From: ssoxer <ssoxer@users.noreply.github.com>
Date: Thu, 13 Jun 2019 19:34:36 -0600
Subject: [PATCH 1/3] Add a test to check that client and server IVs are
 different.

---
 shadowsocks/conn_test.go | 89 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 shadowsocks/conn_test.go

diff --git a/shadowsocks/conn_test.go b/shadowsocks/conn_test.go
new file mode 100644
index 00000000..b24aed75
--- /dev/null
+++ b/shadowsocks/conn_test.go
@@ -0,0 +1,89 @@
+package shadowsocks
+
+import (
+	"bytes"
+	"io"
+	"net"
+	"testing"
+)
+
+func mustNewCipher(method string) *Cipher {
+	const testPassword = "password"
+	cipher, err := NewCipher(method, testPassword)
+	if err != nil {
+		panic(err)
+	}
+	return cipher
+}
+
+type transcriptConn struct {
+	net.Conn
+	ReadTranscript []byte
+}
+
+func (conn *transcriptConn) Read(p []byte) (int, error) {
+	n, err := conn.Conn.Read(p)
+	conn.ReadTranscript = append(conn.ReadTranscript, p[:n]...)
+	return n, err
+}
+
+func connIVs(method string) (clientIV, serverIV []byte, err error) {
+	// underlying network connection
+	clientConn, serverConn := net.Pipe()
+	// make a transcript of bytes at the network level
+	clientTranscriptConn := &transcriptConn{Conn: clientConn}
+	serverTranscriptConn := &transcriptConn{Conn: serverConn}
+	// connection at the ShadowSocks level
+	clientSSConn := NewConn(clientTranscriptConn, mustNewCipher(method))
+	serverSSConn := NewConn(serverTranscriptConn, mustNewCipher(method))
+
+	clientToServerData := []byte("clientToServerData")
+	serverToClientData := []byte("serverToClientData")
+
+	go func() {
+		defer serverSSConn.Close()
+		buf := make([]byte, len(clientToServerData))
+		// read the client IV
+		_, err := io.ReadFull(serverSSConn, buf)
+		if err != nil {
+			return
+		}
+		// send the server IV
+		_, err = serverSSConn.Write(serverToClientData)
+		if err != nil {
+			return
+		}
+	}()
+
+	// send the client IV
+	_, err = clientSSConn.Write(clientToServerData)
+	if err != nil {
+		return
+	}
+	// read the server IV
+	buf := make([]byte, len(serverToClientData))
+	_, err = io.ReadFull(clientSSConn, buf)
+	if err != nil {
+		return
+	}
+
+	// pull the IVs out of the network transcripts
+	clientIV = serverTranscriptConn.ReadTranscript[:clientSSConn.Cipher.info.ivLen]
+	serverIV = clientTranscriptConn.ReadTranscript[:serverSSConn.Cipher.info.ivLen]
+
+	return
+}
+
+func TestIndependentIVs(t *testing.T) {
+	for method := range cipherMethod {
+		clientIV, serverIV, err := connIVs(method)
+		if err != nil {
+			t.Errorf("%s connection error: %s", method, err)
+			continue
+		}
+		if bytes.Equal(clientIV, serverIV) {
+			t.Errorf("%s equal client and server IVs", method)
+			continue
+		}
+	}
+}

From c3326cd0c0ac3b371d687fc3bdcd22bb9d69fc0f Mon Sep 17 00:00:00 2001
From: ssoxer <ssoxer@users.noreply.github.com>
Date: Thu, 13 Jun 2019 18:26:29 -0600
Subject: [PATCH 2/3] Generate a fresh server IV, do not copy the client's.

Copying the client IV makes connections easily detectable, and allows
decryption of one direction if the plaintext of the other direction can
be guessed.
---
 shadowsocks/conn.go | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/shadowsocks/conn.go b/shadowsocks/conn.go
index 84b461a3..e6e3ce95 100644
--- a/shadowsocks/conn.go
+++ b/shadowsocks/conn.go
@@ -99,9 +99,6 @@ func (c *Conn) Read(b []byte) (n int, err error) {
 		if err = c.initDecrypt(iv); err != nil {
 			return
 		}
-		if len(c.iv) == 0 {
-			c.iv = iv
-		}
 	}
 
 	cipherData := c.readBuf

From 1c9f75773bcd72a5bbeecfb2f31529582ffcfccf Mon Sep 17 00:00:00 2001
From: ssoxer <ssoxer@users.noreply.github.com>
Date: Thu, 13 Jun 2019 18:52:30 -0600
Subject: [PATCH 3/3] Remove `iv` from the `Cipher` type.

It is dangerous to have a single `iv` member, because there must be
separate IVs for encryption and decryption. Instead, store the IVs
implicitly inside the `cipher.Stream` objects. The `GetIv` and `GetKey`
functions were unused leftovers from OTA support, added in commit
89460d2e476940f0b702e4bdff4991152d9cdbbe.
---
 shadowsocks/conn.go    | 12 ------------
 shadowsocks/encrypt.go | 12 +++---------
 2 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/shadowsocks/conn.go b/shadowsocks/conn.go
index e6e3ce95..5d264b74 100644
--- a/shadowsocks/conn.go
+++ b/shadowsocks/conn.go
@@ -78,18 +78,6 @@ func Dial(addr, server string, cipher *Cipher) (c *Conn, err error) {
 	return DialWithRawAddr(ra, server, cipher)
 }
 
-func (c *Conn) GetIv() (iv []byte) {
-	iv = make([]byte, len(c.iv))
-	copy(iv, c.iv)
-	return
-}
-
-func (c *Conn) GetKey() (key []byte) {
-	key = make([]byte, len(c.key))
-	copy(key, c.key)
-	return
-}
-
 func (c *Conn) Read(b []byte) (n int, err error) {
 	if c.dec == nil {
 		iv := make([]byte, c.info.ivLen)
diff --git a/shadowsocks/encrypt.go b/shadowsocks/encrypt.go
index eee4c366..60790d8a 100644
--- a/shadowsocks/encrypt.go
+++ b/shadowsocks/encrypt.go
@@ -188,7 +188,6 @@ type Cipher struct {
 	dec  cipher.Stream
 	key  []byte
 	info *cipherInfo
-	iv   []byte
 }
 
 // NewCipher creates a cipher that can be used in Dial() etc.
@@ -215,14 +214,9 @@ func NewCipher(method, password string) (c *Cipher, err error) {
 
 // Initializes the block cipher with CFB mode, returns IV.
 func (c *Cipher) initEncrypt() (iv []byte, err error) {
-	if c.iv == nil {
-		iv = make([]byte, c.info.ivLen)
-		if _, err := io.ReadFull(rand.Reader, iv); err != nil {
-			return nil, err
-		}
-		c.iv = iv
-	} else {
-		iv = c.iv
+	iv = make([]byte, c.info.ivLen)
+	if _, err := io.ReadFull(rand.Reader, iv); err != nil {
+		return nil, err
 	}
 	c.enc, err = c.info.newStream(c.key, iv, Encrypt)
 	return