Fix HTTP-01 IPv6 to IPv4 fallback with fresh dialer per conn. (#2852)

The implementation of the dialer used by the HTTP01 challenge, constructed with `resolveAndConstructDialer`, used the same wrapped `net.Dialer` for both the initial IPv6 connection, and any subsequent IPv4 fallback connections. This caused the IPv4 fallback to never succeed for cases where the initial IPv6 connection expended the `validationTimeout`.

This commit updates the http01Dialer (newly renamed from `dialer` since it is in fact specific to HTTP01 challenges) to use a fresh dialer for each connection. To facilitate testing the http01Dialer maintains
a count of how many dialer instances it has constructed. We use this in a unit test to ensure the correct behaviour without a great deal of new mocking/interfaces.

Resolves #2770

va/va.go

@@ -173,13 +173,33 @@ func (va ValidationAuthorityImpl) getAddr(ctx context.Context, hostname string)
 	return addr, addrs, nil
 }
 
-type dialer struct {
-	record core.ValidationRecord
-	stats  metrics.Scope
+// http01Dialer is a struct that exists to provide a dialer like object with
+// a `Dial` method that can be given to an http.Transport for HTTP-01
+// validation. The primary purpose of the http01Dialer's Dial method is to
+// circumvent traditional DNS lookup and to use the IP addresses provided in the
+// inner `record` member populated by the `resolveAndConstructDialer` function.
+type http01Dialer struct {
+	record      core.ValidationRecord
+	stats       metrics.Scope
+	dialerCount int
 }
 
-func (d *dialer) Dial(_, _ string) (net.Conn, error) {
-	realDialer := net.Dialer{Timeout: validationTimeout}
+// realDialer is used to create a true `net.Dialer` that can be used once an IP
+// address to connect to is determined. It increments the `dialerCount` integer
+// to track how many "fresh" dialer instances have been created during a `Dial`
+// for testing purposes.
+func (d *http01Dialer) realDialer() *net.Dialer {
+	// Record that we created a new instance of a real net.Dialer
+	d.dialerCount++
+	return &net.Dialer{Timeout: validationTimeout}
+}
+
+// Dial processes the IP addresses from the inner validation record, using
+// `realDialer` to make connections as required. If `features.IPv6First` is
+// enabled then for dual-homed hosts an initial IPv6 connection will be made
+// followed by a IPv4 connection if there is a failure with the IPv6 connection.
+func (d *http01Dialer) Dial(_, _ string) (net.Conn, error) {
+	var realDialer *net.Dialer
 
 	// Split the available addresses into v4 and v6 addresses
 	v4, v6 := availableAddresses(d.record)
@@ -194,6 +214,7 @@ func (d *dialer) Dial(_, _ string) (net.Conn, error) {
 		}
 		address := net.JoinHostPort(addresses[0].String(), d.record.Port)
 		d.record.AddressUsed = addresses[0]
+		realDialer = d.realDialer()
 		return realDialer.Dial("tcp", address)
 	}
 
@@ -202,6 +223,7 @@ func (d *dialer) Dial(_, _ string) (net.Conn, error) {
 	if features.Enabled(features.IPv6First) && len(v6) > 0 {
 		address := net.JoinHostPort(v6[0].String(), d.record.Port)
 		d.record.AddressUsed = v6[0]
+		realDialer = d.realDialer()
 		conn, err := realDialer.Dial("tcp", address)
 
 		// If there is no error, return immediately
@@ -230,6 +252,7 @@ func (d *dialer) Dial(_, _ string) (net.Conn, error) {
 	// talking to the first IPv6 address, try the first IPv4 address
 	address := net.JoinHostPort(v4[0].String(), d.record.Port)
 	d.record.AddressUsed = v4[0]
+	realDialer = d.realDialer()
 	return realDialer.Dial("tcp", address)
 }
 
@@ -248,8 +271,8 @@ func availableAddresses(rec core.ValidationRecord) (v4 []net.IP, v6 []net.IP) {
 
 // resolveAndConstructDialer gets the preferred address using va.getAddr and returns
 // the chosen address and dialer for that address and correct port.
-func (va *ValidationAuthorityImpl) resolveAndConstructDialer(ctx context.Context, name string, port int) (dialer, *probs.ProblemDetails) {
-	d := dialer{
+func (va *ValidationAuthorityImpl) resolveAndConstructDialer(ctx context.Context, name string, port int) (http01Dialer, *probs.ProblemDetails) {
+	d := http01Dialer{
 		record: core.ValidationRecord{
 			Hostname: name,
 			Port:     strconv.Itoa(port),

va/va_test.go

@@ -1192,6 +1192,48 @@ func TestAvailableAddresses(t *testing.T) {
 	}
 }
 
+// TestHTTP01DialerFallback tests the underlying dialer used by HTTP01
+// challenges. In particular it ensures that both the first IPv6 request and the
+// subsequent IPv4 request get a new dialer each.
+func TestHTTP01DialerFallback(t *testing.T) {
+	// Create a new challenge to use for the httpSrv
+	chall := core.HTTPChallenge01()
+	setChallengeToken(&chall, core.NewToken())
+
+	// Create an IPv4 test server
+	hs := httpSrv(t, chall.Token)
+	defer hs.Close()
+
+	// Set the IPv6First feature flag
+	_ = features.Set(map[string]bool{"IPv6First": true})
+	defer features.Reset()
+
+	// Create a test VA
+	va, _ := setup(hs, 0)
+
+	// Create a test dialer for the dual homed host. There is only an IPv4 httpSrv
+	// so the IPv6 address returned in the AAAA record will always fail.
+	d, _ := va.resolveAndConstructDialer(context.Background(), "ipv4.and.ipv6.localhost", va.httpPort)
+
+	// Try to dial the dialer
+	_, dialProb := d.Dial("", "ipv4.and.ipv6.localhost")
+
+	// There shouldn't be a problem from this dial
+	test.AssertEquals(t, dialProb, nil)
+
+	// We should have constructed two inner dialers, one for each connection
+	test.AssertEquals(t, d.dialerCount, 2)
+
+	// We expect one validation record to be present
+	test.AssertNotNil(t, d.record, "there should be a non-nil validaiton record on the dialer")
+	// We expect that the address used was the IPv4 localhost address
+	test.AssertEquals(t, d.record.AddressUsed.String(), "127.0.0.1")
+	// We expect that one address was tried before the address used
+	test.AssertEquals(t, len(d.record.AddressesTried), 1)
+	// We expect that IPv6 address was tried before the address used
+	test.AssertEquals(t, d.record.AddressesTried[0].String(), "::1")
+}
+
 func TestFallbackDialer(t *testing.T) {
 	// Create a new challenge to use for the httpSrv
 	chall := core.HTTPChallenge01()