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

proposal: x/net/http2: support padding at a higher, user-accessible layer #26900

Closed
sergeyfrolov opened this issue Aug 9, 2018 · 8 comments
Closed

Comments

@sergeyfrolov
Copy link
Contributor

@sergeyfrolov sergeyfrolov commented Aug 9, 2018

Abstract

This proposal describes a change to allow http/2 padding to be configured with Golang http/2 client and servers. The change is minimalistic, only changes the http2 package, and provides flexibility of the padding scheme.

Background

HTTP/2 spec says that “Padding can be used to obscure the exact size of frame content and is provided to mitigate specific attacks within HTTP, for example, attacks where compressed content includes both attacker-controlled plaintext and secret data (e.g., [BREACH]).” Padding may help against traffic analysis attack that detect which webpage on a certain website was visited. In addition to those use-cases, http/2 padding would be valuable when proxying: proxies may provide privacy protections against network adversaries that eavesdrop on connections between client and intermediary, but size of proxied packets may give out actual website/page/etc visited. Proxying of protocols with fixed-size payload, such as Tor cells, makes the proxied protocol immediately obvious on the wire, since the majority of packets will be of size TOR_CELL_SIZE*N + CONST_OVERHEAD.

However, specification warns that “Incorrectly implemented padding schemes can be easily defeated.”, and that even correctly implemented padding schemes may not provide as much protection, as it may seem. Spec also points out that “Correct application can depend on having specific knowledge of the data that is being padded”, underlying the importance of leaving specific application developers in charge of the padding scheme used.

Currently, http2 package exposes only WriteDataPadded method of low-level struct Framer, which in practice is not accessible from the http client or server structs. In order to use it, the developer would have to reimplement server and/or client, which is an untrivial and error-prone venture. Without changes to the standard library, Golang developers will not be able to use HTTP/2 padding and reap its benefits. For that reason we would like to add HTTP/2 padding support to the standard Golang http2 Server and ClientConn.

Proposal

All the proposed changes go into http2, which isn’t a part of stdlib, and, apparently, has a lower bar for knobs.
Padding scheme both on the client and server side will be set on the per-connection basis. Proposed func paddingScheme(int) uint8, that will be set by the application developer, takes the size of the data in the frame, and returns how much padding should be used. uint8 effectively enforces http2 padding limit of 255 bytes.

Client

http2.ClientConn struct will be expanded with paddingScheme func(int) uint8

Then the user would be able to set a preferred padding scheme using a new function:
func (cc *ClientConn) SetPaddingScheme(s func(int) uint8)

And in func (cs *clientStream) writeRequestBody(body io.Reader, bodyCloser io.Closer) we swap

err = cc.fr.WriteData(cs.ID, sentEnd, data)

(which just does WriteDataPadded(cs.ID, sentEnd, data, nil))

for

var padding []byte
if cc.paddingScheme != nil {
    padding = make([]byte, cc.paddingScheme(len(data)))
}
err = cc.fr.WriteDataPadded(cs.ID, sentEnd, data, padding)

Server

In order to enable http2 padding for http.Server, which doesn’t seem to expose desired knobs, we will add a SetPaddingScheme function to the http2.responseWriter. Then, the application developer should be able to wrap the http.HandleFunc with a check for the existence of SetPaddingScheme via interface cast, and set the scheme, when it’s an http2.responseWriter:

responseWriterState struct will be expanded with
paddingScheme func(int) uint8
and the http2.responseWriter, which holds responseWriterState will have a corresponding setter function:
func (w *responseWriter) SetPaddingScheme(scheme func(int) uint8)
Thus, func (rws *responseWriterState) writeChunk(p []byte) (n int, err error) will be able to access its own paddingScheme

writeData struct will be expanded with byte []padding
writeData is created in the writeDataFromHandler function, which doesn’t have access to paddingScheme, so we will create padding buffer outside of this function, and change its signature from

func (sc *serverConn) writeDataFromHandler(stream *stream, data []byte, endStream bool) error

to

func (sc *serverConn) writeDataFromHandler(stream *stream, data []byte,  padding []byte, endStream bool) error

writeDataFromHandler is (only) called from func (rws *responseWriterState) writeChunk(p []byte) (n int, err error), which does have access to the paddingScheme so we can create the padding, and pass it into writeDataFromHandler.

This should be enough to add padding extension to Caddy Web Server, for instance.

Rationale

Padding scheme is configured on per-connection basis in order to provide better flexibility, and avoid adding extra knobs to high-level structs, such as http.Server. This approach does require additional code, but setting of the padding Scheme may be wrapped with the HandlerFunc, for all connections, if desired. This approach also allows developer to create a variable to count the frames, and close on it, allowing paddingScheme to depend on the data frame counter on per-connection basis, if desired.

PaddingScheme takes int for the size of the frame data, and returns uint8 to enforce the 255 byte padding limit. We can return int, but it is difficult to report error from ServeHTTP function, if padding size requested isn’t between 0 and 255. Alternatively, if requested padding size exceeds 255, we can generate new frames with padding and no data.

Currently, padding is dynamically allocated on each frame write, but only when paddingScheme is set. There are ways to optimize this for codepaths with padding, but they come with trade-offs. Global static array const zeroPadding [255]byte will avoid allocations, but will affect non-padding codepath. We can also have a global empty slice and sync.Once to allocate it once, but that also adds extra code and some extra overhead to non-padding codepath.

Compatibility

The change does not violate expectations of compatibility guidelines. It only adds new exported methods and unexported fields to exported structs.
Performance of the existing programs, that do not use padding, should not be affected substantially: there’s an additional check without lock acquisition for whether paddingScheme is set on every frame sent, but correct branch should be predicted consistently.

Implementation

Change itself is already implemented. Corresponding tests haven’t been written yet, awaiting the acceptance of design, but should be done in following weeks and is expected to be finished before November code freeze.

@gopherbot gopherbot added this to the Proposal milestone Aug 9, 2018
@gopherbot gopherbot added the Proposal label Aug 9, 2018
@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Aug 9, 2018

I think the Client & Server use cases & APIs are different enough that they probably warrant separate bugs.

But while it's all here for now, is func paddingScheme(int) uint8 the right API? It seems one might want to choose what padding is used in an http.Handler per each ResponseWriter.Write call. But then it's intermingled with the Flush mechanics.

But you could imagine keeping our existing Write API but saying that we look at the bytes between the Write argument's length and capacity and treat them as the padding length if those bytes are filled with some sentinel value like PADPADPADPAD. (then write them out in the padded frames as zeros or random data or whatever)

@bradfitz bradfitz changed the title Proposal: HTTP/2 Padding in Golang proposal: x/net/http2: support padding at a higher, user-accessible layer Aug 9, 2018
@bemasc

This comment has been minimized.

Copy link
Contributor

@bemasc bemasc commented Aug 9, 2018

It seems one might want to choose what padding is used in an http.Handler per each ResponseWriter.Write call. But then it's intermingled with the Flush mechanics.

It sounds like you're considering a finer level of granularity here, with padding per-Write instead of per-ResponseWriter. Yes, we could achieve that by adding a WriteDataPadded function on http2.responseWriter. WriteDataPadded would just need to call Flush, and then write the additional frame directly into the serverConn.

But you could imagine keeping our existing Write API but saying that we look at the bytes between the Write argument's length and capacity and treat them as the padding length if those bytes are filled with some sentinel value like PADPADPADPAD.

This seems like a more fragile approach than the proposal, but I'm OK with any approach that you think we can land. However, I think it also has the same bad interaction with Flusher.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Aug 10, 2018

But you could imagine keeping our existing Write API but saying that we look at the bytes between the Write argument's length and capacity and treat them as the padding length if those bytes are filled with some sentinel value like PADPADPADPAD.

That would be very complicated to implement robustly: io.Writer does not document any reads beyond the length of the slice, so callers are allowed to (for example) send one portion of a slice to a Write call while concurrently writing to the bytes just beyond its length. Naïvely snooping those bytes could introduce a data race.

Even worse, I don't think we require that the bytes beyond the length of a slice be valid in general; in some cases (e.g. #13656), we have explicitly recommended that people construct subslices with artificially large capacities in order to alias memory from outside the Go runtime. Because we do not enforce validity beyond the length of the slice anywhere else in the runtime, we have no guarantee that the memory beyond the length of the slice is even mapped: attempting to read it may cause an unhandled fault.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Aug 13, 2018

What do other HTTP/2 libraries or servers do? How can we evaluate whether the proposed API supports other known use cases? It doesn't seem like Go has to lead here, and if others are leading then we should make sure we are at least consciously deviating.

@bemasc

This comment has been minimized.

Copy link
Contributor

@bemasc bemasc commented Aug 13, 2018

Good point. The proposal here is similar to node.js's PADDING_STRATEGY_CALLBACK option.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Aug 20, 2018

It's unclear to me how users will actually use this. The middleware layers are not going to preserve this option. Are there best practices for how to set the padding amount? If so, why not just do it in the HTTP/2 library by default? Otherwise, what are examples of applications that would control padding, and how? And is it OK for them to lose their padding if there are any middleware layers providing their own ResponseWriters?

@sergeyfrolov

This comment has been minimized.

Copy link
Contributor Author

@sergeyfrolov sergeyfrolov commented Aug 22, 2018

The middleware layers are not going to preserve this option.

That depends on the webserver, but true, some of them might have to do some changes in order to make padding work. Apparently, most popular Golang web server Caddy already supports this without any changes. Caddy middleware exposes following function to the plugins:

func (h MyHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error)

The underlying type of the w here is *httpserver.ResponseRecorder, which embeds *httpserver.ResponseWriterWrapper, which in turn embeds standard http.ResponseWrapper.

I added following code to my Caddy plugin in order to determine the precise type of deeply embedded w http.ResponseWriter:

      switch w1 := w.(type) {
      case *httpserver.ResponseRecorder:
             fmt.Printf("deeply embedded type of ResponseWriter: %v\n",
                    reflect.TypeOf(w1.ResponseWriterWrapper.ResponseWriter))
      default:
             fmt.Printf("unexpected type of ResponseWriter: %v\n", w1)
      }

the result is:
deeply embedded type of ResponseWriter: *http.http2responseWriter

which is a type auto-generated from http2.ResponseWriter and bundled with http package. Therefore, developer of padding plugin could simply cast w to interface with SetPaddingScheme()

However, even if this wasn’t already the case, I suspect @mholt -- author of Caddy -- would be happy to accept a reasonable PR to make it work.

Are there best practices for how to set the padding amount?

There is no silver bullet, as the http2 spec points out: “Correct application can depend on having specific knowledge of the data that is being padded”.
There are some more or less generally useful padding schemes, good one being “pad up to the multiple of 256”. However, such scheme gives away the fact that padding is used at all, and I would personally find it useful to use different padding schemes in my projects.

Otherwise, what are examples of applications that would control padding, and how?

Proxying of protocols with fixed-size payload, such as Tor cells, makes the proxied protocol immediately obvious on the wire, since the majority of packets will be of size TOR_CELL_SIZE*N + CONST_OVERHEAD. With padding up to the multiple of 256, situation isn’t much better, and it’s clear that padding is used, which is bad, since certain network actors might aspire to censor the Tor.

And is it OK for them to lose their padding if there are any middleware layers providing their own ResponseWriters?

Could be OK, could be not OK. It’s up to application developer to decide what to do if cast to interface with SetPaddingScheme() fails: could shut down with error. Either way, this won't fail silently due to explicit cast.
That’s a good point, since this cast might fail due to underlying responseWriter being http1, or that web server stopped embedding http2 responseWriter in a desired way. Regression test to ensure correct embedding would be useful for specific applications.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 10, 2018

In general we try not to have knobs for every possible customization (think garbage collection settings in Go vs other languages). If there is a Right Thing to do with respect to padding, then let's do that. But it sounds like the jury is out on that, in which case it seems like we should just wait until the security and HTTP/2 community figures that out.

Brad suggests trying to figure out instead a general way to turn an http.ResponseWriter into an http2.Framer, which might have broader applications. But it's still unclear how that would interact with middleware. Perhaps a new proposal to figure out a way to do that would be a way forward.

But a padding-specific change seems not worthwhile right now.

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