[wp-trac] [WordPress Trac] #46130: Only pause extensions for users with recovery access

[WordPress Trac]

#46130: Only pause extensions for users with recovery access
------------------------------------------------+--------------------------
 Reporter:  flixos90                            |       Owner:
                                                |  TimothyBlynJacobs
     Type:  defect (bug)                        |      Status:  assigned
 Priority:  highest omg bbq                     |   Milestone:  5.1
Component:  Bootstrap/Load                      |     Version:  trunk
 Severity:  normal                              |  Resolution:
 Keywords:  needs-patch servehappy 2nd-opinion  |     Focuses:
------------------------------------------------+--------------------------

Comment (by TimothyBlynJacobs):

 I've uploaded a patch which requires users to enter into Recovery Mode via
 a link.

 The Recovery Mode is broken down into a number of parts.

 1) Requesting and Sending a link to enable recovery mode.
 2) Validating the recovery link.
 3) Handling the recovery cookie.

 ** Part 1**
 When a fatal error occurs, we add a link to `wp_die` generated by
 `get_recovery_mode_request_url()`.

 When the user visits that URL, `handle_recovery_mode_actions()` sends an
 email to the `admin_email` via `maybe_send_recovery_mode_email()`. This
 function rate limits the email to only be sent at most once an hour. We
 then immediately `wp_die()` with a message indicating whether the email
 was sent or not.

 The email generates a "recovery key" with
 `generate_and_store_recovery_mode_key()`. This generates a random string
 with `wp_generate_password()`. ( The length of the string is 20 to match
 the behavior in `get_password_reset_key()` but could be increased ). The
 string is then hashed by `PasswordHash` and that hash saved to the
 `recovery_key` site option along with the creation time.

 This un-hashed key is added as a query arg to `wp_login_url()` along with
 an `action` set to `begin_recovery_mode()`.

 ** Part 2 **
 When the user clicks the link, `handle_recovery_mode_actions()` verifies
 that the key is valid and not expired. The persistent recovery cookie is
 then set. We then `wp_die()` with a JavaScript based redirect to the login
 page.

 This is done so that the cookie can actually be persisted on the client's
 machine. In testing, just setting a cookie and redirecting didn't work.
 Additionally, from the brief research I had time to do, it looks like
 different cache configurations might make persisting a cookie and doing a
 redirect at once not possible.

 The user is redirect to the login page with an `action` set to
 `begun_recovery_mode`. This is so we can then print a message indicating
 that recovery mode was enabled successfully on the login page.

 ** Part 3 **
 The cookie value is generated by `generate_recovery_mode_cookie()`. The
 majority of the security related functions are defined in `pluggable.php`.
 When we load `pluggable.php` in other calls, we can immediately `die()` or
 `wp_redirect`. Since we have to validate this cookie on every page load,
 those functions aren't an option.

 After discussing with @aaroncampbell, it looked like making a version of
 `wp_salt` that has its own fallbacks for invalid salts was a variable
 option.

 A separate function, `recovery_mode_hash()` is introduced that performs a
 `hash_hmac` specific to recovery mode. This tries to use the `AUTH_KEY`
 and `AUTH_SALT` values, but if they aren't available, it will load
 `pluggable.php` and then generate random values and store them to site
 options, similarly to `wp_salt()`.

 The cookie value is an hmac with the data set to a string of
 `recovery_mode|%1$s|%2$s`. 1. is the current time, and 2. is a randomly
 generated value by `wp_generate_password()`. The signature generated by
 `hash_hmac` is then appended to the string.

 Validating the cookie is done by recalculating the expected hash, and
 checking if the hashes are equal. Additionally, the code checks that the
 cookie has not expired. By default the cookie value is valid for 7 days.
 The cookie itself expires at the end of the session. This should be
 discussed.

 The actual cookie implementation beyond using a version of `wp_salt` ''was
 not'' reviewed by @aaroncampbell. This should be reviewed and alternatives
 considered.

 Some possible alternatives:

 a) Use the `PasswordHash` library directly. The hash would be stored in a
 site option, with the creation time. This would require the secret to be
 stored in plain text in the cookie. This might not be terrible in this
 scenario since it's not a user's password, but a randomly generated value.
 b) Just use a shared plaintext secret that is stored in the DB with the
 creation time there.
 c) Same as b) but use `recovery_mode_hash` to hash the shared secret on
 either side so that the Recovery Mode sessions would expire when the
 `AUTH*` salts are changed.

 Moving on, if the cookie is not validated, it is cleared and we `wp_die`
 with the reason why and a prompt to request a new link.

 If the cookie is valid, set a constant `WP_RECOVERY_MODE` to `true`. The
 `wp_is_recovery_mode()` function checks for this constant, and allows
 filtering its output.

 The cookie is cleared when the user logs out by `wp_clear_auth_cookie()`.

 ** Additional Changes **
 - Plugins/Themes are not paused unless `wp_is_recovery_mode()`
 - The redirection mechanism to discover multiple plugins at fault in one
 go now only happens when `wp_is_recovery_mode()` otherwise no progress can
 be made since the plugins aren't being paused one by one.
 - All errors are now stored, regardless of if they occurred in a protected
 endpoint. Alongside the error we record if the error happened in a
 protected endpoint and adjust the language in the Plugins List Table.

 ----

 Additionally, after discussion with @flixos90, we decided to drop Network
 Activated plugins from pausing. There are a number of things that
 WordPress loads between MU/Network Activated plugins being loaded and
 regular plugins being loaded. Most importantly to us, the cookie
 constants. Also from @flixos90, pausing network plugins open their whole
 own permission/scope can of worms and is multisite is generally run by
 more technical users.

 When handling the error, the email is sent automatically. I'm not sure if
 this should stay or not, but I included it to demonstrate.

 `handle_recovery_mode_actions()` is called immediately before loading
 regular plugins.

 Initially, I tried to match this to a user and store the keys in user
 meta. After developing it further, it didn't appear workable because this
 code is executed before we can work with the current user and often
 without a user available ( such as when trying to login ).

 ----

 After writing this, I think we can drop the
 `recovery_mode_email_last_sent` option and just use the creation time from
 `generate_and_store_recovery_mode_key()`.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/46130#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform