review fixes

pkg/protocols/http/httpcache/httpcache.go

@@ -94,10 +94,21 @@ func NewTransportWrapper() func(http.RoundTripper) http.RoundTripper {
 			},
 			DisableWarningHeader: true,
 		}
-		return transport
+		return &unwrappingTransport{Transport: transport}
 	}
 }
 
+// unwrappingTransport lets code that must not use the cache, such as reporting
+// clients that need TLS verification, reach the wrapped transport.
+type unwrappingTransport struct {
+	*httpcache.Transport
+}
+
+// Unwrap returns the transport the cache wraps.
+func (t *unwrappingTransport) Unwrap() http.RoundTripper {
+	return t.Transport.Transport
+}
+
 func openDiskCache(dir string, maxBytes int64) (httpcache.Cache, *leveldb.DB, error) {
 	if err := os.MkdirAll(dir, 0o700); err != nil {
 		return nil, nil, fmt.Errorf("create cache dir: %w", err)

pkg/protocols/http/httpcache/httpcache_test.go

@@ -1,6 +1,7 @@
 package httpcache
 
 import (
+	"crypto/tls"
 	"fmt"
 	"io"
 	"net/http"
@@ -10,6 +11,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/tlsverify"
 	libcache "github.com/sandrolain/httpcache"
 )
 
@@ -178,3 +180,29 @@ func TestDiskCacheRespectsSizeCap(t *testing.T) {
 		t.Fatal("stored a response over a 1-byte directory cap")
 	}
 }
+
+// Reporting clients send tokens, so tlsverify must reach the scan transport
+// under the cache wrapper and turn certificate verification on.
+func TestTransportWrapperUnwrapsForTLSVerification(t *testing.T) {
+	cacheMu.Lock()
+	saved := cache
+	cache = libcache.NewMemoryCache()
+	cacheMu.Unlock()
+	t.Cleanup(func() {
+		cacheMu.Lock()
+		cache = saved
+		cacheMu.Unlock()
+	})
+
+	base := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}
+	wrapped := NewTransportWrapper()(base)
+	u, ok := wrapped.(interface{ Unwrap() http.RoundTripper })
+	if !ok || u.Unwrap() != base {
+		t.Fatal("the cache wrapper does not unwrap to the scan transport")
+	}
+
+	got, ok := tlsverify.Client(&http.Client{Transport: wrapped}).Transport.(*http.Transport)
+	if !ok || got.TLSClientConfig.InsecureSkipVerify {
+		t.Fatal("tlsverify did not turn verification on under the cache wrapper")
+	}
+}

pkg/protocols/http/httpclientpool/clientpool.go

@@ -166,6 +166,11 @@ func (t *connTrackingTransport) RoundTrip(req *http.Request) (*http.Response, er
 	return resp, err
 }
 
+// Unwrap returns the underlying transport.
+func (t *connTrackingTransport) Unwrap() http.RoundTripper {
+	return t.base
+}
+
 func (t *connTrackingTransport) CloseIdleConnections() {
 	type closeIdler interface{ CloseIdleConnections() }
 	if ci, ok := t.base.(closeIdler); ok {

pkg/reporting/reporting.go

@@ -20,6 +20,7 @@ import (
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/exporters/pdf"
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/exporters/sarif"
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/exporters/splunk"
+	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/tlsverify"
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/trackers/filters"
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/trackers/gitea"
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/trackers/github"
@@ -27,6 +28,7 @@ import (
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/trackers/jira"
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/trackers/linear"
 	"github.com/projectdiscovery/nuclei/v3/pkg/utils/yaml"
+	"github.com/projectdiscovery/retryablehttp-go"
 	"github.com/projectdiscovery/utils/errkit"
 	fileutil "github.com/projectdiscovery/utils/file"
 	"go.uber.org/multierr"
@@ -76,6 +78,17 @@ type IssueTrackerStats struct {
 func New(options *Options, db string, doNotDedupe bool) (Client, error) {
 	client := &ReportingClient{options: options}
 
+	// The caller can pass the scan client, which skips TLS verification;
+	// trackers and exporters send tokens and passwords, so they get a copy
+	// that verifies certificates.
+	if options.HttpClient != nil && options.HttpClient.HTTPClient != nil {
+		if hc := tlsverify.Client(options.HttpClient.HTTPClient); hc != options.HttpClient.HTTPClient {
+			opts := retryablehttp.DefaultOptionsSingle
+			opts.Timeout = hc.Timeout // keep the caller's timeout, also when it is 0
+			options.HttpClient = retryablehttp.NewWithHTTPClient(hc, opts)
+		}
+	}
+
 	if options.GitHub != nil {
 		options.GitHub.HttpClient = options.HttpClient
 		options.GitHub.OmitRaw = options.OmitRaw

pkg/reporting/reporting_tls_test.go

@@ -0,0 +1,98 @@
+package reporting_test
+
+import (
+	"io"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+
+	"github.com/projectdiscovery/nuclei/v3/pkg/model"
+	"github.com/projectdiscovery/nuclei/v3/pkg/model/types/severity"
+	"github.com/projectdiscovery/nuclei/v3/pkg/output"
+	"github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolstate"
+	"github.com/projectdiscovery/nuclei/v3/pkg/protocols/http/httpclientpool"
+	"github.com/projectdiscovery/nuclei/v3/pkg/reporting"
+	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/exporters/es"
+	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/exporters/splunk"
+	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/trackers/gitea"
+	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/trackers/github"
+	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/trackers/gitlab"
+	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/trackers/jira"
+	"github.com/projectdiscovery/nuclei/v3/pkg/types"
+	"github.com/stretchr/testify/require"
+)
+
+// connectProxy tunnels every CONNECT to target, whatever host the client asked
+// for, so no name is resolved and nothing leaves 127.0.0.1.
+func connectProxy(t *testing.T, target string) *httptest.Server {
+	p := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodConnect {
+			http.Error(w, "connect only", http.StatusMethodNotAllowed)
+			return
+		}
+		up, err := net.Dial("tcp", target)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadGateway)
+			return
+		}
+		c, _, err := w.(http.Hijacker).Hijack()
+		if err != nil {
+			_ = up.Close()
+			return
+		}
+		_, _ = c.Write([]byte("HTTP/1.1 200 Connection established\r\n\r\n"))
+		go func() { _, _ = io.Copy(up, c); _ = up.Close() }()
+		go func() { _, _ = io.Copy(c, up); _ = c.Close() }()
+	}))
+	t.Cleanup(p.Close)
+	return p
+}
+
+// With -pi and a proxy, the runner gives reporting the scan client, which skips
+// TLS verification. No tracker or exporter may send its secret to a server with
+// an untrusted certificate.
+func TestNewVerifiesTLSWithScanClient(t *testing.T) {
+	var hits atomic.Int32
+	srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		hits.Add(1)
+		w.WriteHeader(http.StatusUnauthorized)
+	}))
+	t.Cleanup(srv.Close)
+	proxy := connectProxy(t, srv.Listener.Addr().String())
+
+	opts := types.DefaultOptions()
+	opts.SetExecutionID("reporting-tls-test")
+	opts.ProxyInternal = true
+	opts.AliveHttpProxy = proxy.URL
+	require.NoError(t, protocolstate.Init(opts))
+	t.Cleanup(func() { protocolstate.Close(opts.ExecutionId) })
+	scanClient, err := httpclientpool.Get(opts, &httpclientpool.Configuration{}, "")
+	require.NoError(t, err)
+
+	event := &output.ResultEvent{
+		TemplateID: "test-template",
+		Host:       "https://target.example",
+		Matched:    "https://target.example/",
+		Info:       model.Info{Name: "test", SeverityHolder: severity.Holder{Severity: severity.High}},
+	}
+	configs := map[string]*reporting.Options{
+		"github":        {GitHub: &github.Options{BaseURL: "https://github.victim.example/", Username: "u", Owner: "o", ProjectName: "r", Token: "test-token"}},
+		"gitlab":        {GitLab: &gitlab.Options{BaseURL: "https://gitlab.victim.example/", Username: "u", Token: "test-token", ProjectName: "1"}},
+		"gitea":         {Gitea: &gitea.Options{BaseURL: "https://gitea.victim.example/", Token: "test-token", ProjectOwner: "o", ProjectName: "r"}},
+		"jira":          {Jira: &jira.Options{Cloud: true, URL: "https://jira.victim.example", AccountID: "a", Email: "[email protected]", Token: "test-token", ProjectName: "P", IssueType: "Bug"}},
+		"elasticsearch": {ElasticsearchExporter: &es.Options{Host: "es.victim.example", Port: 443, SSL: true, Username: "u", Password: "test-password", IndexName: "n"}},
+		"splunk":        {SplunkExporter: &splunk.Options{Host: "splunk.victim.example", Port: 443, SSL: true, Token: "test-token", IndexName: "n"}},
+	}
+	for name, ro := range configs {
+		ro.HttpClient = scanClient
+		ro.ExecutionId = opts.ExecutionId
+		client, err := reporting.New(ro, "", true)
+		if err == nil {
+			_ = client.CreateIssue(event)
+			client.Close()
+		}
+		require.Zero(t, hits.Load(), "%s sent a request to a server with an untrusted certificate", name)
+	}
+}

pkg/reporting/tlsverify/tlsverify.go

@@ -0,0 +1,68 @@
+// Package tlsverify gives trackers and exporters an HTTP client that verifies
+// TLS certificates, since they send tokens and passwords.
+package tlsverify
+
+import (
+	"crypto/tls"
+	"errors"
+	"net/http"
+)
+
+// errTLSDialerOnly is returned when a transport dials HTTPS only through a
+// custom TLS dialer: dropping that dialer would also drop its route or address
+// policy, so the copy refuses requests instead.
+var errTLSDialerOnly = errors.New("tlsverify: the transport has a custom TLS dialer and no DialContext, so certificate verification cannot be enforced")
+
+type refusingTransport struct{}
+
+func (refusingTransport) RoundTrip(*http.Request) (*http.Response, error) {
+	return nil, errTLSDialerOnly
+}
+
+// Client returns c, or a copy of c whose transport verifies TLS certificates
+// when c can skip verification (the scan client can). The copy keeps the proxy
+// and the dialer, so the address policy still applies, but it drops a custom
+// TLS dialer, because that dialer can use the scan TLS config. So the token
+// never goes over an unverified connection.
+//
+// A transport with an Unwrap() http.RoundTripper method declares that its
+// RoundTrip only observes or caches requests and enforces no policy; the copy
+// uses the unwrapped transport. Other transports are returned unchanged.
+func Client(c *http.Client) *http.Client {
+	rt := c.Transport
+	for {
+		u, ok := rt.(interface{ Unwrap() http.RoundTripper })
+		if !ok {
+			break
+		}
+		rt = u.Unwrap()
+	}
+	t, ok := rt.(*http.Transport)
+	if !ok {
+		return c
+	}
+	insecure := t.TLSClientConfig != nil && t.TLSClientConfig.InsecureSkipVerify
+	tlsDialer := t.DialTLSContext != nil || t.DialTLS != nil //nolint:staticcheck // DialTLS is deprecated but still used
+	if !insecure && !tlsDialer {
+		return c
+	}
+	copied := *c
+	if tlsDialer && t.DialContext == nil && t.Dial == nil { //nolint:staticcheck // Dial is deprecated but still used
+		copied.Transport = refusingTransport{}
+		return &copied
+	}
+	verifying := t.Clone()
+	verifying.DialTLSContext = nil
+	verifying.DialTLS = nil //nolint:staticcheck // DialTLS is deprecated but still used
+	if insecure {
+		// Keep the caller's roots, client certificates and checks; the server
+		// name of the scan config belongs to a scan target, not to this host.
+		verifying.TLSClientConfig.InsecureSkipVerify = false
+		verifying.TLSClientConfig.ServerName = ""
+		if verifying.TLSClientConfig.MinVersion < tls.VersionTLS12 {
+			verifying.TLSClientConfig.MinVersion = tls.VersionTLS12
+		}
+	}
+	copied.Transport = verifying
+	return &copied
+}

pkg/reporting/tlsverify/tlsverify_test.go

@@ -0,0 +1,67 @@
+package tlsverify
+
+import (
+	"context"
+	"crypto/tls"
+	"crypto/x509"
+	"errors"
+	"net"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// The scan client can skip TLS verification; the tracker must not send the
+// token over such a connection, and must not change the caller's client.
+func TestClientTurnsOnTLSVerification(t *testing.T) {
+	insecure := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}
+	caller := &http.Client{Transport: insecure}
+
+	got := Client(caller)
+	gotTransport, ok := got.Transport.(*http.Transport)
+	require.True(t, ok)
+	require.False(t, gotTransport.TLSClientConfig.InsecureSkipVerify)
+	require.True(t, insecure.TLSClientConfig.InsecureSkipVerify, "the caller's transport changed")
+
+	secure := &http.Client{Transport: &http.Transport{}}
+	require.Same(t, secure, Client(secure))
+}
+
+func TestClientKeepsCallerTLSSettings(t *testing.T) {
+	pinned := func(tls.ConnectionState) error { return nil }
+	insecure := &http.Transport{
+		DialTLSContext: func(context.Context, string, string) (net.Conn, error) { return nil, errors.New("scan dialer") },
+		DialContext:    (&net.Dialer{}).DialContext,
+		TLSClientConfig: &tls.Config{
+			InsecureSkipVerify: true,
+			ServerName:         "scan-target.example",
+			MinVersion:         tls.VersionTLS10,
+			RootCAs:            x509.NewCertPool(),
+			VerifyConnection:   pinned,
+		},
+	}
+	got := Client(&http.Client{Transport: insecure}).Transport.(*http.Transport)
+	require.Nil(t, got.DialTLSContext)
+	require.NotNil(t, got.DialContext)
+	cfg := got.TLSClientConfig
+	require.False(t, cfg.InsecureSkipVerify)
+	require.Empty(t, cfg.ServerName)
+	require.Equal(t, uint16(tls.VersionTLS12), cfg.MinVersion)
+	require.Same(t, insecure.TLSClientConfig.RootCAs, cfg.RootCAs)
+	require.NotNil(t, cfg.VerifyConnection)
+	require.True(t, insecure.TLSClientConfig.InsecureSkipVerify, "the caller's config changed")
+}
+
+// A custom TLS dialer may carry the only route or address policy, so a
+// transport without DialContext cannot be rebuilt and must refuse requests.
+func TestClientRefusesTLSDialerWithoutDialContext(t *testing.T) {
+	var dialed bool
+	tlsOnly := &http.Transport{DialTLSContext: func(context.Context, string, string) (net.Conn, error) {
+		dialed = true
+		return nil, errors.New("policy dialer")
+	}}
+	_, err := Client(&http.Client{Transport: tlsOnly}).Get("https://tracker.example/")
+	require.ErrorIs(t, err, errTLSDialerOnly)
+	require.False(t, dialed)
+}

pkg/reporting/trackers/github/github.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net/http"
 	"strconv"
 
 	"github.com/google/go-github/v92/github"
@@ -12,6 +11,7 @@ import (
 	"github.com/projectdiscovery/nuclei/v3/pkg/output"
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/exporters/markdown/util"
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/format"
+	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/tlsverify"
 	"github.com/projectdiscovery/nuclei/v3/pkg/reporting/trackers/filters"
 	"github.com/projectdiscovery/retryablehttp-go"
 	"golang.org/x/oauth2"
@@ -55,17 +55,16 @@ type Options struct {
 // New creates a new issue tracker integration client based on options.
 func New(options *Options) (*Integration, error) {
 	ctx := context.Background()
+	// Build the oauth2 client on top of the caller's client, so its transport
+	// (proxy, address policy) and redirect rules apply to every request.
+	if options.HttpClient != nil && options.HttpClient.HTTPClient != nil {
+		ctx = context.WithValue(ctx, oauth2.HTTPClient, tlsverify.Client(options.HttpClient.HTTPClient))
+	}
 	ts := oauth2.StaticTokenSource(
 		&oauth2.Token{AccessToken: options.Token},
 	)
 	tc := oauth2.NewClient(ctx, ts)
 
-	if options.HttpClient != nil && options.HttpClient.HTTPClient != nil {
-		if tcTransport, ok := tc.Transport.(*http.Transport); ok {
-			tcTransport.Proxy = options.HttpClient.HTTPClient.Transport.(*http.Transport).Proxy
-		}
-	}
-
 	clientOptions := []github.ClientOptionsFunc{github.WithHTTPClient(tc)}
 	if options.BaseURL != "" {
 		clientOptions = append(clientOptions, github.WithURLs(&options.BaseURL, nil))

pkg/reporting/trackers/github/github_test.go

@@ -0,0 +1,126 @@
+package github
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+
+	"github.com/projectdiscovery/nuclei/v3/pkg/model"
+	"github.com/projectdiscovery/nuclei/v3/pkg/model/types/severity"
+	"github.com/projectdiscovery/nuclei/v3/pkg/output"
+	"github.com/projectdiscovery/retryablehttp-go"
+	"github.com/stretchr/testify/require"
+)
+
+type roundTripFunc func(*http.Request) (*http.Response, error)
+
+func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
+
+func testEvent() *output.ResultEvent {
+	return &output.ResultEvent{
+		TemplateID: "test-template",
+		Host:       "https://target.example",
+		Info:       model.Info{Name: "test", SeverityHolder: severity.Holder{Severity: severity.High}},
+	}
+}
+
+func clientWithTransport(rt http.RoundTripper) *retryablehttp.Client {
+	c := retryablehttp.NewClient(retryablehttp.DefaultOptionsSingle)
+	c.HTTPClient = &http.Client{Transport: rt}
+	return c
+}
+
+// The tracker must send every request through the caller's client, so a
+// caller's address policy also covers the GitHub API base URL.
+func TestNewUsesCallerHTTPClient(t *testing.T) {
+	var serverHits atomic.Int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		serverHits.Add(1)
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusCreated)
+		_, _ = w.Write([]byte(`{"number":1,"html_url":"https://github.example/o/r/issues/1"}`))
+	}))
+	defer srv.Close()
+
+	t.Run("requests go through the caller's transport", func(t *testing.T) {
+		var seen atomic.Int32
+		rt := roundTripFunc(func(r *http.Request) (*http.Response, error) {
+			seen.Add(1)
+			require.Equal(t, "Bearer test-token", r.Header.Get("Authorization"))
+			return http.DefaultTransport.RoundTrip(r)
+		})
+		tracker, err := New(&Options{BaseURL: srv.URL + "/", Username: "u", Owner: "o", ProjectName: "r", Token: "test-token", HttpClient: clientWithTransport(rt)})
+		require.NoError(t, err)
+		_, err = tracker.CreateIssue(testEvent())
+		require.NoError(t, err)
+		require.Positive(t, seen.Load())
+	})
+
+	t.Run("a refusing transport stops the request", func(t *testing.T) {
+		before := serverHits.Load()
+		denied := errors.New("denied by address policy")
+		rt := roundTripFunc(func(*http.Request) (*http.Response, error) { return nil, denied })
+		tracker, err := New(&Options{BaseURL: srv.URL + "/", Username: "u", Owner: "o", ProjectName: "r", Token: "test-token", HttpClient: clientWithTransport(rt)})
+		require.NoError(t, err)
+		_, err = tracker.CreateIssue(testEvent())
+		require.ErrorContains(t, err, "denied by address policy")
+		require.Equal(t, before, serverHits.Load(), "the request reached the server")
+	})
+}
+
+func TestNewRefusesUntrustedCertificateFromInsecureCallerClient(t *testing.T) {
+	srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusCreated)
+		_, _ = w.Write([]byte(`{"number":1}`))
+	}))
+	defer srv.Close()
+
+	insecure := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}
+	tracker, err := New(&Options{BaseURL: srv.URL + "/", Username: "u", Owner: "o", ProjectName: "r", Token: "test-token", HttpClient: clientWithTransport(insecure)})
+	require.NoError(t, err)
+	_, err = tracker.CreateIssue(testEvent())
+	require.ErrorContains(t, err, "certificate")
+}
+
+type wrappingTransport struct{ base http.RoundTripper }
+
+func (w *wrappingTransport) RoundTrip(r *http.Request) (*http.Response, error) {
+	return w.base.RoundTrip(r)
+}
+func (w *wrappingTransport) Unwrap() http.RoundTripper { return w.base }
+
+// The scan client wraps its transport and, with a SOCKS proxy, does TLS in its
+// own dialer with the scan TLS config, which skips verification.
+func TestNewRefusesUntrustedCertificateFromInsecureTLSDialer(t *testing.T) {
+	srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusCreated)
+		_, _ = w.Write([]byte(`{"number":1}`))
+	}))
+	defer srv.Close()
+
+	var dialed atomic.Int32
+	var dialer net.Dialer
+	base := &http.Transport{
+		DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
+			dialed.Add(1)
+			return dialer.DialContext(ctx, network, addr)
+		},
+		DialTLSContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
+			conn, err := dialer.DialContext(ctx, network, addr)
+			if err != nil {
+				return nil, err
+			}
+			return tls.Client(conn, &tls.Config{InsecureSkipVerify: true}), nil
+		},
+	}
+	tracker, err := New(&Options{BaseURL: srv.URL + "/", Username: "u", Owner: "o", ProjectName: "r", Token: "test-token", HttpClient: clientWithTransport(&wrappingTransport{base: base})})
+	require.NoError(t, err)
+	_, err = tracker.CreateIssue(testEvent())
+	require.ErrorContains(t, err, "certificate")
+	require.Positive(t, dialed.Load(), "the caller's dialer was not used")
+}