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

HTTP::Client throws OpenSSL::SSL::Error for FQDN (domain with trailing dot) #12777

Closed
compumike opened this issue Nov 22, 2022 · 2 comments · Fixed by #12778
Closed

HTTP::Client throws OpenSSL::SSL::Error for FQDN (domain with trailing dot) #12777

compumike opened this issue Nov 22, 2022 · 2 comments · Fixed by #12778
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:crypto

Comments

@compumike
Copy link
Contributor

When an HTTP::Client request is issued for a URL with a domain with a trailing period such as https://example.com./, certificate verification fails. It is expected that the request should succeed.

My use case is to specify fully-resolved domains with a trailing dot, so to be sure that resolv.conf-style search domains are not applied (in a Kubernetes context).

As shown below, other programming languages and software implement this successfully.

How to reproduce

  1. docker run --rm -it crystallang/crystal:1.6.2-alpine

  2. apk add --update --no-cache ca-certificates

  3. crystal eval 'require "http/client"; puts HTTP::Client.get("https://example.com/")'

    This request works fine:
    #<HTTP::Client::Response:0x7f0f708a3dc0>

  4. crystal eval 'require "http/client"; puts HTTP::Client.get("https://example.com./")'

    This request fails:

Unhandled exception: SSL_connect: error:1416F086:SSL routines:tls_process_server_certificate:certificate verify failed (OpenSSL::SSL::Error)
  from usr/share/crystal/src/openssl/ssl/socket.cr:34:11 in 'initialize'
  from usr/share/crystal/src/openssl/ssl/socket.cr:3:5 in 'new:context:sync_close:hostname'
  from usr/share/crystal/src/http/client.cr:800:5 in 'io'
  from usr/share/crystal/src/http/client.cr:676:19 in 'send_request'
  from usr/share/crystal/src/http/client.cr:607:5 in 'exec_internal_single'
  from usr/share/crystal/src/http/client.cr:590:18 in 'exec_internal'
  from usr/share/crystal/src/http/client.cr:583:7 in 'exec'
  from usr/share/crystal/src/http/client.cr:719:5 in 'exec'
  from usr/share/crystal/src/http/client.cr:751:7 in 'exec'
  from usr/share/crystal/src/http/client.cr:408:3 in 'get'
  from eval:1:29 in '__crystal_main'
  from usr/share/crystal/src/crystal/main.cr:115:5 in 'main_user_code'
  from usr/share/crystal/src/crystal/main.cr:101:7 in 'main'
  from usr/share/crystal/src/crystal/main.cr:127:3 in 'main'
  from /lib/ld-musl-x86_64.so.1 in '??'

Expected behavior

The full example.com. should be used for DNS resolution, but the trailing dot should be ignored for SSL certificate verification and only example.com should be checked against the certificate.

Note that the HTTP Host: header should preserve the trailing period.

How it's handled in other software / languages

Firefox

Succeeds: https://example.com./

Chrome

Succeeds: https://example.com./

wget

Succeeds: apk add wget && wget -O - https://example.com./

(note: busybox version of wget fails)

curl

Succeeds: apk add curl && curl https://example.com./

https://github.com/curl/curl/blob/master/lib/vtls/hostcheck.c#L66

Ruby

Succeeds: require 'net/http' ; Net::HTTP.get(URI.parse("https://example.com./"))

Python3

Succeeds: import requests ; requests.get("https://example.com./")

Go

Trailing . is stripped explicitly during certificate verification: https://github.com/golang/go/blob/master/src/crypto/x509/verify.go#L1017

Related issues

This issue was discussed thoroughly in Python: https://bugs.python.org/issue31997 and in curl: curl/curl#8290

@compumike compumike added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Nov 22, 2022
@compumike
Copy link
Contributor Author

I suspect that within the HTTP::Client#io method, the line https://github.com/crystal-lang/crystal/blob/master/src/http/client.cr#L804

io = OpenSSL::SSL::Socket::Client.new(tcp_socket, context: tls, sync_close: true, hostname: @host)

should simply be replaced with:

io = OpenSSL::SSL::Socket::Client.new(tcp_socket, context: tls, sync_close: true, hostname: @host.chomp('.'))

Can anyone more familiar with the code confirm that this change looks okay? Thanks.

@compumike
Copy link
Contributor Author

I created a pull request that fixes this issue: #12778 Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:crypto
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants