auth
132:163ce22fa4c9 Browse Files
Enable CSRF protection, add expiration to sessions. Sessions gain a CSRF token, which is passed as a parameter to the login page. The login page now checks for that CSRF token, and logs a CSRF attempt if the token does not match. I also added an expiration to sessions, so they don't last forever. Sessions should be pretty short--we just need to stay logged in for long enough to approve the OAuth request. Everything after that should be cookie based. Finally, I added a configuration parameter to control whether the session cookie should be set to Secure, requiring the use of HTTPS. For production use, this flag is a requirement, but it makes testing extremely difficult, so we need a way to disable it.
authd/templates/simple.gotmpl config.go oauth2.go oauth2_test.go session.go session_test.go
1.1 --- a/authd/templates/simple.gotmpl Sat Jan 24 10:34:33 2015 -0500 1.2 +++ b/authd/templates/simple.gotmpl Wed Jan 28 07:27:32 2015 -0500 1.3 @@ -28,6 +28,7 @@ 1.4 <p>{{ .client.Name }} is requesting access to your account. if you grant it, you'll be redirected to {{ .redirectURL }}. Their access will be limited to {{ .scope }}. You are granting access for {{ .profile.Name }}.</p>{{ end }}{{ end }} 1.5 <form method="POST"> 1.6 <input type="submit" name="grant" value="approved"> 1.7 + <input type="hidden" name="csrftoken" value="{{ .csrftoken }}"> 1.8 </form> 1.9 </body> 1.10 </html>{{ end }}
2.1 --- a/config.go Sat Jan 24 10:34:33 2015 -0500 2.2 +++ b/config.go Wed Jan 28 07:27:32 2015 -0500 2.3 @@ -24,6 +24,7 @@ 2.4 Template *template.Template 2.5 LoginURI string 2.6 iterations int 2.7 + secureCookie bool 2.8 } 2.9 2.10 // Init is a function that preps the Config object to be used for Context creation, setting variables
3.1 --- a/oauth2.go Sat Jan 24 10:34:33 2015 -0500 3.2 +++ b/oauth2.go Wed Jan 28 07:27:32 2015 -0500 3.3 @@ -16,7 +16,6 @@ 3.4 ) 3.5 3.6 const ( 3.7 - authCookieName = "auth" 3.8 defaultAuthorizationCodeExpiration = 600 // default to ten minute grant expirations 3.9 getAuthorizationCodeTemplateName = "get_grant" 3.10 ) 3.11 @@ -277,7 +276,14 @@ 3.12 return 3.13 } 3.14 if r.Method == "POST" { 3.15 - // BUG(paddy): We need to implement CSRF protection when obtaining a grant code. 3.16 + if checkCSRF(r, session) != nil { 3.17 + log.Println("CSRF attempt detected.") 3.18 + w.WriteHeader(http.StatusInternalServerError) 3.19 + context.Render(w, getAuthorizationCodeTemplateName, map[string]interface{}{ 3.20 + "error": template.HTML("There was an error authenticating your request."), 3.21 + }) 3.22 + return 3.23 + } 3.24 if r.PostFormValue("grant") == "approved" { 3.25 var fragment bool 3.26 switch responseType { 3.27 @@ -352,6 +358,7 @@ 3.28 "redirectURL": redirectURL, 3.29 "scope": scope, 3.30 "profile": profile, 3.31 + "csrftoken": session.CSRFToken, 3.32 }) 3.33 } 3.34
4.1 --- a/oauth2_test.go Sat Jan 24 10:34:33 2015 -0500 4.2 +++ b/oauth2_test.go Wed Jan 28 07:27:32 2015 -0500 4.3 @@ -71,6 +71,8 @@ 4.4 ID: "testsession", 4.5 Active: true, 4.6 ProfileID: profile.ID, 4.7 + CSRFToken: "nosurf", 4.8 + Expires: time.Now().Add(time.Hour), 4.9 } 4.10 err = testContext.CreateSession(session) 4.11 if err != nil { 4.12 @@ -117,6 +119,7 @@ 4.13 w = httptest.NewRecorder() 4.14 data := url.Values{} 4.15 data.Set("grant", "approved") 4.16 + data.Set("csrftoken", session.CSRFToken) 4.17 body := bytes.NewBufferString(data.Encode()) 4.18 req.Body = ioutil.NopCloser(body) 4.19 GetAuthorizationCodeHandler(w, req, testContext) 4.20 @@ -129,6 +132,7 @@ 4.21 t.Fatalf(`Being redirected to a non-URL "%s" threw error "%s" for "%s"\n`, redirectedTo, err, req.URL.String()) 4.22 } 4.23 t.Log("Redirected to", redirectedTo) 4.24 + t.Log("Body", w.Body.String()) 4.25 if red.Query().Get("code") == "" { 4.26 t.Fatalf(`Expected code param in redirect URL to be set, but it wasn't for %s`, req.URL.String()) 4.27 } 4.28 @@ -173,8 +177,10 @@ 4.29 t.Fatal("Can't store client:", err) 4.30 } 4.31 session := Session{ 4.32 - ID: "testsession", 4.33 - Active: true, 4.34 + ID: "testsession", 4.35 + Active: true, 4.36 + CSRFToken: "nosurf", 4.37 + Expires: time.Now().Add(time.Hour), 4.38 } 4.39 err = testContext.CreateSession(session) 4.40 if err != nil { 4.41 @@ -246,8 +252,10 @@ 4.42 t.Fatal("Can't store client:", err) 4.43 } 4.44 session := Session{ 4.45 - ID: "testsession", 4.46 - Active: true, 4.47 + ID: "testsession", 4.48 + Active: true, 4.49 + CSRFToken: "nosurf", 4.50 + Expires: time.Now().Add(time.Hour), 4.51 } 4.52 err = testContext.CreateSession(session) 4.53 if err != nil { 4.54 @@ -362,8 +370,10 @@ 4.55 t.Fatal("Can't store endpoint:", err) 4.56 } 4.57 session := Session{ 4.58 - ID: "testsession", 4.59 - Active: true, 4.60 + ID: "testsession", 4.61 + Active: true, 4.62 + CSRFToken: "nosurf", 4.63 + Expires: time.Now().Add(time.Hour), 4.64 } 4.65 err = testContext.CreateSession(session) 4.66 if err != nil { 4.67 @@ -465,8 +475,10 @@ 4.68 t.Fatal("Can't store endpoint:", err) 4.69 } 4.70 session := Session{ 4.71 - ID: "testsession", 4.72 - Active: true, 4.73 + ID: "testsession", 4.74 + Active: true, 4.75 + CSRFToken: "nosurf", 4.76 + Expires: time.Now().Add(time.Hour), 4.77 } 4.78 err = testContext.CreateSession(session) 4.79 if err != nil { 4.80 @@ -489,8 +501,10 @@ 4.81 params.Set("state", "my super secure state string") 4.82 data := url.Values{} 4.83 data.Set("grant", "denied") 4.84 + data.Set("csrftoken", session.CSRFToken) 4.85 req.URL.RawQuery = params.Encode() 4.86 req.Body = ioutil.NopCloser(bytes.NewBufferString(data.Encode())) 4.87 + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") 4.88 req.Method = "POST" 4.89 w := httptest.NewRecorder() 4.90 GetAuthorizationCodeHandler(w, req, testContext) 4.91 @@ -566,8 +580,10 @@ 4.92 t.Errorf("Expected ErrNoSession, got %s", err) 4.93 } 4.94 session = Session{ 4.95 - ID: "testsession", 4.96 - Active: true, 4.97 + ID: "testsession", 4.98 + Active: true, 4.99 + CSRFToken: "nosurf", 4.100 + Expires: time.Now().Add(time.Hour), 4.101 } 4.102 err = testContext.CreateSession(session) 4.103 if err != nil {
5.1 --- a/session.go Sat Jan 24 10:34:33 2015 -0500 5.2 +++ b/session.go Wed Jan 28 07:27:32 2015 -0500 5.3 @@ -1,7 +1,9 @@ 5.4 package auth 5.5 5.6 import ( 5.7 + "crypto/rand" 5.8 "crypto/sha256" 5.9 + "encoding/base64" 5.10 "encoding/hex" 5.11 "encoding/json" 5.12 "errors" 5.13 @@ -16,6 +18,7 @@ 5.14 ) 5.15 5.16 const ( 5.17 + authCookieName = "auth" 5.18 loginTemplateName = "login" 5.19 ) 5.20 5.21 @@ -38,6 +41,8 @@ 5.22 ErrInvalidSession = errors.New("session is not valid") 5.23 // ErrSessionAlreadyExists is returned when a sessionStore tries to store a Session with an ID that already exists in the sessionStore. 5.24 ErrSessionAlreadyExists = errors.New("session already exists") 5.25 + // ErrCSRFAttempt is returned when a CSRF attempt is detected. 5.26 + ErrCSRFAttempt = errors.New("CSRF attempt") 5.27 5.28 passphraseSchemes = map[int]passphraseScheme{ 5.29 1: { 5.30 @@ -63,7 +68,9 @@ 5.31 ProfileID uuid.ID 5.32 Login string 5.33 Created time.Time 5.34 + Expires time.Time 5.35 Active bool 5.36 + CSRFToken string 5.37 } 5.38 5.39 type sortedSessions []Session 5.40 @@ -145,6 +152,13 @@ 5.41 // BUG(paddy): We need to implement a handler for terminating sessions. 5.42 } 5.43 5.44 +func checkCSRF(r *http.Request, s Session) error { 5.45 + if r.PostFormValue("csrftoken") != s.CSRFToken { 5.46 + return ErrCSRFAttempt 5.47 + } 5.48 + return nil 5.49 +} 5.50 + 5.51 func checkCookie(r *http.Request, context Context) (Session, error) { 5.52 cookie, err := r.Cookie(authCookieName) 5.53 if err == http.ErrNoCookie { 5.54 @@ -162,6 +176,9 @@ 5.55 if !sess.Active { 5.56 return Session{}, ErrInvalidSession 5.57 } 5.58 + if time.Now().After(sess.Expires) { 5.59 + return Session{}, ErrInvalidSession 5.60 + } 5.61 return sess, nil 5.62 } 5.63 5.64 @@ -233,7 +250,6 @@ 5.65 5.66 // CreateSessionHandler allows the user to log into their account and create their session. 5.67 func CreateSessionHandler(w http.ResponseWriter, r *http.Request, context Context) { 5.68 - // BUG(paddy): Creating a session needs CSRF protection, right? This whole thing should get a security audit 5.69 errors := []error{} 5.70 if r.Method == "POST" { 5.71 profile, err := authenticate(r.PostFormValue("login"), r.PostFormValue("passphrase"), context) 5.72 @@ -242,14 +258,32 @@ 5.73 if ip == "" { 5.74 ip = r.RemoteAddr 5.75 } 5.76 + sessionID := make([]byte, 32) 5.77 + csrfToken := make([]byte, 32) 5.78 + _, err = rand.Read(sessionID) 5.79 + if err != nil { 5.80 + log.Println("Error reading CSPRNG for session ID:", err) 5.81 + w.WriteHeader(http.StatusInternalServerError) 5.82 + w.Write([]byte("Internal error")) 5.83 + return 5.84 + } 5.85 + _, err = rand.Read(csrfToken) 5.86 + if err != nil { 5.87 + log.Println("Error reading CSPRNG for CSRF token:", err) 5.88 + w.WriteHeader(http.StatusInternalServerError) 5.89 + w.Write([]byte("internal error")) 5.90 + return 5.91 + } 5.92 session := Session{ 5.93 - ID: uuid.NewID().String(), 5.94 + ID: base64.StdEncoding.EncodeToString(sessionID), 5.95 IP: ip, 5.96 UserAgent: r.UserAgent(), 5.97 ProfileID: profile.ID, 5.98 Login: r.PostFormValue("login"), 5.99 Created: time.Now(), 5.100 + Expires: time.Now().Add(time.Hour), 5.101 Active: true, 5.102 + CSRFToken: base64.StdEncoding.EncodeToString(csrfToken), 5.103 } 5.104 err = context.CreateSession(session) 5.105 if err != nil { 5.106 @@ -257,12 +291,13 @@ 5.107 w.Write([]byte(err.Error())) 5.108 return 5.109 } 5.110 - // BUG(paddy): We really need to do a security audit on our cookie. 5.111 + // BUG(paddy): We really need to do a security audit on our cookies. 5.112 cookie := http.Cookie{ 5.113 Name: authCookieName, 5.114 Value: session.ID, 5.115 - Expires: time.Now().Add(24 * 7 * time.Hour), 5.116 + Expires: session.Expires, 5.117 HttpOnly: true, 5.118 + Secure: context.config.secureCookie, 5.119 } 5.120 http.SetCookie(w, &cookie) 5.121 redirectTo := r.URL.Query().Get("from")
6.1 --- a/session_test.go Sat Jan 24 10:34:33 2015 -0500 6.2 +++ b/session_test.go Wed Jan 28 07:27:32 2015 -0500 6.3 @@ -25,12 +25,18 @@ 6.4 if !session1.Created.Equal(session2.Created) { 6.5 return false, "Created", session1.Created, session2.Created 6.6 } 6.7 + if !session1.Expires.Equal(session2.Expires) { 6.8 + return false, "Expires", session1.Expires, session2.Expires 6.9 + } 6.10 if session1.Login != session2.Login { 6.11 return false, "Login", session1.Login, session2.Login 6.12 } 6.13 if session1.Active != session2.Active { 6.14 return false, "Active", session1.Active, session2.Active 6.15 } 6.16 + if session1.CSRFToken != session2.CSRFToken { 6.17 + return false, "CSRFToken", session1.CSRFToken, session2.CSRFToken 6.18 + } 6.19 return true, "", nil, nil 6.20 } 6.21