va: Support IP address identifiers #8020
Conversation
…nstead; remove invalid bad port test
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
jprenken
changed the title
…Pv6 in URL construction; add test cases
|
I've done an initial review of the Let's Encrypt CP/CPS, the Baseline Requirements, and RFC 8738. I didn't find anything that would prevent validating IP addresses using these same functions, or that would prevent accepting HTTP redirects to bare IP addresses. |
| @@ -838,51 +915,33 @@ func TestFetchHTTP(t *testing.T) { | |||
| }, | |||
| }, | |||
| { | |||
| Name: "Connecting to bad port", | |||
There was a problem hiding this comment.
Removing this test. What it really tested was a doubled port: example.com:80:80. This is no longer structurally possible now that we more fully separate the port from the hostname inside the VA.
va/va.go
Outdated
| ident, err := identifier.FromProtoOrName(req.Identifier, req.DnsName) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if core.IsAnyNilOrZero(req, ident, req.Challenge, req.Authz, req.ExpectedKeyAuthorization) { |
There was a problem hiding this comment.
Here an elsewhere, I would expect these two to be swapped. First we want to check that the request we received contained all of the required fields, then we want to validate those fields and perform the necessary conversions.
There was a problem hiding this comment.
I'd gotten some feedback about this structure that I think is mildly conflicting: #7961 (comment)
But I've reworked FromProtoOrName to be simpler (and also more closely track the other PR's approach) and added TODOs to these call sites that will let us restore this order pretty shortly. Did this end up looking OK, or would you still prefer a re-order here?
Add an
identifierfield to theva.PerformValidationRequestproto, which will soon replace itsdnsNamefield.Accept and prefer the
identifierfield in every VA function that uses this struct. Don't (yet) assume it will be present.Throughout the VA, accept and handle the IP address identifier type. Handling is similar to DNS names, except that
getAddrsis not called, and consider that:x509.Certificatestruct..arpa) names in SNI for TLS-ALPN-01 challenge requests.For HTTP-01 challenges, accept redirects to bare IP addresses, which were previously rejected.
Fixes #2706
Part of #7311