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

crypto/tls: add VerifyPeerCertificate to tls.Config #16363

Closed
jboelter opened this issue Jul 13, 2016 · 2 comments
Closed

crypto/tls: add VerifyPeerCertificate to tls.Config #16363

jboelter opened this issue Jul 13, 2016 · 2 comments
Assignees
Milestone

Comments

@jboelter
Copy link

@jboelter jboelter commented Jul 13, 2016

It would be useful to have a callback in tls.Config called after the TLS handshake to allow the callee to perform additional or alternate certificate verification on the peer. This would enable custom cert checking and/or explicit certificate pinning/whitelisting (e.g. by cert hash).

I propose calling VerifyPeerCertificate (if !nil) during the respective client and server handshakes (doFullHandshake and processPeerCertsFromClient) after the peer certificates have been received.

The func is introduced to tls.Config

    // VerifyPeerCertificate returns an error if the peer should not be
    // trusted. It will be called after the initial handshake and before
    // any other verification checks on the cert or chain are performed.
    // This provides the callee an opportunity to augment the certificate
    // verification.
    //
    // If VerifyPeerCertificate is not nil and returns an error,
    // then the handshake will fail.
    VerifyPeerCertificate func(peer []*x509.Certificate) error

This works with both http and http2. It appears that tls.Config is cloned from the initial transport for TLSNextProto -- is this intentional or an artifact of the current impl?

    c := &http.Client{
        Transport: &http.Transport{
            Proxy: http.ProxyFromEnvironment,
            Dial: (&net.Dialer{
                Timeout:   60 * time.Second,
                KeepAlive: 60 * time.Second,
            }).Dial,
            TLSHandshakeTimeout:   10 * time.Second,
            ExpectContinueTimeout: 1 * time.Second,
            TLSClientConfig: &tls.Config{
                // inject our callback to check the peer here
                NextProtos:            []string{"h2"},
                VerifyPeerCertificate: verifyPeerCertificate,
            },
            TLSNextProto: map[string]func(authority string, c *tls.Conn) http.RoundTripper{
                "h2": func(authority string, c *tls.Conn) http.RoundTripper {
                    return http.DefaultTransport
                },
            },
        },
    }

I have a changset ready if this is a reasonable approach.

Fixes #9126
Fixes #9451

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 13, 2016

CC @agl

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Aug 10, 2016

CL https://golang.org/cl/26654 mentions this issue.

@agl agl self-assigned this Aug 19, 2016
@quentinmit quentinmit added the NeedsFix label Oct 5, 2016
@gopherbot gopherbot closed this in 426c287 Oct 24, 2016
@mholt mholt mentioned this issue Feb 18, 2017
9 of 10 tasks complete
@golang golang locked and limited conversation to collaborators Oct 24, 2017
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
VerifyPeerCertificate returns an error if the peer should not be
trusted. It will be called after the initial handshake and before
any other verification checks on the cert or chain are performed.
This provides the callee an opportunity to augment the certificate
verification.

If VerifyPeerCertificate is not nil and returns an error,
then the handshake will fail.

Fixes golang#16363

Change-Id: I6a22f199f0e81b6f5d5f37c54d85ab878216bb22
Reviewed-on: https://go-review.googlesource.com/26654
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
VerifyPeerCertificate returns an error if the peer should not be
trusted. It will be called after the initial handshake and before
any other verification checks on the cert or chain are performed.
This provides the callee an opportunity to augment the certificate
verification.

If VerifyPeerCertificate is not nil and returns an error,
then the handshake will fail.

Fixes golang#16363

Change-Id: I6a22f199f0e81b6f5d5f37c54d85ab878216bb22
Reviewed-on: https://go-review.googlesource.com/26654
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.