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

DialTLS: use tls.Dial instead of doing an explicit handshake #94

Closed
ericchiang opened this issue Nov 15, 2016 · 3 comments
Closed

DialTLS: use tls.Dial instead of doing an explicit handshake #94

ericchiang opened this issue Nov 15, 2016 · 3 comments
Milestone

Comments

@ericchiang
Copy link

This was brought up in dexidp/dex#689

Currently DialTLS() uses net.Dial then performs an explicit handshake.[0]

We noticed this because tls.Dial does a few other things like inferring the ServerName[1], but since the LDAP client doesn't use that code path, users have to set it explicitly.

Is there a reason this client needs to do its own handshake?

[0]

ldap/conn.go

Lines 124 to 138 in d0a5ced

func DialTLS(network, addr string, config *tls.Config) (*Conn, error) {
dc, err := net.DialTimeout(network, addr, DefaultTimeout)
if err != nil {
return nil, NewError(ErrorNetwork, err)
}
c := tls.Client(dc, config)
err = c.Handshake()
if err != nil {
// Handshake error, close the established connection before we return an error
dc.Close()
return nil, NewError(ErrorNetwork, err)
}
conn := NewConn(c, true)
conn.Start()
return conn, nil

[1] https://github.com/golang/go/blob/go1.7.3/src/crypto/tls/tls.go#L134-L141

@liggitt
Copy link
Contributor

liggitt commented Nov 15, 2016

https://github.com/go-ldap/ldap/pull/65/files switches to use DialWithDialer, but depends on a newer version of go. Will switch to that for v3 when we bump minimum supported go versions

@liggitt liggitt added this to the v3 milestone Nov 15, 2016
@ericchiang
Copy link
Author

cool, thanks.

@stefanmcshane
Copy link
Contributor

Closing as this can be implementing using DialWithDialer DialOpt with DialURL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants