FAQ
I'd like to resolve "read size limit" part of this issue:
https://github.com/golang/go/issues/5082 and would appreciate some advice
on proper API exposure of extra parameter before submitting a CL.

The root cause of the issue is ioutil.ReadAll usage on a network
connection:
https://github.com/golang/net/blob/30db96677b74e24b967e23f911eb3364fc61a011/websocket/websocket.go#L326
— this can be abused by client to take down server by exhausting its memory.

Most consumers of this package are likely using websocket.Codec struct
instances implementing custom message (un)marshalling; it is this struct's
Receive method that has this unbound ReadAll call.

My idea is to introduce an extra integer field to Codec struct, that would
specify max. size of payload that is allowed to be passed to Unmarshal
function (which receives []byte). Codec's Receive method (the one that
calls Unmarshal) would then use something similar to io.LimitedReader over
the message payload if Codec has non-zero limit set; if this limit is hit,
Receive would return with reasonable error so websocket handler won't
allocate unbound amount of memory.

Package users expecting malicious input would then have to set this
threshold to some safe value. It would probably make sense to set this
field to some safe non-zero value for default predefined Codecs:
websocket.Message and websocket.JSON.

Before proceeding with CL, I would like to have some feedback on this
approach. Does this API change look reasonable? Is introducing a non-zero
payload size limit to predefined Codecs (Message and JSON) a good idea for
this particular case, provided that it would protect server from possible
denial of service, although having a chance of breaking existing programs
relying on current behavior (unlimited payload size)?

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Search Discussions

  • Ian Lance Taylor at May 31, 2016 at 5:38 am

    On Mon, May 30, 2016 at 1:50 PM, wrote:
    I'd like to resolve "read size limit" part of this issue:
    https://github.com/golang/go/issues/5082 and would appreciate some advice on
    proper API exposure of extra parameter before submitting a CL.

    The root cause of the issue is ioutil.ReadAll usage on a network connection:
    https://github.com/golang/net/blob/30db96677b74e24b967e23f911eb3364fc61a011/websocket/websocket.go#L326
    — this can be abused by client to take down server by exhausting its memory.

    Most consumers of this package are likely using websocket.Codec struct
    instances implementing custom message (un)marshalling; it is this struct's
    Receive method that has this unbound ReadAll call.

    My idea is to introduce an extra integer field to Codec struct, that would
    specify max. size of payload that is allowed to be passed to Unmarshal
    function (which receives []byte). Codec's Receive method (the one that calls
    Unmarshal) would then use something similar to io.LimitedReader over the
    message payload if Codec has non-zero limit set; if this limit is hit,
    Receive would return with reasonable error so websocket handler won't
    allocate unbound amount of memory.

    Package users expecting malicious input would then have to set this
    threshold to some safe value. It would probably make sense to set this field
    to some safe non-zero value for default predefined Codecs: websocket.Message
    and websocket.JSON.

    Before proceeding with CL, I would like to have some feedback on this
    approach. Does this API change look reasonable? Is introducing a non-zero
    payload size limit to predefined Codecs (Message and JSON) a good idea for
    this particular case, provided that it would protect server from possible
    denial of service, although having a chance of breaking existing programs
    relying on current behavior (unlimited payload size)?
    Does net/http have a parameter somewhere that limits the size of a
    request? Or does it just have a fixed limit?

    It's too bad websocket.Codec was not written in terms of streams.
    Though I suppose that would have other complexities.

    Another possible API would be (Codec) LimitedReceive(*Conn, uint64,
    interface{}) and have Receive simply call LimitedReceive with some
    plausible value that would usually work. Most connections would
    presumably work just fine, ones that required large input messages
    could call LimitedReceive.

    Ian

    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.
  • Artyom Pervukhin at May 31, 2016 at 8:32 am

    On May 31, 2016, at 08:38, Ian Lance Taylor wrote:
    On Mon, May 30, 2016 at 1:50 PM, wrote:
    I'd like to resolve "read size limit" part of this issue:
    https://github.com/golang/go/issues/5082 and would appreciate some advice on
    proper API exposure of extra parameter before submitting a CL.

    The root cause of the issue is ioutil.ReadAll usage on a network connection:
    https://github.com/golang/net/blob/30db96677b74e24b967e23f911eb3364fc61a011/websocket/websocket.go#L326
    — this can be abused by client to take down server by exhausting its memory.

    Most consumers of this package are likely using websocket.Codec struct
    instances implementing custom message (un)marshalling; it is this struct's
    Receive method that has this unbound ReadAll call.

    My idea is to introduce an extra integer field to Codec struct, that would
    specify max. size of payload that is allowed to be passed to Unmarshal
    function (which receives []byte). Codec's Receive method (the one that calls
    Unmarshal) would then use something similar to io.LimitedReader over the
    message payload if Codec has non-zero limit set; if this limit is hit,
    Receive would return with reasonable error so websocket handler won't
    allocate unbound amount of memory.

    Package users expecting malicious input would then have to set this
    threshold to some safe value. It would probably make sense to set this field
    to some safe non-zero value for default predefined Codecs: websocket.Message
    and websocket.JSON.

    Before proceeding with CL, I would like to have some feedback on this
    approach. Does this API change look reasonable? Is introducing a non-zero
    payload size limit to predefined Codecs (Message and JSON) a good idea for
    this particular case, provided that it would protect server from possible
    denial of service, although having a chance of breaking existing programs
    relying on current behavior (unlimited payload size)?
    Does net/http have a parameter somewhere that limits the size of a
    request? Or does it just have a fixed limit?
    net/http’s Server has only configurable limit for headers in place (with sane default); as request bodies are available to handlers as io.Reader, it’s up to handler implementors to process reader as they see fit. websocket package also provides access to Conn as io.Reader interface to read payload of distinct frames, but Codec provides much more user-friendly interface while relying on non-exported Conn attributes, so re-implementing Codec by user of the package is not trivial, if possible at all.
    It's too bad websocket.Codec was not written in terms of streams.
    Though I suppose that would have other complexities.

    Another possible API would be (Codec) LimitedReceive(*Conn, uint64,
    interface{}) and have Receive simply call LimitedReceive with some
    plausible value that would usually work. Most connections would
    presumably work just fine, ones that required large input messages
    could call LimitedReceive.

    Ian
    Hmm, this approach really looks cleaner, will proceed with implementation then.

    Thank you for the feedback!

    --
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/d/optout.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedMay 31, '16 at 5:26a
activeMay 31, '16 at 8:32a
posts3
users2
websitegolang.org

People

Translate

site design / logo © 2021 Grokbase