From cd83f9f0ebeefafac6d4be15abda83a0a5e1d1f4 Mon Sep 17 00:00:00 2001 From: mutantmonkey Date: Sun, 11 Oct 2015 23:31:57 -0700 Subject: [PATCH 1/5] fix CSP referrer policy The policy of "referrer none" was incorrect and was nonfunctional. With this change, the CSP referrer policy is set to origin, which will causes only the origin to be sent for requests made from the main site. A fix was also needed for referrer checks in two places. --- csrf.go | 6 ++++-- fileserve.go | 3 ++- server.go | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/csrf.go b/csrf.go index 5f8ca48..61799db 100644 --- a/csrf.go +++ b/csrf.go @@ -12,11 +12,13 @@ func strictReferrerCheck(r *http.Request, prefix string, whitelistHeaders []stri } } - if referrer := r.Header.Get("Referer"); !strings.HasPrefix(referrer, prefix) { + p := strings.TrimSuffix(prefix, "/") + + if referrer := r.Header.Get("Referer"); !strings.HasPrefix(referrer, p) { return false } - if origin := r.Header.Get("Origin"); origin != "" && !strings.HasPrefix(origin, strings.TrimSuffix(prefix, "/")) { + if origin := r.Header.Get("Origin"); origin != "" && !strings.HasPrefix(origin, p) { return false } diff --git a/fileserve.go b/fileserve.go index d41523d..cc682ca 100644 --- a/fileserve.go +++ b/fileserve.go @@ -26,7 +26,8 @@ func fileServeHandler(c web.C, w http.ResponseWriter, r *http.Request) { if !Config.allowHotlink { referer := r.Header.Get("Referer") - if referer != "" && !strings.HasPrefix(referer, Config.siteURL) { + prefix := strings.TrimSuffix(Config.siteURL, "/") + if referer != "" && !strings.HasPrefix(referer, prefix) { w.WriteHeader(403) return } diff --git a/server.go b/server.go index 4cb88e4..6f1b63e 100644 --- a/server.go +++ b/server.go @@ -184,10 +184,10 @@ func main() { flag.StringVar(&Config.remoteAuthFile, "remoteauthfile", "", "path to a file containing newline-separated scrypted auth keys for remote uploads") flag.StringVar(&Config.contentSecurityPolicy, "contentsecuritypolicy", - "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; referrer none;", + "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; referrer origin;", "value of default Content-Security-Policy header") flag.StringVar(&Config.fileContentSecurityPolicy, "filecontentsecuritypolicy", - "default-src 'none'; img-src 'self'; object-src 'self'; media-src 'self'; sandbox; referrer none;", + "default-src 'none'; img-src 'self'; object-src 'self'; media-src 'self'; sandbox; referrer origin;", "value of Content-Security-Policy header for file access") flag.StringVar(&Config.xFrameOptions, "xframeoptions", "SAMEORIGIN", "value of X-Frame-Options header") From 61147554a92c3ddfcf68ec05c43802245c56d9a5 Mon Sep 17 00:00:00 2001 From: mutantmonkey Date: Mon, 12 Oct 2015 00:02:09 -0700 Subject: [PATCH 2/5] update CSP flags in readme --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d7e78a3..7f765b4 100644 --- a/README.md +++ b/README.md @@ -25,8 +25,8 @@ Command-line options - ```-maxsize 4294967296``` maximum upload file size in bytes (default 4GB) - ```-certfile path/to/your.crt``` -- Path to the ssl certificate (required if you want to use the https server) - ```-keyfile path/to/your.key``` -- Path to the ssl key (required if you want to use the https server) -- ```-contentsecuritypolicy "..."``` -- Content-Security-Policy header for pages (default is "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; referrer none;") -- ```-filecontentsecuritypolicy "..."``` -- Content-Security-Policy header for files (default is "default-src 'none'; img-src 'self'; object-src 'self'; media-src 'self'; sandbox; referrer none;"") +- ```-contentsecuritypolicy "..."``` -- Content-Security-Policy header for pages (default is "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; referrer origin;") +- ```-filecontentsecuritypolicy "..."``` -- Content-Security-Policy header for files (default is "default-src 'none'; img-src 'self'; object-src 'self'; media-src 'self'; sandbox; referrer origin;"") - ```-xframeoptions "..." ``` -- X-Frame-Options header (default is "SAMEORIGIN") - ```-remoteuploads``` -- (optionally) enable remote uploads (/upload?url=https://...) - ```-realip``` -- (optionally) let linx-server know you (nginx, etc) are providing the X-Real-IP and/or X-Forwarded-For headers. From a7ae455ac1949388c8dd1f9ee57f714121209162 Mon Sep 17 00:00:00 2001 From: mutantmonkey Date: Mon, 12 Oct 2015 00:28:01 -0700 Subject: [PATCH 3/5] strict referrer check improvements * Always check Origin if it is present, regardless of headers sent * Whitelist X-Requested-With header --- csrf.go | 11 +++++------ upload.go | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/csrf.go b/csrf.go index 61799db..fdf3d93 100644 --- a/csrf.go +++ b/csrf.go @@ -6,21 +6,20 @@ import ( ) func strictReferrerCheck(r *http.Request, prefix string, whitelistHeaders []string) bool { + p := strings.TrimSuffix(prefix, "/") + if origin := r.Header.Get("Origin"); origin != "" && !strings.HasPrefix(origin, p) { + return false + } + for _, header := range whitelistHeaders { if r.Header.Get(header) != "" { return true } } - p := strings.TrimSuffix(prefix, "/") - if referrer := r.Header.Get("Referer"); !strings.HasPrefix(referrer, p) { return false } - if origin := r.Header.Get("Origin"); origin != "" && !strings.HasPrefix(origin, p) { - return false - } - return true } diff --git a/upload.go b/upload.go index 63582d9..a4a9032 100644 --- a/upload.go +++ b/upload.go @@ -46,7 +46,7 @@ type Upload struct { } func uploadPostHandler(c web.C, w http.ResponseWriter, r *http.Request) { - if !strictReferrerCheck(r, Config.siteURL, []string{"Linx-Delete-Key", "Linx-Expiry", "Linx-Randomize"}) { + if !strictReferrerCheck(r, Config.siteURL, []string{"Linx-Delete-Key", "Linx-Expiry", "Linx-Randomize", "X-Requested-With"}) { badRequestHandler(c, w, r) return } @@ -145,7 +145,7 @@ func uploadRemote(c web.C, w http.ResponseWriter, r *http.Request) { } } else { // strict referrer checking is mandatory without remote auth keys - if !strictReferrerCheck(r, Config.siteURL, []string{"Linx-Delete-Key", "Linx-Expiry", "Linx-Randomize"}) { + if !strictReferrerCheck(r, Config.siteURL, []string{"Linx-Delete-Key", "Linx-Expiry", "Linx-Randomize", "X-Requested-With"}) { badRequestHandler(c, w, r) return } From 0a1aa869e4654092457f57b98fb5fdc582499f0a Mon Sep 17 00:00:00 2001 From: mutantmonkey Date: Mon, 12 Oct 2015 01:03:02 -0700 Subject: [PATCH 4/5] nicer 400 error page --- pages.go | 5 ++++- templates.go | 3 ++- templates/400.html | 5 +++++ 3 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 templates/400.html diff --git a/pages.go b/pages.go index 7481bc4..9d24bba 100644 --- a/pages.go +++ b/pages.go @@ -86,7 +86,10 @@ func oopsHandler(c web.C, w http.ResponseWriter, r *http.Request, rt RespType, m func badRequestHandler(c web.C, w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadRequest) - http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) + err := Templates["400.html"].ExecuteWriter(pongo2.Context{}, w) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } } func unauthorizedHandler(c web.C, w http.ResponseWriter, r *http.Request) { diff --git a/templates.go b/templates.go index 6575881..d8cd7bf 100644 --- a/templates.go +++ b/templates.go @@ -45,8 +45,9 @@ func populateTemplatesMap(tSet *pongo2.TemplateSet, tMap map[string]*pongo2.Temp "index.html", "paste.html", "API.html", - "404.html", + "400.html", "401.html", + "404.html", "oops.html", "display/audio.html", diff --git a/templates/400.html b/templates/400.html new file mode 100644 index 0000000..11f3e87 --- /dev/null +++ b/templates/400.html @@ -0,0 +1,5 @@ +{% extends "base.html" %} + +{% block content %} +400 Bad Request +{% endblock %} From a3723d3665480a784af8dd1c4b181641d2b4f257 Mon Sep 17 00:00:00 2001 From: mutantmonkey Date: Mon, 12 Oct 2015 01:23:06 -0700 Subject: [PATCH 5/5] short-circuit on origin header If the Origin header is present, we can check it and skip the other checks. --- csrf.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/csrf.go b/csrf.go index fdf3d93..b70215b 100644 --- a/csrf.go +++ b/csrf.go @@ -7,8 +7,9 @@ import ( func strictReferrerCheck(r *http.Request, prefix string, whitelistHeaders []string) bool { p := strings.TrimSuffix(prefix, "/") - if origin := r.Header.Get("Origin"); origin != "" && !strings.HasPrefix(origin, p) { - return false + if origin := r.Header.Get("Origin"); origin != "" { + // if there's an Origin header, check it and ignore the rest + return strings.HasPrefix(origin, p) } for _, header := range whitelistHeaders {