Merge pull request #2756 from letsencrypt/cpu-makes-err-msg-mistake-is-sorry

Rewords IPv6 to IPv4 fallback message

bdns/mocks.go

@@ -80,6 +80,11 @@ func (mock *MockDNSResolver) LookupHost(_ context.Context, hostname string) ([]n
 			net.ParseIP("127.0.0.1"),
 		}, nil
 	}
+	if hostname == "ipv6.localhost" {
+		return []net.IP{
+			net.ParseIP("::1"),
+		}, nil
+	}
 	ip := net.ParseIP("127.0.0.1")
 	return []net.IP{ip}, nil
 }

va/va.go

@@ -152,7 +152,7 @@ func (d *dialer) Dial(_, _ string) (net.Conn, error) {
 		addresses := append(v4, v6...)
 		// This shouldn't happen, but be defensive about it anyway
 		if len(addresses) < 1 {
-			return nil, fmt.Errorf("No available addresses for dialer to dial")
+			return nil, fmt.Errorf("no IP addresses found for %q", d.record.Hostname)
 		}
 		address := net.JoinHostPort(addresses[0].String(), d.record.Port)
 		d.record.AddressUsed = addresses[0]
@@ -399,15 +399,17 @@ func (va *ValidationAuthorityImpl) tryGetTLSSNICerts(ctx context.Context, identi
 
 	// Split the available addresses into v4 and v6 addresses
 	v4, v6 := availableAddresses(*thisRecord)
+	addresses := append(v4, v6...)
+
+	// This shouldn't happen, but be defensive about it anyway
+	if len(addresses) < 1 {
+		return nil, validationRecords, probs.Malformed(
+			fmt.Sprintf("no IP addresses found for %q", identifier.Value))
+	}
 
 	// If the IPv6 first feature isn't enabled then combine available IPv4 and
 	// IPv6 addresses and connect to the first IP in the combined list
 	if !features.Enabled(features.IPv6First) {
-		addresses := append(v4, v6...)
-		// This shouldn't happen, but be defensive about it anyway
-		if len(addresses) < 1 {
-			return nil, validationRecords, probs.Malformed("No available addresses for getTLSSNICerts to dial")
-		}
 		address := net.JoinHostPort(addresses[0].String(), thisRecord.Port)
 		thisRecord.AddressUsed = addresses[0]
 		certs, err := va.getTLSSNICerts(address, identifier, challenge, zName)
@@ -432,9 +434,13 @@ func (va *ValidationAuthorityImpl) tryGetTLSSNICerts(ctx context.Context, identi
 		va.stats.Inc("IPv4Fallback", 1)
 	}
 
-	// This shouldn't happen, but be defensive about it anyway
+	// If there are no v4 addresses then return an error about there being no
+	// usable addresses found. We don't say "no IP addresses found" here because
+	// we may have tried an IPv6 address before this point, had it fail, and then
+	// found no fallbacks.
 	if len(v4) < 1 {
-		return nil, validationRecords, probs.Malformed("No available addresses for getTLSSNICerts to dial")
+		return nil, validationRecords, probs.Malformed(
+			fmt.Sprintf("no working IP addresses found for %q", identifier.Value))
 	}
 
 	// Otherwise if there are no IPv6 addresses, or there was an error

va/va_test.go

@@ -1320,4 +1320,27 @@ func TestFallbackTLS(t *testing.T) {
 	test.AssertEquals(t, len(records[0].AddressesTried), 1)
 	// We expect that IPv6 localhost address was tried before the address used
 	test.AssertEquals(t, records[0].AddressesTried[0].String(), "::1")
+
+	// Now try a validation for an IPv6 only host. E.g. one without an IPv4
+	// address. The IPv6 will fail without a server and we expect the overall
+	// validation to fail since there is no IPv4 address/listener to fall back to.
+	host = "ipv6.localhost"
+	ident = core.AcmeIdentifier{Type: core.IdentifierDNS, Value: host}
+	va.stats = metrics.NewStatsdScope(mocks.NewStatter(), "VA")
+	records, prob = va.validateChallenge(ctx, ident, chall)
+
+	// The validation is expected to fail since there is no IPv4 to fall back to
+	// and a broken IPv6
+	records, prob = va.validateChallenge(ctx, ident, chall)
+	test.Assert(t, prob != nil, "validation succeeded with broken IPv6 and no IPv4 fallback")
+	// We expect that the problem has the correct error message about working IPs
+	test.AssertEquals(t, prob.Detail, "no working IP addresses found for \"ipv6.localhost\"")
+	// We expect one validation record to be present
+	test.AssertEquals(t, len(records), 1)
+	// We expect that the address eventually used was the IPv6 localhost address
+	test.AssertEquals(t, records[0].AddressUsed.String(), "::1")
+	// We expect that one address was tried
+	test.AssertEquals(t, len(records[0].AddressesTried), 1)
+	// We expect that IPv6 localhost address was tried
+	test.AssertEquals(t, records[0].AddressesTried[0].String(), "::1")
 }