Browse Source

Merge pull request #34 from rod-hynes/master

Bug fixes
Rod Hynes 11 years ago
parent
commit
bc22a41e79
5 changed files with 27 additions and 12 deletions
  1. 2 1
      psiphon/meekConn.go
  2. 1 0
      psiphon/obfuscatedSshConn.go
  3. 2 1
      psiphon/obfuscator.go
  4. 21 9
      psiphon/socksProxy.go
  5. 1 1
      psiphon/utils.go

+ 2 - 1
psiphon/meekConn.go

@@ -515,7 +515,8 @@ func makeCookie(serverEntry *ServerEntry, sessionId string) (cookie *http.Cookie
 	// The format is <random letter 'A'-'Z'>=<base64 data>, which is intended to match common cookie formats.
 	A := int('A')
 	Z := int('Z')
-	letterIndex, err := MakeSecureRandomInt(Z - A)
+	// letterIndex is integer in range [int('A'), int('Z')]
+	letterIndex, err := MakeSecureRandomInt(Z - A + 1)
 	if err != nil {
 		return nil, ContextError(err)
 	}

+ 1 - 0
psiphon/obfuscatedSshConn.go

@@ -304,6 +304,7 @@ func (conn *ObfuscatedSshConn) transformAndWrite(buffer []byte) (err error) {
 			// See RFC 4253 sec. 6 for constraints
 			possiblePaddings := (SSH_MAX_PADDING_LENGTH - paddingLength) / SSH_PADDING_MULTIPLE
 			if possiblePaddings > 0 {
+				// selectedPadding is integer in range [0, possiblePaddings)
 				selectedPadding, err := MakeSecureRandomInt(possiblePaddings)
 				if err != nil {
 					return ContextError(err)

+ 2 - 1
psiphon/obfuscator.go

@@ -125,7 +125,8 @@ func deriveKey(seed, keyword, iv []byte) ([]byte, error) {
 }
 
 func makeSeedMessage(maxPadding int, seed []byte, clientToServerCipher *rc4.Cipher) ([]byte, error) {
-	paddingLength, err := MakeSecureRandomInt(maxPadding)
+	// paddingLength is integer in range [0, maxPadding]
+	paddingLength, err := MakeSecureRandomInt(maxPadding + 1)
 	if err != nil {
 		return nil, ContextError(err)
 	}

+ 21 - 9
psiphon/socksProxy.go

@@ -31,10 +31,11 @@ import (
 // the tunnel SSH client and relays traffic through the port
 // forward.
 type SocksProxy struct {
-	tunneler       Tunneler
-	listener       *socks.SocksListener
-	serveWaitGroup *sync.WaitGroup
-	openConns      *Conns
+	tunneler               Tunneler
+	listener               *socks.SocksListener
+	serveWaitGroup         *sync.WaitGroup
+	openConns              *Conns
+	stopListeningBroadcast chan struct{}
 }
 
 // NewSocksProxy initializes a new SOCKS server. It begins listening for
@@ -47,10 +48,11 @@ func NewSocksProxy(config *Config, tunneler Tunneler) (proxy *SocksProxy, err er
 		return nil, ContextError(err)
 	}
 	proxy = &SocksProxy{
-		tunneler:       tunneler,
-		listener:       listener,
-		serveWaitGroup: new(sync.WaitGroup),
-		openConns:      new(Conns),
+		tunneler:               tunneler,
+		listener:               listener,
+		serveWaitGroup:         new(sync.WaitGroup),
+		openConns:              new(Conns),
+		stopListeningBroadcast: make(chan struct{}),
 	}
 	proxy.serveWaitGroup.Add(1)
 	go proxy.serve()
@@ -61,6 +63,7 @@ func NewSocksProxy(config *Config, tunneler Tunneler) (proxy *SocksProxy, err er
 // Close terminates the listener and waits for the accept loop
 // goroutine to complete.
 func (proxy *SocksProxy) Close() {
+	close(proxy.stopListeningBroadcast)
 	proxy.listener.Close()
 	proxy.serveWaitGroup.Wait()
 	proxy.openConns.CloseAll()
@@ -86,15 +89,24 @@ func (proxy *SocksProxy) socksConnectionHandler(localConn *socks.SocksConn) (err
 func (proxy *SocksProxy) serve() {
 	defer proxy.listener.Close()
 	defer proxy.serveWaitGroup.Done()
+loop:
 	for {
 		// Note: will be interrupted by listener.Close() call made by proxy.Close()
 		socksConnection, err := proxy.listener.AcceptSocks()
+		// Can't check for the exact error that Close() will cause in Accept(),
+		// (see: https://code.google.com/p/go/issues/detail?id=4373). So using an
+		// explicit stop signal to stop gracefully.
+		select {
+		case <-proxy.stopListeningBroadcast:
+			break loop
+		default:
+		}
 		if err != nil {
 			Notice(NOTICE_ALERT, "SOCKS proxy accept error: %s", err)
 			if e, ok := err.(net.Error); ok && !e.Temporary() {
 				proxy.tunneler.SignalFailure()
 				// Fatal error, stop the proxy
-				break
+				break loop
 			}
 			// Temporary error, keep running
 			continue

+ 1 - 1
psiphon/utils.go

@@ -42,7 +42,7 @@ func Contains(list []string, target string) bool {
 }
 
 // MakeSecureRandomInt is a helper function that wraps
-// crypto/rand.Int.
+// crypto/rand.Int, which returns a uniform random value in [0, max).
 func MakeSecureRandomInt(max int) (int, error) {
 	randomInt, err := rand.Int(rand.Reader, big.NewInt(int64(max)))
 	if err != nil {