fasthttp / session Goto Github PK

Using the memory provider a couple users are reporting an index out of range issue. Wondering if you have any idea what we can do to figure out the underlying cause?

authelia/authelia#3751

In the context of testing a handler performing a Regenerate, I'd like to be able to Get the new store (after regenerate) from the testing code but that does not work because the Get method retrieves the old instance of store instead.

This issue is due to the fact that Get retrieves the session ID from the request cookie

@savsgio , what do you think?

Hi @savsgio , it's me again.
I set expiration as 10 * time.Minute and my provider(Docker container with image mariadb/10.4.12-bionic ) stored it as 600.
but setting 'last_active' as nano made SQLGC's where clausemismatch, so my session doesn't keep alive. Below is my session table.

mariadb root@localhost:dev> select * from session;
+----------------------------------+--------------------------------------------------------------------------------------------------+---------------------+--------------+
| id | data | last_active | expiration |
|----------------------------------+--------------------------------------------------------------------------------------------------+---------------------+--------------|
| nqMriPTLKiLRguvobaRoaRBdWgGWtzxB | gaFEkoKjS2V5xANmb2+lVmFsdWWjYmFygqNLZXnEGl9fc3RvcmU6ZXhwaXJhdGlvbjp0eVlTQl9fpVZhbHVl0wAAAIuyyXAA | 1588249643693065881 | 600 |
+----------------------------------+--------------------------------------------------------------------------------------------------+---------------------+--------------+
1 row in set
Time: 0.004s
mariadb root@localhost:dev>

Using the example provided and settingcfg.Expiration to -1 causes the following error to appear when loading any of the pages.
Error 1264: Out of range value for column 'expiration' at row 1

Setting it to 0 turns it into a session cookie. While any positive number works as intended.

Visiting /getAll still outputs to console so I'm assuming it must be part of the saving process.

defer func() {
	if err := serverSession.Save(ctx, store); err != nil {
		ctx.Error(err.Error(), fasthttp.StatusInternalServerError)
	}
}()

Behavior

Environment

I'm getting this error when forked. Anyone tried? I only use the "memory" option for storage.

2019/06/09 18:25:26 session set provider error, memory not registered!

How to resolve?

@savsgio , when fasthttp server is behind a proxy and there is no TLS link between the proxy and the server because let say the server and the proxy are on the same host, the secure flag of the cookie should legitimately be set to true to enhance security.
I propose to add a configuration attribute called TrustedProxies being a list of strings being known and trusted IPs of proxies in order to mimic the behavior of the Express framework as described in this documentation: http://expressjs.com/en/guide/behind-proxies.html.

So what I propose is to check whether the connection is over TLS || (the remote IP is a trusted proxy && X-Forwarde-Proto is set to https) by modifying the following line:

Would that sound reasonable to you?

Hi I found an error at 'Save' method of 'session.go while I was using this package'.
I added log.Println and panic like code below and got log below.
It is also happens when I use 'memory' as provider. I think tinylib/msgp should be ready for time.Duration treating it as int64 or session to use int64 instead of time.Duration.

I think returning nil when there is an error is inappropriate too. '30s setting' didn't work every time and '2hours' was returned constantly and it made me to take some time to find out like this.

Thank you for such a nice library.

data, err := s.config.EncodeFunc(store.GetAll())
	if err != nil {
		log.Println(err)
		panic(err)
		return nil
	}

2020/04/29 20:16:25 Starting example with provider: mysql
2020/04/29 20:16:25 Session example server listen: http://0.0.0.0:8086
2020/04/29 20:16:32 msgp: type "time.Duration" not supported at D/0/Value
panic: msgp: type "time.Duration" not supported at D/0/Value

goroutine 22 [running]:
github.com/fasthttp/session/v2.(*Session).Save(0xc000286000, 0xc0002b8000, 0xc00021a080, 0x0, 0x0)
/home/yeongju/go/pkg/mod/github.com/fasthttp/session/[email protected]/session.go:191 +0x369
main.setExpirationHandler(0xc0002b8000)
/home/yeongju/go/src/github.com/979156/shrello/handler.go:190 +0x15d
github.com/fasthttp/router.(*Router).Handler(0xc0001e0180, 0xc0002b8000)
/home/yeongju/go/pkg/mod/github.com/fasthttp/[email protected]/router.go:443 +0xefd
github.com/valyala/fasthttp.(*Server).serveConn(0xc00029a000, 0x849840, 0xc0001ee060, 0x0, 0x0)
/home/yeongju/go/pkg/mod/github.com/valyala/[email protected]/server.go:2087 +0x567
github.com/valyala/fasthttp.(*workerPool).workerFunc(0xc0002ac000, 0xc000282080)
/home/yeongju/go/pkg/mod/github.com/valyala/[email protected]/workerpool.go:223 +0xc0
github.com/valyala/fasthttp.(*workerPool).getCh.func1(0xc0002ac000, 0xc000282080, 0x73b200, 0xc000282080)
/home/yeongju/go/pkg/mod/github.com/valyala/[email protected]/workerpool.go:195 +0x35
created by github.com/valyala/fasthttp.(*workerPool).getCh
/home/yeongju/go/pkg/mod/github.com/valyala/[email protected]/workerpool.go:194 +0x101

While testing my implementation, I found out a potential security issue at HEAD (or maybe it's a feature). I found out that when a user saves data in the store, the latest activity is marked with the current timestamp but the cookie expiration time is not changed accordingly. Therefore, it might happen that a cookie expires at some point and the data related to is still present in the store and retrievable from the session ID.

Let say the generator of session ID has not enough randomness and the size is not big enough to avoid collisions, then an attacker could exploit a system by regenerating as many session as possible and eventually recover a store with some data from an expired cookie (because the new cookie has the same ID as a previously generated cookie).

In order to be safe it would be better to not update the last activity to let the data expires with the cookie.

Hi @savsgio, could you tag a new release containing the SameSite fix?

Thank you, keep up the great work!

It'll be really cool if you could add the capability to set a custom session like we have in PHP with the session_id() function.
This is especially useful in a non-browser context, like server-to-server communication with a need for session management based on a custom identifier.

Hi, @savsgio !
Can you add a version of Get that doesn't attempt to regenerate a new id and returns a *Store only if it is present? Or make getSessionID method public?

I can wait for a late reply and submit a pr, if needed.

Hi @savsgio, I noticed that when we use the MYSQL provider the table gets dropped on initialization ( https://github.com/fasthttp/session/blob/master/providers/mysql/provider.go#L13 ).

So it's not possible to save session data in case of a server crash, I'm wondering if this is a design choice?

Thank you!

Just letting you know we solved this issue upstream by implementing our own session ID generator, but figure there is likely an underlying cause you may want to investigate:

authelia/authelia#2349

Basically looks like under some conditions is it causes an index out of range panic, I suspect heavy load.

how can i make it base64url friendly? sorry i can't seem to find the line to change it.

From what I can tell from the upstream library, you need to instantiate a client with NewClusterClient/NewSentinelClient in order to setup connections to these types, I've not confirmed this. But was wondering if you either knew before I worked on supporting something like this, and if you had a preference for how we would set it up.

I think realistically it would be better to make it their own providers since the options are vastly different for Sentinel and for Cluster configurations.

Panic at https://github.com/fasthttp/session/blob/master/internal/sql/provider.go#L150 should be removed to avoid crashing production services, when the error can just be ignored silently.

Hi there, I had a question to maybe implement multiple additional session providers.
I am asking your opinion to add the providers listed below.

• etcd - etcd-io/etcd (https://etcd.io)
• LevelDB - syndtr/goleveldb (the official golang repository is a WIP; golang/leveldb)
• BadgerDB - dgraph-io/badger

Thanks!

I have not yet 100% confirmed the version this occurred in, but I think it was the upgrade from 1.1.8 to 2.0.0.

Basically if you use the redis provider in a mutli-instance app in something like kubernetes the redis provider does not seem to obtain the correct session expiration (after using Store.SetExpiration() and Session.Save())from the redis store, leading to the situation where your session expiration is not the same between sessions.

I have also played with using Traefik to enable sticky load balancing. In this instance as soon as the load balancer cookie expires, and the client his a new pod, it appears as if the second pod is completely unaware of the change to the expiration.

To me it makes sense the session expiration should be available to all instances, but maybe I'm incorrect?

instead of the "random" generator
would like to use
"github.com/oklog/ulid"
do u know how to get it done elegantly? can this be an enhancement feature request?

Please immediately declare the copyright and the license statement! !

I found that the project is copying my project, but there is no authorization from me ！！！

Please be sure to include a author copyright notice and a license notice！！！

My project: https://github.com/phachon/fasthttpsession （I found that you have a PR in my project !）

Please, can you include a way to set: "SameSite=Strict"?

Thanks.

It seems the int value is not stored correctly in session.
When serving with the following test code, GET from /, the value of session['incr'] is expected to increment each time, but it always keep the same.

The code uses sqlite3 as the provider.

Is the code written wrongly or session does not support int type, or it is a bug of the session provider?

Please have a look at this code:

package main

import (
    "fmt"
    "log"

    "github.com/fasthttp/router"
    "github.com/fasthttp/session/v2"
    "github.com/fasthttp/session/v2/providers/sqlite3"
    "github.com/valyala/fasthttp"
)

var serverSession *session.Session

func incrHandler(ctx *fasthttp.RequestCtx) {
    store, err := serverSession.Get(ctx)
    if err != nil {
        ctx.Error(err.Error(), fasthttp.StatusInternalServerError)
        return
    }   
    defer func() {
        if err := serverSession.Save(ctx, store); err != nil {
            ctx.Error(err.Error(), fasthttp.StatusInternalServerError)
        }   
    }() 

    cval,ok := store.Get("incr").(int)
    if ok {
        cval++
    }else{
        log.Println("get incr failed,cval",cval)
        cval = 1 
    }   

    store.Set("incr", cval)

    sessionID := store.GetSessionID()
    ctx.SetBodyString(fmt.Sprintf("Session(%s) SET: incr='%d' --> OK", sessionID, store.Get("incr").(int)))
}

func init() {
    provider, err := sqlite3.New(sqlite3.NewConfigWith("test.db", "session"))
    if err != nil {
        log.Fatal(err)
    }

    cfg := session.NewDefaultConfig()
    cfg.EncodeFunc = session.Base64Encode
    cfg.DecodeFunc = session.Base64Decode
    serverSession = session.New(cfg)

    if err = serverSession.SetProvider(provider); err != nil {
        log.Fatal(err)
    }
}

func main() {
    r := router.New()
    r.GET("/", incrHandler)

    addr := "0.0.0.0:8086"
    log.Println("Now Listen on %s",addr)

    err := fasthttp.ListenAndServe(addr, r.Handler)
    if err != nil {
        log.Fatal(err)
    }
}

can you provide more example? i realised there are a lot of things that can be done for configuration setting but it's not written.

how to make the cookie domain be .domain.com? it's domain.com now.

any example on how to do cross cookie domain?

I have a feeling it's just going to be more logical to implement this downstream rather than trying to implement it in fasthttp/session, but I figured I'd ask what you think anyway - plus, maybe this idea is really desirable for other users.

Authelia is currently using the memory/redis providers of this lib. Basically what we will be intending to do is give users the ability to lookup their active sessions as well as information about the sessions and optionally destroy them. The complication is that we'd have to store and keep track of ID's belonging to each user which likely is fairly impractical.

My only idea in working around this is to implement additional funcs in the interface, that allow getting a list of session ID's belonging to a specific lookup value, which would also require we add funcs to save with the lookup value. Additionally ideally it would include a specific EncodeFunc to encode the lookup key, though this reasonably can be done locally too.

In the example below GetWithUserIdentifier would return all session data for a given UserIdentifier, GetSessionIDs would return a list of session ID's for a given UserIdentifier to be used with Get, SaveWithUserIdentifer would obviously operate the same as Save except it would include the UserIdentifier which would be required to use the other funcs, DestroyWithUserIdentifier would destroy the given session with the given ID provided the given UserIdentifier matched.

type Provider interface {
	Get(id []byte) ([]byte, error)
	GetWithUserIdentifier(userIdentifier []byte) (sessions [][]byte, error)
	GetSessionIDs(userIdentifier []byte) (ids [][]byte, error)
	Save(id, data []byte, expiration time.Duration) error
	SaveWithUserIdentifier(id, userIdentifer, data []byte, expiration time.Duration) error
	Destroy(id []byte) error
	DestroyWithUserIdentifier(id, userIdentifer) error
	Regenerate(id, newID []byte, expiration time.Duration) error
	Count() int
	NeedGC() bool
	GC() error
}

the current redis implementation with go-redis is very limited. possible to change to github.com/gomodule/redigo
?