Browse Source

fix(fetcher): allow connections to configured private proxies

Treat user-configured proxies (feed proxy, application proxy, proxy
rotator) as trusted hops so that the private-network restriction does
not block requests routed through a proxy listening on a private
address. Direct requests and redirects still enforce the check.
Frédéric Guillot 18 hours ago
parent
commit
15a532b991

+ 70 - 18
internal/reader/fetcher/request_builder.go

@@ -4,6 +4,7 @@
 package fetcher // import "miniflux.app/v2/internal/reader/fetcher"
 
 import (
+	"context"
 	"crypto/tls"
 	"encoding/base64"
 	"errors"
@@ -13,6 +14,7 @@ import (
 	"net/http"
 	"net/url"
 	"slices"
+	"strings"
 	"syscall"
 	"time"
 
@@ -139,18 +141,40 @@ func (r *RequestBuilder) WithoutCompression() *RequestBuilder {
 }
 
 func (r *RequestBuilder) ExecuteRequest(requestURL string) (*http.Response, error) {
-	dialer := &net.Dialer{
+	var clientProxyURL *url.URL
+
+	switch {
+	case r.feedProxyURL != "":
+		var err error
+		clientProxyURL, err = url.Parse(r.feedProxyURL)
+		if err != nil {
+			return nil, fmt.Errorf(`fetcher: invalid feed proxy URL %q: %w`, r.feedProxyURL, err)
+		}
+	case r.useClientProxy && r.clientProxyURL != nil:
+		clientProxyURL = r.clientProxyURL
+	case r.proxyRotator != nil && r.proxyRotator.HasProxies():
+		clientProxyURL = r.proxyRotator.GetNextProxy()
+	}
+
+	directDialer := &net.Dialer{
 		Timeout:   10 * time.Second, // Default is 30s.
 		KeepAlive: 15 * time.Second, // Default is 30s.
 	}
 
+	proxyDialer := &net.Dialer{
+		Timeout:   10 * time.Second, // Default is 30s.
+		KeepAlive: 15 * time.Second, // Default is 30s.
+	}
+
+	proxyDialAddress := normalizeProxyDialAddress(clientProxyURL)
+
 	// Perform the private-network check inside the dialer's Control callback,
 	// which fires after DNS resolution but before the TCP connection is made.
 	// This eliminates TOCTOU / DNS-rebinding vulnerabilities: the resolved IP
 	// that is checked is exactly the IP that will be connected to.
 	allowPrivateNetworks := config.Opts == nil || config.Opts.FetcherAllowPrivateNetworks()
 	if !allowPrivateNetworks {
-		dialer.Control = func(network, address string, c syscall.RawConn) error {
+		directDialer.Control = func(network, address string, c syscall.RawConn) error {
 			host, _, err := net.SplitHostPort(address)
 			if err != nil {
 				return err
@@ -169,11 +193,23 @@ func (r *RequestBuilder) ExecuteRequest(requestURL string) (*http.Response, erro
 		Proxy: http.ProxyFromEnvironment,
 		// Setting `DialContext` disables HTTP/2, this option forces the transport to try HTTP/2 regardless.
 		ForceAttemptHTTP2: true,
-		DialContext:       dialer.DialContext,
 		MaxIdleConns:      50,               // Default is 100.
 		IdleConnTimeout:   10 * time.Second, // Default is 90s.
 	}
 
+	transport.DialContext = directDialer.DialContext
+	if !allowPrivateNetworks && proxyDialAddress != "" {
+		// Explicitly configured proxies are a trusted hop. Keep the private-network
+		// check for direct requests and redirects, but allow the connection to the proxy itself.
+		transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+			if normalizeDialAddress(addr) == proxyDialAddress {
+				return proxyDialer.DialContext(ctx, network, addr)
+			}
+
+			return directDialer.DialContext(ctx, network, addr)
+		}
+	}
+
 	if r.ignoreTLSErrors {
 		//  Add insecure ciphers if we are ignoring TLS errors. This allows to connect to badly configured servers anyway
 		ciphers := slices.Concat(tls.CipherSuites(), tls.InsecureCipherSuites())
@@ -195,21 +231,6 @@ func (r *RequestBuilder) ExecuteRequest(requestURL string) (*http.Response, erro
 		transport.TLSNextProto = map[string]func(string, *tls.Conn) http.RoundTripper{}
 	}
 
-	var clientProxyURL *url.URL
-
-	switch {
-	case r.feedProxyURL != "":
-		var err error
-		clientProxyURL, err = url.Parse(r.feedProxyURL)
-		if err != nil {
-			return nil, fmt.Errorf(`fetcher: invalid feed proxy URL %q: %w`, r.feedProxyURL, err)
-		}
-	case r.useClientProxy && r.clientProxyURL != nil:
-		clientProxyURL = r.clientProxyURL
-	case r.proxyRotator != nil && r.proxyRotator.HasProxies():
-		clientProxyURL = r.proxyRotator.GetNextProxy()
-	}
-
 	var clientProxyURLRedacted string
 	if clientProxyURL != nil {
 		transport.Proxy = http.ProxyURL(clientProxyURL)
@@ -261,3 +282,34 @@ func (r *RequestBuilder) ExecuteRequest(requestURL string) (*http.Response, erro
 
 	return client.Do(req)
 }
+
+func normalizeDialAddress(addr string) string {
+	host, port, err := net.SplitHostPort(addr)
+	if err != nil {
+		return ""
+	}
+
+	return net.JoinHostPort(strings.ToLower(host), port)
+}
+
+func normalizeProxyDialAddress(proxyURL *url.URL) string {
+	if proxyURL == nil {
+		return ""
+	}
+
+	port := proxyURL.Port()
+	if port == "" {
+		switch strings.ToLower(proxyURL.Scheme) {
+		case "", "http":
+			port = "80"
+		case "https":
+			port = "443"
+		case "socks5", "socks5h":
+			port = "1080"
+		default:
+			return ""
+		}
+	}
+
+	return net.JoinHostPort(strings.ToLower(proxyURL.Hostname()), port)
+}

+ 75 - 0
internal/reader/fetcher/request_builder_test.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"miniflux.app/v2/internal/config"
+	"miniflux.app/v2/internal/proxyrotator"
 )
 
 func TestNewRequestBuilder(t *testing.T) {
@@ -436,6 +437,80 @@ func TestRequestBuilder_AllowPrivateNetworkWhenEnabled(t *testing.T) {
 	defer resp.Body.Close()
 }
 
+func TestRequestBuilder_AllowPrivateConfiguredProxy(t *testing.T) {
+	configureFetcherAllowPrivateNetworksOption(t, "0")
+
+	tests := []struct {
+		name      string
+		configure func(t *testing.T, builder *RequestBuilder, proxyURL string) *RequestBuilder
+	}{
+		{
+			name: "feed proxy",
+			configure: func(t *testing.T, builder *RequestBuilder, proxyURL string) *RequestBuilder {
+				return builder.WithCustomFeedProxyURL(proxyURL)
+			},
+		},
+		{
+			name: "application proxy",
+			configure: func(t *testing.T, builder *RequestBuilder, proxyURL string) *RequestBuilder {
+				t.Helper()
+
+				parsedProxyURL, err := url.Parse(proxyURL)
+				if err != nil {
+					t.Fatalf("Unable to parse proxy URL: %v", err)
+				}
+
+				return builder.WithCustomApplicationProxyURL(parsedProxyURL).UseCustomApplicationProxyURL(true)
+			},
+		},
+		{
+			name: "proxy rotator",
+			configure: func(t *testing.T, builder *RequestBuilder, proxyURL string) *RequestBuilder {
+				t.Helper()
+
+				rotator, err := proxyrotator.NewProxyRotator([]string{proxyURL})
+				if err != nil {
+					t.Fatalf("Unable to create proxy rotator: %v", err)
+				}
+
+				return builder.WithProxyRotator(rotator)
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			targetURL := "http://feed.invalid/rss.xml"
+			proxyRequests := make(chan string, 1)
+			proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				select {
+				case proxyRequests <- r.URL.String():
+				default:
+				}
+
+				w.WriteHeader(http.StatusOK)
+			}))
+			defer proxyServer.Close()
+
+			builder := tt.configure(t, NewRequestBuilder(), proxyServer.URL)
+			resp, err := builder.ExecuteRequest(targetURL)
+			if err != nil {
+				t.Fatalf("Expected private proxy request to succeed: %v", err)
+			}
+			defer resp.Body.Close()
+
+			select {
+			case gotURL := <-proxyRequests:
+				if gotURL != targetURL {
+					t.Fatalf("Expected proxy request URL to be %q, got %q", targetURL, gotURL)
+				}
+			default:
+				t.Fatal("Expected request to be sent through the proxy")
+			}
+		})
+	}
+}
+
 func TestRequestBuilder_RefusePrivateNetworkOnRedirect(t *testing.T) {
 	configureFetcherAllowPrivateNetworksOption(t, "0")