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: Config.GetConfigForClient is not sufficient for Listen #29139

Closed
marten-seemann opened this issue Dec 7, 2018 · 3 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@marten-seemann
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.11.2 darwin/amd64

What did you do?

Assume that I have a valid tls.Config (with Certificates set), saved in the variable conf.

Then I can start listening on a new connection by running

tls.Listen("tcp", "locahost:0", conf)

Now I want to build a more sophisticated tls.Config, which in the simplest case takes the following form

c := &tls.Config{
	GetConfigForClient: func(*tls.ClientHelloInfo) (*tls.Config, error) {
		return conf, nil
	},
}

Now

tls.Listen("tcp", "locahost:0", c)

returns tls: neither Certificates nor GetCertificate set in Config.

What did you expect to see?

tls.Listen should accept a tls.Config that has GetConfigForClient set, even if Certificates and GetCertificate is not set.
It should use the tls.Config returned by that callback, and close the connection with an error in case the returned tls.Config is nil or doesn't have any certificate configured, depending on the SNI.

What did you see instead?

tls.Listen didn't accept the tls.Config and returned an error.

@agnivade
Copy link
Contributor

agnivade commented Dec 8, 2018

/cc @FiloSottile

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. FeatureRequest labels Dec 19, 2018
@bcmills bcmills added this to the Go1.13 milestone Dec 19, 2018
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@FiloSottile
Copy link
Contributor

Yep, see also #18377. Will fix.

@FiloSottile FiloSottile added NeedsFix The path to resolution is known, but the work has not been done. and removed FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 1, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@FiloSottile FiloSottile modified the milestones: Backlog, Go1.14 Oct 23, 2019
@gopherbot
Copy link

Change https://golang.org/cl/205059 mentions this issue: crypto/tls: select only compatible chains from Certificates

@golang golang locked and limited conversation to collaborators Nov 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants