Ivan/http private ip fix

node/config/def.go

@@ -214,7 +214,8 @@ func DefaultBoost() *Boost {
 			},
 		},
 		HttpDownload: HttpDownloadConfig{
-			NChunks: 5,
+			NChunks:         5,
+			AllowPrivateIPs: false,
 		},
 	}
 	return cfg

node/config/types.go

@@ -428,4 +428,7 @@ type HttpDownloadConfig struct {
 	// which improves the overall download speed. NChunks is always equal to 1 for libp2p transport because libp2p server
 	// doesn't support range requests yet. NChunks must be greater than 0 and less than 16, with the default of 5.
 	NChunks int
+	// AllowPrivateIPs defines whether boost should allow HTTP downloads from private IPs as per https://en.wikipedia.org/wiki/Private_network.
+	// The default is false.
+	AllowPrivateIPs bool
 }

node/modules/storageminer.go

@@ -622,7 +622,7 @@ func NewStorageMarketProvider(provAddr address.Address, cfg *config.Boost) func(
 			SealingPipelineCacheTimeout: time.Duration(cfg.Dealmaking.SealingPipelineCacheTimeout),
 		}
 		dl := logs.NewDealLogger(logsDB)
-		tspt := httptransport.New(h, dl, httptransport.NChunksOpt(cfg.HttpDownload.NChunks))
+		tspt := httptransport.New(h, dl, httptransport.NChunksOpt(cfg.HttpDownload.NChunks), httptransport.AllowPrivateIPsOpt(cfg.HttpDownload.AllowPrivateIPs))
 		prov, err := storagemarket.NewProvider(prvCfg, sqldb, dealsDB, fundMgr, storageMgr, a, dp, provAddr, secb, commpc,
 			sps, cdm, df, logsSqlDB.db, logsDB, piecedirectory, ip, lp, &signatureVerifier{a}, dl, tspt)
 		if err != nil {

transport/httptransport/http_transport.go

@@ -5,7 +5,9 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"net"
 	"net/http"
+	"net/url"
 	"os"
 	"strconv"
 	"sync"
@@ -59,6 +61,12 @@ func NChunksOpt(nChunks int) Option {
 	}
 }
 
+func AllowPrivateIPsOpt(b bool) Option {
+	return func(h *httpTransport) {
+		h.allowPrivateIPs = b
+	}
+}
+
 type httpTransport struct {
 	libp2pHost   host.Host
 	libp2pClient *http.Client
@@ -68,7 +76,8 @@ type httpTransport struct {
 	backOffFactor        float64
 	maxReconnectAttempts float64
 
-	nChunks int
+	nChunks         int
+	allowPrivateIPs bool
 
 	dl *logs.DealLogger
 }
@@ -119,6 +128,16 @@ func (h *httpTransport) Execute(ctx context.Context, transportInfo []byte, dealI
 	}
 	tInfo.URL = u.Url
 
+	if !h.allowPrivateIPs {
+		pu, err := url.Parse(u.Url)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse the request url: %w", err)
+		}
+		if ip := net.ParseIP(pu.Hostname()); ip != nil && ip.IsPrivate() {
+			return nil, fmt.Errorf("downloading from private ip addresses is not allowed")
+		}
+	}
+
 	// check that the outputFile exists
 	fi, err := os.Stat(dealInfo.OutputFile)
 	if err != nil {
@@ -188,7 +207,13 @@ func (h *httpTransport) Execute(ctx context.Context, transportInfo []byte, dealI
 			h.libp2pHost.ConnManager().Unprotect(u.PeerID, tag)
 		})
 	} else {
-		t.client = http.DefaultClient
+		// do not follow http redirects for security reasons
+		t.client = &http.Client{
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				return http.ErrUseLastResponse
+			},
+		}
+
 		h.dl.Infow(duuid, "http url", "url", tInfo.URL)
 	}
 

transport/httptransport/http_transport_test.go

@@ -316,6 +316,73 @@ func TestConcurrentTransfers(t *testing.T) {
 	}
 }
 
+func TestDownloadFromPrivateIPs(t *testing.T) {
+	ctx := context.Background()
+	of := getTempFilePath(t)
+	// deal info is irrelevant in this test
+	dealInfo := &types.TransportDealInfo{
+		OutputFile: of,
+		DealSize:   1000,
+	}
+	// ht := New(nil, newDealLogger(t, ctx), NChunksOpt(nChunks))
+	bz, err := json.Marshal(types.HttpRequest{URL: "http://192.168.0.1/blah"})
+	require.NoError(t, err)
+
+	// do not allow download from private IP addresses by default
+	_, err = New(nil, newDealLogger(t, ctx), NChunksOpt(nChunks)).Execute(ctx, bz, dealInfo)
+	require.Error(t, err, "downloading from private addresses is not allowed")
+	// allow download from private addresses if explicitly enabled
+	_, err = New(nil, newDealLogger(t, ctx), NChunksOpt(nChunks), AllowPrivateIPsOpt(true)).Execute(ctx, bz, dealInfo)
+	require.NoError(t, err)
+}
+
+func TestDontFollowHttpRedirects(t *testing.T) {
+	// we should not follow http redirects for security reasons. If the target URL tries to redirect, the client should return 303 response instead.
+	// This test sets up two servers, with one redirecting to the other. Without the redirect check the download would have been completed successfully.
+	rawSize := (100 * readBufferSize) + 30
+	ctx := context.Background()
+	st := newServerTest(t, rawSize)
+	carSize := len(st.carBytes)
+
+	var handler http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) {
+		switch r.Method {
+		case http.MethodGet:
+			offset := r.Header.Get("Range")
+
+			startend := strings.Split(strings.TrimPrefix(offset, "bytes="), "-")
+			start, _ := strconv.ParseInt(startend[0], 10, 64)
+			end, _ := strconv.ParseInt(startend[1], 10, 64)
+
+			w.WriteHeader(200)
+			if end == 0 {
+				w.Write(st.carBytes[start:]) //nolint:errcheck
+			} else {
+				w.Write(st.carBytes[start:end]) //nolint:errcheck
+			}
+		case http.MethodHead:
+			addContentLengthHeader(w, len(st.carBytes))
+		}
+	}
+	svr := httptest.NewServer(handler)
+	defer svr.Close()
+
+	var redirectHandler http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, svr.URL, http.StatusSeeOther)
+	}
+	redirectSvr := httptest.NewServer(redirectHandler)
+	defer redirectSvr.Close()
+
+	of := getTempFilePath(t)
+	th := executeTransfer(t, ctx, New(nil, newDealLogger(t, ctx), NChunksOpt(nChunks)), carSize, types.HttpRequest{URL: redirectSvr.URL}, of)
+	require.NotNil(t, th)
+
+	evts := waitForTransferComplete(th)
+	require.NotEmpty(t, evts)
+	require.Equal(t, 1, len(evts))
+	require.EqualValues(t, 0, evts[0].NBytesReceived)
+	require.Contains(t, evts[0].Error.Error(), "303 See Other")
+}
+
 func TestCompletionOnMultipleAttemptsWithSameFile(t *testing.T) {
 	ctx := context.Background()
 	of := getTempFilePath(t)