iris
iris copied to clipboard
kataras
[BUG] Recreating session (cookie) with securecookie enabled passes plain cookie value sometimes
Describe the bug When using securecookie for session it fails sometime because plain (decoded) cookie value is passed to decode function.
securecookie decode fails with "securecookie: the value is not valid" on MAC check/decode: https://github.com/gorilla/securecookie/blob/master/securecookie.go#L323
To Reproduce
package main
import (
"net/http"
"github.com/gorilla/securecookie"
"github.com/kataras/iris/v12"
"github.com/kataras/iris/v12/context"
"github.com/kataras/iris/v12/sessions"
)
const cookieNameForSessionID = "session_id_cookie"
var sess *sessions.Sessions
func secret(ctx iris.Context) {
session := getSession(ctx, false)
// Check if user is authenticated
if auth, _ := session.GetBoolean("authenticated"); !auth {
ctx.StatusCode(iris.StatusForbidden)
ctx.ContentType("text/html")
ctx.WriteString(`Not logged in or session invalid <a href="/logout">logout</a>`)
return
}
// Print secret message
ctx.ContentType("text/html")
ctx.WriteString(`The cake is a lie! <a href="/logout">logout</a>`)
}
func login(ctx iris.Context) {
session := getSession(ctx, false)
// Authentication goes here
// ...
// Set user as authenticated
session = getSession(ctx, true)
session.Set("authenticated", true)
ctx.StatusCode(http.StatusTemporaryRedirect)
ctx.ContentType("text/html")
ctx.Header("Location", "/secret")
}
func logout(ctx iris.Context) {
sess.Destroy(ctx)
ctx.ContentType("text/html")
ctx.WriteString(`<a href="/login">login</a>`)
}
func getSession(ctx iris.Context, recreate bool) *sessions.Session {
cookieOptionList := []context.CookieOption{
func(ctx *context.Context, cookie *http.Cookie, op uint8) {
// c.applySessionSetting(ctx, cookie, op)
},
}
if recreate {
sess.Destroy(ctx)
}
s := sess.Start(ctx, cookieOptionList...)
return s
}
func main() {
app := iris.New()
secureCookie := securecookie.New(
[]byte("IGVneS5eZ0b5jTgwe0l38nON0bW5awxr"),
[]byte("r4GFiLvBtYCohUxt"),
)
sess = sessions.New(sessions.Config{
Cookie: cookieNameForSessionID,
Encoding: secureCookie,
CookieSecureTLS: false,
AllowReclaim: true,
DisableSubdomainPersistence: true,
})
app.Use(sess.Handler())
// ^ or comment this line and use sess.Start(ctx) inside your handlers
// instead of sessions.Get(ctx).
app.Get("/secret", secret)
app.Get("/login", login)
app.Get("/logout", logout)
app.Listen(":8080")
}
and add alter Decode function in securecookie:
func (s *SecureCookie) Decode(name, value string, dst interface{}) error {
fmt.Println(value)
result is:
MTY1Nzk3Nzg4MnxOTy0tV2JmWi1xbExTRlAzQXRMbXJJWV82S0F4dDJ5eGpSZWg5ODRydk1qdDBpVm1ZeXpKWUpMSExCWUY0M2ItZFZfZkQ2bm8yUUU9fAmHNHj68vBKoujiboz1UMf5r2IguTm-zoHQJ0DSJDpM
MTY1Nzk3Nzg4MnxOTy0tV2JmWi1xbExTRlAzQXRMbXJJWV82S0F4dDJ5eGpSZWg5ODRydk1qdDBpVm1ZeXpKWUpMSExCWUY0M2ItZFZfZkQ2bm8yUUU9fAmHNHj68vBKoujiboz1UMf5r2IguTm-zoHQJ0DSJDpM
a95828e7-a194-49e8-89cf-d76a987faa2e
a95828e7-a194-49e8-89cf-d76a987faa2e
a95828e7-a194-49e8-89cf-d76a987faa2e
MTY1Nzk3Nzg4OXxPSGs2YVNhQThFRXYyVmkwWkl5OTQ1bm5vNDVNRmxsUHRZUFNCdGFBRW12aGZIZ3dnaW1naVNDZjkyTXZLRWU1Nkc1QkxTVWdoc2M9fG38bjG-RwEYLoostgZErggXc07I_a28N2vKS-99Uf1P
MTY1Nzk3Nzg4OXxPSGs2YVNhQThFRXYyVmkwWkl5OTQ1bm5vNDVNRmxsUHRZUFNCdGFBRW12aGZIZ3dnaW1naVNDZjkyTXZLRWU1Nkc1QkxTVWdoc2M9fG38bjG-RwEYLoostgZErggXc07I_a28N2vKS-99Uf1P
Desktop (please complete the following information):
- OS: osx
iris.Version
-
v12.2.0-beta3 -
master
I haven't found the code where this happens but my rough guess: When a new session is created it's passed to the decode function which fails because the session was never encoded.
Hello @mblaschke,
In your code example you initialize the session on each handler through:
app.Use(sess.Handler())
Then, on your getSession function, it calls the Destroy and if getSession is called from the login function, it calls the session's Start on the same request-response lifecycle, can you explain me why? Sessions are not supposed to work like this, please describe the issue so I can provide a better solution. Meanwhile, I will keep investigate why the plain cookie value is passed when securecookie is enabled.
i reused the example from https://github.com/kataras/iris/issues/1877
should i avoid using?
app.Use(sess.Handler())
I tried to verify the issue why sometimes sessions are dropped (still working on an example) so i found the issue behind it that plain text session values are passed to securecookie.
You should use (app.Use(sess.Handler()) and sessions.Get) or Start - not both. I can't re-produce the issue. I see encoded values on securecookie.Decode method:
SecureCookie.Encode: name = session_id_cookie, value = "6ef75d2a-adba-4c77-a9e9-645548786364"
SecureCookie.Decode: name = session_id_cookie, value = MTY1Nzk5MTI1N3xwOUdRbDBRS2NWbWVFcmZCcTdfU3REWDEzYmp5Tm83VTRmbko4eDFpSnd4SjBZclJHYzI2VXMybmpFVmRzY2JlREhOMTNWU2R4ZU09fFR9KtqAGcJRW_xQJSmr2pkFjyfxAE-64q2SSYdM582w
SecureCookie.Decode: name = session_id_cookie, value = MTY1Nzk5MTI1N3xwOUdRbDBRS2NWbWVFcmZCcTdfU3REWDEzYmp5Tm83VTRmbko4eDFpSnd4SjBZclJHYzI2VXMybmpFVmRzY2JlREhOMTNWU2R4ZU09fFR9KtqAGcJRW_xQJSmr2pkFjyfxAE-64q2SSYdM582w
@mblaschke the code part, the securecookie's Decode is called is at: https://github.com/kataras/iris/blob/cf42f37169c43b5bc7b5911614ba51da403b38e9/context/context.go#L5314-L5318
If the decode is failed, the cookie value is set to empty string for security reasons. This may be happening if the previous Encode is not called for some reason, probably because you call Destroy and Start in the same handler for the same request-response lifecycle.
@mblaschke this is a working example which destroys and re-initializes the session in the same request-response lifecycle (I don't recommend it) but here you are:
package main
import (
"fmt"
"net/http"
"github.com/gorilla/securecookie"
"github.com/kataras/iris/v12"
"github.com/kataras/iris/v12/sessions"
)
const cookieNameForSessionID = "session_id_cookie"
func secret(ctx iris.Context) {
session := getSession(ctx, false)
// Check if user is authenticated
if auth, _ := session.GetBoolean("authenticated"); !auth {
ctx.StatusCode(iris.StatusForbidden)
ctx.ContentType("text/html")
ctx.WriteString(`Not logged in or session invalid <a href="/logout">logout</a>`)
return
}
// Print secret message
ctx.ContentType("text/html")
ctx.WriteString(`The cake is a lie! <a href="/logout">logout</a>`)
}
func login(ctx iris.Context) {
// Authentication goes here
// ...
// Set user as authenticated
session := getSession(ctx, true)
session.Set("authenticated", true)
ctx.StatusCode(http.StatusTemporaryRedirect)
ctx.ContentType("text/html")
ctx.Header("Location", "/secret")
}
func logout(ctx iris.Context) {
sessions.Get(ctx).Destroy()
ctx.ContentType("text/html")
ctx.WriteString(`<a href="/login">login</a>`)
}
func getSession(ctx iris.Context, recreate bool) *sessions.Session {
// cookieOptionList := []context.CookieOption{
// func(ctx *context.Context, cookie *http.Cookie, op uint8) {
// // c.applySessionSetting(ctx, cookie, op)
// },
// }
if recreate {
sessions.Get(ctx).Destroy()
fmt.Println("getSession.recreate: destroy called")
}
// s := sess.Start(ctx, cookieOptionList...)
// return s
return sessions.Get(ctx)
}
func main() {
app := iris.New()
secureCookie := securecookie.New(
[]byte("IGVneS5eZ0b5jTgwe0l38nON0bW5awxr"),
[]byte("r4GFiLvBtYCohUxt"),
)
sess := sessions.New(sessions.Config{
Cookie: cookieNameForSessionID,
Encoding: secureCookie,
CookieSecureTLS: false,
AllowReclaim: true,
DisableSubdomainPersistence: true,
})
app.Use(sess.Handler())
app.Get("/secret", secret)
app.Get("/login", login)
app.Get("/logout", logout)
app.Listen(":8080")
}
It's the same but you use app.Use/sessions.Get(ctx) and sessions.Get(ctx).Destroy instead.
@mblaschke,
I found the origin of the "issue". This is happening because the login handler has decoded the request cookie's value already - but this is not a problem, the server never sends the decoded value back to the client.
I can confirm the session id is not send back to the client but the session is invalidated at that point.
The reason for recreation of session is https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#renew-the-session-id-after-any-privilege-level-change
Also I want to change cookies samesite mode from lax (for oauth login) to strict (for application).
Hello @mblaschke,
Also I want to change cookies samesite mode from lax (for oauth login) to strict (for application).
That's easy, you can do it using a middleware (for all cookies) and cookie options via ctx.AddCookieOptions or pass the cookie option(s) when calling the sessions.Handler method. There is a builtin cookie option called CookieSameSite in the context sub-package but you can define your own, example:
var strictSameSiteOpt = func(_ iris.Context, c *http.Cookie, op uint8) {
if op == 1 /* context.OpCookieSet */ {
c.SameSite = http.SameSiteStrictMode
}
}
sess := sessions.New(sessions.Config{
Cookie: cookieNameForSessionID,
Encoding: secureCookie,
CookieSecureTLS: false,
AllowReclaim: true,
DisableSubdomainPersistence: true,
})
app.Use(sess.Handler(strictSameSiteOpt))
The reason for recreation of session is https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#renew-the-session-id-after-any-privilege-level-change
Can you run this and provide some feedback? It works here:
package main
import (
"github.com/gorilla/securecookie"
"github.com/kataras/iris/v12"
"github.com/kataras/iris/v12/sessions"
)
var strictSameSiteOpt = func(_ iris.Context, c *http.Cookie, op uint8) {
if op == 1 /* context.OpCookieSet */ {
c.SameSite = http.SameSiteStrictMode
}
}
var sessManager *sessions.Sessions
const sessionCookieName = "session_id"
func main() {
app := iris.New()
secureCookie := securecookie.New(
[]byte("IGVneS5eZ0b5jTgwe0l38nON0bW5awxr"),
[]byte("r4GFiLvBtYCohUxt"),
)
sessManager = sessions.New(sessions.Config{
Cookie: sessionCookieName,
Encoding: secureCookie,
CookieSecureTLS: false,
AllowReclaim: true,
DisableSubdomainPersistence: true,
})
app.Use(sessManager.Handler(strictSameSiteOpt))
app.Get("/login", login)
app.Get("/logout", logout)
app.Get("/secret", secret)
app.Listen(":8080")
}
func login(ctx iris.Context) {
// Authentication goes here
session := sessions.Get(ctx)
// ...
oldSessionID := session.ID()
ctx.Application().Logger().Infof("old session id = %s", oldSessionID)
// Set user as authenticated
// If AllowReclaim is false:
// manually remove the request cookie, so the new Start creates a new session:
// ctx.Request().Header.Del(sessionCookieName)
// Use session's manager instead of session.Destroy so
// it destroys the response AND request (if AllowReclaim is true)
// cookie information.
sessManager.Destroy(ctx)
session = sessManager.Start(ctx, strictSameSiteOpt)
session.Set("authenticated", true)
ctx.Application().Logger().Infof("new session id = %s", session.ID())
ctx.Redirect("/secret", iris.StatusTemporaryRedirect)
}
func logout(ctx iris.Context) {
sessManager.Destroy(ctx)
ctx.HTML(`<a href="/login">login</a>`)
}
func secret(ctx iris.Context) {
session := sessions.Get(ctx)
// Check if user is authenticated.
if auth, _ := session.GetBoolean("authenticated"); !auth {
ctx.StatusCode(iris.StatusForbidden)
ctx.HTML(`Not logged in or session invalid <a href="/logout">logout</a>`)
return
}
// Print secret message.
ctx.HTML(`The cake is a lie! <a href="/logout">logout</a>`)
}
It's working but i found a way to reproduce (but not with an example app) it so i'm currently digging into the iris code (12.2.0-beta4) to find a clue what's happening.
If the user doesn't have a session, everything is fine.
When the user is redirected to oauth provider and I restart the app (using internal session storage) the login fails.
with the error message i'm using c.session.Destroy(ctx) to destroy the session but the session is NOT destroyed and instead creates a new session:
Set-Cookie: sid=29b213ad-e8a4-4208-a0d7-d4f5e5f76500; Path=/; Expires=Sat, 23 Jul 2022 17:12:36 GMT; Max-Age=7199; HttpOnly; SameSite=Strict
on the login page (where i start oauth and redirect to user to the provider) i then get a for whatever reason:
Set-Cookie: devconsole-sid=; Path=/; Expires=Tue, 10 Nov 2009 23:00:00 GMT; Max-Age=0; HttpOnly
so the user is trapped in an endless invalid session loop.
also app.Use(sessManager.Handler(strictSameSiteOpt)) sets session ids for app.HandleDir requests which should never happen. I guess that's a major issue because that recreates a strict cookie again.
while trying several things sometimes i encounter a invalid memory address or nil pointer dereference:
runtime error: invalid memory address or nil pointer dereference
/opt/homebrew/Cellar/go/1.18.4/libexec/src/runtime/panic.go:838
/opt/homebrew/Cellar/go/1.18.4/libexec/src/runtime/panic.go:220
/opt/homebrew/Cellar/go/1.18.4/libexec/src/runtime/signal_unix.go:818
/app/vendor/github.com/kataras/iris/v12/sessions/session.go:63
/app/vendor/github.com/kataras/iris/v12/sessions/session.go:135
to fix the endless invalid session loop i've moved RemoveCookie up before the cookieValue check. this at least solved to kill the invalid session.
github.com/kataras/iris/v12/sessions/sessions.go:
// Destroy removes the session data, the associated cookie
// and the Context's session value.
// Next calls of `sessions.Get` will occur to a nil Session,
// use `Sessions#Start` method for renewal
// or use the Session's Destroy method which does keep the session entry with its values cleared.
func (s *Sessions) Destroy(ctx *context.Context) {
cookieValue := s.getCookieValue(ctx, nil)
ctx.RemoveCookie(s.config.Cookie, s.cookieOptions...)
if cookieValue == "" { // nothing to destroy
return
}
ctx.Values().Remove(sessionContextKey)
//ctx.RemoveCookie(s.config.Cookie, s.cookieOptions...)
s.provider.Destroy(cookieValue)
}
found one issue:
i've forgot to enable AllowReclaim. Then it's working but only without securecookie 🤔
also app.Use(sessManager.Handler(strictSameSiteOpt)) sets session ids for app.HandleDir requests which should never happen. I guess that's a major issue because that recreates a strict cookie again.
It should happen if you have allow it, you should register the middleware after the app.HandleDir to prevent it from running to your app.HandleDir.
i've forgot to enable AllowReclaim. Then it's working but only without securecookie 🤔
You mean that the loop you said, is still valid? If so, could you please post an example with steps to re-produce it and test the solution by myself?