Skip to content

Commit

Permalink
admin: Disallow websockets
Browse files Browse the repository at this point in the history
No currently-known exploit here, just being conservative
  • Loading branch information
mholt committed May 21, 2020
1 parent 452d472 commit 1dc4ec2
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,14 @@ func (h adminHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// be called more than once per request, for example if a request
// is rewritten (i.e. internal redirect).
func (h adminHandler) serveHTTP(w http.ResponseWriter, r *http.Request) {
if strings.Contains(r.Header.Get("Upgrade"), "websocket") {
// I've never been able demonstrate a vulnerability myself, but apparently
// WebSocket connections originating from browsers aren't subject to CORS
// restrictions, so we'll just be on the safe side
h.handleError(w, r, fmt.Errorf("websocket connections aren't allowed"))
return
}

if h.enforceHost {
// DNS rebinding mitigation
err := h.checkHost(r)
Expand Down

2 comments on commit 1dc4ec2

@luisalvarado
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Matt, will there be an option to bypass this from the caddy config file. For example, the site owner aware of this issue can still enable websockets because of the needed functionality?

@francislavoie
Copy link
Member

@francislavoie francislavoie commented on 1dc4ec2 May 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The admin API doesn't use websockets, so I don't see why that would be necessary. This is only for the admin API.

Please sign in to comment.