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

crypto/tls: "unexpected message" when CT extension enabled with no SCTs #17958

woodsaj opened this issue Nov 17, 2016 · 7 comments
Assignees

Comments

@woodsaj
Copy link
Contributor

@woodsaj 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

This comment has been minimized.

Copy link
Contributor Author

@woodsaj 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 added 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

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 17, 2016

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

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Nov 17, 2016

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

@agl

This comment has been minimized.

Copy link
Contributor

@agl 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.

@gopherbot gopherbot closed this in c099459 Nov 17, 2016
@woodsaj

This comment has been minimized.

Copy link
Contributor Author

@woodsaj 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

This comment has been minimized.

Copy link
Contributor

@agl 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

This comment has been minimized.

Copy link
Contributor

@agl 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>
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
4 participants
You can’t perform that action at this time.