Custom cookie name breaks redis for session #978

thekoma · 2021-01-07T15:50:52Z

Expected Behavior

Login succeed

Current Behavior

If you specify a custom cookie name while using redis to manage sessions we are unable to decrypt the cookie:

oauth2-proxy      | [2021/01/07 15:44:47] [stored_session.go:75] Error loading cookied session: failed to decode ticket, removing session
oauth2-proxy      | [2021/01/07 15:44:47] [stored_session.go:78] Error removing session: error decoding ticket to clear session: failed to decode ticket
oauth2-proxy      | [2021/01/07 15:44:47] [oauthproxy.go:506] Error clearing session cookie: error decoding ticket to clear session: failed to decode ticket
oauth2-proxy      | 172.18.0.10 - - [2021/01/07 15:44:47] sonarr.koma.link GET - "/oauth2/sign_in" HTTP/1.1 "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.107 Safari/537.36" 500 394 0.000
oauth2-proxy      | [2021/01/07 15:44:47] [stored_session.go:75] Error loading cookied session: http: named cookie not present, removing session

Possible Solution

Ignore the cookie name parameter when session is managed by redis
Decrypt using the correct cookie name

The text was updated successfully, but these errors were encountered:

JoelSpeed · 2021-01-07T15:58:25Z

This definitely used to work as I've been using it in the past for production instances, could you confirm which version of the proxy you are using?

thekoma · 2021-01-07T20:47:32Z

@JoelSpeed compiled the master a couple hour before filling the bug

NickMeves · 2021-01-09T03:26:22Z

Hmmm - this cookie change just merged a few days ago: #970

Regression potentially?

If you built from commits before that PR merged, do you still get this bug?

NickMeves · 2021-01-10T18:30:03Z

So based on your error message, you have a valid cookie at this point that passed signature validation. But the inner content isn't formatted correctly so decodeTicket fails:

oauth2-proxy/pkg/sessions/persistence/ticket.go

Line 70 in 597ffeb

    
           func decodeTicket(encTicket string, cookieOpts *options.Cookie) (*ticket, error) {

Do you have a sample on what your cookie value looked like?

Ticket sessions have 2 parts in the cookie split by a . - the Ticket ID & the Ticket Secret. The Ticket ID is used as the key in your store to find the encrypted and encoded session object in your data store. The Ticket Secret is the decryption key (makes it so a compromised session store can't steal sessions, it has a per session secret that is stored with the uses in their cookies).

NickMeves · 2021-01-10T18:35:03Z

oh I wonder... Does your custom name have a . in it?

NickMeves · 2021-01-10T18:51:46Z

That would then make the encoded ticket value have more than 2 entries after a split on . and give that error message you saw. Since the Ticket ID includes the cookie name as a base (either for debugging help or to really ensure no conflicts in a shared redis DB even though 16 byte random keys shouldn't collide probabilistically -- I don't know the original design intent).

ticketID := fmt.Sprintf("%s-%s", cookieOpts.Name, hex.EncodeToString(rawID))

Would explain your error message from this:

ticketParts := strings.Split(encTicket, ".")
if len(ticketParts) != 2 {
	return nil, errors.New("failed to decode ticket")
}

thekoma · 2021-01-11T18:19:17Z

yes it has

NickMeves · 2021-01-11T18:49:40Z

Cool thanks - that's the issue. At the moment I recommend you just use a different character in your custom cookie name to get over this issue.

Long term, I've tagged this as a bug for us to fix the encoding scheme so it is aware of . in the cookie name. This change will need to be aware of legacy & new encoding schemes simultaneously so it isn't a breaking change for existing Redis users.

thekoma · 2021-01-11T19:30:29Z

Cool! Thank you

github-actions · 2021-03-13T00:15:38Z

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

thekoma · 2021-03-15T08:50:38Z

This is still a bug @NickMeves ?

github-actions · 2021-05-15T00:04:19Z

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

github-actions · 2021-07-16T00:03:15Z

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

github-actions · 2021-09-15T00:03:18Z

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

thekoma · 2021-09-15T06:59:22Z

Is this still interesting?

JoelSpeed · 2021-09-15T08:14:04Z

Yeah we still need to get round to finding a fix for this 🙂

github-actions · 2021-11-15T00:04:19Z

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

thekoma · 2023-07-18T08:01:34Z

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

. This issue gas a pr ti dolce the problema that is still there 3y layer.

github-actions · 2023-11-17T00:06:23Z

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

thekoma · 2023-11-17T12:26:51Z

/unstale

tuunit · 2023-11-18T10:03:56Z

Hi @thekoma,

I've never looked into this issue before. I would like to give this a fresh look. Could you provide the following details:

Your configuration for oauth2-proxy and redis. Which version this occurs on and if you have tested it with the latest version?

thekoma · 2023-11-18T10:12:57Z

I don't use oauth2proxy at the moment the issue is 3 years old 😅.
I can check for the old config maybe.

thekoma · 2023-11-18T10:21:04Z

Just checked and the config is lost in time. If it's necessary to proceed I will need time to produce it again.

* Issue 978: Fix Custom cookie name breaks redis for session (see #978) * Issue 978: Fix Custom cookie name breaks redis for session (see #978) * Update CHANGELOG.md * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Update CHANGELOG.md --------- Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

…oxy#1949) * Issue 978: Fix Custom cookie name breaks redis for session (see oauth2-proxy#978) * Issue 978: Fix Custom cookie name breaks redis for session (see oauth2-proxy#978) * Update CHANGELOG.md * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Update CHANGELOG.md --------- Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

* Add GitHub groups (orgs/teams) support * align code of getTeams with getOrgs to support Github Enterprise Server instances with different domain * add documentation * add missing import after rebase * add nightly build and push (#2297) * add nightly build and push * add date based nightly build tags * only keep single multiarch image build and push * add changelog * add images to internal docs static files * add docu for nightly builds * remove unnecessary spaces * update nightly repository * Issue 978: Fix Custom cookie name breaks redis for session (#1949) * Issue 978: Fix Custom cookie name breaks redis for session (see #978) * Issue 978: Fix Custom cookie name breaks redis for session (see #978) * Update CHANGELOG.md * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Update CHANGELOG.md --------- Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Support http.AllowQuerySemicolons (#2248) * Support http.AllowQuerySemicolons * Docs * Make it clear we are overriding the handler * Update documentation for allow-query-semicolons * Fix changelog format * Fix formatting --------- Co-authored-by: MickMake <github@mickmake.com> * Add GitHub groups (orgs/teams) support * align code of getTeams with getOrgs to support Github Enterprise Server instances with different domain * add documentation * fix changelog & documentation * fix missing import --------- Co-authored-by: Tobias Mayer <github@tobiasm.de> Co-authored-by: Nuno Miguel Micaelo Borges <miguelborges99@gmail.com> Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> Co-authored-by: Tim White <tim.white@su.org.au> Co-authored-by: MickMake <github@mickmake.com>

github-actions · 2024-01-25T00:06:39Z

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

thekoma · 2024-01-25T08:27:44Z

/unstale

github-actions · 2024-03-29T00:06:31Z

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

JoelSpeed added the bug label Jan 7, 2021

Christoph-Raab mentioned this issue Jan 19, 2021

Error loading cookied session: http: named cookie not present, removing session #998

Closed

github-actions bot added the Stale label Mar 13, 2021

JoelSpeed removed the Stale label Mar 15, 2021

github-actions bot added the Stale label May 15, 2021

JoelSpeed removed the Stale label May 16, 2021

github-actions bot added the Stale label Jul 16, 2021

JoelSpeed removed the Stale label Jul 16, 2021

github-actions bot added the Stale label Sep 15, 2021

JoelSpeed removed the Stale label Sep 15, 2021

github-actions bot added the Stale label Nov 15, 2021

github-actions bot closed this as completed Nov 23, 2021

JoelSpeed removed the Stale label Nov 23, 2021

github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 18, 2023

thekoma mentioned this issue Jul 18, 2023

Please reopen #978 #2164

Closed

tuunit reopened this Sep 17, 2023

tuunit removed the Stale label Sep 17, 2023

shaheislam mentioned this issue Oct 12, 2023

500 Internal Server Error Switching between Prometheus & Alertmanager #2267

Closed

github-actions bot added the Stale label Nov 17, 2023

braunsonm removed the Stale label Nov 17, 2023

tuunit added help wanted configuration labels Nov 18, 2023

github-actions bot added the Stale label Jan 25, 2024

github-actions bot removed the Stale label Jan 27, 2024

github-actions bot added the Stale label Mar 29, 2024

github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 5, 2024

tuunit removed the Stale label Apr 5, 2024

tuunit reopened this Apr 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom cookie name breaks redis for session #978

Custom cookie name breaks redis for session #978

thekoma commented Jan 7, 2021

JoelSpeed commented Jan 7, 2021

thekoma commented Jan 7, 2021

NickMeves commented Jan 9, 2021

NickMeves commented Jan 10, 2021

NickMeves commented Jan 10, 2021

NickMeves commented Jan 10, 2021

thekoma commented Jan 11, 2021

NickMeves commented Jan 11, 2021

thekoma commented Jan 11, 2021

github-actions bot commented Mar 13, 2021

thekoma commented Mar 15, 2021

github-actions bot commented May 15, 2021

github-actions bot commented Jul 16, 2021

github-actions bot commented Sep 15, 2021

thekoma commented Sep 15, 2021

JoelSpeed commented Sep 15, 2021

github-actions bot commented Nov 15, 2021

thekoma commented Jul 18, 2023

github-actions bot commented Nov 17, 2023

thekoma commented Nov 17, 2023

tuunit commented Nov 18, 2023

thekoma commented Nov 18, 2023

thekoma commented Nov 18, 2023

github-actions bot commented Jan 25, 2024

thekoma commented Jan 25, 2024

github-actions bot commented Mar 29, 2024

Custom cookie name breaks redis for session #978

Custom cookie name breaks redis for session #978

Comments

thekoma commented Jan 7, 2021

Expected Behavior

Current Behavior

Possible Solution

JoelSpeed commented Jan 7, 2021

thekoma commented Jan 7, 2021

NickMeves commented Jan 9, 2021

NickMeves commented Jan 10, 2021

NickMeves commented Jan 10, 2021

NickMeves commented Jan 10, 2021

thekoma commented Jan 11, 2021

NickMeves commented Jan 11, 2021

thekoma commented Jan 11, 2021

github-actions bot commented Mar 13, 2021

thekoma commented Mar 15, 2021

github-actions bot commented May 15, 2021

github-actions bot commented Jul 16, 2021

github-actions bot commented Sep 15, 2021

thekoma commented Sep 15, 2021

JoelSpeed commented Sep 15, 2021

github-actions bot commented Nov 15, 2021

thekoma commented Jul 18, 2023

github-actions bot commented Nov 17, 2023

thekoma commented Nov 17, 2023

tuunit commented Nov 18, 2023

thekoma commented Nov 18, 2023

thekoma commented Nov 18, 2023

github-actions bot commented Jan 25, 2024

thekoma commented Jan 25, 2024

github-actions bot commented Mar 29, 2024