Make login cookie SameSite=None

This is currently necessary for FedCM. In order to try and protect
against unanticipated security issues from this, I added a new lax
cookie that is set on all requests. I then gated all non-FedCM access to
the login cookie behind the getLoginCookie function. This function
verifies that the lax cookie is present, which indicates the
request is not cross-site. We can't use the Origin header for this
because FedCM doesn't send it for privacy reasons.

Hopefully FedCM eventually changes to allow SameSite=lax so we
don't have to worry about this. See
w3c-fedid/FedCM#587

add_identity_email.go

@@ -65,8 +65,6 @@ func NewAddIdentityEmailHandler(db Database, cluster *Cluster, tmpl *template.Te
 	prefix, err := db.GetPrefix()
 	checkErr(err)
 
-	loginKeyName := prefix + "login_key"
-
 	// Periodically clean up expired shares
 	go func() {
 		for {
@@ -410,7 +408,7 @@ func NewAddIdentityEmailHandler(db Database, cluster *Cluster, tmpl *template.Te
 		email := pendingLogin.Email
 
 		cookieValue := ""
-		loginKeyCookie, err := r.Cookie(loginKeyName)
+		loginKeyCookie, err := getLoginCookie(db, r)
 		if err == nil {
 			cookieValue = loginKeyCookie.Value
 		}
@@ -431,8 +429,12 @@ func NewAddIdentityEmailHandler(db Database, cluster *Cluster, tmpl *template.Te
 			return
 		}
 
-		w.Header().Add("Set-Login", "logged-in")
-		http.SetCookie(w, cookie)
+		err = setLoginCookie(w, cookie)
+		if err != nil {
+			w.WriteHeader(500)
+			fmt.Fprintf(os.Stderr, err.Error())
+			return
+		}
 
 		returnUri, err := getReturnUriCookie(db, r)
 		if err != nil {

add_identity_fedcm.go

@@ -23,11 +23,6 @@ func NewAddIdentityFedCmHandler(db Database, tmpl *template.Template, jose *JOSE
 		mux: mux,
 	}
 
-	prefix, err := db.GetPrefix()
-	checkErr(err)
-
-	loginKeyName := prefix + "login_key"
-
 	mux.HandleFunc("/login-fedcm", func(w http.ResponseWriter, r *http.Request) {
 
 		r.ParseForm()
@@ -127,7 +122,7 @@ func NewAddIdentityFedCmHandler(db Database, tmpl *template.Template, jose *JOSE
 		}
 
 		cookieValue := ""
-		loginKeyCookie, err := r.Cookie(loginKeyName)
+		loginKeyCookie, err := getLoginCookie(db, r)
 		if err == nil {
 			cookieValue = loginKeyCookie.Value
 		}

add_identity_gaml.go

@@ -31,8 +31,6 @@ func NewAddIdentityGamlHandler(db Database, cluster *Cluster, tmpl *template.Tem
 	prefix, err := db.GetPrefix()
 	checkErr(err)
 
-	loginKeyName := prefix + "login_key"
-
 	mux.HandleFunc("/login-gaml", func(w http.ResponseWriter, r *http.Request) {
 
 		templateData := struct {
@@ -193,7 +191,7 @@ func NewAddIdentityGamlHandler(db Database, cluster *Cluster, tmpl *template.Tem
 		}
 
 		cookieValue := ""
-		loginKeyCookie, err := r.Cookie(loginKeyName)
+		loginKeyCookie, err := getLoginCookie(db, r)
 		if err == nil {
 			cookieValue = loginKeyCookie.Value
 		}

add_identity_oauth2.go

@@ -66,8 +66,6 @@ func NewAddIdentityOauth2Handler(db Database, oauth2MetaMan *OAuth2MetadataManag
 	prefix, err := db.GetPrefix()
 	checkErr(err)
 
-	loginKeyName := prefix + "login_key"
-
 	mux.HandleFunc("/login-oauth2", func(w http.ResponseWriter, r *http.Request) {
 
 		r.ParseForm()
@@ -333,7 +331,7 @@ func NewAddIdentityOauth2Handler(db Database, oauth2MetaMan *OAuth2MetadataManag
 		deleteReturnUriCookie(r.Host, db, w)
 
 		cookieValue := ""
-		loginKeyCookie, err := r.Cookie(loginKeyName)
+		loginKeyCookie, err := getLoginCookie(db, r)
 		if err == nil {
 			cookieValue = loginKeyCookie.Value
 		}
@@ -354,8 +352,12 @@ func NewAddIdentityOauth2Handler(db Database, oauth2MetaMan *OAuth2MetadataManag
 			return
 		}
 
-		w.Header().Add("Set-Login", "logged-in")
-		http.SetCookie(w, cookie)
+		err = setLoginCookie(w, cookie)
+		if err != nil {
+			w.WriteHeader(500)
+			fmt.Fprintf(os.Stderr, err.Error())
+			return
+		}
 
 		redirUrl := fmt.Sprintf("%s", returnUri)
 

fedcm.go

@@ -117,7 +117,7 @@ func NewFedCmHandler(db Database, loginEndpoint string, jose *JOSE) *FedCmHandle
 			return
 		}
 
-		idents, _ := getIdentities(db, r)
+		idents, _ := getIdentitiesFedCm(db, r)
 
 		if len(idents) == 0 {
 			w.WriteHeader(401)
@@ -213,7 +213,7 @@ func NewFedCmHandler(db Database, loginEndpoint string, jose *JOSE) *FedCmHandle
 			return
 		}
 
-		idents, _ := getIdentities(db, r)
+		idents, _ := getIdentitiesFedCm(db, r)
 
 		accountId := r.Form.Get("account_id")
 		pkceCodeChallenge := r.Form.Get("nonce")

obligator.go

@@ -164,6 +164,25 @@ func (s *ObligatorMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	cookieDomain, err := buildCookieDomain(r.Host)
+	if err != nil {
+		w.WriteHeader(500)
+		io.WriteString(w, err.Error())
+		return
+	}
+
+	crossSiteDetectorCookie := &http.Cookie{
+		Domain:   cookieDomain,
+		Name:     "obligator_not_cross_site",
+		Value:    "true",
+		Secure:   true,
+		HttpOnly: true,
+		MaxAge:   86400 * 365,
+		Path:     "/",
+		SameSite: http.SameSiteLaxMode,
+	}
+	http.SetCookie(w, crossSiteDetectorCookie)
+
 	fmt.Println(fmt.Sprintf("%s\t%s\t%s\t%s\t%s", timestamp, remoteIp, r.Method, r.Host, r.URL.Path))
 	s.mux.ServeHTTP(w, r)
 }
@@ -468,14 +487,7 @@ func validate(db Database, r *http.Request, jose *JOSE) (*Validation, error) {
 		return nil, err
 	}
 
-	prefix, err := db.GetPrefix()
-	if err != nil {
-		return nil, err
-	}
-
-	loginKeyName := prefix + "login_key"
-
-	loginKeyCookie, err := r.Cookie(loginKeyName)
+	loginKeyCookie, err := getLoginCookie(db, r)
 	if err != nil {
 		return checkErrPassthrough(err, passthrough)
 	}

qr.go

@@ -50,10 +50,7 @@ func NewQrHandler(db Database, cluster *Cluster, tmpl *template.Template, jose *
 
 	const ShareTimeout = 2 * time.Minute
 
-	prefix, err := db.GetPrefix()
-	checkErr(err)
-
-	loginKeyName := prefix + "login_key"
+	var err error
 
 	// Periodically clean up expired shares
 	go func() {
@@ -272,7 +269,7 @@ func NewQrHandler(db Database, cluster *Cluster, tmpl *template.Template, jose *
 		}
 
 		cookie := &http.Cookie{}
-		loginKeyCookie, err := r.Cookie(loginKeyName)
+		loginKeyCookie, err := getLoginCookie(db, r)
 		if err == nil {
 			cookie = loginKeyCookie
 		}

utils.go

@@ -138,6 +138,38 @@ func validUser(id string, users []*User) bool {
 	return false
 }
 
+func getLoginCookie(db Database, r *http.Request) (*http.Cookie, error) {
+
+	prefix, err := db.GetPrefix()
+	if err != nil {
+		return nil, err
+	}
+
+	loginKeyName := prefix + "login_key"
+
+	loginKeyCookie, err := r.Cookie(loginKeyName)
+	if err != nil {
+		return nil, err
+	}
+
+	// This cookie unlocks the main one. This is necessary for FedCM
+	crossSiteDetectorCookieName := "obligator_not_cross_site"
+	_, err = r.Cookie(crossSiteDetectorCookieName)
+	if err != nil {
+		return nil, err
+	}
+
+	return loginKeyCookie, nil
+}
+
+func setLoginCookie(w http.ResponseWriter, cookie *http.Cookie) error {
+
+	w.Header().Add("Set-Login", "logged-in")
+	http.SetCookie(w, cookie)
+
+	return nil
+}
+
 func addIdentToCookie(domain string, db Database, cookieValue string, newIdent *Identity, jose *JOSE) (*http.Cookie, error) {
 
 	idents := []*Identity{newIdent}
@@ -203,12 +235,6 @@ func addIdentToCookie(domain string, db Database, cookieValue string, newIdent *
 
 	loginKeyName := prefix + "login_key"
 
-	sameSiteMode := http.SameSiteLaxMode
-
-	if os.Getenv("FEDCM_ENABLED") == "true" {
-		sameSiteMode = http.SameSiteNoneMode
-	}
-
 	cookie := &http.Cookie{
 		Domain:   cookieDomain,
 		Name:     loginKeyName,
@@ -217,9 +243,7 @@ func addIdentToCookie(domain string, db Database, cookieValue string, newIdent *
 		HttpOnly: true,
 		MaxAge:   86400 * 365,
 		Path:     "/",
-		SameSite: sameSiteMode,
-		//SameSite: http.SameSiteLaxMode,
-		//SameSite: http.SameSiteStrictMode,
+		SameSite: http.SameSiteNoneMode,
 	}
 
 	return cookie, nil
@@ -236,7 +260,7 @@ func addLoginToCookie(db Database, r *http.Request, clientId string, newLogin *L
 
 	loginKeyName := prefix + "login_key"
 
-	loginKeyCookie, err := r.Cookie(loginKeyName)
+	loginKeyCookie, err := getLoginCookie(db, r)
 	if err != nil {
 		return nil, errors.New("Only logged-in users can access this endpoint")
 	}
@@ -462,9 +486,9 @@ func getRemoteIp(r *http.Request, behindProxy bool) (string, error) {
 	return remoteIp, nil
 }
 
-func getIdentities(db Database, r *http.Request) ([]*Identity, error) {
-
-	identities := []*Identity{}
+// This function doesn't check against cross site requests as compared to
+// the normal version
+func getIdentitiesFedCm(db Database, r *http.Request) ([]*Identity, error) {
 
 	prefix, err := db.GetPrefix()
 	if err != nil {
@@ -475,9 +499,26 @@ func getIdentities(db Database, r *http.Request) ([]*Identity, error) {
 
 	loginKeyCookie, err := r.Cookie(loginKeyName)
 	if err != nil {
-		return identities, err
+		return nil, err
+	}
+
+	return getIdentitiesCommon(db, r, loginKeyCookie)
+}
+
+func getIdentities(db Database, r *http.Request) ([]*Identity, error) {
+
+	loginKeyCookie, err := getLoginCookie(db, r)
+	if err != nil {
+		return []*Identity{}, err
 	}
 
+	return getIdentitiesCommon(db, r, loginKeyCookie)
+}
+
+func getIdentitiesCommon(db Database, r *http.Request, loginKeyCookie *http.Cookie) ([]*Identity, error) {
+
+	identities := []*Identity{}
+
 	jwtStr := loginKeyCookie.Value
 
 	if jwtStr == "" {
@@ -503,14 +544,7 @@ func getIdentities(db Database, r *http.Request) ([]*Identity, error) {
 
 func getLogins(db Database, r *http.Request) (map[string][]*Login, error) {
 
-	prefix, err := db.GetPrefix()
-	if err != nil {
-		return nil, err
-	}
-
-	loginKeyName := prefix + "login_key"
-
-	loginKeyCookie, err := r.Cookie(loginKeyName)
+	loginKeyCookie, err := getLoginCookie(db, r)
 	if err != nil {
 		return nil, err
 	}