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

x/net/websocket: allow configurable read size limit, and MaxHeaderBytes #5082

Closed
slimsag opened this issue Mar 19, 2013 · 12 comments
Closed

x/net/websocket: allow configurable read size limit, and MaxHeaderBytes #5082

slimsag opened this issue Mar 19, 2013 · 12 comments
Milestone

Comments

@slimsag
Copy link

@slimsag slimsag commented Mar 19, 2013

What steps will reproduce the problem?
    Currently go.net/websocket provides no way to limit how much data an websocket server reads, if an client sends very large amounts of data (messages, frames) the websocket server will allocate that memory, and will continue to do so, until the server runs out of memory or the client crashes.

[In someone else's tests] it has been observed that there is also no header limit
provided (for the same issue). (See discussion:
https://groups.google.com/forum/?fromgroups=#!topic/golang-nuts/2Tge6U8-QYI)


What is the expected output?
go.net/websocket should provide an way to set an maximum read size and maximum header
read size, to avoid attacks from potentially malicious clients.
@robpike
Copy link
Contributor

@robpike robpike commented Aug 4, 2013

Comment 2:

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

@rsc
Copy link
Contributor

@rsc rsc commented Nov 27, 2013

Comment 3:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 4:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 5:

Labels changed: added repo-net.

@mikioh mikioh changed the title go.net/websocket: allow configurable read size limit, and MaxHeaderBytes x/net/websocket: allow configurable read size limit, and MaxHeaderBytes Dec 23, 2014
@mikioh mikioh changed the title x/net/websocket: allow configurable read size limit, and MaxHeaderBytes websocket: allow configurable read size limit, and MaxHeaderBytes Jan 4, 2015
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc changed the title websocket: allow configurable read size limit, and MaxHeaderBytes x/net/websocket: allow configurable read size limit, and MaxHeaderBytes Apr 14, 2015
@rsc rsc modified the milestones: Unreleased, Unplanned Apr 14, 2015
@rsc rsc removed the repo-net label Apr 14, 2015
@gdamore
Copy link

@gdamore gdamore commented Oct 20, 2015

This actually is a tragic failing. It means that Go websockets cannot be used safely in an unprotected network, since malefactor can simply request a giant message to DoS your service.

@gdamore
Copy link

@gdamore gdamore commented May 30, 2016

Bump. This is a SECURITY FAILURE. Its a trivial remote DoS for websocket, meaning that you should not use native Golang websockets pretty much anywhere.

@gdamore
Copy link

@gdamore gdamore commented May 30, 2016

In fact, this is probably the only reason that I have to use 3rd party Gorilla websocket instead of native golang one. Which angers me as the Gorilla folks have busted compatibility for Go versions < 1.5 recently.

@artyom
Copy link
Member

@artyom artyom commented May 30, 2016

One option to limit incoming payload size can be introduction of optional "max.received message payload size" field to websocket.Codec struct, which, if non-zero, should be treated by Codec.Receive method as indication to wrap frame io.Reader into something similar to io.LimitedReader (that also has to report ErrUnexpectedEOF to recognize incomplete reads) right before the call to ioutil.ReadAll, which is a culprit here.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 30, 2016

Anybody want to send in a fix? If you are unsure about the approach, discuss on the golang-dev mailing list first.

@artyom
Copy link
Member

@artyom artyom commented May 30, 2016

@ianlancetaylor will open a mailing list thread then, ready to get my hands on this.

@gdamore
Copy link

@gdamore gdamore commented May 30, 2016

Yay! Glad to see progress here!

On Mon, May 30, 2016 at 12:08 PM, Artyom Pervukhin <notifications@github.com

wrote:

@ianlancetaylor https://github.com/ianlancetaylor will open a mailing
list thread then, ready to get my hands on this.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#5082 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABPDfc8ZPx0J5BpVVAeGq9hyAntN9idGks5qGzWjgaJpZM4GSWx9
.

@gopherbot
Copy link

@gopherbot gopherbot commented May 31, 2016

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

@golang golang locked and limited conversation to collaborators Oct 12, 2017
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
7 participants
You can’t perform that action at this time.