Make WordPress Core

Opened 11 years ago

Closed 6 years ago

Last modified 3 years ago

#25446 closed enhancement (wontfix)

Return HTTP status code 401 upon failed login

Reported by: raoulbhatia's profile raoulbhatia Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.6
Component: Login and Registration Keywords: dev-feedback needs-patch 2nd-opinion
Focuses: Cc:

Description

Hi!

Please make the login return a HTTP status code of 401 (or the like) in case the login fails.

This would greatly enhance the possibility to create appropriate filters for e.g. mod_security.

Thanks,
Raoul

Attachments (1)

25446.diff (297 bytes) - added by kovshenin 11 years ago.

Download all attachments as: .zip

Change History (29)

#2 @SergeyBiryukov
11 years ago

  • Type changed from feature request to enhancement
  • Version changed from trunk to 3.6

#3 @HypertextRanch
11 years ago

I'm not sure 401 is the appropriate status code here. Per RFC2616 the response code appears to be reserved for HTTP auth only:

10.4.2 401 Unauthorized

The request requires user authentication. The response MUST include a
WWW-Authenticate header field (section 14.47) containing a challenge
applicable to the requested resource. The client MAY repeat the
request with a suitable Authorization header field (section 14.8)...

If we must return a non 200 response 400 seems the most applicable although I'm not sure if a wrong username/password combination should be considered "malformed syntax".

@kovshenin
11 years ago

#4 @kovshenin
11 years ago

  • Keywords has-patch added

400 is something the server did not understand. In our case we understood the request, we just didn't accept the login and password. I'm leaning towards 403 or 401, though 401 seems to be designed around HTTP authentication, which is not the case with wp-login.php.

My vote is for 403, besides, our XML-RPC methods that require authentication use the 403 error code for failed logins too. Patch for wp_signon in 25446.diff.

#5 @dd32
11 years ago

The HTTP response spec was never really designed for Web-login forms, assuming everyone would want to use the HTTP Authentication spec instead, as such it defines:

401 Unauthorized- The request requires user authentication. The response MUST include a WWW-Authenticate header field (section 14.47) containing a challenge applicable to the requested resource. The client MAY repeat the request with a suitable Authorization header field (section 14.8). If the request already included Authorization credentials, then the 401 response indicates that authorization has been refused for those credentials.
403 Forbidden - The server understood the request, but is refusing to fulfill it. Authorization will not help and the request SHOULD NOT be repeated.

so technically 403 is invalid for authentication purposes - however, if you read "Authorization" as the HTTP Authorization spec, then it could be seen as allowed for a web form (Don't retry this request with that POST data.. that combination is forbidden), but then "refusing to fulfill it" doesn't apply either, as WordPress is fulfilling the request just denying the authentication.. which is why a 200 response is completely valid here.

I believe the XML-RPC spec even specifies all responses must be 200 OK (with an error object in the response) too.

That being said, the web has moved far from the origins of the HTTP specs, and I'm OK with returning a 403 here, assuming it doesn't break anything.

One thing that needs to be kept in mind: Certain browsers *cough*IE*cough* do the whole "smart HTTP error" messages where they don't display the server output, but rather their user-friendly output. I'm not sure what browsers/versions would be affected here, but it's worth mentioning.

#6 @rmccue
11 years ago

401 is the correct error to return here. 403 means you don't have access to the resource, 401 means you need to authenticate.

401 should work fine in terms of the standard, since it's not just for Basic authentication. To be really compliant, we can also send a WWW-Authenticate header, which should probably look something like:

WWW-Authenticate: WordPress location="http://example.com/wp-login.php"

(There's no real standard for what the header should look like, but it's usually "<scheme> <scheme specific parts>")

#7 follow-up: @nacin
11 years ago

Does *anyone* send a non-200 for a failed web login? I can't say I've ever noticed this in practice.

#8 @RavanH
11 years ago

  • Version 3.6 deleted

Landed on this ticket looking for hooks to design a Fail2Ban access log filter to block brute-force login attempts. One approach is described by Konstantin Kovshenin on http://kovshenin.com/2014/fail2ban-wordpress-nginx/ where he needs a response header other than 200 to reliable recognize brute-force attempts. He suggests a 403 response but I agree with others in this ticket thread that it is not really valid as header for the wp-login.php page. Not even after a failed authentication, simply because the request is for the login page which IS accessible and thus SHOULD respond with a 200 status header.

So the underlying issue here is in my view the fact that WordPress uses a separate login form/page URL. This makes any of the few likely response header candidates -- 401 and 403 -- unsuitable.

This is what happens currently:

  1. Trying to access the Dashboard (/wp-admin/) when not authenticated, the request is met with a 302 (temporary redirect) forwarding the user the the login form (/wp-login.php) which in turn responds with a 200 OK header.
  2. After submitting the form with incorrect login data, the form comes up again with a 200 OK response.
  3. After submitting the form with correct login data, the request is met with a 302 redirecting back to the Dashboard which now responds with a 200 OK, obviously.

This is all according to logic and (in my view) there are no other suitable responses in this scenario. And thus, it does not leave much room for recognizable access log patterns to be used by security mod filters.

But...

What if the whole WP authentication flow was changed? For example like this:

  1. Trying to access the Dashboard (/wp-admin/) when not authenticated, the request is met with a 401 response and shows a login form instead of the Dashboard.
  2. Now after submitting that form with incorrect login data, the request is again met with a 401 response (after an X number of failed logins this might be turned into a 403 by a plugin like Limit Login Attempts) presenting the form again.
  3. After submitting with correct login data, the request is met with a 200 OK and now shows the Dashboard.

In this scenario, there are no redirects and the request URL does not change. Less confusing and allows different response headers according to different visitor credentials like auth cookies, session or POST data. And now there would be recognizable patterns in the access log for sec filters :)

Last edited 11 years ago by RavanH (previous) (diff)

#9 @Viper007Bond
11 years ago

  • Version set to 3.6

Restoring ticket's version number.

#10 follow-up: @s19n
10 years ago

+1 for this feature, or anything else could be used to build a (much needed) security filter in front of WordPress.

#11 in reply to: ↑ 10 @RavanH
10 years ago

Replying to s19n:

+1 for this feature, or anything else could be used to build a (much needed) security filter in front of WordPress.

Much needed indeed. Install a plugin like Limit Login Attempts and turn on e-mail notification on lockouts... It's amazing how many (automated) brute-force attacks are hitting even the smallest of blog sites on a daily basis! I think in this day and age, and as widely used as WordPress is, any security measure -- like limiting login attempts, two-factor authentification or a logical login scenario where a server-side solution like a fail2ban filter can depend on --should be built-in.

#12 @ticoombs
10 years ago

  • Component changed from General to XML-RPC

+1 for 401

Plugins like fail2ban are useless, and any searching along these paths turn up a few blogs with people just blocking all requests to /xmlrpc.php via a fail2ban regex. Webservers need to know if this is denied access, or was successful.

After receiving a generous 18000 unique ip's (over a week) trying to bruteforce xmlrpc, I think its time for this to get merged.

#13 @johnbillion
10 years ago

  • Component changed from XML-RPC to Login and Registration

#14 @lumpysimon
10 years ago

+1 for returning a different status code on failed logins.

The workaround I'm currently using is to hook into the wp_login_failed action to generate an entry in the error log, then monitor these with fail2ban: http://forum.bytemark.co.uk/t/running-both-fail2ban-and-symbiosis-firewall/2017/9

#15 @toddlahman
10 years ago

+1 for 401
+1 for WWW-Authenticate header

Hope this gets added to WP 4.2

This ticket was mentioned in Slack in #core by shelob9. View the logs.


10 years ago

#17 in reply to: ↑ 7 ; follow-up: @sippis
10 years ago

Replying to nacin:

Does *anyone* send a non-200 for a failed web login? I can't say I've ever noticed this in practice.

Nope. Example Twitter, Facebook, GitHub, Google and BitBucket all returns 200. So I'll +1 for staying with 200 because seems that everyone else is doing the same, and HTTP response spec lacks decent status code for failed web login.

But what about adding op-in custom log for this purpose? I'm not so familiar with fail2ban, but i think that it can use custom logs.

#18 @toddlahman
10 years ago

@nacin

Although HTTP status codes like 401 are most often applied to APIs, they should also be applied when a response would provide a useful/usable response. If a login fails, via a login form, the response is currently a 302 redirect, then a 200 succeeded. Neither of those communicate what actually happened to the client, which leaves ambiguity. The end result should be a 401, rather than a 200 status code, since a 401 communicates useful/usable information to the client, just as an API would, so the client can react accordingly. For example, after receiving a 401 the client could try to login again automatically. Thinking forward, forms will need to react as an API would. Erroring on the side of clearly communicating via an HTTP response code seems like a step in the right direction.

#19 @Ipstenu
10 years ago

Just as a point of order, there is a plugin called fail2ban but also a server module which is similar to iptables, and is regularly used in conjunction with mod_security to wrangle bad behavior (like 18,000 hits to Xmlrpc from one ip in 5 minutes...). In case anyone was confused here.

#20 in reply to: ↑ 17 @RavanH
10 years ago

Replying to sippis:

Nope. Example Twitter, Facebook, GitHub, Google and BitBucket all returns 200. So I'll +1 for staying with 200 because seems that everyone else is doing the same, and HTTP response spec lacks decent status code for failed web login.

But what about adding op-in custom log for this purpose? I'm not so familiar with fail2ban, but i think that it can use custom logs.

Yes, fail2ban (talking about the server service, not a plugin) can monitor custom logs. However, it would mean that a generic WordPress rule cannot be added to the available fal2ban preset rules. So only those that have their own server/VPS could add their own custimized rule and set up fail2ban to monitor this extra WP log. Plus, it will mean even more server load as fail2ban is not that friendly on CPU usage. A small price to pay for added security and not bad for starters but it'd be better if the failed login attempts would be recognizable straight in the access logs that are already monitored.

Maybe an entry in the error log instead of a dedicated login log would be better? Not very elegant but probably less demanding on server resources...

Replying to nacin:

Does *anyone* send a non-200 for a failed web login? I can't say I've ever noticed this in practice.

The login flow of sites like Gmail, Facebook, Twitter etc. is the same as the current flow in WordPress. Accessing the admin will first do a 302 temp redirect to a seperate login form page which then responds with 200 ok status. Subsequent failed attempts reload that same login form page which responds each time with a 200 ok. And that's all according to logic. The form page loads and responds correctly so it should return a 200 OK.

That is why I proposed in comment:8 to change the login flow to something where a 401 status can be a fitting response on a failed login. A flow that no longer redirects to a separate login page (login.php) but instead return an 'in-page' login form might work.

A smaller adaptation of the current flow might suffice too:

Consider an attacker submits login data to login.php he currently gets a 200 status and knows this means a failed attempt because on success, the response is a 301 (with header Location: /wp-admin/)... the attacker tries again and again, filling the access log with status 200 responses. But what if one extra step is added? Let's say a submission to login.php always responds with a 301 (either to /wp-admin/ or the back-end page that was originally requested) but then depending on the login credentials the user gets again a 301 back to the login.php form on failed login.

This would mean that a successful login would only generate two 302 responses. And a legit user that forgot his password will likely try a limited amount of times before finally resorting to using the Lost Password form so he would create - let's say - max twenty 302 responses over a period of 30 to 60 seconds. While a brute force attack would fill the logs quickly with hundreds 301 (instead of 200 as it is now) status request in just a few seconds. This can then easily be recognized as brute force attempts by fail2ban...

Unless, of course, many other pages on the domain respond with a 302. In that case, the risk is blocking legit visitors or search spiders.

#21 follow-up: @swissspidy
9 years ago

  • Keywords needs-patch added; has-patch removed

As per #6:

401 is the correct error to return here.

401 should work fine in terms of the standard, since it's not just for Basic authentication. To be really compliant, we can also send a WWW-Authenticate header

Version 0, edited 9 years ago by swissspidy (next)

#22 in reply to: ↑ 21 @RavanH
9 years ago

Replying to swissspidy:

As per #6:

401 is the correct error to return here.

401 should work fine in terms of the standard, since it's not just for Basic authentication. To be really compliant, we can also send a WWW-Authenticate header

Although I'm all for a switch to a 401 response (because it would make it easier for server processes like fail2ban to recognise a brute force attack from access logs) I don't agree that it's - strictly speaking - a correct response. Be aware that the current 200 response (either on first access or on failed login) is the response that comes after requesting wp-login.php. This is the login page that is and should always be accessible without authentication and should therefore always respond with 200 status. Sending a 401 status response is essentially saying the client is not authorized to access the resource.

Imagine having to authenticate before access the authentication form is granted? That would be a nice catch-22 :D

This confusion is why I proposed to change the whole login logic (redirect form /wp-admin/ to the login page, then a redirect back to admin after succes) to something simpler that indeed would warrant a 401 response when access to a particular resource is not granted. See my TL;DR reply above ;)

Last edited 9 years ago by RavanH (previous) (diff)

#23 @dejayc
8 years ago

Perhaps I'm missing something, but isn't it sufficient to setup fail2ban to look out for HTTP POST requests to wp-login that result in HTTP STATUS CODE 200?

POST is only generated by the browser when it submits a login attempt - all other requests are GET.

And 200 is only generated by the server in the event of login failure - success results in 301.

#25 @slaFFik
6 years ago

  • Status changed from new to reopened

@iseulde

I understand that this a template answer that you posted, but this issue has everything described in both description, and comments, and valuable discussion, and even a patch (that needs to be updated, perhaps @kovshenin?). It's just no one considered to actually make a decision and implement.

#26 @iseulde
6 years ago

  • Keywords dev-feedback 2nd-opinion added
  • Milestone set to Awaiting Review

Sure, no problem at all to reopen! :)

Would be great if a decision could be taken.

#27 @mihdan
6 years ago

+1 for this feature

#28 @chrigu99
6 years ago

+1 for the feature
+1 for 401 - even when it's not strict perfect.

#29 @johnbillion
6 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

I've just checked the HTTP response code for the failed login screen for a whole load of popular online services, and every single one of them returns a 200.

While I can certainly see the benefit of switching to a 40x, nobody else is doing it, it's not technically correct, and there are other means of identifying failed login attempts internally (for example by hooking into the wp_login_errors or wp_login_failed actions) and externally (for example by looking for POST requests that return a 200 as pointed out by @dejayc).

To that end I'm going to make an executive decision and close this as wontfix.

Note: See TracTickets for help on using tickets.