fix: remove forgeable remember-me cookie, persist sessions by default (#8289)

CHANGELOG.md

@@ -13,6 +13,7 @@ All notable changes to Gogs are documented in this file.
 - _Security:_ Denial of service in repository and wiki file listing pages via crafted file names. [#8116](https://github.com/gogs/gogs/pull/8116) - [GHSA-3qq3-668m-v9mj](https://github.com/gogs/gogs/security/advisories/GHSA-3qq3-668m-v9mj)
 - _Security:_ Reverse proxy authentication header was honored from any remote address, allowing user impersonation when Gogs was reachable directly. The header is now only trusted from addresses listed in `[auth] TRUSTED_PROXY_IPS`. [#8264](https://github.com/gogs/gogs/pull/8264) - [GHSA-w6j9-vw59-27wv](https://github.com/gogs/gogs/security/advisories/GHSA-w6j9-vw59-27wv)
 - _Security:_ Server-side request forgery in webhook deliveries via HTTP redirects to local network addresses. [#8263](https://github.com/gogs/gogs/pull/8263) - [GHSA-c4v7-xg93-qf8g](https://github.com/gogs/gogs/security/advisories/GHSA-c4v7-xg93-qf8g)
+- _Security:_ The "remember me" auto-login cookie was derived from database columns, so an attacker with a database dump could forge a valid cookie for any user. The auto-login cookie path has been removed entirely. Persistence is now provided by the server-issued session cookie. [#8289](https://github.com/gogs/gogs/pull/8289) - [GHSA-4pph-25p3-pw73](https://github.com/gogs/gogs/security/advisories/GHSA-4pph-25p3-pw73)
 
 ### Removed
 

cmd/gogs/internal/web/web.go

@@ -536,6 +536,7 @@ func Run(configPath string, portOverride int) error {
 			Gclifetime:     conf.Session.GCInterval,
 			Maxlifetime:    conf.Session.MaxLifeTime,
 			Secure:         conf.Session.CookieSecure,
+			CookieLifeTime: 86400 * conf.Security.LoginRememberDays,
 		}),
 		csrf.Csrfer(csrf.Options{
 			Secret:         conf.Security.SecretKey,

cmd/gogs/internal/web/webapi.go

@@ -237,7 +237,6 @@ type userSignInRequest struct {
 	Username    string `json:"username" validate:"required,max=254"`
 	Password    string `json:"password" validate:"required,max=255"`
 	LoginSource int64  `json:"loginSource"`
-	Remember    bool   `json:"remember"`
 }
 
 type userSignInResponse struct {
@@ -265,29 +264,21 @@ func postUserSignIn(r *http.Request, sess session.Store, mc *macaron.Context, l
 	}
 
 	if database.Handle.TwoFactors().IsEnabled(r.Context(), u.ID) {
-		_ = sess.Set("mfaRemember", req.Remember)
 		_ = sess.Set("mfaUserID", u.ID)
 		return http.StatusOK, &userSignInResponse{MFA: true}, nil
 	}
 
-	completeSignIn(sess, mc, u, req.Remember)
+	completeSignIn(sess, mc, u)
 	return http.StatusOK, &userSignInResponse{}, nil
 }
 
 // completeSignIn finalizes the sign-in session for u: writes the auth session,
-// clears any in-flight MFA session, and sets remember-me / login-status
+// clears any in-flight MFA state, and sets the login-status cookie. The
-// cookies. The caller is responsible for navigating to a post-login
+// caller is responsible for navigating to a post-login destination via
-// destination via /redirect?to=.
+// /redirect?to=.
-func completeSignIn(sess session.Store, mc *macaron.Context, u *database.User, remember bool) {
+func completeSignIn(sess session.Store, mc *macaron.Context, u *database.User) {
-	if remember {
-		days := 86400 * conf.Security.LoginRememberDays
-		mc.SetCookie(conf.Security.CookieUsername, u.Name, days, conf.Server.Subpath, "", conf.Security.CookieSecure, true)
-		mc.SetSuperSecureCookie(u.Rands+u.Password, conf.Security.CookieRememberName, u.Name, days, conf.Server.Subpath, "", conf.Security.CookieSecure, true)
-	}
-
 	_ = sess.Set("uid", u.ID)
 	_ = sess.Set("uname", u.Name)
-	_ = sess.Delete("mfaRemember")
 	_ = sess.Delete("mfaUserID")
 
 	mc.SetCookie(conf.Session.CSRFCookieName, "", -1, conf.Server.Subpath)
@@ -349,8 +340,7 @@ func postUserMFA(r *http.Request, sess session.Store, mc *macaron.Context, ca ca
 		return http.StatusInternalServerError, nil, errors.Wrap(err, "get user by ID")
 	}
 
-	remember, _ := sess.Get("mfaRemember").(bool)
+	completeSignIn(sess, mc, u)
-	completeSignIn(sess, mc, u, remember)
 	return http.StatusOK, &userMFAResponse{}, nil
 }
 
@@ -381,8 +371,7 @@ func postUserMFARecovery(r *http.Request, sess session.Store, mc *macaron.Contex
 		return http.StatusInternalServerError, nil, errors.Wrap(err, "get user by ID")
 	}
 
-	remember, _ := sess.Get("mfaRemember").(bool)
+	completeSignIn(sess, mc, u)
-	completeSignIn(sess, mc, u, remember)
 	return http.StatusOK, &userMFAResponse{}, nil
 }
 
@@ -410,8 +399,6 @@ func getUserInfo(user *database.User) (statusCode int, resp *userInfo, err error
 func postUserSignOut(sess session.Store, mc *macaron.Context) (statusCode int, resp any, err error) {
 	_ = sess.Flush()
 	_ = sess.Destory(mc)
-	mc.SetCookie(conf.Security.CookieUsername, "", -1, conf.Server.Subpath)
-	mc.SetCookie(conf.Security.CookieRememberName, "", -1, conf.Server.Subpath)
 	mc.SetCookie(conf.Session.CSRFCookieName, "", -1, conf.Server.Subpath)
 	return http.StatusNoContent, nil, nil
 }

conf/app.ini

@@ -164,12 +164,8 @@ INSTALL_LOCK = false
 ; The secret to encrypt cookie values, 2FA code, etc.
 ; !!CHANGE THIS TO KEEP YOUR USER DATA SAFE!!
 SECRET_KEY = !#@FDEWREWR&*(
-; The days remembered for auto-login.
+; The number of days a sign-in session persists across browser restarts.
 LOGIN_REMEMBER_DAYS = 7
-; The cookie name to store auto-login information.
-COOKIE_REMEMBER_NAME = gogs_incredible
-; The cookie name to store logged in username.
-COOKIE_USERNAME = gogs_awesome
 ; Whether to set secure cookie.
 COOKIE_SECURE = false
 ; Whether to set cookie to indicate user login status.
@@ -255,8 +251,10 @@ COOKIE_NAME = i_like_gogs
 COOKIE_SECURE = false
 ; The GC interval in seconds for session data.
 GC_INTERVAL = 3600
-; The maximum life time in seconds for a session.
+; The maximum idle time in seconds before a session record is garbage-collected.
-MAX_LIFE_TIME = 86400
+; Set lower than `[security] LOGIN_REMEMBER_DAYS * 86400` to enforce a sliding
+; idle timeout. Otherwise the session lives for the full cookie lifetime.
+MAX_LIFE_TIME = 604800
 ; The cookie name for CSRF token.
 CSRF_COOKIE_NAME = _csrf
 

conf/locale/locale_en-US.ini

@@ -183,7 +183,6 @@ disable_register_prompt = Sorry, registration has been disabled. Please contact
 disable_register_mail = Sorry, email services are disabled. Please contact the site administrator.
 auth_source = Authentication source
 local = Local
-remember_me = Remember me
 forgot_password= Forgot Password
 forget_password = Forgot password?
 sign_up_now = Create a new account
@@ -1269,8 +1268,6 @@ config.db.max_idle_conns = Maximum idle connections
 
 config.security_config = Security configuration
 config.security.login_remember_days = Login remember days
-config.security.cookie_remember_name = Remember cookie
-config.security.cookie_username = Username cookie
 config.security.cookie_secure = Enable secure cookie
 config.security.reverse_proxy_auth_user = Reverse proxy authentication header
 config.security.enable_login_status_cookie = Enable login status cookie

internal/conf/static.go

@@ -40,8 +40,6 @@ var (
 		InstallLock             bool
 		SecretKey               string
 		LoginRememberDays       int
-		CookieRememberName      string
-		CookieUsername          string
 		CookieSecure            bool
 		EnableLoginStatusCookie bool
 		LoginStatusCookieName   string

internal/conf/testdata/TestInit.golden.ini

@@ -74,8 +74,6 @@ MAX_IDLE_CONNS=30
 INSTALL_LOCK=false
 SECRET_KEY=`!#@FDEWREWR&*(`
 LOGIN_REMEMBER_DAYS=7
-COOKIE_REMEMBER_NAME=gogs_incredible
-COOKIE_USERNAME=gogs_awesome
 COOKIE_SECURE=false
 ENABLE_LOGIN_STATUS_COOKIE=false
 LOGIN_STATUS_COOKIE_NAME=login_status

internal/context/auth.go

@@ -86,18 +86,6 @@ func Toggle(options *ToggleOptions) macaron.Handler {
 			}
 		}
 
-		// Redirect to log in page if auto-signin info is provided and has not signed in.
-		if !options.SignOutRequired && !c.IsLogged && !isAPIPath(c.Req.URL.Path) &&
-			len(c.GetCookie(conf.Security.CookieUsername)) > 0 {
-			if isWebPath(c.Req.URL.Path) {
-				c.ServeWeb()
-				return
-			}
-			c.SetCookie("redirect_to", url.QueryEscape(conf.Server.Subpath+c.Req.RequestURI), 0, conf.Server.Subpath)
-			c.RedirectSubpath("/user/sign-in")
-			return
-		}
-
 		if options.AdminRequired {
 			if !c.User.IsAdmin {
 				c.Status(http.StatusForbidden)

internal/route/home.go

@@ -28,13 +28,6 @@ func Home(c *context.Context) {
 		return
 	}
 
-	// Check auto-login.
-	uname := c.GetCookie(conf.Security.CookieUsername)
-	if uname != "" {
-		c.Redirect(conf.Server.Subpath + "/user/sign-in")
-		return
-	}
-
 	c.ServeWeb()
 }
 

internal/route/user/auth.go

@@ -28,8 +28,6 @@ const (
 func SignOut(c *context.Context) {
 	_ = c.Session.Flush()
 	_ = c.Session.Destory(c.Context)
-	c.SetCookie(conf.Security.CookieUsername, "", -1, conf.Server.Subpath)
-	c.SetCookie(conf.Security.CookieRememberName, "", -1, conf.Server.Subpath)
 	c.SetCookie(conf.Session.CSRFCookieName, "", -1, conf.Server.Subpath)
 	if conf.Auth.CustomLogoutURL != "" {
 		c.Redirect(conf.Auth.CustomLogoutURL)

templates/admin/config.tmpl

@@ -202,10 +202,6 @@
 					<dl class="dl-horizontal admin-dl-horizontal">
 						<dt>{{.i18n.Tr "admin.config.security.login_remember_days"}}</dt>
 						<dd>{{.Security.LoginRememberDays}}</dd>
-						<dt>{{.i18n.Tr "admin.config.security.cookie_remember_name"}}</dt>
-						<dd>{{.Security.CookieRememberName}}</dd>
-						<dt>{{.i18n.Tr "admin.config.security.cookie_username"}}</dt>
-						<dd>{{.Security.CookieUsername}}</dd>
 						<dt>{{.i18n.Tr "admin.config.security.cookie_secure"}}</dt>
 						<dd><i class="fa fa{{if .Security.CookieSecure}}-check{{end}}-square-o"></i></dd>
 						<dt>{{.i18n.Tr "admin.config.security.enable_login_status_cookie"}}</dt>

web/src/locales/en-US.json

@@ -31,7 +31,6 @@
   "password_placeholder": "Enter your password",
   "auth_source": "Authentication source",
   "local": "Local",
-  "remember_me": "Remember me",
   "forget_password": "Forgot password?",
   "sign_up_now": "Create a new account",
   "sign_in_submitting": "Signing in...",

web/src/pages/SignIn.tsx

@@ -5,7 +5,6 @@ import { useTranslation } from "react-i18next";
 
 import { Button } from "@/components/ui/button";
 import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card";
-import { Checkbox } from "@/components/ui/checkbox";
 import { Input } from "@/components/ui/input";
 import { Label } from "@/components/ui/label";
 import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select";
@@ -46,7 +45,6 @@ export function SignIn() {
   const [username, setUsername] = useState("");
   const [password, setPassword] = useState("");
   const [loginSource, setLoginSource] = useState<number>(defaultSource?.id ?? 0);
-  const [remember, setRemember] = useState(false);
   const [showPassword, setShowPassword] = useState(false);
   const [submitting, setSubmitting] = useState(false);
   const [formError, setFormError] = useState<string | null>(null);
@@ -65,7 +63,7 @@ export function SignIn() {
           method: "POST",
           credentials: "same-origin",
           headers: { "Content-Type": "application/json" },
-          body: JSON.stringify({ username, password, loginSource, remember }),
+          body: JSON.stringify({ username, password, loginSource }),
         });
         if (!res.ok) {
           const body = (await res.json().catch(() => ({}))) as SignInErrorResponse;
@@ -158,7 +156,7 @@ export function SignIn() {
                     <Button variant="link" size="inline" asChild>
                       <a
                         href={subUrl("/user/forget_password")}
-                        tabIndex={submitting ? -1 : 7}
+                        tabIndex={submitting ? -1 : 6}
                         aria-disabled={submitting || undefined}
                         className={submitting ? "pointer-events-none opacity-50" : undefined}
                         onClick={(e) => {
@@ -231,26 +229,14 @@ export function SignIn() {
                   </div>
                 )}
 
-                <div className="flex items-center gap-2">
-                  <Checkbox
-                    id="remember"
-                    tabIndex={5}
-                    checked={remember}
-                    onCheckedChange={(v) => setRemember(v === true)}
-                  />
-                  <Label htmlFor="remember" className="cursor-pointer font-normal">
-                    {t("remember_me")}
-                  </Label>
-                </div>
-
                 <div className="mt-2 flex flex-col gap-3">
-                  <Button type="submit" disabled={submitting} tabIndex={6} className="w-full">
+                  <Button type="submit" disabled={submitting} tabIndex={5} className="w-full">
                     {submitting ? t("sign_in_submitting") : t("sign_in")}
                   </Button>
                   <Button variant="link" size="inline" asChild className="self-center">
                     <a
                       href={subUrl("/user/sign_up")}
-                      tabIndex={submitting ? -1 : 8}
+                      tabIndex={submitting ? -1 : 7}
                       aria-disabled={submitting || undefined}
                       className={submitting ? "pointer-events-none opacity-50" : undefined}
                       onClick={(e) => {