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: "unexpected message" when CT extension enabled with no SCTs #17958

Closed
woodsaj opened this issue Nov 17, 2016 · 7 comments
Closed
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@woodsaj
Copy link
Contributor

woodsaj commented Nov 17, 2016

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

all since go1.5

What operating system and processor architecture are you using (go env)?

all

What did you do?

Try and negotiate a TLS connection with a server that has Certificate Transparency TLS extension enabled but provides no signed certificate timestamps

What did you expect to see?

The TLS handshake to succeed.

What did you see instead?

Error: "local error: tls: unexpected message"

woodsaj pushed a commit to woodsaj/go that referenced this issue Nov 17, 2016
@woodsaj
Copy link
Contributor Author

woodsaj commented Nov 17, 2016

https://github.com/golang/go/blob/master/src/crypto/tls/handshake_messages.go#L809

By calling "continue" the byte slice "data" is never reduced by the extension length. "break" should be called instead.

woodsaj pushed a commit to woodsaj/go that referenced this issue Nov 17, 2016
When the CT extension is enabled but no SCTs are present, the
existing code calls "continue" which causes resizing the data
byte slice to be skipped.

fixes golang#17958
@gopherbot
Copy link

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

@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Nov 17, 2016
@bradfitz
Copy link
Contributor

To @agl for decision on what Go should do here.

@agl
Copy link
Contributor

agl commented Nov 17, 2016

Where did you come across a server that returns an empty SCT list? The spec says that the list must not be empty, so it's actually something that should be rejected. The code, as is, will mostly reject it—albeit because of a bug. Thankfully the extension type and length are already sliced off so its not an infinite loop.

@woodsaj
Copy link
Contributor Author

woodsaj commented Nov 17, 2016

Had a user of one of our monitoring tools report that our tool was unable to connect to their site even though their site loaded correctly in browsers. I cant give you their site address without their permission, but the site referenced in #7953 (comment) is an example you can test with.

I believe that these sites are using https://github.com/grahamedgecombe/nginx-ct

I am not all familiar with this TLS feature, i just came across the handling bug after spending a big chunk of my day tracking down the cause of the vague "unexpected message" error. I assumed that the empty SCT list was valid as the sites loaded in the browser (Chrome) fine.

So though the Spec is explicit about the SCT list have at least 1 entry, perhaps the Go TLS library could be as forgiving as browsers.

@agl
Copy link
Contributor

agl commented Nov 17, 2016

Thank you for the pointer. I'll see whether I can get nginx-ct fixed as the best approach here isn't to make Go more permissive but rather to fix the browsers to match.

@agl
Copy link
Contributor

agl commented Dec 1, 2016

nginx-ct have produced a new release (1.3.2) which includes a fix for this.

@golang golang locked and limited conversation to collaborators Dec 1, 2017
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
When the CT extension is enabled but no SCTs are present, the existing
code calls "continue" which causes resizing the data byte slice to be
skipped. In fact, such extensions should be rejected.

Fixes golang#17958

Change-Id: Iad12da10d1ea72d04ae2e1012c28bb2636f06bcd
Reviewed-on: https://go-review.googlesource.com/33265
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
When the CT extension is enabled but no SCTs are present, the existing
code calls "continue" which causes resizing the data byte slice to be
skipped. In fact, such extensions should be rejected.

Fixes golang#17958

Change-Id: Iad12da10d1ea72d04ae2e1012c28bb2636f06bcd
Reviewed-on: https://go-review.googlesource.com/33265
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@rsc rsc unassigned agl Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

4 participants