Bypass using bogus Upgrade headers

[Enrico204]

I see that the middleware is ignoring websocket requests here: https://github.com/acouvreur/traefik-modsecurity-plugin/blob/19cdb477b8cee1966ad95278d168ae90a93df663/modsecurity.go#L56

Why is that? A request for WebSocket is actually a valid HTTP request, which may contains spurious/malicious data.

Also, there is another problem with this technique: an attacker can send a Upgrade header while issuing a normal request to any web page, as the server can ignore the Upgrade header for any reason (e.g. no websocket is expected?) and continue to process the request as if the header does not exists. Which means that an attacker can add the Upgrade header to any request to bypass the security check.

I didn't test the code with WebSockets, so I don't know if there is any issue (probably something with the body?). In that case I propose two possible solutions:

1. have a configuration parameter where WebSocket endpoints can be specified; or
2. when a websocket is detected, the body buffering is skipped, and the request to the mod-security backend will be made without body

The 2nd solution is less secure, as we're still skipping the body in presence of an header, but we don't know if there is any WebSocket or not. So I prefer the 1st.

I may contribute in few weeks if you like, I'm quite busy at the moment :-)

[acouvreur]

Because of the middleware architecture, and how it communicates with the modsecurity backend, I'm not sure that this would work.

But your first option is definitely something to do.

As you send the request to the modsec backend, it tries to upgrade to Websocket, but in this case, as it doesn't really act as a proxy but more of a sandbox place before forwarding the request, I don't know how to do that.

Please contribute, I'd like to of course! :)

[Enrico204]

I think that we can come up with something like a config parameter where the user can specify the path(s) of the websocket endpoint(s). For multiple values we may use a separator, like the | symbol which is not allowed in URIs (so it should not create any problem).

Something like:

- traefik.http.middlewares.waf.plugin.traefik-modsecurity-plugin.webSocketPaths=/websocket|/other/websocket

Internally, we can create a list and match the inbound request for both Upgrade header and the path configured. If there is a match, we can skip buffering the body (however we can still read the whole request, create a duplicate for the WAF excluding the Upgrade header and see what the WAF thinks about it). And, of course, in the README we will explain this behavior (aka: the content of the websocket stream is excluded).

In this way the only part that is "unprotected" is content of the websocket stream. However, due to the architecture of Traefik middlewares, I think that we can't do better than that.

What do you think?