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

Question: Is it possible to use self-signed certs but NOT use InsecureSkipVerify with rafthttp #8578

Closed
robin865 opened this issue Sep 19, 2017 · 14 comments

Comments

@robin865
Copy link

Background:

I have nodes with self-signed certs, and am testing setting up a raft cluster. If I want to use TLS, and use my own custom http clients I can get this to work by doing the following for the client TLSClientConfig:

  1. Set the RootCAs to include my root CA, include the sender's cert in "Certificates", and then set the "ServerName" value to be equivalent to the "CN" used for the destinations self signed cert (this destination CN is not the actual host name, but something else related to the server).

Ie. Node111 and Node222. Sending from Node111 to Node222; I can set the ServerName to '222' and this will all client cert verification to work. (again, 222 is NOT the host name)

However, in rafthttp; the only reference to self signed certs is the option that sets InsecureSkipVerify to true (which I don't want to do). But there doesn't seem to be a way to customize per-client/per-peer TLS. The only control I get is constructing the TLSInfo object.

So my question is; is there a way to get around this? Essentially I need a way to customize the TLS config based on the peer being communicated to.

@gyuho
Copy link
Contributor

gyuho commented Sep 19, 2017

@Rsm10 How do you construct your TLSInfo? Do you use transport.SelfCert https://github.com/coreos/etcd/blob/master/pkg/transport/listener.go#L89?

@robin865
Copy link
Author

I can get it to work when I use transport.SelfCert; but then it won't do hostname verification (which I want).

@robin865
Copy link
Author

Ok, let me re-phrase. A bit new to http authentication. Using "SelfCert" will disable hostname verification (by setting InsecureSkipVerify=true). But it will still authenticate using the provided cert.pem and key.pem correct?

@gyuho
Copy link
Contributor

gyuho commented Sep 20, 2017

I am not sure how you use pkg/transport and rafthttp, but I would try call func (info TLSInfo) ClientConfig() from SelfCert, and try overwriting ServerName field in tls.Config as you wish.

@gyuho
Copy link
Contributor

gyuho commented Sep 20, 2017

Using "SelfCert" will disable hostname verification (by setting InsecureSkipVerify=true).

Yes, as in the code above.

Certs will still be verified when no hostname is provided, but they won't be checked against any hostname.

@robin865
Copy link
Author

So InsecureSkipVerify still verifies the tls certs? Is InsecureSkipVerify equivalent to "skip hostname verification" or does it skip other steps as well? I guess what I really want is just the ability to skip hostname verification.

@gyuho
Copy link
Contributor

gyuho commented Sep 20, 2017

I meant, you still need valid certs for encryption. But InsecureSkipVerify will skip host name checks.

@robin865
Copy link
Author

    // InsecureSkipVerify controls whether a client verifies the
    // server's certificate chain and host name.
    // If InsecureSkipVerify is true, TLS accepts any certificate
    // presented by the server and any host name in that certificate.
    // In this mode, TLS is susceptible to man-in-the-middle attacks.
    // This should be used only for testing.

So my client and server have a common CA, and I still want to verify that the certs used are signed by this CA; just not do the host name check. Doing a bit of testing seems like when InsecureSkipVerify is true, this is NOT done.

Ex: I replaced certs on one of my servers with certs not signed by my common CA; but everything still works.

This is my tslInfo:

tlsInfo, err := transport.SelfCert("/usr/local/certs/", []string{})
if err != nil {
	logWarning(fmt.Sprintf("failed to get SelfCert %v", err))
}
tlsInfo.CAFile = "/usr/local/certs/cacert.pem"
tlsInfo.ClientCertAuth = true

@robin865
Copy link
Author

robin865 commented Sep 20, 2017

Looking a bit deeper into the go crypto library, what I really want to be able to do is leave ServerName empty but also have InsecureSkipVerify set to false. At a higher level, this seems to be prevented (empty ServerName gets set to the host) but when I look in "crypto/x509/verify.go"; it seems like this would do exactly what I want: Skip the "VerifyHostname" check but do the rest of the validation (ServerName is translated into the DNSName config option).

@gyuho
Copy link
Contributor

gyuho commented Sep 20, 2017

what I really want to be able to do is leave ServerName empty but also have InsecureSkipVerify set to false.

You can't. See golang/go@fca335e.

@robin865
Copy link
Author

OK thanks. Filed a new feature request to add support for this. We can close this thread.

golang/go#21971

@robin865
Copy link
Author

Just a note that the change we'd need to support my initial ask would be to expose setting "VerifyPeerCertificate" directly in TLSInfo and propagate it to the tls.Config struct.

As per a comment on my feature request (golang/go#21971):

"You can currently do this by using VerifyPeerCertificate and (*Certificate).Verify (and remembering to put the remaining rawCerts into VerifyOptions.Intermediates).

https://github.com/golang/go/blob/39e523792e33a0bd9217161ca53c6c0cb2324a99/src/crypto/tls/handshake_client.go#L315-L344"

@stale
Copy link

stale bot commented Apr 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 7, 2020
@NikolayUvarov
Copy link

I've got same problem. I need a way to use self-signed serts, because thay are intended to use in closed network without internet.

@stale stale bot closed this as completed May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants