Project

General

Profile

Bug #7736

Change to prevent unauthenticated requests for CSRF modified login behaviour as well

Added by Jan Pazdziora about 6 years ago. Updated about 6 years ago.

Status:
Rejected
Priority:
Normal
Assignee:
-
Category:
Security
Target version:
-
Difficulty:
Triaged:
No
Bugzilla link:
Pull request:
Fixed in Releases:
Found in Releases:

Description

The change that went into Foreman as 4e3a7e7a2a5 included definition of handle_unverified_request which in turn affected not just the logout functionality, but login as well. The net effect is that it's now not possible to log in to Foreman WebUI via its logon form at /users/login without GETing that URL first, to get the authenticity_token.

Since issue #6999 was attempting to only change the logout behaviour, I'd call this a regression, unless there is a specific reason why the exception should be raised for the unauthenticated login page where just resetting the session and proceeding with the request was enough.


Related issues

Related to Foreman - Bug #6999: CVE-2014-3590 - User logout susceptible to CSRF attackClosed2014-08-08

History

#1 Updated by Dominic Cleal about 6 years ago

  • Subject changed from Change 4e3a7e7a2a542435686a667773eafc73c92e557b for issue 6999 modified login behaviour as well to Change to prevent unauthenticated requests for CSRF modified login behaviour as well
  • Category changed from Web Interface to Security

#2 Updated by Dominic Cleal about 6 years ago

  • Related to Bug #6999: CVE-2014-3590 - User logout susceptible to CSRF attack added

#3 Updated by Dominic Cleal about 6 years ago

Thanks for the report. Did this affect one of the login features you added? (I'm thinking the form intercept method perhaps?)

#4 Updated by Dominic Cleal about 6 years ago

  • Legacy Backlogs Release (now unused) set to 22

#5 Updated by Jan Pazdziora about 6 years ago

Dominic Cleal wrote:

Thanks for the report. Did this affect one of the login features you added? (I'm thinking the form intercept method perhaps?)

It did affect my tests of that feature. It's fairly easy to change the tests but I'm concerned that there was a wholesale change in behaviour even if the issue/commit message only point to a very narrow case (logout). Hence this issue, to spark a discussion about the intended behaviour in all the other cases.

#6 Updated by Dominic Cleal about 6 years ago

  • Status changed from New to Rejected
  • Legacy Backlogs Release (now unused) deleted (22)

Yeah, the commit message, as is typical for us, was overly narrow. We realised the scope was broader than logout only, as any cross-domain request that was made missing CSRF tokens would result in the user being "logged out" by way of their session being invalidated.

Looking at what we do during the login process, we reset the user's session immediately before we go any further, as part of the fix for another CVE (#4457). So as not to leave another case where a user could be attacked and get their session deliberately reset, I think we need to retain the CSRF authenticity check on login.

I think we can say this is deliberate and we're preventing an identical attack on the login route.

Also available in: Atom PDF