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: optional ocspStapling #8549

Closed
brad-burch opened this issue Aug 19, 2014 · 9 comments
Closed

crypto/tls: optional ocspStapling #8549

brad-burch opened this issue Aug 19, 2014 · 9 comments
Assignees
Milestone

Comments

@brad-burch
Copy link
Contributor

@brad-burch brad-burch commented Aug 19, 2014

go version go1.3

I have a server outside my control that fails to respond properly when OSCP stapling is
enabled and causes a "tls: received unexpected handshake message..." error.

I would like to be able to set a tls.Config{} option to disable OSCP stapling instead of
recompiling the package.  I am attaching the rudimentary patch I have been compiling
into crypto/tls.

Attachments:

  1. DisableOSCPStapling.patch (1166 bytes)
@griesemer

This comment has been minimized.

Copy link
Contributor

@griesemer griesemer commented Oct 1, 2014

Comment 1:

Labels changed: added release-none, repo-crypto.

@brad-burch brad-burch added new labels Oct 1, 2014
@brad-burch

This comment has been minimized.

Copy link
Contributor Author

@brad-burch brad-burch commented Dec 9, 2014

go version go1.4rc2

Attaching a different patch which resolves my issue and follows the wording in RFC4366 more precisely.

https://gist.github.com/brad-burch/148a7dfe258c3e6fd68f

@brad-burch brad-burch changed the title crypto/tls: disabling ocspStapling crypto/tls: optional ocspStapling Dec 9, 2014
@bradfitz bradfitz removed the new label Dec 18, 2014
@mikioh mikioh added repo-main and removed repo-crypto labels Jan 7, 2015
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc removed release-none labels Apr 10, 2015
@mberhault

This comment has been minimized.

Copy link

@mberhault mberhault commented Jan 3, 2018

Any movement on this? It has caused some confusion on this stack overflow question. Given that the patch proposes to accurately implement the RFC, is there any downside in integrating it?

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Jan 3, 2018

@mberhault, any movement would be reflected in this bug, so I would say there's been no movement. And no patch was formally sent via https://golang.org/doc/contribute.html

@agl, any opinion on making OCSP stapling optional?

@bradfitz bradfitz modified the milestones: Unplanned, Go1.11 Jan 3, 2018
@agl

This comment has been minimized.

Copy link
Contributor

@agl agl commented Jan 3, 2018

We don't want to accrete lots of options to work around buggy servers. OCSP stapling isn't obscure or anything.

@mberhault

This comment has been minimized.

Copy link

@mberhault mberhault commented Jan 3, 2018

The updated patch (not the original) is about properly handling a certificateStatusMsg response as optional, it no longer changes tls.Config.
I can't claim to be that knowledgeable about OCSP stapling, but the quoted RFC section seems to justify the change.

@agl

This comment has been minimized.

Copy link
Contributor

@agl agl commented Jan 3, 2018

Ah, I see. The CertificateStatus message being optional is a mistake in the spec, but it is in the spec. I don't want to look at a patch that isn't CLAed, but it sounds perfectly reasonable. (Possibly for 1.10, even.)

@mberhault

This comment has been minimized.

Copy link

@mberhault mberhault commented Jan 3, 2018

Sounds reasonable. @brad-burch: would you mind taking your patch through https://golang.org/doc/contribute.html?

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 4, 2018

Change https://golang.org/cl/86115 mentions this issue: crypto/tls: optional "certificate_status" with OCSP

@gopherbot gopherbot closed this in 100bd43 Jan 4, 2018
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
Follows the wording in RFC4366 more precisely which allows a server
to optionally return a "certificate_status" when responding to a
client hello containing "status_request" extension.

fixes golang#8549

Change-Id: Ib02dc9f972da185b25554568fe6f8bc411d9c0b7
Reviewed-on: https://go-review.googlesource.com/86115
Reviewed-by: Adam Langley <agl@golang.org>
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
Follows the wording in RFC4366 more precisely which allows a server
to optionally return a "certificate_status" when responding to a
client hello containing "status_request" extension.

fixes golang#8549

Change-Id: Ib02dc9f972da185b25554568fe6f8bc411d9c0b7
Reviewed-on: https://go-review.googlesource.com/86115
Reviewed-by: Adam Langley <agl@golang.org>
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
Follows the wording in RFC4366 more precisely which allows a server
to optionally return a "certificate_status" when responding to a
client hello containing "status_request" extension.

fixes golang#8549

Change-Id: Ib02dc9f972da185b25554568fe6f8bc411d9c0b7
Reviewed-on: https://go-review.googlesource.com/86115
Reviewed-by: Adam Langley <agl@golang.org>
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
Follows the wording in RFC4366 more precisely which allows a server
to optionally return a "certificate_status" when responding to a
client hello containing "status_request" extension.

fixes golang#8549

Change-Id: Ib02dc9f972da185b25554568fe6f8bc411d9c0b7
Reviewed-on: https://go-review.googlesource.com/86115
Reviewed-by: Adam Langley <agl@golang.org>
@golang golang locked and limited conversation to collaborators Jan 4, 2019
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
8 participants
You can’t perform that action at this time.