fix(oohelperd): make sure endpoints don't connect to 127.0.0.1 (#1463)

This diff adds extra tests and logging showing that we're not connecting
to 127.0.0.1 in endpoints code. We still need to deal with making sure
we don't connect to 127.0.0.1 in HTTP code. In principle, this is
already true, but the QA framework constructs the oohelperd differently
than with the default constructor, and we'll need to improve in this
department to make sure the results obtained in the QA suite also give
us confidence on the general oohelperd behavior. For now, QA tests
attempting to use 127.0.0.1 do not produce the correct result because
oohelperd misbehaves.

Extracted from #1462.

Part of ooni/probe#2652.

Part of ooni/probe#1517.

internal/oohelperd/ipinfo.go

@@ -74,7 +74,7 @@ type endpointInfo struct {
 // whether an IP address is valid for a domain;
 //
 // 4. otherwise, we don't generate any endpoint to measure.
-func ipInfoToEndpoints(URL *url.URL, ipinfo map[string]*model.THIPInfo) []endpointInfo {
+func ipInfoToEndpoints(logger model.Logger, URL *url.URL, ipinfo map[string]*model.THIPInfo) []endpointInfo {
 	var ports []string
 
 	if port := URL.Port(); port != "" {
@@ -88,6 +88,7 @@ func ipInfoToEndpoints(URL *url.URL, ipinfo map[string]*model.THIPInfo) []endpoi
 	out := []endpointInfo{}
 	for addr, info := range ipinfo {
 		if (info.Flags & model.THIPInfoFlagIsBogon) != 0 {
+			logger.Infof("IPInfo: skip bogon IP address: %s", addr)
 			continue // as documented
 		}
 		for _, port := range ports {

internal/oohelperd/ipinfo_test.go

@@ -4,6 +4,7 @@ import (
 	"net/url"
 	"testing"
 
+	"github.com/apex/log"
 	"github.com/google/go-cmp/cmp"
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
@@ -84,6 +85,32 @@ func Test_newIPInfo(t *testing.T) {
 			addrs: []string{},
 		},
 		want: map[string]*model.THIPInfo{},
+	}, {
+		name: "with localhost",
+		args: args{
+			creq: &model.THRequest{
+				HTTPRequest:        "",
+				HTTPRequestHeaders: map[string][]string{},
+				TCPConnect: []string{
+					"127.0.0.1:443",
+					"[::1]:443",
+				},
+			},
+			addrs: []string{
+				"127.0.0.1",
+				"::1",
+			},
+		},
+		want: map[string]*model.THIPInfo{
+			"127.0.0.1": {
+				ASN:   0,
+				Flags: model.THIPInfoFlagIsBogon | model.THIPInfoFlagResolvedByProbe | model.THIPInfoFlagResolvedByTH,
+			},
+			"::1": {
+				ASN:   0,
+				Flags: model.THIPInfoFlagIsBogon | model.THIPInfoFlagResolvedByProbe | model.THIPInfoFlagResolvedByTH,
+			},
+		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -137,6 +164,10 @@ func Test_ipInfoToEndpoints(t *testing.T) {
 					ASN:   15169,
 					Flags: model.THIPInfoFlagResolvedByTH,
 				},
+				"127.0.0,1": {
+					ASN:   0,
+					Flags: model.THIPInfoFlagIsBogon | model.THIPInfoFlagResolvedByProbe,
+				},
 			},
 		},
 		want: []endpointInfo{{
@@ -239,7 +270,7 @@ func Test_ipInfoToEndpoints(t *testing.T) {
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := ipInfoToEndpoints(tt.args.URL, tt.args.ipinfo)
+			got := ipInfoToEndpoints(log.Log, tt.args.URL, tt.args.ipinfo)
 			if diff := cmp.Diff(tt.want, got); diff != "" {
 				t.Fatal(diff)
 			}

internal/oohelperd/measure.go

@@ -80,7 +80,7 @@ func measure(ctx context.Context, config *Handler, creq *ctrlRequest) (*ctrlResp
 
 	// obtain IP info and figure out the endpoints measurement plan
 	cresp.IPInfo = newIPInfo(creq, cresp.DNS.Addrs)
-	endpoints := ipInfoToEndpoints(URL, cresp.IPInfo)
+	endpoints := ipInfoToEndpoints(logger, URL, cresp.IPInfo)
 
 	// tcpconnect: start over all the endpoints
 	tcpconnch := make(chan *tcpResultPair, len(endpoints))