-
Notifications
You must be signed in to change notification settings - Fork 83
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
Further secure TLS communications #97
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: saroshali-dbx <saroshali@dropbox.com>
Signed-off-by: saroshali-dbx <saroshali@dropbox.com>
475e00a
to
69f5c39
Compare
gentle ping @roidelapluie @SuperQ :) For context, here's the issue created for the downstream dependency - weaveworks/common#242 |
I thought most certificate validation has moved to verifying SANs, not CNs. |
Yeah, thats the intention with SANs to be able to get around the limitation of only being able to specify a single server-name in CN. I ported this functionality from etcd -- and my organization still utilizes common-name -- but I can update this PR to check SANs first and then fallback to CNs. How does that sound? |
Actually that's a good point @SuperQ , even go uses SAN's and has removed support for CN. I think we should follow go and use SANs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Common Name is deprecated for validation. This needs to use SANs.
From RFC 2818, May 2000:
Although the use of the Common Name is existing practice, it is deprecated and Certification Authorities are encouraged to use the dNSName instead.
Chrome-based browsers dropped CN-only in 2017.
@saroshali-dbx are you likely to return to this, or shall we close it? |
Just a question: isn't SAN or any other DNS unusual for client certs? I usually create them with
Thus given the 3.2 leads me to |
Closes #96