diff --git a/CHANGELOG.md b/CHANGELOG.md index 90e2d8dc7..936190996 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cmd/gogs/internal/web/web.go b/cmd/gogs/internal/web/web.go index b91270a9a..218360fef 100644 --- a/cmd/gogs/internal/web/web.go +++ b/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, diff --git a/cmd/gogs/internal/web/webapi.go b/cmd/gogs/internal/web/webapi.go index 50a31ccc1..5a1d92dbf 100644 --- a/cmd/gogs/internal/web/webapi.go +++ b/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 -// cookies. The caller is responsible for navigating to a post-login -// destination via /redirect?to=. -func completeSignIn(sess session.Store, mc *macaron.Context, u *database.User, remember bool) { - 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) - } - +// clears any in-flight MFA state, and sets the login-status cookie. The +// caller is responsible for navigating to a post-login destination via +// /redirect?to=. +func completeSignIn(sess session.Store, mc *macaron.Context, u *database.User) { _ = 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, remember) + completeSignIn(sess, mc, u) 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, remember) + completeSignIn(sess, mc, u) 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 } diff --git a/conf/app.ini b/conf/app.ini index f00280cff..303d3571c 100644 --- a/conf/app.ini +++ b/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. -MAX_LIFE_TIME = 86400 +; The maximum idle time in seconds before a session record is garbage-collected. +; 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 diff --git a/conf/locale/locale_en-US.ini b/conf/locale/locale_en-US.ini index fbe5bfe0a..8e9890ff2 100644 --- a/conf/locale/locale_en-US.ini +++ b/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 diff --git a/internal/conf/static.go b/internal/conf/static.go index 31ac9426f..f18aebaab 100644 --- a/internal/conf/static.go +++ b/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 diff --git a/internal/conf/testdata/TestInit.golden.ini b/internal/conf/testdata/TestInit.golden.ini index 19e7311d8..84210b57b 100644 --- a/internal/conf/testdata/TestInit.golden.ini +++ b/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 diff --git a/internal/context/auth.go b/internal/context/auth.go index 7d4825750..6094547c0 100644 --- a/internal/context/auth.go +++ b/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) diff --git a/internal/route/home.go b/internal/route/home.go index 771316e0f..5f8a3e49b 100644 --- a/internal/route/home.go +++ b/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() } diff --git a/internal/route/user/auth.go b/internal/route/user/auth.go index 0503cef36..d198cff89 100644 --- a/internal/route/user/auth.go +++ b/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) diff --git a/templates/admin/config.tmpl b/templates/admin/config.tmpl index a7b19c332..836d30b56 100644 --- a/templates/admin/config.tmpl +++ b/templates/admin/config.tmpl @@ -202,10 +202,6 @@
{{.i18n.Tr "admin.config.security.login_remember_days"}}
{{.Security.LoginRememberDays}}
-
{{.i18n.Tr "admin.config.security.cookie_remember_name"}}
-
{{.Security.CookieRememberName}}
-
{{.i18n.Tr "admin.config.security.cookie_username"}}
-
{{.Security.CookieUsername}}
{{.i18n.Tr "admin.config.security.cookie_secure"}}
{{.i18n.Tr "admin.config.security.enable_login_status_cookie"}}
diff --git a/web/src/locales/en-US.json b/web/src/locales/en-US.json index 80d31b579..e1097bf35 100644 --- a/web/src/locales/en-US.json +++ b/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...", diff --git a/web/src/pages/SignIn.tsx b/web/src/pages/SignIn.tsx index 742becf84..9ef6f48a8 100644 --- a/web/src/pages/SignIn.tsx +++ b/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(defaultSource?.id ?? 0); - const [remember, setRemember] = useState(false); const [showPassword, setShowPassword] = useState(false); const [submitting, setSubmitting] = useState(false); const [formError, setFormError] = useState(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() {