WordPress.org

Make WordPress Core

Opened 22 months ago

Last modified 6 months ago

#25446 new enhancement

Return HTTP status code 401 upon failed login

Reported by: raoulbhatia Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.6
Component: Login and Registration Keywords: has-patch
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 19 months ago.

Download all attachments as: .zip

Change History (21)

comment:2 @SergeyBiryukov20 months ago

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

comment:3 @HypertextRanch20 months 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".

@kovshenin19 months ago

comment:4 @kovshenin19 months 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.

comment:5 @dd3219 months 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.

comment:6 @rmccue19 months 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>")

comment:7 follow-up: @nacin19 months ago

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

comment:8 @RavanH16 months 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 16 months ago by RavanH (previous) (diff)

comment:9 @Viper007Bond16 months ago

  • Version set to 3.6

Restoring ticket's version number.

comment:10 follow-up: @s19n13 months ago

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

comment:11 in reply to: ↑ 10 @RavanH13 months 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.

comment:12 @ticoombs12 months 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.

comment:13 @johnbillion11 months ago

  • Component changed from XML-RPC to Login and Registration

comment:14 @lumpysimon11 months 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

comment:15 @toddlahman6 months ago

+1 for 401
+1 for WWW-Authenticate header

Hope this gets added to WP 4.2

comment:16 @slackbot6 months ago

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

comment:17 in reply to: ↑ 7 ; follow-up: @sippis6 months 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.

comment:18 @toddlahman6 months 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.

comment:19 @Ipstenu6 months 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.

comment:20 in reply to: ↑ 17 @RavanH6 months 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.

Note: See TracTickets for help on using tickets.