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

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.

4 participants