HTTP proxy authentication fixes (#300)

- Only accepts HTTP 1.x protocols
- Only checks the credentials when the method is `CONNECT` or the request URL is absolute
- More logging on authorization failures
- Removes the authorization headers before forwarding the HTTP(s) requests
- Refers to #298

cmd/gluetun/main.go

@@ -243,7 +243,7 @@ func _main(background context.Context, args []string) int { //nolint:gocognit,go
 	go publicIPLooper.RunRestartTicker(ctx, wg)
 	publicIPLooper.SetPeriod(allSettings.PublicIPPeriod) // call after RunRestartTicker
 
-	httpProxyLooper := httpproxy.NewLooper(httpClient, logger, allSettings.HTTPProxy)
+	httpProxyLooper := httpproxy.NewLooper(logger, allSettings.HTTPProxy)
 	wg.Add(1)
 	go httpProxyLooper.Run(ctx, wg)
 

internal/httpproxy/accept.go

@@ -0,0 +1,24 @@
+package httpproxy
+
+import (
+	"fmt"
+	"net/http"
+)
+
+func (h *handler) isAccepted(responseWriter http.ResponseWriter, request *http.Request) bool {
+	// Not compatible with HTTP < 1.0 or HTTP >= 2.0 (see https://github.com/golang/go/issues/14797#issuecomment-196103814)
+	const (
+		minimalMajorVersion = 1
+		minimalMinorVersion = 0
+		maximumMajorVersion = 2
+		maximumMinorVersion = 0
+	)
+	if !request.ProtoAtLeast(minimalMajorVersion, minimalMinorVersion) ||
+		request.ProtoAtLeast(maximumMajorVersion, maximumMinorVersion) {
+		message := fmt.Sprintf("http version not supported: %s", request.Proto)
+		h.logger.Info("%s, from %s", message, request.RemoteAddr)
+		http.Error(responseWriter, message, http.StatusBadRequest)
+		return false
+	}
+	return true
+}

internal/httpproxy/auth.go

@@ -6,10 +6,13 @@ import (
 	"strings"
 )
 
-func isAuthorized(responseWriter http.ResponseWriter, request *http.Request,
+func (h *handler) isAuthorized(responseWriter http.ResponseWriter, request *http.Request) (authorized bool) {
-	username, password string) (authorized bool) {
+	if len(h.username) == 0 || (request.Method != "CONNECT" && !request.URL.IsAbs()) {
+		return true
+	}
 	basicAuth := request.Header.Get("Proxy-Authorization")
 	if len(basicAuth) == 0 {
+		h.logger.Info("Proxy-Authorization header not found from %s", request.RemoteAddr)
 		responseWriter.Header().Set("Proxy-Authenticate", `Basic realm="Access to Gluetun over HTTP"`)
 		responseWriter.WriteHeader(http.StatusProxyAuthRequired)
 		return false
@@ -17,6 +20,8 @@ func isAuthorized(responseWriter http.ResponseWriter, request *http.Request,
 	b64UsernamePassword := strings.TrimPrefix(basicAuth, "Basic ")
 	b, err := base64.StdEncoding.DecodeString(b64UsernamePassword)
 	if err != nil {
+		h.logger.Info("Cannot decode Proxy-Authorization header value from %s: %s",
+			request.RemoteAddr, err.Error())
 		responseWriter.WriteHeader(http.StatusUnauthorized)
 		return false
 	}
@@ -26,7 +31,9 @@ func isAuthorized(responseWriter http.ResponseWriter, request *http.Request,
 		responseWriter.WriteHeader(http.StatusBadRequest)
 		return false
 	}
-	if username != usernamePassword[0] && password != usernamePassword[1] {
+	if h.username != usernamePassword[0] || h.password != usernamePassword[1] {
+		h.logger.Info("Username or password mismatch from %s", request.RemoteAddr)
+		h.logger.Debug("username provided %q and password provided %q", usernamePassword[0], usernamePassword[1])
 		responseWriter.WriteHeader(http.StatusUnauthorized)
 		return false
 	}

internal/httpproxy/handler.go

@@ -9,20 +9,18 @@ import (
 	"github.com/qdm12/golibs/logging"
 )
 
-func newHandler(ctx context.Context, wg *sync.WaitGroup,
+func newHandler(ctx context.Context, wg *sync.WaitGroup, logger logging.Logger,
-	client *http.Client, logger logging.Logger,
 	stealth, verbose bool, username, password string) http.Handler {
-	const relayTimeout = 10 * time.Second
+	const httpTimeout = 24 * time.Hour
 	return &handler{
-		ctx:          ctx,
+		ctx:      ctx,
-		wg:           wg,
+		wg:       wg,
-		client:       client,
+		client:   &http.Client{Timeout: httpTimeout},
-		logger:       logger,
+		logger:   logger,
-		relayTimeout: relayTimeout,
+		verbose:  verbose,
-		verbose:      verbose,
+		stealth:  stealth,
-		stealth:      stealth,
+		username: username,
-		username:     username,
+		password: password,
-		password:     password,
 	}
 }
 
@@ -31,16 +29,20 @@ type handler struct {
 	wg                 *sync.WaitGroup
 	client             *http.Client
 	logger             logging.Logger
-	relayTimeout       time.Duration
 	verbose, stealth   bool
 	username, password string
 }
 
 func (h *handler) ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) {
-	if len(h.username) > 0 && !isAuthorized(responseWriter, request, h.username, h.password) {
+	if !h.isAccepted(responseWriter, request) {
-		h.logger.Info("%s unauthorized", request.RemoteAddr)
 		return
 	}
+	if !h.isAuthorized(responseWriter, request) {
+		return
+	}
+	request.Header.Del("Proxy-Connection")
+	request.Header.Del("Proxy-Authenticate")
+	request.Header.Del("Proxy-Authorization")
 	switch request.Method {
 	case http.MethodConnect:
 		h.handleHTTPS(responseWriter, request)

internal/httpproxy/http.go

@@ -1,7 +1,6 @@
 package httpproxy
 
 import (
-	"context"
 	"fmt"
 	"io"
 	"net"
@@ -18,9 +17,7 @@ func (h *handler) handleHTTP(responseWriter http.ResponseWriter, request *http.R
 		return
 	}
 
-	ctx, cancel := context.WithTimeout(h.ctx, h.relayTimeout)
+	request = request.WithContext(h.ctx)
-	defer cancel()
-	request = request.WithContext(ctx)
 
 	request.RequestURI = ""
 

internal/httpproxy/https.go

@@ -9,7 +9,7 @@ import (
 )
 
 func (h *handler) handleHTTPS(responseWriter http.ResponseWriter, request *http.Request) {
-	dialer := net.Dialer{Timeout: h.relayTimeout}
+	dialer := net.Dialer{}
 	destinationConn, err := dialer.DialContext(h.ctx, "tcp", request.Host)
 	if err != nil {
 		http.Error(responseWriter, err.Error(), http.StatusServiceUnavailable)

internal/httpproxy/loop.go

@@ -3,7 +3,6 @@ package httpproxy
 import (
 	"context"
 	"fmt"
-	"net/http"
 	"sync"
 
 	"github.com/qdm12/gluetun/internal/settings"
@@ -20,7 +19,6 @@ type Looper interface {
 }
 
 type looper struct {
-	client        *http.Client
 	settings      settings.HTTPProxy
 	settingsMutex sync.RWMutex
 	logger        logging.Logger
@@ -29,10 +27,8 @@ type looper struct {
 	stop          chan struct{}
 }
 
-func NewLooper(client *http.Client, logger logging.Logger,
+func NewLooper(logger logging.Logger, settings settings.HTTPProxy) Looper {
-	settings settings.HTTPProxy) Looper {
 	return &looper{
-		client:   client,
 		settings: settings,
 		logger:   logger.WithPrefix("http proxy: "),
 		restart:  make(chan struct{}),
@@ -104,7 +100,7 @@ func (l *looper) Run(ctx context.Context, wg *sync.WaitGroup) {
 		settings := l.GetSettings()
 		address := fmt.Sprintf("0.0.0.0:%d", settings.Port)
 
-		server := New(ctx, address, l.logger, l.client, settings.Stealth, settings.Log, settings.User, settings.Password)
+		server := New(ctx, address, l.logger, settings.Stealth, settings.Log, settings.User, settings.Password)
 
 		runCtx, runCancel := context.WithCancel(context.Background())
 		runWg := &sync.WaitGroup{}

internal/httpproxy/server.go

@@ -20,13 +20,12 @@ type server struct {
 	internalWG *sync.WaitGroup
 }
 
-func New(ctx context.Context, address string,
+func New(ctx context.Context, address string, logger logging.Logger,
-	logger logging.Logger, client *http.Client,
 	stealth, verbose bool, username, password string) Server {
 	wg := &sync.WaitGroup{}
 	return &server{
 		address:    address,
-		handler:    newHandler(ctx, wg, client, logger, stealth, verbose, username, password),
+		handler:    newHandler(ctx, wg, logger, stealth, verbose, username, password),
 		logger:     logger,
 		internalWG: wg,
 	}