http/client: Don't verify IP

[daurnimator]

OpenSSL's API does not allow for removing an IP verification once set (see openssl/openssl#2673)

This means in turn that (without this PR) you can't use an IP as the connect address if you don't want to verify that it presents a certificate for it (e.g. if you did the dns lookup out of band).

local http_request = require "http.request"
local http_tls = require "http.tls"
local openssl_verify_param = require "openssl.x509.verify_param"

local r = http_request.new_from_uri("https://google.com")
r.host = "216.58.199.78" -- manually looked up IP for google
r.sendname = "google.com"
r.ctx = http_tls.new_client_context()
local params = openssl_verify_param.new()
params:setHost("google.com")
r.ctx:setParam(params)

print(r:go()) -- nil	starttls: error:14090086:SSL routines:ssl3_get_server_certificate:certificate verify failed:IP address mismatch	-1935895353

[daurnimator]

As an alternative: perhaps don't add the IP check if .sendname is set?

[KellerFuchs]

What happens if ip is set, here? NVM, misread some of the code.

[KellerFuchs]

Also, assign me as a reviewer. :P

[daurnimator]

What happens if ip is set, here?

Set where?

Also, assign me as a reviewer. :P

Github isn't letting me. I assume because you haven't contributed to the project, and they want to prevent spam?

[KellerFuchs]

This seems quite sane.
However, the breaking change (plainly disabling cert validation if an IP is passed) is very un-nice, especially since it will break silently (i.e. things will still “work” but become insecure), even though certificates for specific IPs are relatively uncommon (at least on the Internet).

Would it be possible instead to take an optional parameter to specify what the cert should be valid for, which could be either an hostname, an IP or nil (disables CN/altNames validation)?

[daurnimator]

the breaking change (plainly disabling cert validation if an IP is passed) is very un-nice,

It's not disabling cert validation, just hostname validation (but... yeah, still bad).

even though certificates for specific IPs are relatively uncommon (at least on the Internet)

Yeah I use certs with an IP from an internal CA for my gear in our community WISP: I don't love disabling it. (or I should say: making it opt-in instead of opt-out)

Would it be possible instead to take an optional parameter to specify what the cert should be valid for, which could be either an hostname, an IP or nil (disables CN/altNames validation)?

Well that's what the openssl tls_verify_param object is meant to be in the first place. However super annoyingly, it's missing the operation to disable an IP after one is added.

In general the current openssl API makes it possible to opt-in. but after you opt-in, you can't opt-out again. Which makes my pattern of "sensible defaults => merge in user configuration" unable to opt-out of IP verification.

[daurnimator]

Maybe instead I should add an options flag add_hostname_verification?
Though I feel like that'll turn into an ugly legacy to carry around.

[daurnimator]

I think a better resolution is to push the custom (out of band) dns lookup into a callback or similar.

Possibly related to wahern/cqueues#78

[daurnimator]

Closing as #120 seems like the better path forward.