sso icon indicating copy to clipboard operation
sso copied to clipboard

Support alternative cookie storage

Open sporkmonger opened this issue 6 years ago • 12 comments

Azure AD has massive tokens. It's obnoxious and unnecessary, but Microsoft isn't going to change this. Cookie compression helps, and cookie spanning solves most of the rest of the problem, but I still find that there are edge cases where it's a problem. For implementors that have these issues, I think it would be beneficial to just simply stop trying to do stateless session storage. If you can't reliably fit everything into 4kb, you really cannot guarantee that you won't have sign-in failures. Sometimes the failures are from browsers, sometimes it's from load balancers and other proxies enforcing their own limits. Regardless of where the limit is enforced, unfortunately, the user experience for a cookie size limit failure is bad.

I think SSO should support usage of a Redis backend. It ought to be pretty straightforward to point SSO to e.g. Elasticache Redis or an equivalent hosted option for most people who run into this issue, and that's going to be much more reliable than trying to jam the whole session directly into cookie state.

sporkmonger avatar Feb 16 '19 04:02 sporkmonger

@sporkmonger see also #141 for a discussion on the matter :)

victornoel avatar Feb 26 '19 08:02 victornoel

Hi there,

Is anyone interested on working on this ? I'd do it myslef unfortunately I have no experience with Redis nor SSO knowledge.

Regards.

sylr avatar Mar 26 '19 19:03 sylr

Hi @sylr I don't believe anyone is working on this at the moment.

We'd be happy to accept and work through any contributions on this front!

jphines avatar Mar 26 '19 21:03 jphines

This commit has an implementation of a Redis-backed cookie store I implemented in a fork of pusher/oauth2_proxy:

https://github.com/lsst-dm/oauth2_proxy/commit/29494561251efc7b1f066ef45455eb934630bcef

This implementation introduces a ServerCookiesStore interface:

type ServerCookiesStore interface {
	Store(responseCookie *http.Cookie, requestCookie *http.Cookie) (string, error)
	Clear(requestCookie *http.Cookie) error
	Load(requestCookie *http.Cookie) (string, error)
}

To which there's one implementation, a RedisCookiesStore.

This implementation introduces the concept of a Ticket, which is composed of three parts:

{cookieName}-{uniqueId}.{iv}

Where: * cookieName is the same name configured to store the cookies in the browser.

  • uniqueId is a random ID - it's found by just taking the sha1 of the cookie, but it could just be a unique ID/UUID or something * iv is the initialization vector used to encrypt/decrypt the cookie in redis

On Store, it takes the cookie it would have sent to be browser, encrypts it, and sends ticket back as the new cookie value back (It could potentially just return a cookie, but it doesn't have access to makeCookie). Only the client has the initialization vector - this isn't perfect security, but it means the Redis store can't be SCAN'd to dump all the sessions, so this is roughly equivalent to the security oauth2_proxy has by storing the encrypted cookies in the browser. It also stores the cookie in Redis with SETEX.

In LoadCookiedSession in oauthproxy.go, if there's a ServerCookiesStore, it will perform try a Load instead to the cookie store with the ticket, using the handle and then decrypting with the initialization vector (which is URL encoded base64)

I'm not sure if I will have time to get this into this repo, because I'm working with pusher/oauth2_proxy, but I've been using this with success (although I just did a rebase, and I need to test it some more).

brianv0 avatar Apr 24 '19 02:04 brianv0

In the pusher fork of oauth2_proxy, a new session storage interface was implemented, along with a Redis session storage interface. Additional interfaces could be implemented, probably using the Redis interface as a template.

It might be a decent chunk of work to merge these as development in the pusher/oauth2_proxy fork has been active lately, but these pull requests can give you an idea on what was changed:

https://github.com/pusher/oauth2_proxy/pull/147 https://github.com/pusher/oauth2_proxy/pull/148 https://github.com/pusher/oauth2_proxy/pull/155

brianv0 avatar Jun 05 '19 17:06 brianv0

I'm starting to work on this now, will take a close look at the Redis stuff @brianv0 mentioned.

sporkmonger avatar Jun 19 '19 20:06 sporkmonger

I probably won't make this change in the upcoming PR, but I think there's a good case to be made to rename CookieSecret/COOKIE_SECRET to SessionSecret/SESSION_SECRET subsequent to this PR. Also CookieCipher to SessionCipher. The RedisStore will still use a cookie, but the way in which the cipher is deployed is different as the cookie value is now plaintext (opaque random token) but the server side value is encrypted.

sporkmonger avatar Jun 26 '19 21:06 sporkmonger

@sporkmonger that makes sense -- we've done some work to change up the configuration on the authenticator side with the intent that it would be easier to extend in the future.

We haven't gotten to doing that yet on the proxy side. You might take a look there to get some idea of where the config options will be going, which should hopefully make it easier to supply configuration for various different session storage backends.

jphines avatar Jun 27 '19 19:06 jphines

I initially started porting the code @brianv0 mentioned, but ended up finding it hard to reason about the security guarantees it seemed to be trying to make. Opted to write from scratch with a simpler approach that should be easier to understand and reason about.

sporkmonger avatar Jun 27 '19 20:06 sporkmonger

The main security guarantee is that

  • Each session is individually encrypted in Redis
  • The key to encrypt a session exists solely in the user's cookie, in the form of a ticket
  • Sessions are retrieved and decrypted on-demand using the cookie (ticket)

The CookieSecret/session secret could actually be ignored in this scheme, but it's used to (somewhat unnecessarily) encrypt the tokens of a session object when serializing the session. That is necessary when storing the session in a cookie, and we reuse the serialization methods. The cookie secret is also used to sign the cookies in both scenarios (again, one cookie is the stored session in the normal mode, and in Redis store mode the cookie is a ticket), but I don't think the signature is actually necessary for the Redis mode.

It may be more complicated than necessary. When I was implementing this, there was some concern that storing all sessions (and thus, tokens) in Redis without the sessions being individually encrypted would introduce operational risk that doesn't exist in when storing the sessions in cookies, namely that having access to Redis and the CookieSecret allows you to decrypt all sessions stored in Redis.

brianv0 avatar Jun 28 '19 14:06 brianv0

Took some doing, but I got Redis sessions working. PR coming soon.

sporkmonger avatar Jul 12 '19 00:07 sporkmonger

@jphines Some minor feedback: The mapstructure / go-micro config stuff is way too magical with way too many non-obvious gotchas. If it wasn't for dlv I'm not sure I ever would have realized what was going wrong. At the very least, needs the weird stuff documented, like "thou shalt not use multi-word configuration keys".

sporkmonger avatar Jul 12 '19 00:07 sporkmonger