Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TCP+TLS tries to validate IP address instead of hostname #225

Closed
marcan opened this issue Sep 11, 2017 · 2 comments · Fixed by #231
Closed

TCP+TLS tries to validate IP address instead of hostname #225

marcan opened this issue Sep 11, 2017 · 2 comments · Fixed by #231

Comments

@marcan
Copy link

marcan commented Sep 11, 2017

It seems plain TCP TLS mode doesn't correctly validate the hostname. It appears to be resolving the hostname then passing the IP down to the TLS layer for hostname verification, which is wrong.

This works:
/probe?target=https://google.com&module=http_2xx

But this doesn't:
/probe?target=google.com:443&module=tls_connect

level=error msg="Error dialing TCP: x509: cannot validate certificate for 216.58.197.174 because it doesn't contain any IP SANs" source="tcp.go:78"

This isn't the same as #224, as google.com serves a valid cert even without SNI. It isn't something weird about Google's cert either: the same thing happens with euskalencounter.org, which by default (without SNI) serves a valid Let's Encrypt cert for that hostname.

Version: 0.8.1
Config:

  http_2xx:
    prober: http
    timeout: 5s
    http:
      preferred_ip_protocol: ip4
      tls_config:
        insecure_skip_verify: false
  tls_connect:
    prober: tcp
    timeout: 5s
    tcp:
      preferred_ip_protocol: ip4
      tls: true
      tls_config:
        insecure_skip_verify: false
@brian-brazil
Copy link
Contributor

Sounds like we need to set the ServerName in the TCP probe.

@thz
Copy link
Contributor

thz commented Sep 14, 2017

Of course the sever-name can be set in tls_config. In my opinion it makes sense to get the TLS peer-name from the target if none is specified explicitly (in tls_config). Would look like this:

if tlsConfig.ServerName == "" {
  // Use target-hostname as default for TLS-servername.
  targetAddress, _, _ := net.SplitHostPort(target)
  tlsConfig.ServerName = targetAddress
}

I did exactly this for the TLS upgrade during TCP query/response (starttls) in pull #220.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants