HTTP auth improvements complete

psiphon/upstreamproxy/auth_digest.go

@@ -36,9 +36,10 @@ const (
 )
 
 type DigestHttpAuthenticator struct {
-	state    DigestHttpAuthState
-	username string
-	password string
+	state         DigestHttpAuthState
+	username      string
+	password      string
+	digestHeaders *DigestHeaders
 }
 
 func newDigestAuthenticator(username, password string) *DigestHttpAuthenticator {
@@ -70,7 +71,6 @@ type DigestHeaders struct {
 // ApplyAuth adds proper auth header to the passed request
 func (d *DigestHeaders) ApplyAuth(req *http.Request) {
 	d.Nc += 0x1
-	d.Cnonce = randomKey()
 	d.Method = req.Method
 	d.digestChecksum()
 	response := h(strings.Join([]string{d.HA1, d.Nonce, fmt.Sprintf("%08x", d.Nc),
@@ -158,25 +158,38 @@ func (a *DigestHttpAuthenticator) Authenticate(req *http.Request, resp *http.Res
 
 	algorithm := digestParams["algorithm"]
 
-	d := &DigestHeaders{}
-	if req.Method == "CONNECT" {
-		d.Uri = req.URL.Host
-	} else {
-		d.Uri = req.URL.Scheme + "://" + req.URL.Host + req.URL.RequestURI()
+	if _, ok := digestParams["stale"]; ok {
+		// Server indicated that the nonce is stale
+		// Reset auth cache and state
+		a.digestHeaders = nil
+		a.state = DIGEST_HTTP_AUTH_STATE_CHALLENGE_RECEIVED
+		return nil
 	}
-	d.Realm = digestParams["realm"]
-	d.Qop = digestParams["qop"]
-	d.Nonce = digestParams["nonce"]
-	d.Opaque = digestParams["opaque"]
-	if algorithm == "" {
-		d.Algorithm = "MD5"
-	} else {
-		d.Algorithm = digestParams["algorithm"]
+
+	if a.digestHeaders == nil {
+		d := &DigestHeaders{}
+		if req.Method == "CONNECT" {
+			d.Uri = req.URL.Host
+		} else {
+			d.Uri = req.URL.Scheme + "://" + req.URL.Host + req.URL.RequestURI()
+		}
+		d.Realm = digestParams["realm"]
+		d.Qop = digestParams["qop"]
+		d.Nonce = digestParams["nonce"]
+		d.Opaque = digestParams["opaque"]
+		if algorithm == "" {
+			d.Algorithm = "MD5"
+		} else {
+			d.Algorithm = digestParams["algorithm"]
+		}
+		d.Nc = 0x0
+		d.Cnonce = randomKey()
+		d.Username = a.username
+		d.Password = a.password
+		a.digestHeaders = d
 	}
-	d.Nc = 0x0
-	d.Username = a.username
-	d.Password = a.password
-	d.ApplyAuth(req)
+
+	a.digestHeaders.ApplyAuth(req)
 	a.state = DIGEST_HTTP_AUTH_STATE_RESPONSE_GENERATED
 	return nil
 }
@@ -194,5 +207,8 @@ func (a *DigestHttpAuthenticator) Reset() {
 }
 
 func (a *DigestHttpAuthenticator) PreAuthenticate(req *http.Request) error {
+	if a.digestHeaders != nil {
+		a.digestHeaders.ApplyAuth(req)
+	}
 	return nil
 }

psiphon/upstreamproxy/transport_proxy_auth.go

@@ -42,7 +42,6 @@ type ProxyAuthTransport struct {
 	Username      string
 	Password      string
 	Authenticator HttpAuthenticator
-	authState     HttpAuthState
 	mu            sync.Mutex
 }
 
@@ -51,7 +50,7 @@ func NewProxyAuthTransport(rawTransport *http.Transport) (*ProxyAuthTransport, e
 	if dialFn == nil {
 		dialFn = net.Dial
 	}
-	tr := &ProxyAuthTransport{Dial: dialFn, authState: HTTP_AUTH_STATE_UNCHALLENGED}
+	tr := &ProxyAuthTransport{Dial: dialFn}
 	proxyUrlFn := rawTransport.Proxy
 	if proxyUrlFn != nil {
 		wrappedDialFn := tr.wrapTransportDial()
@@ -76,6 +75,8 @@ func NewProxyAuthTransport(rawTransport *http.Transport) (*ProxyAuthTransport, e
 }
 
 func (tr *ProxyAuthTransport) preAuthenticateRequest(req *http.Request) error {
+	tr.mu.Lock()
+	defer tr.mu.Unlock()
 	if tr.Authenticator == nil {
 		return nil
 	}
@@ -91,10 +92,10 @@ func (tr *ProxyAuthTransport) RoundTrip(req *http.Request) (resp *http.Response,
 		return nil, err
 	}
 
-	//Clone request early because RoundTrip will destroy request Body
 	var ha HttpAuthenticator = nil
+
+	//Clone request early because RoundTrip will destroy request Body
 	newReq := cloneRequest(req)
-	//authState := HTTP_AUTH_STATE_UNCHALLENGED
 
 	resp, err = tr.Transport.RoundTrip(newReq)
 
@@ -103,16 +104,19 @@ func (tr *ProxyAuthTransport) RoundTrip(req *http.Request) (resp *http.Response,
 	}
 
 	if resp.StatusCode == 407 {
-		fmt.Println("407!")
 		tr.mu.Lock()
 		defer tr.mu.Unlock()
+		if err != nil {
+			//already wrapped in proxyError
+			return nil, err
+		}
 		if tr.Authenticator == nil {
 			ha, err = NewHttpAuthenticator(resp, tr.Username, tr.Password)
 			if err != nil {
 				return nil, err
 			}
 			if ha.IsConnectionBased() {
-				return nil, proxyError(fmt.Errorf("Connection based auth was not handled by transportConn"))
+				return nil, proxyError(fmt.Errorf("Connection based auth was not handled by transportConn!"))
 			}
 			tr.Authenticator = ha
 		}
@@ -133,6 +137,7 @@ func (tr *ProxyAuthTransport) RoundTrip(req *http.Request) (resp *http.Response,
 					tr.Authenticator.Reset()
 				}
 				break authenticationLoop
+			} else {
 			}
 		}
 	}
@@ -155,8 +160,6 @@ func (tr *ProxyAuthTransport) wrapTransportDial() DialFunc {
 	}
 }
 
-// cloneRequest returns a clone of the provided *http.Request. The clone is a
-// shallow copy of the struct and its Header map.
 func cloneRequest(r *http.Request) *http.Request {
 	// shallow copy of the struct
 	r2 := new(http.Request)
@@ -170,8 +173,10 @@ func cloneRequest(r *http.Request) *http.Request {
 	if r.Body != nil {
 		body, _ := ioutil.ReadAll(r.Body)
 		defer r.Body.Close()
-		//restore original request Body
+		// restore original request Body
+		// drained by ReadAll()
 		r.Body = ioutil.NopCloser(bytes.NewReader(body))
+
 		r2.Body = ioutil.NopCloser(bytes.NewReader(body))
 	}
 	return r2
@@ -182,12 +187,9 @@ type transportConn struct {
 	requestInterceptor io.Writer
 	reqDone            chan struct{}
 	errChannel         chan error
-	// last written request holder
-	lastRequest   *http.Request
-	authenticator HttpAuthenticator
-	authState     HttpAuthState
-	transport     *ProxyAuthTransport
-	//mutex         *sync.Mutex
+	lastRequest        *http.Request
+	authenticator      HttpAuthenticator
+	transport          *ProxyAuthTransport
 }
 
 func newTransportConn(c net.Conn, tr *ProxyAuthTransport) *transportConn {
@@ -227,6 +229,8 @@ func newTransportConn(c net.Conn, tr *ProxyAuthTransport) *transportConn {
 
 // Read peeks into the new response and checks if the proxy requests authentication
 // If so, the last intercepted request is authenticated against the response
+// in case of connection based auth scheme(i.e. NTLM)
+// All the non-connection based schemes are handled by the ProxyAuthTransport.RoundTrip()
 func (tc *transportConn) Read(p []byte) (n int, read_err error) {
 	n, read_err = tc.Conn.Read(p)
 	if n < HTTP_STAT_LINE_LENGTH {
@@ -251,14 +255,15 @@ func (tc *transportConn) Read(p []byte) (n int, read_err error) {
 				return 0, err
 			}
 			// If connection based auth is requested, we are going to
-			// authenticate this very connection
-
+			// authenticate request on this very connection
+			// otherwise just return what we read
 			if !ha.IsConnectionBased() {
 				return
 			}
 
 			// Drain the rest of the response
 			// in order to perform auth handshake
+			// on the connection
 			readBufferReader.Seek(0, 0)
 			responseReader = bufio.NewReader(io.MultiReader(readBufferReader, tc.Conn))
 			resp, err = http.ReadResponse(responseReader, nil)