crypto/tls: feature request: add option to JUST skip hostname verification #21971

robin865 · 2017-09-21T22:10:40Z

I understand in "fca335e" we made changes to enforce either requiring ServerName or InsecureSkipVerify to be set in our tls libraries.

What about making this the default behaviour but having a "SkipHostnameVerification" option? I have a situation where we are using certs signed by our own private CA. The hostname verification won't work since we pre-generate certs using a CN that isn't an actual DSN/Host name. I still want to validate that other servers certs are at least signed by the same private CA as the client but JUST want to skip the hostnameVerify check. Today that doesn't seem possible.

The text was updated successfully, but these errors were encountered:

ianlancetaylor · 2017-09-21T22:26:34Z

CC @agl @FiloSottile

FiloSottile · 2017-09-28T00:36:31Z

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

go/src/crypto/tls/handshake_client.go

Lines 315 to 344 in 39e5237

    
           certs := make([]*x509.Certificate, len(certMsg.certificates)) 
        
           for i, asn1Data := range certMsg.certificates { 
        
           	cert, err := x509.ParseCertificate(asn1Data) 
        
           	if err != nil { 
        
           		c.sendAlert(alertBadCertificate) 
        
           		return errors.New("tls: failed to parse certificate from server: " + err.Error()) 
        
           	} 
        
           	certs[i] = cert 
        
           } 
        
           if !c.config.InsecureSkipVerify { 
        
           	opts := x509.VerifyOptions{ 
        
           		Roots:         c.config.RootCAs, 
        
           		CurrentTime:   c.config.time(), 
        
           		DNSName:       c.config.ServerName, 
        
           		Intermediates: x509.NewCertPool(), 
        
           	} 
        
           	for i, cert := range certs { 
        
           		if i == 0 { 
        
           			continue 
        
           		} 
        
           		opts.Intermediates.AddCert(cert) 
        
           	} 
        
           	c.verifiedChains, err = certs[0].Verify(opts) 
        
           	if err != nil { 
        
           		c.sendAlert(alertBadCertificate) 
        
           		return err 
        
           	} 
        
           }

I don't think this is common enough to add another verification option to the already crowded tls.Config.

robin865 · 2017-10-05T05:50:40Z

Problem is I want to use a library (rafthttp) that only exposes a transport.TLSInfo (https://github.com/coreos/etcd/blob/master/pkg/transport/listener.go) , not a tls Config (https://golang.org/pkg/crypto/tls/#Config). So this sort of manual workaround would need to be plumbed up the stack for me to use it. If there was a simple option, it would be easier to plumb this through. [https://avatars2.githubusercontent.com/u/3730757?v=4&s=400]<https://github.com/coreos/etcd/blob/master/pkg/transport/listener.go> etcd/listener.go at master · coreos/etcd · GitHub<https://github.com/coreos/etcd/blob/master/pkg/transport/listener.go> github.com etcd - Distributed reliable key-value store for the most critical data of a distributed system Package tls - The Go Programming Language<https://golang.org/pkg/crypto/tls/#Config> golang.org Constants. A list of cipher suite IDs that are, or have been, implemented by this package. Taken from http://www.iana.org/assignments/tls-parameters/tls-parameters.xml

…

________________________________ From: Filippo Valsorda <notifications@github.com> Sent: September 27, 2017 5:37 PM To: golang/go Cc: rsm10; Author Subject: Re: [golang/go] crypto/tls: feature request: add option to JUST skip hostname verification (#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 [https://avatars3.githubusercontent.com/u/4314092?v=4&s=400]<https://github.com/golang/go/blob/39e523792e33a0bd9217161ca53c6c0cb2324a99/src/crypto/tls/handshake_client.go#L315-L344> golang/go<https://github.com/golang/go/blob/39e523792e33a0bd9217161ca53c6c0cb2324a99/src/crypto/tls/handshake_client.go#L315-L344> github.com go - The Go programming language I don't think this is common enough to add another verification option to the already crowded tls.Config. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#21971 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAvL8o2bbdzxc8ptfc343eWUqY16IHmQks5smupMgaJpZM4Pf-6w>.

robin865 · 2017-10-17T07:00:32Z

As to your original suggestion, curious why on lines 334-336 we seem to skip the first cert in the chain?

	if i == 0 { 
		continue 
	}

FiloSottile · 2017-10-17T14:30:40Z

The first cert is the leaf, and it’s used on line 339. All the others are intermediates and are used on line 337.

rsc · 2017-10-23T20:33:34Z

Problem is I want to use a library (rafthttp) that only exposes a transport.TLSInfo (https://github.com/coreos/etcd/blob/master/pkg/transport/listener.go) , not a tls Config

This is a very special case. I would suggest working with the library maintainers (or just having your own fork) to expose the tls.Config. We shouldn't add a second way to do something in the standard library (especially a somewhat confusing security knob) just because one downstream package hides the first way.

benma · 2018-08-14T11:07:45Z

I am also interested in skipping the hostname verification only. The problem is that if the ServerName is unset, it ill be inferred. As stated above, one way to achieve it is to set InsecureSkipVerify to true and copy/paste the code from the stdlib to config.VerifyPeerCertificate, but that is suboptimal, as one would miss fixes done to this part of the code in the stdlib.

👍 for a SkipHostnameVerification option.

sikemullivan · 2018-11-23T14:55:01Z

👍 for a SkipHostnameVerification option as well.

Trying to get client authentication working with MySQL. Using a self-signed cert because I'm running everything out of VPS machines. Everything is IP based. Has anyone built the code for the suggested work around? I don't want to reinvent the wheel.

benma · 2018-11-23T15:18:37Z

@sikemullivan

yes, here: https://github.com/digitalbitbox/bitbox-wallet-app/blob/b04bd07852d5b37939da75b3555b5a1e34a976ee/backend/coins/btc/electrum/electrum.go#L76-L111

sikemullivan · 2018-11-23T15:31:34Z

Awesome, thanks for the quick response.

sikemullivan · 2018-11-23T15:42:38Z

@benma

Should line 109 set the verified chains? Or does it not matter?

Now:

_, err := certs[0].Verify(opts)
return err

Then:

verifiedChains, err := certs[0].Verify(opts)
return err

benma · 2018-11-23T15:54:27Z

Based on a quick look through the stdlib, I would say it would make sense, yes. Seems to be exposed only in conn.ConnectionState(), and conn.VerifyHostname(). Not sure why I didn't put it in back then.

That was too quick. I looked again, and verifiedChains is input to the function, not output. The api/callback doesn't support setting verifiedChains (which should probably be fixed upstream).

sikemullivan · 2018-11-23T16:09:36Z

Agree, everything seems to work without it getting returned. Thanks for your help.

gopherbot · 2019-09-05T20:35:30Z

Change https://golang.org/cl/193620 mentions this issue: crypto/tls: add ExampleConfig_VerifyPeerCertificate

Setting InsecureSkipVerify and VerifyPeerCertificate is the recommended way to customize and override certificate validation. However, there is boilerplate involved and it usually requires first reimplementing the default validation strategy to then customize it. Provide an example that does the same thing as the default as a starting point. Examples of where we directed users to do something similar are in issues #35467, #31791, #28754, #21971, and #24151. Fixes #31792 Change-Id: Id033e9fa3cac9dff1f7be05c72dfb34b4f973fd4 Reviewed-on: https://go-review.googlesource.com/c/go/+/193620 Reviewed-by: Adam Langley <agl@golang.org>

ParseConfig currently treats the libpq "verify-ca" SSL mode as "verify-full". This is okay from a security standpoint because "verify-full" performs certificate verification and hostname verification, whereas "verify-ca" only performs certificate verification. The downside to this approach is that checking the hostname is unnecessary when the server's certificate has been signed by a private CA. It can also cause the SSL handshake to fail when connecting to an instance by IP. For example, a Google Cloud SQL instance typically doesn't have a hostname and uses its own private CA to sign its server and client certs. This change uses the tls.Config.VerifyPeerCertificate function to perform certificate verification without checking the hostname when the "verify-ca" SSL mode is set. This brings pgconn's behavior closer to that of libpq. See golang/go#21971 (comment) and https://pkg.go.dev/crypto/tls?tab=doc#example-Config-VerifyPeerCertificate for more details on how this is implemented.

ianlancetaylor changed the title ~~feature request: add option to JUST skip hostname verification~~ crypto/tls: feature request: add option to JUST skip hostname verification Sep 21, 2017

ianlancetaylor added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 21, 2017

ianlancetaylor added this to the Unplanned milestone Sep 21, 2017

robin865 mentioned this issue Sep 21, 2017

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

Closed

rsc closed this as completed Oct 23, 2017

shotahino mentioned this issue Dec 18, 2017

How to bypass hostname verification? grpc/grpc-java#3880

Closed

FiloSottile mentioned this issue May 1, 2019

crypto/tls: add an example for using VerifyPeerCertificate to customize verification logic #31792

Closed

laf-rge mentioned this issue Sep 21, 2019

HTTP client can't be configured to accept invalid hostnames newrelic/infra-integrations-sdk#185

Closed

14 tasks

EronWright mentioned this issue Jan 11, 2020

Enable TLS hostname verification by default apache/pulsar-client-go#171

Closed

sijie mentioned this issue Jan 11, 2020

ISSUE-171: Enable TLS hostname verification by default streamnative/pulsar-client-go#21

Open

gcurtis mentioned this issue Mar 18, 2020

Implement "verify-ca" SSL mode jackc/pgconn#32

Merged

zzzming mentioned this issue May 3, 2020

Implement TLS VerifyPeerCertificate callback to skip hostname verfication apache/pulsar-client-go#238

Open

golang locked and limited conversation to collaborators Sep 4, 2020

gopherbot added the FrozenDueToAge label Sep 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

crypto/tls: feature request: add option to JUST skip hostname verification #21971

crypto/tls: feature request: add option to JUST skip hostname verification #21971

robin865 commented Sep 21, 2017

ianlancetaylor commented Sep 21, 2017

FiloSottile commented Sep 28, 2017

robin865 commented Oct 5, 2017 via email

robin865 commented Oct 17, 2017

FiloSottile commented Oct 17, 2017

rsc commented Oct 23, 2017

benma commented Aug 14, 2018 •

edited

sikemullivan commented Nov 23, 2018

benma commented Nov 23, 2018

sikemullivan commented Nov 23, 2018

sikemullivan commented Nov 23, 2018

benma commented Nov 23, 2018 •

edited

sikemullivan commented Nov 23, 2018

gopherbot commented Sep 5, 2019

crypto/tls: feature request: add option to JUST skip hostname verification #21971

crypto/tls: feature request: add option to JUST skip hostname verification #21971

Comments

robin865 commented Sep 21, 2017

ianlancetaylor commented Sep 21, 2017

FiloSottile commented Sep 28, 2017

robin865 commented Oct 5, 2017 via email

robin865 commented Oct 17, 2017

FiloSottile commented Oct 17, 2017

rsc commented Oct 23, 2017

benma commented Aug 14, 2018 • edited

sikemullivan commented Nov 23, 2018

benma commented Nov 23, 2018

sikemullivan commented Nov 23, 2018

sikemullivan commented Nov 23, 2018

benma commented Nov 23, 2018 • edited

sikemullivan commented Nov 23, 2018

gopherbot commented Sep 5, 2019

benma commented Aug 14, 2018 •

edited

benma commented Nov 23, 2018 •

edited