cleanup: doh.powerdns.org is not working anymore (#924)

While there, `.../internal/sessionresolver` => `.../sessionresolver`

See https://github.com/ooni/probe/issues/2255

internal/engine/sessionresolver/doc.go

@@ -0,0 +1,24 @@
+// Package sessionresolver contains the resolver used by the session. This
+// resolver will try to figure out which is the best service for running
+// domain name resolutions and will consistently use it.
+//
+// Occasionally this code will also swap the best resolver with other
+// ~good resolvers to give them a chance to perform.
+//
+// The penalty/reward mechanism is strongly derivative, so the code should
+// adapt ~quickly to changing network conditions. Occasionally, we will
+// have longer resolutions when trying out other resolvers.
+//
+// At the beginning we randomize the known resolvers so that we do not
+// have any preferential ordering. The initial resolutions may be slower
+// if there are many issues with resolvers.
+//
+// The system resolver is given the lowest priority at the beginning
+// but it will of course be the most popular resolver if anything else
+// is failing us. (We will still occasionally probe for other working
+// resolvers and increase their score on success.)
+//
+// We also support a socks5 proxy. When such a proxy is configured,
+// the code WILL skip http3 resolvers AS WELL AS the system
+// resolver, in an attempt to avoid leaking your queries.
+package sessionresolver

internal/engine/sessionresolver/errwrapper.go

@@ -0,0 +1,40 @@
+package sessionresolver
+
+//
+// Error wrapping
+//
+
+import (
+	"errors"
+	"fmt"
+)
+
+// errWrapper wraps an error to include the URL of the
+// resolver that we're currently using.
+type errWrapper struct {
+	err error
+	url string
+}
+
+// newErrWrapper creates a new err wrapper.
+func newErrWrapper(err error, URL string) *errWrapper {
+	return &errWrapper{
+		err: err,
+		url: URL,
+	}
+}
+
+// Error implements error.Error.
+func (ew *errWrapper) Error() string {
+	return fmt.Sprintf("<%s> %s", ew.url, ew.err.Error())
+}
+
+// Is allows consumers to query for the type of the underlying error.
+func (ew *errWrapper) Is(target error) bool {
+	return errors.Is(ew.err, target)
+}
+
+// Unwrap returns the underlying error.
+func (ew *errWrapper) Unwrap() error {
+	return ew.err
+}

internal/engine/sessionresolver/errwrapper_test.go

@@ -0,0 +1,24 @@
+package sessionresolver
+
+import (
+	"errors"
+	"io"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestErrWrapper(t *testing.T) {
+	ew := newErrWrapper(io.EOF, "https://dns.quad9.net/dns-query")
+	o := ew.Error()
+	expect := "<https://dns.quad9.net/dns-query> EOF"
+	if diff := cmp.Diff(expect, o); diff != "" {
+		t.Fatal(diff)
+	}
+	if !errors.Is(ew, io.EOF) {
+		t.Fatal("not the sub-error we expected")
+	}
+	if errors.Unwrap(ew) != io.EOF {
+		t.Fatal("unwrap failed")
+	}
+}

internal/engine/sessionresolver/integration_test.go

@@ -0,0 +1,32 @@
+package sessionresolver_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/engine/sessionresolver"
+	"github.com/ooni/probe-cli/v3/internal/kvstore"
+)
+
+func TestSessionResolverGood(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skip test in short mode")
+	}
+	reso := &sessionresolver.Resolver{
+		KVStore: &kvstore.Memory{},
+	}
+	defer reso.CloseIdleConnections()
+	if reso.Network() != "sessionresolver" {
+		t.Fatal("unexpected Network")
+	}
+	if reso.Address() != "" {
+		t.Fatal("unexpected Address")
+	}
+	addrs, err := reso.LookupHost(context.Background(), "google.com")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(addrs) < 1 {
+		t.Fatal("expected some addrs here")
+	}
+}

internal/engine/sessionresolver/jsoncodec.go

@@ -0,0 +1,37 @@
+package sessionresolver
+
+//
+// JSON codec
+//
+
+import "encoding/json"
+
+// jsonCodec encodes to/decodes from JSON.
+type jsonCodec interface {
+	// Encode encodes v as a JSON stream of bytes.
+	Encode(v interface{}) ([]byte, error)
+
+	// Decode decodes b from a JSON stream of bytes.
+	Decode(b []byte, v interface{}) error
+}
+
+// codec always returns a valid jsonCodec.
+func (r *Resolver) codec() jsonCodec {
+	if r.jsonCodec != nil {
+		return r.jsonCodec
+	}
+	return &jsonCodecStdlib{}
+}
+
+// jsonCodecStdlib is the default codec.
+type jsonCodecStdlib struct{}
+
+// Decode implements jsonCodec.Decode.
+func (*jsonCodecStdlib) Decode(b []byte, v interface{}) error {
+	return json.Unmarshal(b, v)
+}
+
+// Encode implements jsonCodec.Encode.
+func (*jsonCodecStdlib) Encode(v interface{}) ([]byte, error) {
+	return json.Marshal(v)
+}

internal/engine/sessionresolver/jsoncodec_test.go

@@ -0,0 +1,48 @@
+package sessionresolver
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+type jsonCodecMockable struct {
+	EncodeData []byte
+	EncodeErr  error
+	DecodeErr  error
+}
+
+func (c *jsonCodecMockable) Encode(v interface{}) ([]byte, error) {
+	return c.EncodeData, c.EncodeErr
+}
+
+func (c *jsonCodecMockable) Decode(b []byte, v interface{}) error {
+	return c.DecodeErr
+}
+
+func TestJSONCodecCustom(t *testing.T) {
+	c := &jsonCodecMockable{}
+	reso := &Resolver{jsonCodec: c}
+	if r := reso.codec(); r != c {
+		t.Fatal("not the codec we expected")
+	}
+}
+
+func TestJSONCodecDefault(t *testing.T) {
+	reso := &Resolver{}
+	in := resolverinfo{
+		URL:   "https://dns.google/dns.query",
+		Score: 0.99,
+	}
+	data, err := reso.codec().Encode(in)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var out resolverinfo
+	if err := reso.codec().Decode(data, &out); err != nil {
+		t.Fatal(err)
+	}
+	if diff := cmp.Diff(in, out); diff != "" {
+		t.Fatal(diff)
+	}
+}

internal/engine/sessionresolver/lookup.go

@@ -0,0 +1,41 @@
+package sessionresolver
+
+//
+// Actual lookup code
+//
+
+import (
+	"context"
+	"time"
+
+	"github.com/ooni/probe-cli/v3/internal/model"
+)
+
+// defaultTimeLimitedLookupTimeout is the default timeout the code should
+// pass to the timeLimitedLookup function.
+//
+// This algorithm is similar to Firefox using TRR2 mode. See:
+// https://wiki.mozilla.org/Trusted_Recursive_Resolver#DNS-over-HTTPS_Prefs_in_Firefox
+//
+// We use a higher timeout than Firefox's timeout (1.5s) to be on the safe side
+// and therefore see to use DoH more often.
+const defaultTimeLimitedLookupTimeout = 4 * time.Second
+
+// timeLimitedLookup performs a time-limited lookup using the given re.
+func timeLimitedLookup(ctx context.Context, re model.Resolver, hostname string) ([]string, error) {
+	return timeLimitedLookupWithTimeout(ctx, re, hostname, defaultTimeLimitedLookupTimeout)
+}
+
+// timeLimitedLookupWithTimeout is like timeLimitedLookup but with explicit timeout.
+func timeLimitedLookupWithTimeout(ctx context.Context, re model.Resolver,
+	hostname string, timeout time.Duration) ([]string, error) {
+	// In https://github.com/ooni/probe-cli/pull/807, I modified this code to
+	// run in a background goroutine and this resulted in a data race, see
+	// https://github.com/ooni/probe/issues/2135#issuecomment-1149840579. While
+	// I could not reproduce the data race in a simple way, the race itself
+	// seems to happen inside the http3 package. For now, I am going to revert
+	// the change causing the race and I'll investigate later.
+	ctx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
+	return re.LookupHost(ctx, hostname)
+}

internal/engine/sessionresolver/lookup_test.go

@@ -0,0 +1,44 @@
+package sessionresolver
+
+import (
+	"context"
+	"errors"
+	"io"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/model/mocks"
+)
+
+func TestTimeLimitedLookupSuccess(t *testing.T) {
+	expected := []string{"8.8.8.8", "8.8.4.4"}
+	re := &mocks.Resolver{
+		MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
+			return expected, nil
+		},
+	}
+	ctx := context.Background()
+	out, err := timeLimitedLookup(ctx, re, "dns.google")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if diff := cmp.Diff(expected, out); diff != "" {
+		t.Fatal(diff)
+	}
+}
+
+func TestTimeLimitedLookupFailure(t *testing.T) {
+	re := &mocks.Resolver{
+		MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
+			return nil, io.EOF
+		},
+	}
+	ctx := context.Background()
+	out, err := timeLimitedLookup(ctx, re, "dns.google")
+	if !errors.Is(err, io.EOF) {
+		t.Fatal("not the error we expected", err)
+	}
+	if out != nil {
+		t.Fatal("expected nil here")
+	}
+}

internal/engine/sessionresolver/resolver.go

@@ -0,0 +1,194 @@
+package sessionresolver
+
+//
+// Implementation of Resolver
+//
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"math/rand"
+	"net"
+	"net/url"
+	"sync"
+	"time"
+
+	"github.com/ooni/probe-cli/v3/internal/bytecounter"
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/multierror"
+	"github.com/ooni/probe-cli/v3/internal/runtimex"
+)
+
+// Resolver is the session resolver. Resolver will try to use
+// a bunch of DoT/DoH resolvers before falling back to the
+// system resolver. The relative priorities of the resolver
+// are stored onto the KVStore such that we can remember them
+// and therefore we can generally give preference to underlying
+// DoT/DoH resolvers that work better.
+//
+// Make sure you fill the mandatory fields (indicated below)
+// before using this data structure.
+//
+// You MUST NOT modify public fields of this structure once it
+// has been created, because that MAY lead to data races.
+type Resolver struct {
+	// ByteCounter is the OPTIONAL byte counter. It will count
+	// the bytes used by any child resolver except for the
+	// system resolver, whose bytes ARE NOT counted. If this
+	// field is not set, then we won't count the bytes.
+	ByteCounter *bytecounter.Counter
+
+	// KVStore is the MANDATORY key-value store where you
+	// want us to write statistics about which resolver is
+	// working better in your network.
+	KVStore model.KeyValueStore
+
+	// Logger is the OPTIONAL logger you want us to use
+	// to emit log messages.
+	Logger model.Logger
+
+	// ProxyURL is the OPTIONAL URL of the socks5 proxy
+	// we should be using. If not set, then we WON'T use
+	// any proxy. If set, then we WON'T use any http3
+	// based resolvers and we WON'T use the system resolver.
+	ProxyURL *url.URL
+
+	// jsonCodec is the OPTIONAL JSON Codec to use. If not set,
+	// we will construct a default codec.
+	jsonCodec jsonCodec
+
+	// mu provides synchronisation of internal fields.
+	mu sync.Mutex
+
+	// newChildResolverFn is the OPTIONAL function to override
+	// the construction of a new resolver in unit tests
+	newChildResolverFn func(h3 bool, URL string) (model.Resolver, error)
+
+	// once ensures that CloseIdleConnection is
+	// run just once.
+	once sync.Once
+
+	// res maps a URL to a child resolver. We will
+	// construct child resolvers just once and we
+	// will track them into this field.
+	res map[string]model.Resolver
+}
+
+// CloseIdleConnections closes the idle connections, if any. This
+// function is guaranteed to be idempotent.
+func (r *Resolver) CloseIdleConnections() {
+	r.once.Do(r.closeall)
+}
+
+// Stats returns stats about the session resolver.
+func (r *Resolver) Stats() string {
+	data, err := json.Marshal(r.readstatedefault())
+	runtimex.PanicOnError(err, "json.Marshal should not fail here")
+	return fmt.Sprintf("sessionresolver: %s", string(data))
+}
+
+// errLookupNotImplemented indicates a given lookup type is not implemented.
+var errLookupNotImplemented = errors.New("sessionresolver: lookup not implemented")
+
+// LookupHTTPS implements Resolver.LookupHTTPS.
+func (r *Resolver) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPSSvc, error) {
+	return nil, errLookupNotImplemented
+}
+
+// LookupNS implements Resolver.LookupNS.
+func (r *Resolver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) {
+	return nil, errLookupNotImplemented
+}
+
+// ErrLookupHost indicates that LookupHost failed.
+var ErrLookupHost = errors.New("sessionresolver: LookupHost failed")
+
+// LookupHost implements Resolver.LookupHost. This function returns a
+// multierror.Union error on failure, so you can see individual errors
+// and get a better picture of what's been going wrong.
+func (r *Resolver) LookupHost(ctx context.Context, hostname string) ([]string, error) {
+	state := r.readstatedefault()
+	r.maybeConfusion(state, time.Now().UnixNano())
+	defer r.writestate(state)
+	me := multierror.New(ErrLookupHost)
+	for _, e := range state {
+		if r.ProxyURL != nil && r.shouldSkipWithProxy(e) {
+			r.logger().Infof("sessionresolver: skipping with proxy: %+v", e)
+			continue // we cannot proxy this URL so ignore it
+		}
+		addrs, err := r.lookupHost(ctx, e, hostname)
+		if err == nil {
+			return addrs, nil
+		}
+		me.Add(newErrWrapper(err, e.URL))
+	}
+	return nil, me
+}
+
+func (r *Resolver) shouldSkipWithProxy(e *resolverinfo) bool {
+	URL, err := url.Parse(e.URL)
+	if err != nil {
+		return true // please skip
+	}
+	switch URL.Scheme {
+	case "https", "dot", "tcp":
+		return false // we can handle this
+	default:
+		return true // please skip
+	}
+}
+
+func (r *Resolver) lookupHost(ctx context.Context, ri *resolverinfo, hostname string) ([]string, error) {
+	const ewma = 0.9 // the last sample is very important
+	re, err := r.getresolver(ri.URL)
+	if err != nil {
+		r.logger().Warnf("sessionresolver: getresolver: %s", err.Error())
+		ri.Score = 0 // this is a hard error
+		return nil, err
+	}
+	addrs, err := timeLimitedLookup(ctx, re, hostname)
+	if err == nil {
+		r.logger().Infof("sessionresolver: %s... %v", ri.URL, model.ErrorToStringOrOK(nil))
+		ri.Score = ewma*1.0 + (1-ewma)*ri.Score // increase score
+		return addrs, nil
+	}
+	r.logger().Warnf("sessionresolver: %s... %s", ri.URL, err.Error())
+	ri.Score = ewma*0.0 + (1-ewma)*ri.Score // decrease score
+	return nil, err
+}
+
+// maybeConfusion will rearrange the  first elements of the vector
+// with low probability, so giving other resolvers a chance
+// to run and show that they are also viable. We do not fully
+// reorder the vector because that could lead to long runtimes.
+//
+// The return value is only meaningful for testing.
+func (r *Resolver) maybeConfusion(state []*resolverinfo, seed int64) int {
+	rng := rand.New(rand.NewSource(seed))
+	const confusion = 0.3
+	if rng.Float64() >= confusion {
+		return -1
+	}
+	switch len(state) {
+	case 0, 1: // nothing to do
+		return 0
+	case 2:
+		state[0], state[1] = state[1], state[0]
+		return 2
+	default:
+		state[0], state[2] = state[2], state[0]
+		return 3
+	}
+}
+
+// Network implements Resolver.Network.
+func (r *Resolver) Network() string {
+	return "sessionresolver"
+}
+
+// Address implements Resolver.Address.
+func (r *Resolver) Address() string {
+	return ""
+}

internal/engine/sessionresolver/resolver_test.go

@@ -0,0 +1,386 @@
+package sessionresolver
+
+import (
+	"context"
+	"errors"
+	"net"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/atomicx"
+	"github.com/ooni/probe-cli/v3/internal/kvstore"
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/model/mocks"
+	"github.com/ooni/probe-cli/v3/internal/multierror"
+)
+
+func TestNetworkWorks(t *testing.T) {
+	reso := &Resolver{}
+	if reso.Network() != "sessionresolver" {
+		t.Fatal("unexpected value returned by Network")
+	}
+}
+
+func TestAddressWorks(t *testing.T) {
+	reso := &Resolver{}
+	if reso.Address() != "" {
+		t.Fatal("unexpected value returned by Address")
+	}
+}
+
+func TestTypicalUsageWithFailure(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // fail immediately
+	reso := &Resolver{KVStore: &kvstore.Memory{}}
+	addrs, err := reso.LookupHost(ctx, "ooni.org")
+	if !errors.Is(err, ErrLookupHost) {
+		t.Fatal("not the error we expected", err)
+	}
+	var me *multierror.Union
+	if !errors.As(err, &me) {
+		t.Fatal("cannot convert error")
+	}
+	for _, child := range me.Children {
+		// net.DNSError does not include the underlying error
+		// but just a string representing the error. This
+		// means that we need to go down hunting what's the
+		// real error that occurred and use more verbose code.
+		{
+			var ew *errWrapper
+			if !errors.As(child, &ew) {
+				t.Fatal("not an instance of errwrapper")
+			}
+			var de *net.DNSError
+			if errors.As(ew, &de) {
+				if !strings.HasSuffix(de.Err, "operation was canceled") {
+					t.Fatal("not the error we expected", de.Err)
+				}
+				continue
+			}
+		}
+		// otherwise just unwrap and check whether it's
+		// a real context.Canceled error.
+		if !errors.Is(child, context.Canceled) {
+			t.Fatal("unexpected sub-error", child)
+		}
+	}
+	if addrs != nil {
+		t.Fatal("expected nil here")
+	}
+	if len(reso.res) < 1 {
+		t.Fatal("expected to see some resolvers here")
+	}
+	if reso.Stats() == "" {
+		t.Fatal("expected to see some string returned by stats")
+	}
+	reso.CloseIdleConnections()
+	if len(reso.res) != 0 {
+		t.Fatal("expected to see no resolvers after CloseIdleConnections")
+	}
+}
+
+func TestTypicalUsageWithSuccess(t *testing.T) {
+	expected := []string{"8.8.8.8", "8.8.4.4"}
+	ctx := context.Background()
+	reso := &Resolver{
+		KVStore: &kvstore.Memory{},
+		newChildResolverFn: func(h3 bool, URL string) (model.Resolver, error) {
+			reso := &mocks.Resolver{
+				MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
+					return expected, nil
+				},
+			}
+			return reso, nil
+		},
+	}
+	addrs, err := reso.LookupHost(ctx, "dns.google")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if diff := cmp.Diff(expected, addrs); diff != "" {
+		t.Fatal(diff)
+	}
+}
+
+func TestLittleLLookupHostWithInvalidURL(t *testing.T) {
+	reso := &Resolver{}
+	ctx := context.Background()
+	ri := &resolverinfo{URL: "\t\t\t", Score: 0.99}
+	addrs, err := reso.lookupHost(ctx, ri, "ooni.org")
+	if err == nil || !strings.HasSuffix(err.Error(), "invalid control character in URL") {
+		t.Fatal("not the error we expected", err)
+	}
+	if addrs != nil {
+		t.Fatal("expected nil addrs here")
+	}
+	if ri.Score != 0 {
+		t.Fatal("unexpected ri.Score", ri.Score)
+	}
+}
+
+func TestLittleLLookupHostWithSuccess(t *testing.T) {
+	expected := []string{"8.8.8.8", "8.8.4.4"}
+	reso := &Resolver{
+		newChildResolverFn: func(h3 bool, URL string) (model.Resolver, error) {
+			reso := &mocks.Resolver{
+				MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
+					return expected, nil
+				},
+			}
+			return reso, nil
+		},
+	}
+	ctx := context.Background()
+	ri := &resolverinfo{URL: "dot://dns-nonexistent.ooni.org", Score: 0.1}
+	addrs, err := reso.lookupHost(ctx, ri, "dns.google")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if diff := cmp.Diff(expected, addrs); diff != "" {
+		t.Fatal(diff)
+	}
+	if ri.Score < 0.88 || ri.Score > 0.92 {
+		t.Fatal("unexpected score", ri.Score)
+	}
+}
+
+func TestLittleLLookupHostWithFailure(t *testing.T) {
+	errMocked := errors.New("mocked error")
+	reso := &Resolver{
+		newChildResolverFn: func(h3 bool, URL string) (model.Resolver, error) {
+			reso := &mocks.Resolver{
+				MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
+					return nil, errMocked
+				},
+			}
+			return reso, nil
+		},
+	}
+	ctx := context.Background()
+	ri := &resolverinfo{URL: "dot://dns-nonexistent.ooni.org", Score: 0.95}
+	addrs, err := reso.lookupHost(ctx, ri, "dns.google")
+	if !errors.Is(err, errMocked) {
+		t.Fatal("not the error we expected", err)
+	}
+	if addrs != nil {
+		t.Fatal("expected nil addrs here")
+	}
+	if ri.Score < 0.094 || ri.Score > 0.096 {
+		t.Fatal("unexpected score", ri.Score)
+	}
+}
+
+func TestMaybeConfusionNoConfusion(t *testing.T) {
+	reso := &Resolver{}
+	rv := reso.maybeConfusion(nil, 0)
+	if rv != -1 {
+		t.Fatal("unexpected return value", rv)
+	}
+}
+
+func TestMaybeConfusionNoArray(t *testing.T) {
+	reso := &Resolver{}
+	rv := reso.maybeConfusion(nil, 11)
+	if rv != 0 {
+		t.Fatal("unexpected return value", rv)
+	}
+}
+
+func TestMaybeConfusionSingleEntry(t *testing.T) {
+	reso := &Resolver{}
+	state := []*resolverinfo{{}}
+	rv := reso.maybeConfusion(state, 11)
+	if rv != 0 {
+		t.Fatal("unexpected return value", rv)
+	}
+}
+
+func TestMaybeConfusionTwoEntries(t *testing.T) {
+	reso := &Resolver{}
+	state := []*resolverinfo{{
+		Score: 0.8,
+		URL:   "https://dns.google/dns-query",
+	}, {
+		Score: 0.4,
+		URL:   "http3://dns.google/dns-query",
+	}}
+	rv := reso.maybeConfusion(state, 11)
+	if rv != 2 {
+		t.Fatal("unexpected return value", rv)
+	}
+	if state[0].Score != 0.4 {
+		t.Fatal("unexpected state[0].Score")
+	}
+	if state[0].URL != "http3://dns.google/dns-query" {
+		t.Fatal("unexpected state[0].URL")
+	}
+	if state[1].Score != 0.8 {
+		t.Fatal("unexpected state[1].Score")
+	}
+	if state[1].URL != "https://dns.google/dns-query" {
+		t.Fatal("unexpected state[1].URL")
+	}
+}
+
+func TestMaybeConfusionManyEntries(t *testing.T) {
+	reso := &Resolver{}
+	state := []*resolverinfo{{
+		Score: 0.8,
+		URL:   "https://dns.google/dns-query",
+	}, {
+		Score: 0.4,
+		URL:   "http3://dns.google/dns-query",
+	}, {
+		Score: 0.1,
+		URL:   "system:///",
+	}, {
+		Score: 0.01,
+		URL:   "dot://dns.google",
+	}}
+	rv := reso.maybeConfusion(state, 11)
+	if rv != 3 {
+		t.Fatal("unexpected return value", rv)
+	}
+	if state[0].Score != 0.1 {
+		t.Fatal("unexpected state[0].Score")
+	}
+	if state[0].URL != "system:///" {
+		t.Fatal("unexpected state[0].URL")
+	}
+	if state[1].Score != 0.4 {
+		t.Fatal("unexpected state[1].Score")
+	}
+	if state[1].URL != "http3://dns.google/dns-query" {
+		t.Fatal("unexpected state[1].URL")
+	}
+	if state[2].Score != 0.8 {
+		t.Fatal("unexpected state[2].Score")
+	}
+	if state[2].URL != "https://dns.google/dns-query" {
+		t.Fatal("unexpected state[2].URL")
+	}
+	if state[3].Score != 0.01 {
+		t.Fatal("unexpected state[3].Score")
+	}
+	if state[3].URL != "dot://dns.google" {
+		t.Fatal("unexpected state[3].URL")
+	}
+}
+
+func TestResolverWorksWithProxy(t *testing.T) {
+	var (
+		works      = &atomicx.Int64{}
+		startuperr = make(chan error)
+		listench   = make(chan net.Listener)
+		done       = make(chan interface{})
+	)
+	// proxy implementation
+	go func() {
+		defer close(done)
+		lconn, err := net.Listen("tcp", "127.0.0.1:0")
+		startuperr <- err
+		if err != nil {
+			return
+		}
+		listench <- lconn
+		for {
+			conn, err := lconn.Accept()
+			if err != nil {
+				// We assume this is when we were told to
+				// shutdown by the main goroutine.
+				return
+			}
+			works.Add(1)
+			conn.Close()
+		}
+	}()
+	// make sure we could start the proxy
+	if err := <-startuperr; err != nil {
+		t.Fatal(err)
+	}
+	listener := <-listench
+	// use the proxy
+	reso := &Resolver{
+		ProxyURL: &url.URL{
+			Scheme: "socks5",
+			Host:   listener.Addr().String(),
+		},
+		KVStore: &kvstore.Memory{},
+	}
+	ctx := context.Background()
+	addrs, err := reso.LookupHost(ctx, "ooni.org")
+	// cleanly shutdown the listener
+	listener.Close()
+	<-done
+	// check results
+	if !errors.Is(err, ErrLookupHost) {
+		t.Fatal("not the error we expected", err)
+	}
+	if addrs != nil {
+		t.Fatal("expected nil addrs")
+	}
+	if works.Load() < 1 {
+		t.Fatal("expected to see a positive number of entries here")
+	}
+}
+
+func TestShouldSkipWithProxyWorks(t *testing.T) {
+	expect := []struct {
+		url    string
+		result bool
+	}{{
+		url:    "\t",
+		result: true,
+	}, {
+		url:    "https://dns.google/dns-query",
+		result: false,
+	}, {
+		url:    "dot://dns.google/",
+		result: false,
+	}, {
+		url:    "http3://dns.google/dns-query",
+		result: true,
+	}, {
+		url:    "tcp://dns.google/",
+		result: false,
+	}, {
+		url:    "udp://dns.google/",
+		result: true,
+	}, {
+		url:    "system:///",
+		result: true,
+	}}
+	reso := &Resolver{}
+	for _, e := range expect {
+		out := reso.shouldSkipWithProxy(&resolverinfo{URL: e.url})
+		if out != e.result {
+			t.Fatal("unexpected result for", e)
+		}
+	}
+}
+
+func TestUnimplementedFunctions(t *testing.T) {
+	t.Run("LookupHTTPS", func(t *testing.T) {
+		r := &Resolver{}
+		https, err := r.LookupHTTPS(context.Background(), "dns.google")
+		if !errors.Is(err, errLookupNotImplemented) {
+			t.Fatal("unexpected error", err)
+		}
+		if https != nil {
+			t.Fatal("expected nil result")
+		}
+	})
+
+	t.Run("LookupNS", func(t *testing.T) {
+		r := &Resolver{}
+		ns, err := r.LookupNS(context.Background(), "dns.google")
+		if !errors.Is(err, errLookupNotImplemented) {
+			t.Fatal("unexpected error", err)
+		}
+		if len(ns) > 0 {
+			t.Fatal("expected empty result")
+		}
+	})
+}

internal/engine/sessionresolver/resolvermaker.go

@@ -0,0 +1,120 @@
+package sessionresolver
+
+//
+// Code for creating a new child resolver
+//
+
+import (
+	"math/rand"
+	"strings"
+	"time"
+
+	"github.com/ooni/probe-cli/v3/internal/engine/netx"
+	"github.com/ooni/probe-cli/v3/internal/model"
+)
+
+// resolvemaker contains rules for making a resolver.
+type resolvermaker struct {
+	url   string
+	score float64
+}
+
+// systemResolverURL is the URL of the system resolver.
+const systemResolverURL = "system:///"
+
+// allmakers contains all the makers in a list. We use the http3
+// prefix to indicate we wanna use http3. The code will translate
+// this to https and set the proper netx options.
+var allmakers = []*resolvermaker{{
+	url: "https://cloudflare-dns.com/dns-query",
+}, {
+	url: "http3://cloudflare-dns.com/dns-query",
+}, {
+	url: "https://dns.google/dns-query",
+}, {
+	url: "http3://dns.google/dns-query",
+}, {
+	url: "https://dns.quad9.net/dns-query",
+}, {
+	url: systemResolverURL,
+}, {
+	url: "https://mozilla.cloudflare-dns.com/dns-query",
+}, {
+	url: "http3://mozilla.cloudflare-dns.com/dns-query",
+}}
+
+// allbyurl contains all the resolvermakers by URL
+var allbyurl map[string]*resolvermaker
+
+// init fills allbyname and gives a nonzero initial score
+// to all resolvers except for the system resolver. We set
+// the system resolver score to zero, so that it's less
+// likely than other resolvers in this list.
+func init() {
+	allbyurl = make(map[string]*resolvermaker)
+	rng := rand.New(rand.NewSource(time.Now().UnixNano()))
+	for _, e := range allmakers {
+		allbyurl[e.url] = e
+		if e.url != systemResolverURL {
+			e.score = rng.Float64()
+		}
+	}
+}
+
+// logger returns the configured logger or a default
+func (r *Resolver) logger() model.Logger {
+	return model.ValidLoggerOrDefault(r.Logger)
+}
+
+// newChildResolver creates a new child model.Resolver.
+func (r *Resolver) newChildResolver(h3 bool, URL string) (model.Resolver, error) {
+	if r.newChildResolverFn != nil {
+		return r.newChildResolverFn(h3, URL)
+	}
+	return netx.NewDNSClient(netx.Config{
+		BogonIsError: true,
+		ByteCounter:  r.ByteCounter, // nil is handled by netx
+		HTTP3Enabled: h3,
+		Logger:       r.logger(),
+		ProxyURL:     r.ProxyURL,
+	}, URL)
+}
+
+// newresolver creates a new resolver with the given config and URL. This is
+// where we expand http3 to https and set the h3 options.
+func (r *Resolver) newresolver(URL string) (model.Resolver, error) {
+	h3 := strings.HasPrefix(URL, "http3://")
+	if h3 {
+		URL = strings.Replace(URL, "http3://", "https://", 1)
+	}
+	return r.newChildResolver(h3, URL)
+}
+
+// getresolver returns a resolver with the given URL. This function caches
+// already allocated resolvers so we only allocate them once.
+func (r *Resolver) getresolver(URL string) (model.Resolver, error) {
+	defer r.mu.Unlock()
+	r.mu.Lock()
+	if re, found := r.res[URL]; found {
+		return re, nil // already created
+	}
+	re, err := r.newresolver(URL)
+	if err != nil {
+		return nil, err // config err?
+	}
+	if r.res == nil {
+		r.res = make(map[string]model.Resolver)
+	}
+	r.res[URL] = re
+	return re, nil
+}
+
+// closeall closes the cached resolvers.
+func (r *Resolver) closeall() {
+	defer r.mu.Unlock()
+	r.mu.Lock()
+	for _, re := range r.res {
+		re.CloseIdleConnections()
+	}
+	r.res = nil
+}

internal/engine/sessionresolver/resolvermaker_test.go

@@ -0,0 +1,123 @@
+package sessionresolver
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/bytecounter"
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/model/mocks"
+)
+
+func TestDefaultLogger(t *testing.T) {
+	t.Run("when using a different logger", func(t *testing.T) {
+		logger := &mocks.Logger{}
+		reso := &Resolver{Logger: logger}
+		lo := reso.logger()
+		if lo != logger {
+			t.Fatal("expected another logger here")
+		}
+	})
+
+	t.Run("when no logger is set", func(t *testing.T) {
+		reso := &Resolver{Logger: nil}
+		lo := reso.logger()
+		if lo != model.DiscardLogger {
+			t.Fatal("expected another logger here")
+		}
+	})
+}
+
+func TestGetResolverHTTPSStandard(t *testing.T) {
+	bc := bytecounter.New()
+	URL := "https://dns.google"
+	var closed bool
+	re := &mocks.Resolver{
+		MockCloseIdleConnections: func() {
+			closed = true
+		},
+	}
+	var (
+		savedURL string
+		savedH3  bool
+	)
+	reso := &Resolver{
+		ByteCounter: bc,
+		newChildResolverFn: func(h3 bool, URL string) (model.Resolver, error) {
+			savedURL = URL
+			savedH3 = h3
+			return re, nil
+		},
+	}
+	out, err := reso.getresolver(URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if out != re {
+		t.Fatal("not the result we expected")
+	}
+	o2, err := reso.getresolver(URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if out != o2 {
+		t.Fatal("not the result we expected")
+	}
+	reso.closeall()
+	if closed != true {
+		t.Fatal("was not closed")
+	}
+	if savedURL != URL {
+		t.Fatal("not the URL we expected")
+	}
+	if savedH3 {
+		t.Fatal("expected false")
+	}
+}
+
+func TestGetResolverHTTP3(t *testing.T) {
+	bc := bytecounter.New()
+	URL := "http3://dns.google"
+	var closed bool
+	re := &mocks.Resolver{
+		MockCloseIdleConnections: func() {
+			closed = true
+		},
+	}
+	var (
+		savedURL string
+		savedH3  bool
+	)
+	reso := &Resolver{
+		ByteCounter: bc,
+		newChildResolverFn: func(h3 bool, URL string) (model.Resolver, error) {
+			savedURL = URL
+			savedH3 = h3
+			return re, nil
+		},
+	}
+	out, err := reso.getresolver(URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if out != re {
+		t.Fatal("not the result we expected")
+	}
+	o2, err := reso.getresolver(URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if out != o2 {
+		t.Fatal("not the result we expected")
+	}
+	reso.closeall()
+	if closed != true {
+		t.Fatal("was not closed")
+	}
+	if savedURL != strings.Replace(URL, "http3://", "https://", 1) {
+		t.Fatal("not the URL we expected")
+	}
+	if !savedH3 {
+		t.Fatal("expected true")
+	}
+}

internal/engine/sessionresolver/state.go

@@ -0,0 +1,106 @@
+package sessionresolver
+
+//
+// Persistent on-disk state
+//
+
+import (
+	"errors"
+	"sort"
+)
+
+// storekey is the key used by the key value store to store
+// the state required by this package.
+const storekey = "sessionresolver.state"
+
+// resolverinfo contains info about a resolver.
+type resolverinfo struct {
+	// URL is the URL of a resolver.
+	URL string
+
+	// Score is the score of a resolver.
+	Score float64
+}
+
+// ErrNilKVStore indicates that the KVStore is nil.
+var ErrNilKVStore = errors.New("sessionresolver: kvstore is nil")
+
+// readstate reads the resolver state from disk
+func (r *Resolver) readstate() ([]*resolverinfo, error) {
+	if r.KVStore == nil {
+		return nil, ErrNilKVStore
+	}
+	data, err := r.KVStore.Get(storekey)
+	if err != nil {
+		return nil, err
+	}
+	var ri []*resolverinfo
+	if err := r.codec().Decode(data, &ri); err != nil {
+		return nil, err
+	}
+	return ri, nil
+}
+
+// errNoEntries indicates that no entry remained after we pruned
+// all the available entries in readstateandprune.
+var errNoEntries = errors.New("sessionresolver: no available entries")
+
+// readstateandprune reads the state from disk and removes all the
+// entries that we don't actually support.
+func (r *Resolver) readstateandprune() ([]*resolverinfo, error) {
+	ri, err := r.readstate()
+	if err != nil {
+		return nil, err
+	}
+	var out []*resolverinfo
+	for _, e := range ri {
+		if _, found := allbyurl[e.URL]; !found {
+			continue // we don't support this specific entry
+		}
+		out = append(out, e)
+	}
+	if len(out) <= 0 {
+		return nil, errNoEntries
+	}
+	return out, nil
+}
+
+// sortstate sorts the state by descending score
+func sortstate(ri []*resolverinfo) {
+	sort.SliceStable(ri, func(i, j int) bool {
+		return ri[i].Score >= ri[j].Score
+	})
+}
+
+// readstatedefault reads the state from disk and merges the state
+// so that all supported entries are represented.
+func (r *Resolver) readstatedefault() []*resolverinfo {
+	ri, _ := r.readstateandprune()
+	here := make(map[string]bool)
+	for _, e := range ri {
+		here[e.URL] = true // record what we already have
+	}
+	for _, e := range allmakers {
+		if _, found := here[e.url]; found {
+			continue // already here so no need to add
+		}
+		ri = append(ri, &resolverinfo{
+			URL:   e.url,
+			Score: e.score,
+		})
+	}
+	sortstate(ri)
+	return ri
+}
+
+// writestate writes the state to the kvstore.
+func (r *Resolver) writestate(ri []*resolverinfo) error {
+	if r.KVStore == nil {
+		return ErrNilKVStore
+	}
+	data, err := r.codec().Encode(ri)
+	if err != nil {
+		return err
+	}
+	return r.KVStore.Set(storekey, data)
+}

internal/engine/sessionresolver/state_test.go

@@ -0,0 +1,148 @@
+package sessionresolver
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/kvstore"
+)
+
+func TestReadStateNoKVStore(t *testing.T) {
+	reso := &Resolver{}
+	out, err := reso.readstate()
+	if !errors.Is(err, ErrNilKVStore) {
+		t.Fatal("not the error we expected", err)
+	}
+	if out != nil {
+		t.Fatal("expected nil here")
+	}
+}
+
+func TestReadStateNothingInKVStore(t *testing.T) {
+	reso := &Resolver{KVStore: &kvstore.Memory{}}
+	out, err := reso.readstate()
+	if !errors.Is(err, kvstore.ErrNoSuchKey) {
+		t.Fatal("not the error we expected", err)
+	}
+	if out != nil {
+		t.Fatal("expected nil here")
+	}
+}
+
+func TestReadStateDecodeError(t *testing.T) {
+	errMocked := errors.New("mocked error")
+	reso := &Resolver{
+		KVStore:   &kvstore.Memory{},
+		jsonCodec: &jsonCodecMockable{DecodeErr: errMocked},
+	}
+	if err := reso.KVStore.Set(storekey, []byte(`[]`)); err != nil {
+		t.Fatal(err)
+	}
+	out, err := reso.readstate()
+	if !errors.Is(err, errMocked) {
+		t.Fatal("not the error we expected", err)
+	}
+	if out != nil {
+		t.Fatal("expected nil here")
+	}
+}
+
+func TestReadStateAndPruneReadStateError(t *testing.T) {
+	reso := &Resolver{KVStore: &kvstore.Memory{}}
+	out, err := reso.readstateandprune()
+	if !errors.Is(err, kvstore.ErrNoSuchKey) {
+		t.Fatal("not the error we expected", err)
+	}
+	if out != nil {
+		t.Fatal("expected nil here")
+	}
+}
+
+func TestReadStateAndPruneWithUnsupportedEntries(t *testing.T) {
+	reso := &Resolver{KVStore: &kvstore.Memory{}}
+	var in []*resolverinfo
+	in = append(in, &resolverinfo{})
+	if err := reso.writestate(in); err != nil {
+		t.Fatal(err)
+	}
+	out, err := reso.readstateandprune()
+	if !errors.Is(err, errNoEntries) {
+		t.Fatal("not the error we expected", err)
+	}
+	if out != nil {
+		t.Fatal("expected nil here")
+	}
+}
+
+func TestReadStateDefaultWithMissingEntries(t *testing.T) {
+	reso := &Resolver{KVStore: &kvstore.Memory{}}
+	// let us simulate that we have just one entry here
+	existingURL := "https://dns.google/dns-query"
+	existingScore := 0.88
+	var in []*resolverinfo
+	in = append(in, &resolverinfo{
+		URL:   existingURL,
+		Score: existingScore,
+	})
+	if err := reso.writestate(in); err != nil {
+		t.Fatal(err)
+	}
+	// let us seee what we read
+	out := reso.readstatedefault()
+	if len(out) < 1 {
+		t.Fatal("expected non-empty output")
+	}
+	keys := make(map[string]bool)
+	var found bool
+	for _, e := range out {
+		keys[e.URL] = true
+		if e.URL == existingURL {
+			if e.Score != existingScore {
+				t.Fatal("the score is not what we expected")
+			}
+			found = true
+		}
+	}
+	if !found {
+		t.Fatal("did not found the pre-loaded URL")
+	}
+	for k := range allbyurl {
+		if _, found := keys[k]; !found {
+			t.Fatal("missing key", k)
+		}
+	}
+}
+
+func TestWriteStateNoKVStore(t *testing.T) {
+	reso := &Resolver{}
+	existingURL := "https://dns.google/dns-query"
+	existingScore := 0.88
+	var in []*resolverinfo
+	in = append(in, &resolverinfo{
+		URL:   existingURL,
+		Score: existingScore,
+	})
+	if err := reso.writestate(in); !errors.Is(err, ErrNilKVStore) {
+		t.Fatal("not the error we expected", err)
+	}
+}
+
+func TestWriteStateCannotSerialize(t *testing.T) {
+	errMocked := errors.New("mocked error")
+	reso := &Resolver{
+		jsonCodec: &jsonCodecMockable{
+			EncodeErr: errMocked,
+		},
+		KVStore: &kvstore.Memory{},
+	}
+	existingURL := "https://dns.google/dns-query"
+	existingScore := 0.88
+	var in []*resolverinfo
+	in = append(in, &resolverinfo{
+		URL:   existingURL,
+		Score: existingScore,
+	})
+	if err := reso.writestate(in); !errors.Is(err, errMocked) {
+		t.Fatal("not the error we expected", err)
+	}
+}