Incorrect IPv4 address in error message when IPv6 is not working

[osirisinferi]

Summary:

On the Community, we've seen a few instances where users were showing a time out error with an IPv4 IP address in the returned error, even though the IPv4 address was working properly. (E.g. https://community.letsencrypt.org/t/letsencrypt-reissue-certificate/214220.) In reality, it was the IPv6 address timing out. This problem only occurs if there's a HTTP to HTTPS redirect in place.
For some reason Boulder only retries using IPv4 when there's an issue with IPv6 for the HTTP connection, but not for the HTTPS connection. (Not the scope of this issue.) But when the HTTPS over IPv6 attempt fails, Boulder uses the IPv4 address from the HTTP connection in the error message. Which is not correct.

Steps to reproduce:

• Have a site with HTTP to HTTPS redirect enabled;
• Have a functional IPv4 address;
• Have a non-functional IPv6 address;
• Attempt to get a certificate.

Expected result:

• Time out error with the IPv6 address shown in the error message.

Actual result:

• Time out error with the IPv4 address shown in the error message.

Additional details:

It's really confusing to see a perfectly functional IPv4 address in a time out error message when actually the IPv6 address is the problem.

[aarongable]

I believe the issue is this:

If HTTP-01 validation gets an unrecoverable error, it returns an error decorated with the IP address:

boulder/va/http.go

Lines 631 to 634 in 7ddb2be

	} else if err != nil {
	// if the error was not a fallbackErr then return immediately.
	return nil, records, newIPError(target, err)
	}

That decorator function always uses target.cur as the IP address to put in the error message:

boulder/va/http.go

Lines 438 to 440 in 7ddb2be

	newIPError := func(target *httpValidationTarget, err error) error {
	return ipError{ip: target.cur, err: err}
	}

This means that, if we've been falling back between various IP addresses to try, we'll be including the right IP address. But, if connecting to that IP address resulted in an HTTP 3XX redirect, we will not be including the redirected-to IP address in the error message.

I think the correct fix here is for newIPError to extract the IP Address from the last item in records instead of from target. This way it will reflect any redirects that were followed.

This is probably a low-priority fix for us at the moment, so I'm putting my notes here for the future.