Features like CSP and COOP require a report collector to be installed on the server in order to collect reports.

Implement a handler that properly parses and collects reports, sets a 204 status and passes the parsed data to a user-provided implementation of a collector handler interface.

The plugin should implement the following features:

1. Have a configurable setup

• Enforcement or Report-Only
• Enable strict-dynamic (defaults to disabled)
• Enable unsafe-eval (defaults to disabled)
• Set a reporting endpoint or group (defaults to nothing for the time being, it will be a handler that the plugin installs later)
• Set base-src (defaults to none)
• Add a set of hashes (defaults to empty)

2. Set a Content-Security-Policy header
   The policy should be a strict CSP and nonces should be generated in the Before phase. The plugin will be responsible of putting the nonce in the request context for later retrieval.

Currently, we consider valid booleans to be only the strings "true" and "false", unlike strconv.ParseBool() which correctly converts "1", "t", "T", "true", "TRUE", "True", "0", "f", "F", "false", "FALSE", "False" into bool. We should discuss the implications of this and decide whether we want to use strconv.ParseBool() in the safehttp.Form implementation.

The plugin should validate incoming requests against a FM policy and potentially decide to reject them.

Two policies should be supported: Resource Isolation and Navigation Isolation. It should be possible to selectively disable this plugin on CORS endpoints.

Many PRs have been merged with missing copyright information at the top of source files. We should have a linter in the CI that checks this.

Provide a package similar to net/http/httptest with helper functions that create data structures we'll use when testing the plugins.

Cross-Origin-Opener-Policy is a protection against cross-window side channels. It can assume 3 possible values and has 2 possible keys.

Keys: Cross-Origin-Opener-Policy, Cross-Origin-Opener-Policy-Report-Only
Values: same-origin, same-origin-allow-popups, none

We need a plugin that supports this functionality and installs a default policy of Cross-Origin-Opener-Policy: same-origin-allow-popups.

This is low priority since it is a stretch goal.

The plugin should set some important security headers by default. The initial list should be:

X-Content-Type-Options: nosniff
X-XSS-Protection: 0

The CSP and XSRF plugins currently put tokens in the request context.

• There needs to be a part of the plugins that can, later on, provide that nonce for a template to render.
• This should ideally work easily with different template implementations
• This will likely happen in the Commit phase of both plugins

On CORS enabled endpoints this plugin should handle the incoming headers:

• Origin
• Access-Control-Request-Method
• Access-Control-Request-Headers

And set the response headers accordingly:

• Access-Control-Allow-Origin
• Access-Control-Expose-Headers
• Access-Control-Max-Age
• Access-Control-Allow-Credentials
• Access-Control-Allow-Methods
• Access-Control-Allow-Headers

See https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS for more info

safehttp/form_test.go tests both safehttp/form.go and the functions PostForm and MultipartForm in safehttp/incoming_request.go. I think this should be split into two test files form_test.go and incoming_request_test.go testing the functionality implemented in the files separately. Integration tests could also be added to test the combined functionality of both files.

Framing protection is done in two ways:

X-Frame-Options header, which should by default set to "sameorigin"
A frame-ancestors CSP directive set to "self". This part should somehow interact with the CSP plugin to set the additional directive.

Configuration should only be possible via some gated API that relaxes the framing protection on some endpoints. Note that on those endpoints XFO will be set to "allow" and CSP frame-ancestors to a specified list of allowed origins.

A POST, PATCH or PUT request with the header Content-Type:multipart/form-data allows users to send files as part of the request. This is currently not implemented in the safehttp.MultipartForm type. We need to investigate the behaviour of the current API, provided by net/http, and provide accessor methods for form files in the safehttp API

Using #4, add test scenarios showing how an http.Handler sees the incoming HTTP request headers.

Take into account:

• tricky encoding
• ordering of headers
• case sensitivity
• how different headers interact with each other according to the HTTP spec
• and many others.

Flag potentially problematic behavior.

Some reading material:

• The Tangled Web by Michał Zalewski (2011)
• https://portswigger.net/web-security/request-smuggling

This could be a bit more readable:

Currently, calling ResponseRecorder.Status function returns the status code as an int that we then have to cast to safehttp.StatusCode in every test. We should make it return an safehttp.StatusCode by default and modify the tests.

Using #4, add test scenarios showing how an http.Handler sees the incoming HTTP form values and query parameters.

Take into account:

• tricky encoding
• ordering
• case sensitivity
• how deterministic the results are
• and many others.

Flag potentially problematic behavior.

Some reading material:

• The Tangled Web by Michał Zalewski (2011)
• https://portswigger.net/web-security/request-smuggling

Currently the safehttp.Dispatcher can write a response without providing the Content-Type header. This could potentially lead to security issues. We therefore want to ensure that a Content-Type header is always written on each response.

This could be accomplished by creating a wrapper around the http.ResponseWriter such that the dispatcher needs to first specify a Content-Type before getting access to the responsewriter.

A plugin to provide HSTS with safe default settings.

To document the strange behavior we (@mattiasgrenfeldt and @mihalimara22) have found while testing the current behavior of net/http in #5 and #6 we should do the following: For each behavior we have observed that is strange or undesirable we should:

• Create a separate test function that tests this behavior.
• Create one subtest in the function that documents the current behavior.
• Create one subtest in the function that documents the desired behavior. Use t.Skip() so that the tests pass CI.
• Document the behavior using comments and explain why it is strange or undesirable.

(Issue created on @kele 's request.)

It is just a burden and adds complication and stuff to implement, not all servers will use templates and if we need to create a sugar to allow templates we can always create structs that wrap a template and its data in a safe response.

If we don't remove this function we would need to add a ExecuteTemplateName function to call ExecuteTemplate underneath instead of Execute.

There should be documentation for the HSTS plugin in /docs/. There should also be some document listing all plugins that are provided by the framework.

The plugin should provide infrastructure for uniform error handling (e.g. to prevent accidental leaks or XSS from error responses).

Error responses should not surface internal details or code errors to the end users.

When we have multiple interceptors installed on the ServeMux and one of them panics, what happens? Should After/Commit stages be run? Which ones? In which order? And so on.

This needs to be decided on and tested properly.

(Woo! Issue/PR 100! 💯 )

This needs to be well documented and requires to change all plugins to use it.

We should also think of a mechanism to prevent future code to use safehttp.Result{} instead.

One important security feature is providing safe defaults for the Server type. For example there is a quite outdated post that explains some things that should be added to a Server type before exposing it to the internet.

We should provide a wrapper type that constructs servers with safe defaults.

@FiloSottile can probably help identify the stuff that needs updating from that original post.

Plugins should be configurable per handler. This is especially important for plugins which use interceptors.

Examples:

1. mux.Handler(..., csp.Disable()) -- CSP plugin is installed on the mux, but disabled for a given handler.
2. mux.Handler(..., cors.Enable()) -- CORS plugin is installed on the mux, but by default it is not applied to any handlers, therefore we need to enable it.
3. mux.Handler(..., auth.AllowAnonymous()) -- to change how access control is done.

This need stems from the design decision of not having per-handler plugins. The problems of having per-handler plugins (as opposed to mux-level plugins and per-plugin configurations):

• what should be the order of interceptors execution?
• overriding of plugins installed on handler level by the ones on mux level - how to remove a plugin?

Overall, having configurations seems like an easier option.

Plugins might need to mark a header as immutable and then change it at a later stage. A method called Claim will therefore be implemented that sets the header as immutable and returns something that can later change the header. When Claim is implemented there will be no need for MarkImmutable (for now) and therefore it will be removed.

Currently, to set cookies on a response you have to use the safehttp.Header.SetCookie method which takes a http.Cookie. All other ways to modify header values block the modification of the Set-Cookie header.

I suggest that we remove the safehttp.Header.SetCookie method, implement a safehttp.Cookie type with safe defaults and add a SetCookie method to safehttp.ResponseWriter.

One of the plugins that are part of the go-safeweb will need to provide XSRF protection. This will also include a Fetch Metadata policy in case the browser used has Fetch Metadata support.

This is one of the most critical security features.

Double-writing should likely result in panic.

Now that we have a mux, these are no longer needed.

As of Go 1.15 #59 has been become a problem again. The solution we found in #63 doesn't work. But at least we know what is causing the slowdown.net/http.Server.Shutdowns polling interval for checking if all connections are finished is 0.5 s and we don't have time to shut everything down before the first sleep starts. Therefore we get punished by 0.5 s every time although closing our connections takes way less than 0.5 s.

The current implementation of safehttp.Form type also supports parsing query parameters as a Form object and returning any parsing errors that occurred. This follows the implementation of net/http and prevents users from calling URL.Query(), which silently discards parsing errors. We might want to consider separating the URL from the form type as future work.

Currently, the safehttp.IncomingRequest API doesn't provide methods for retrieving and modifying the context.Context of the underlying http.Request. Plugins need this functionality to add XSRF tokens and CSP nonces.

Problem

Some plugins need to cooperate. For instance: framing plugin needs to cooperate with a csp plugin, cors would usually disable xsrf.

Proposed solution

To avoid the extra complexity of facilitating plugins (their interceptors) cooperation during execution time, we want to introduce orchestrators (name TBD).

At mux startup time, orchestrators would add other plugins to the respective handlers. Example:

mux.Handler(..., framing.Orchestrate(framing.UseCSP, framing.UseXFO))

The framing package would depend on the csp and the xfo packages in order to install and/or amend their configuration to support safe iframing.

HTTP request parsing involves handling:

• headers
• form values
• query parameters

Incorrect or non-deterministic handling of these might lead to security issues such as request smuggling.

The Tangled Web by Michał Zalewski (lcamtuf) (2011), specifically chapters I.2 It Starts with a URL and I.3 Hypertext Transfer Protocol, is a good resource for learning how complex and full of edge cases this task can be.

We need to create a testing harness where, as unit tests, we can send HTTP requests (built as arbitrary byte slices to cover attacks leveraging tricky encoding) and assert on what the http.Handler sees inside http.Request.

The assertions must not assume the client is written in Go.

This issue considers only building the needed infrastructure, not the test scenarios themselves. These are to be tracked in separate issues.

In Go, accessing the form values associated to one key can be done using multiple functions, some with different return values. These can cause inconsistencies in the handlers' implementation of a server. We want to design and implement a way to mitigate this problem.

safehttp.ResponseWriter.ServerError currently has the following signature:

func (w *ResponseWriter) ServerError(code StatusCode, resp Response) Result

It takes a safehttp.Response as an argument, but there might be places where we want to respond with a server error without having the ability to create safe responses. For example if a plugin needs to interrupt the handling of an incoming request. We therefore need to reconsider the signature of ServerError so that it doesn't take a Response or so that it is possible to optionally call without a Response.

Create the ability of adding conformance checks that run after the mux has been set up. These would be able to:

• list all the plugins registered for each handler, together with the method, path and all other information
• ask plugins for their configuration

Conformance checks would need to cooperate with plugins, i.e. there is going to be no complex protocol for information exchange between a plugin and the conformance check. A simple func (p Plugin) Config() interface{} would be enough, as the conformance check needs to have full trust in the plugins anyway.

While a safehttp.ResponseWriter.ServerError method is currently being implemented, there is no support for for returning client errors when a request fails.

ServerError is supposed to handle 5xx errors, this is about 4xx.

CC: @empijei

Running the tests in tests/headers, tests/formparams and tests/queryparams currently takes 30+ seconds. This is very slow. They all use the testing harness in internal/requesttesting/test_harness.go. @empijei suspects that this might be because something is not closed correctly which causes the test harness to have to wait for a timeout to finish on each test.

This needs to be investigated further. The cause of the slowdown needs to identified and fixed.

The CSP plugin (#74 ) should contain a policy that can enable Trusted Types.

I will add this after #93 has been merged.

We need to be able to install plugins on the ServeMux and they should then run when an request reaches the ServeMux.

Imagine that we have two plugins installed. The first one claims and sets a header in the Before phase. The other plugins Before phase runs and an error occurs, so the plugin writes an error response (4xx, 5xx). Now, what happens to the header that the first plugin set? Depending on the plugin this header should probably not be present on error responses. So how do we remove it?

One option would be that all error responses, 4xx and 5xx, automatically remove all headers and set their own ones. But this becomes a problem since we might want CSP on error responses. CSP is handled by a plugin.

Another solution is that all plugins are responsible for removing their headers in the commit phase if it sees that the response is an error.

This needs to be discussed further.

In tests/integration/safehtml/safehtml_test.go and in tests/integration/header/header_test.go (not merged yet as of writing this, see #21 ) the ResponseRecorder from net/http/httptest is used. After calling ResponseRecorder.Result() the response is parsed and a http.Response object is created. This should be removed since it assumes that the client in our tests is written in Go. Instead simpler comparisons should be used so that we don't rely on the implementation of ResponseRecorder.

Now that there is interceptor/plugin infrastructure in the ServeMux there should be integration tests making sure that the HSTS plugin works with the ServeMux.