Skip to content
New issue

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

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

Already on GitHub? # to your account

transport: use reverse lookup to match wildcard DNS SAN #8281

Merged
merged 3 commits into from
Jul 22, 2017

Conversation

heyitsanthony
Copy link
Contributor

Fixes #8268

if err != nil {
errStr = " (" + err.Error() + ")"
}
return fmt.Errorf("tls: %q does not match any of DNSNames %q"+errStr, h, cert.DNSNames)
Copy link
Contributor

Choose a reason for hiding this comment

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

%q "+errStr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

already gets the space from " ("

Copy link
Contributor

Choose a reason for hiding this comment

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

oh I see it now.

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. thanks

@codecov-io
Copy link

Codecov Report

Merging #8281 into master will decrease coverage by 0.21%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8281      +/-   ##
==========================================
- Coverage   76.42%   76.21%   -0.22%     
==========================================
  Files         346      346              
  Lines       27055    27079      +24     
==========================================
- Hits        20676    20637      -39     
- Misses       4901     4963      +62     
- Partials     1478     1479       +1
Impacted Files Coverage Δ
pkg/transport/listener_tls.go 66.44% <0%> (-12.76%) ⬇️
pkg/transport/timeout_conn.go 80% <0%> (-20%) ⬇️
pkg/adt/interval_tree.go 79.27% <0%> (-11.72%) ⬇️
auth/simple_token.go 86.79% <0%> (-6.61%) ⬇️
pkg/testutil/recorder.go 77.77% <0%> (-3.71%) ⬇️
clientv3/concurrency/election.go 80% <0%> (-2.41%) ⬇️
pkg/netutil/netutil.go 62.35% <0%> (-2.36%) ⬇️
etcdserver/cluster_util.go 76.64% <0%> (-1.46%) ⬇️
clientv3/watch.go 94.5% <0%> (-1%) ⬇️
proxy/grpcproxy/lease.go 87.61% <0%> (-0.96%) ⬇️
... and 14 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 608df0f...9aed03f. Read the comment docs.

@heyitsanthony
Copy link
Contributor Author

This didn't work as approved; PTR records will return a trailing '.' that has to be stripped off. I've also added wildcard certs / a container for testing DNS. /cc @gyuho

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. Thanks!

@heyitsanthony heyitsanthony merged commit e9a7f35 into etcd-io:master Jul 22, 2017
@heyitsanthony heyitsanthony deleted the san-rdns branch July 22, 2017 15:03
@pires
Copy link

pires commented Jul 31, 2017

Can we have a 3.2.5 release? Waiting on this PR to re-enable peer authentication on my clusters.

@gyuho
Copy link
Contributor

gyuho commented Jul 31, 2017

We will release this Wednesday or Friday.

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

Successfully merging this pull request may close these issues.

4 participants