From 103fd72cc8254b45fd94b4517d9ebcf54875e123 Mon Sep 17 00:00:00 2001 From: Jordan Wright Date: Wed, 14 Sep 2016 22:24:51 -0500 Subject: [PATCH] Fixing context issues with Go 1.7. --- .travis.yml | 2 ++ auth/auth.go | 14 ++++-------- context/context-legacy.go | 26 +++++++++++++++++++++ context/context.go | 25 ++++++++++++++++++++ context/doc.go | 29 +++++++++++++++++++++++ controllers/api.go | 2 +- controllers/route.go | 48 ++++++++++++++++----------------------- controllers/route_test.go | 18 +++++++++++++++ middleware/middleware.go | 35 +++++++++++++++++++++------- 9 files changed, 152 insertions(+), 47 deletions(-) create mode 100644 context/context-legacy.go create mode 100644 context/context.go create mode 100644 context/doc.go create mode 100644 controllers/route_test.go diff --git a/.travis.yml b/.travis.yml index d9c9dc65..3c61f3dd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,8 @@ sudo: false go: - 1.5 + - 1.6 + - 1.7 - tip install: diff --git a/auth/auth.go b/auth/auth.go index bae2e044..540185f2 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -9,8 +9,8 @@ import ( "crypto/rand" + ctx "github.com/gophish/gophish/context" "github.com/gophish/gophish/models" - ctx "github.com/gorilla/context" "github.com/gorilla/securecookie" "github.com/gorilla/sessions" "golang.org/x/crypto/bcrypt" @@ -39,23 +39,19 @@ var ErrEmptyPassword = errors.New("Password cannot be blank") var ErrPasswordMismatch = errors.New("Passwords must match") // Login attempts to login the user given a request. -func Login(r *http.Request) (bool, error) { +func Login(r *http.Request) (bool, models.User, error) { username, password := r.FormValue("username"), r.FormValue("password") - session, _ := Store.Get(r, "gophish") u, err := models.GetUserByUsername(username) if err != nil && err != models.ErrUsernameTaken { - return false, err + return false, models.User{}, err } //If we've made it here, we should have a valid user stored in u //Let's check the password err = bcrypt.CompareHashAndPassword([]byte(u.Hash), []byte(password)) if err != nil { - ctx.Set(r, "user", nil) - return false, ErrInvalidPassword + return false, models.User{}, ErrInvalidPassword } - ctx.Set(r, "user", u) - session.Values["id"] = u.Id - return true, nil + return true, u, nil } // Register attempts to register the user given a request. diff --git a/context/context-legacy.go b/context/context-legacy.go new file mode 100644 index 00000000..625387bd --- /dev/null +++ b/context/context-legacy.go @@ -0,0 +1,26 @@ +// +build !go1.7 + +package context + +import ( + "net/http" + + "github.com/gorilla/context" +) + +func Get(r *http.Request, key interface{}) interface{} { + return context.Get(r, key) +} + +func Set(r *http.Request, key, val interface{}) *http.Request { + if val == nil { + return r + } + + context.Set(r, key, val) + return r +} + +func Clear(r *http.Request) { + context.Clear(r) +} diff --git a/context/context.go b/context/context.go new file mode 100644 index 00000000..705a0a81 --- /dev/null +++ b/context/context.go @@ -0,0 +1,25 @@ +// +build go1.7 + +package context + +import ( + "net/http" + + "context" +) + +func Get(r *http.Request, key interface{}) interface{} { + return r.Context().Value(key) +} + +func Set(r *http.Request, key, val interface{}) *http.Request { + if val == nil { + return r + } + + return r.WithContext(context.WithValue(r.Context(), key, val)) +} + +func Clear(r *http.Request) { + return +} diff --git a/context/doc.go b/context/doc.go new file mode 100644 index 00000000..ef2f2e2e --- /dev/null +++ b/context/doc.go @@ -0,0 +1,29 @@ +/* +gophish - Open-Source Phishing Framework + +The MIT License (MIT) + +Copyright (c) 2013 Jordan Wright + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +*/ + +// Package context provides the ability to store request-scoped values on an http.Request instance. These values are cleared after every request. +// This package was created due to Go v1.7 moving the context package into the stdlib, which had a net effect of breaking some functionality from the existing gorilla/context. +package context diff --git a/controllers/api.go b/controllers/api.go index 72681c7c..594bb92c 100644 --- a/controllers/api.go +++ b/controllers/api.go @@ -14,10 +14,10 @@ import ( "github.com/PuerkitoBio/goquery" "github.com/gophish/gophish/auth" + ctx "github.com/gophish/gophish/context" "github.com/gophish/gophish/models" "github.com/gophish/gophish/util" "github.com/gophish/gophish/worker" - ctx "github.com/gorilla/context" "github.com/gorilla/mux" "github.com/jinzhu/gorm" "github.com/jordan-wright/email" diff --git a/controllers/route.go b/controllers/route.go index db95d606..73360a26 100644 --- a/controllers/route.go +++ b/controllers/route.go @@ -14,12 +14,12 @@ import ( "github.com/gophish/gophish/auth" "github.com/gophish/gophish/config" + ctx "github.com/gophish/gophish/context" mid "github.com/gophish/gophish/middleware" "github.com/gophish/gophish/models" - ctx "github.com/gorilla/context" + "github.com/gorilla/csrf" "github.com/gorilla/mux" "github.com/gorilla/sessions" - "github.com/justinas/nosurf" ) // Logger is used to send logging messages to stdout. @@ -67,22 +67,11 @@ func CreateAdminRouter() http.Handler { router.PathPrefix("/").Handler(http.FileServer(http.Dir("./static/"))) // Setup CSRF Protection - csrfHandler := nosurf.New(router) - // Exempt API routes and Static files - csrfHandler.ExemptGlob("/api/campaigns") - csrfHandler.ExemptGlob("/api/campaigns/*") - csrfHandler.ExemptGlob("/api/groups") - csrfHandler.ExemptGlob("/api/groups/*") - csrfHandler.ExemptGlob("/api/templates") - csrfHandler.ExemptGlob("/api/templates/*") - csrfHandler.ExemptGlob("/api/pages") - csrfHandler.ExemptGlob("/api/pages/*") - csrfHandler.ExemptGlob("/api/smtp") - csrfHandler.ExemptGlob("/api/smtp/*") - csrfHandler.ExemptGlob("/api/import/*") - csrfHandler.ExemptGlob("/api/util/*") - csrfHandler.ExemptGlob("/static/*") - return Use(csrfHandler.ServeHTTP, mid.GetContext) + csrfHandler := csrf.Protect([]byte(auth.GenerateSecureKey()), + csrf.FieldName("csrf_token"), + csrf.Secure(config.Conf.AdminConf.UseTLS)) + csrfRouter := csrfHandler(router) + return Use(csrfRouter.ServeHTTP, mid.CSRFExceptions, mid.GetContext) } // CreatePhishingRouter creates the router that handles phishing connections. @@ -259,7 +248,7 @@ func Register(w http.ResponseWriter, r *http.Request) { Flashes []interface{} User models.User Token string - }{Title: "Register", Token: nosurf.Token(r)} + }{Title: "Register", Token: csrf.Token(r)} session := ctx.Get(r, "session").(*sessions.Session) switch { case r.Method == "GET": @@ -304,7 +293,7 @@ func Base(w http.ResponseWriter, r *http.Request) { Title string Flashes []interface{} Token string - }{Title: "Dashboard", User: ctx.Get(r, "user").(models.User), Token: nosurf.Token(r)} + }{Title: "Dashboard", User: ctx.Get(r, "user").(models.User), Token: csrf.Token(r)} getTemplate(w, "dashboard").ExecuteTemplate(w, "base", params) } @@ -316,7 +305,7 @@ func Campaigns(w http.ResponseWriter, r *http.Request) { Title string Flashes []interface{} Token string - }{Title: "Campaigns", User: ctx.Get(r, "user").(models.User), Token: nosurf.Token(r)} + }{Title: "Campaigns", User: ctx.Get(r, "user").(models.User), Token: csrf.Token(r)} getTemplate(w, "campaigns").ExecuteTemplate(w, "base", params) } @@ -328,7 +317,7 @@ func CampaignID(w http.ResponseWriter, r *http.Request) { Title string Flashes []interface{} Token string - }{Title: "Campaign Results", User: ctx.Get(r, "user").(models.User), Token: nosurf.Token(r)} + }{Title: "Campaign Results", User: ctx.Get(r, "user").(models.User), Token: csrf.Token(r)} getTemplate(w, "campaign_results").ExecuteTemplate(w, "base", params) } @@ -340,7 +329,7 @@ func Templates(w http.ResponseWriter, r *http.Request) { Title string Flashes []interface{} Token string - }{Title: "Email Templates", User: ctx.Get(r, "user").(models.User), Token: nosurf.Token(r)} + }{Title: "Email Templates", User: ctx.Get(r, "user").(models.User), Token: csrf.Token(r)} getTemplate(w, "templates").ExecuteTemplate(w, "base", params) } @@ -352,7 +341,7 @@ func Users(w http.ResponseWriter, r *http.Request) { Title string Flashes []interface{} Token string - }{Title: "Users & Groups", User: ctx.Get(r, "user").(models.User), Token: nosurf.Token(r)} + }{Title: "Users & Groups", User: ctx.Get(r, "user").(models.User), Token: csrf.Token(r)} getTemplate(w, "users").ExecuteTemplate(w, "base", params) } @@ -364,7 +353,7 @@ func LandingPages(w http.ResponseWriter, r *http.Request) { Title string Flashes []interface{} Token string - }{Title: "Landing Pages", User: ctx.Get(r, "user").(models.User), Token: nosurf.Token(r)} + }{Title: "Landing Pages", User: ctx.Get(r, "user").(models.User), Token: csrf.Token(r)} getTemplate(w, "landing_pages").ExecuteTemplate(w, "base", params) } @@ -376,7 +365,7 @@ func SendingProfiles(w http.ResponseWriter, r *http.Request) { Title string Flashes []interface{} Token string - }{Title: "Sending Profiles", User: ctx.Get(r, "user").(models.User), Token: nosurf.Token(r)} + }{Title: "Sending Profiles", User: ctx.Get(r, "user").(models.User), Token: csrf.Token(r)} getTemplate(w, "sending_profiles").ExecuteTemplate(w, "base", params) } @@ -390,7 +379,7 @@ func Settings(w http.ResponseWriter, r *http.Request) { Flashes []interface{} Token string Version string - }{Title: "Settings", Version: config.Version, User: ctx.Get(r, "user").(models.User), Token: nosurf.Token(r)} + }{Title: "Settings", Version: config.Version, User: ctx.Get(r, "user").(models.User), Token: csrf.Token(r)} getTemplate(w, "settings").ExecuteTemplate(w, "base", params) case r.Method == "POST": err := auth.ChangePassword(r) @@ -419,7 +408,7 @@ func Login(w http.ResponseWriter, r *http.Request) { Title string Flashes []interface{} Token string - }{Title: "Login", Token: nosurf.Token(r)} + }{Title: "Login", Token: csrf.Token(r)} session := ctx.Get(r, "session").(*sessions.Session) switch { case r.Method == "GET": @@ -433,12 +422,13 @@ func Login(w http.ResponseWriter, r *http.Request) { template.Must(templates, err).ExecuteTemplate(w, "base", params) case r.Method == "POST": //Attempt to login - succ, err := auth.Login(r) + succ, u, err := auth.Login(r) if err != nil { Logger.Println(err) } //If we've logged in, save the session and redirect to the dashboard if succ { + session.Values["id"] = u.Id session.Save(r, w) http.Redirect(w, r, "/", 302) } else { diff --git a/controllers/route_test.go b/controllers/route_test.go new file mode 100644 index 00000000..754b59df --- /dev/null +++ b/controllers/route_test.go @@ -0,0 +1,18 @@ +package controllers + +import ( + "fmt" + "net/http" + "net/url" +) + +func (s *ControllersSuite) TestLoginCSRF() { + resp, err := http.PostForm(fmt.Sprintf("%s/login", as.URL), + url.Values{ + "username": {"admin"}, + "password": {"gophish"}, + }) + + s.Equal(resp.StatusCode, 403) + fmt.Println(err) +} diff --git a/middleware/middleware.go b/middleware/middleware.go index f33c6f87..6b429817 100644 --- a/middleware/middleware.go +++ b/middleware/middleware.go @@ -4,12 +4,30 @@ import ( "encoding/json" "fmt" "net/http" + "strings" "github.com/gophish/gophish/auth" + ctx "github.com/gophish/gophish/context" "github.com/gophish/gophish/models" - ctx "github.com/gorilla/context" + "github.com/gorilla/csrf" ) +var CSRFExemptPrefixes = []string{ + "/api", +} + +func CSRFExceptions(handler http.Handler) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + for _, prefix := range CSRFExemptPrefixes { + if strings.HasPrefix(r.URL.Path, prefix) { + r = csrf.UnsafeSkipCheck(r) + break + } + } + handler.ServeHTTP(w, r) + } +} + // GetContext wraps each request in a function which fills in the context for a given request. // This includes setting the User and Session keys and values as necessary for use in later functions. func GetContext(handler http.Handler) http.HandlerFunc { @@ -23,17 +41,18 @@ func GetContext(handler http.Handler) http.HandlerFunc { // Set the context appropriately here. // Set the session session, _ := auth.Store.Get(r, "gophish") - // Put the session in the context so that - ctx.Set(r, "session", session) + // Put the session in the context so that we can + // reuse the values in different handlers + r = ctx.Set(r, "session", session) if id, ok := session.Values["id"]; ok { u, err := models.GetUser(id.(int64)) if err != nil { - ctx.Set(r, "user", nil) + r = ctx.Set(r, "user", nil) } else { - ctx.Set(r, "user", u) + r = ctx.Set(r, "user", u) } } else { - ctx.Set(r, "user", nil) + r = ctx.Set(r, "user", nil) } handler.ServeHTTP(w, r) // Remove context contents @@ -60,8 +79,8 @@ func RequireAPIKey(handler http.Handler) http.HandlerFunc { JSONError(w, 400, "Invalid API Key") return } - ctx.Set(r, "user_id", u.Id) - ctx.Set(r, "api_key", ak) + r = ctx.Set(r, "user_id", u.Id) + r = ctx.Set(r, "api_key", ak) handler.ServeHTTP(w, r) } }