-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: don't overwrite Config.sessionTicketKeys if already set #15421
Labels
Milestone
Comments
Lines 424 to 435 in 57e459e
Lines 387 to 407 in 57e459e
|
mark-kubacki
added a commit
to caddyserver/caddy
that referenced
this issue
Apr 23, 2016
[1] https://github.com/golang/go/blob/57e459e02b4b01567f92542f92cd9afde209e193/src/crypto/tls/common.go#L424 [2] https://github.com/golang/go/blob/57e459e02b4b01567f92542f92cd9afde209e193/src/crypto/tls/common.go#L392-L407 [2] has overwritten the first tls ticket key on round N=0, that has previously been written using [1]. Go's stdlib does not use c.sessionTicketKeys≥1 as indicator if those values had already been set; initializing that lone SessionTicketKey does the job for for now. If c.serverInit() were called in round N+1 all existing tls ticket keys are being overwritten (in round N<4 except the very first one, of course). Member variables for tls.Config are read-only by then, so we cannot just keep updating SessionTicketKey as well. This has been escalated to upstream with golang/go#15421 here: golang/go#15421 Thanks to Matthew Holt for the initial report!
mark-kubacki
added a commit
to caddyserver/caddy
that referenced
this issue
Apr 23, 2016
…#785) [1] https://github.com/golang/go/blob/57e459e02b4b01567f92542f92cd9afde209e193/src/crypto/tls/common.go#L424 [2] https://github.com/golang/go/blob/57e459e02b4b01567f92542f92cd9afde209e193/src/crypto/tls/common.go#L392-L407 [2] has overwritten the first tls ticket key on round N=0, that has previously been written using [1]. Go's stdlib does not use c.sessionTicketKeys≥1 as indicator if those values had already been set; initializing that lone SessionTicketKey does the job for for now. If c.serverInit() were called in round N+1 all existing tls ticket keys would be overwritten (in round N<4 except the very first one, of course). As member variables of tls.Config are read-only by then, we cannot keep updating SessionTicketKey as well. This has been escalated to Go's authors with golang/go#15421 here: golang/go#15421 Thanks to Matthew Holt for the initial report!
bradfitz
changed the title
tls: Don't overwerwrite tls.Config.sessionTicketKeys if already set
crypto/tls: Don't overwerwrite tls.Config.sessionTicketKeys if already set
Apr 23, 2016
bradfitz
changed the title
crypto/tls: Don't overwerwrite tls.Config.sessionTicketKeys if already set
crypto/tls: don't overwrite Config.sessionTicketKeys if already set
Apr 23, 2016
Given that this is a minor bugfix, any chance to see this in Go 1.7 or 1.8? |
CL https://golang.org/cl/27317 mentions this issue. |
FiloSottile
pushed a commit
to FiloSottile/go
that referenced
this issue
Oct 12, 2018
If SetSessionTicketKeys was called on a fresh tls.Config, the configured keys would be overridden with a random key by serverInit. Fixes golang#15421. Change-Id: I5d6cc81fc3e5de4dfa15eb614d102fb886150d1b Reviewed-on: https://go-review.googlesource.com/27317 Reviewed-by: Brad Fitzpatrick <[email protected]>
FiloSottile
pushed a commit
to FiloSottile/go
that referenced
this issue
Oct 12, 2018
If SetSessionTicketKeys was called on a fresh tls.Config, the configured keys would be overridden with a random key by serverInit. Fixes golang#15421. Change-Id: I5d6cc81fc3e5de4dfa15eb614d102fb886150d1b Reviewed-on: https://go-review.googlesource.com/27317 Reviewed-by: Brad Fitzpatrick <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
With Go 1.6.2 and earlier and in
tls.Config
,serverInit()
overwrites effects of a priorSetSessionTicketKeys()
.Initializing
SessionTicketKey
to an arbitrary non-null value does not sort this in case of TLS ticket key rotation:SetSessionTicketKeys()
could have already been called several times without being allowed to update that lone field because »after [tls.Config has] been passed to a TLS function it must not be modified«.I expected
serverInit()
to:The text was updated successfully, but these errors were encountered: