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

transport: resolve DNSNames when SAN checking #7767

Merged

Conversation

heyitsanthony
Copy link
Contributor

The current transport client TLS checking will pass an IP address into
VerifyHostnames if there is DNSNames SAN. However, the go runtime will
not resolve the DNS names to match the client IP. Intead, resolve the
names when checking.

/cc @gyuho

The current transport client TLS checking will pass an IP address into
VerifyHostnames if there is DNSNames SAN. However, the go runtime will
not resolve the DNS names to match the client IP. Intead, resolve the
names when checking.
@heyitsanthony
Copy link
Contributor Author

an alternative to this is to rip out the DNSNames check entirely and only check if IPAddresses is set

Copy link
Contributor

@gyuho gyuho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Tried this for etcd-io/etcdlabs#216 and confirmed that it works.

Thanks!

@codecov-io
Copy link

Codecov Report

Merging #7767 into master will increase coverage by 0.15%.
The diff coverage is 48.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7767      +/-   ##
==========================================
+ Coverage   75.61%   75.77%   +0.15%     
==========================================
  Files         331      331              
  Lines       26053    26085      +32     
==========================================
+ Hits        19700    19765      +65     
+ Misses       4917     4891      -26     
+ Partials     1436     1429       -7
Impacted Files Coverage Δ
pkg/transport/listener_tls.go 73.91% <48.38%> (-10.81%) ⬇️
proxy/grpcproxy/register.go 69.44% <0%> (-13.89%) ⬇️
pkg/testutil/recorder.go 66.66% <0%> (-3.71%) ⬇️
pkg/netutil/netutil.go 77.94% <0%> (-2.95%) ⬇️
rafthttp/transport.go 81.92% <0%> (-1.13%) ⬇️
clientv3/watch.go 94.92% <0%> (-0.26%) ⬇️
etcdserver/raft.go 88.39% <0%> (+0.24%) ⬆️
etcdserver/server.go 80.25% <0%> (+0.59%) ⬆️
clientv3/client.go 81.54% <0%> (+0.73%) ⬆️
etcdserver/v3_server.go 71.19% <0%> (+0.95%) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30552e2...05582ad. Read the comment docs.

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

Successfully merging this pull request may close these issues.

3 participants