WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#17830 closed enhancement (invalid)

[patch] The extension mechanisms related to hashing and storing passwords could be improved

Reported by: monperrus Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: General Keywords:
Focuses: Cc:

Description

Hi,

I've just written a plugin which allows storing passwords in a way that is compatible with HTTP digest (see http://www.monperrus.net/martin/wp-http-digest). The advantages of this approach is that it enables me to build services using HTTP digest authentication on top of standard wordpress accounts.

The hash of HTTP digest passwords is md5(username:realm:password). Since it requires the username, overriding wp_hash_password($password) is not enough.

Eventually, I had to override:

  • wp_check_password to concatenate username:realm:password
  • wp_set_password to correctly update the password
  • wp_new_user_notification to intercept newly created passwords
  • add_action('profile_update', 'action_profile_update' ); to intercept updated passwords.

The last two changes are somehow hacks, if wp_insert_user and wp_update_user would use wp_set_password instead of wp_hash_password directly, I would only have to override wp_check_password and wp_set_password, and it would be fine.

Thus, I suggest to add a call to wp_set_password in wp_insert_user.

Regards,

--Martin

Attachments (1)

wp-ticket-17830.diff (1.9 KB) - added by monperrus 4 years ago.
Patch for Ticket #17830

Download all attachments as: .zip

Change History (8)

comment:1 @scribu4 years ago

I'm no security expert, but it seems to me that the proposed hashing mechanism would be weaker than the current one.

Anyway, a patch would help.

comment:2 follow-up: @nacin4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

The entire authentication system is pluggable. I don't know of anyone crazy enough to do so for this though.

WP.org and WP.com run a patched mod_auth_mysql which supports phpass: http://barry.wordpress.com/2008/05/19/mod_auth_mysql-and-phpass/. (The patch was rejected upstream.)

comment:3 in reply to: ↑ description @westi4 years ago

Replying to monperrus:

Hi,

I've just written a plugin which allows storing passwords in a way that is compatible with HTTP digest (see http://www.monperrus.net/martin/wp-http-digest). The advantages of this approach is that it enables me to build services using HTTP digest authentication on top of standard wordpress accounts.

As nacin pointed out there are already working solutions to this problem:

comment:4 in reply to: ↑ 2 @monperrus4 years ago

Replying to nacin:

The entire authentication system is pluggable.

Indeed it is. I am just saying that overriding wp_new_user_notification to intercept newly created passwords and adding add_action('profile_update', 'action_profile_update' ); to intercept updated passwords is a hack that could be avoided.

WP.org and WP.com run a patched mod_auth_mysql which supports phpass: http://barry.wordpress.com/2008/05/19/mod_auth_mysql-and-phpass/. (The patch was rejected upstream.)

Thanks for the link.

--Martin

comment:5 follow-up: @nacin4 years ago

You are able to replace wp_hash_password(). See wp-includes/pluggable.php.

@monperrus4 years ago

Patch for Ticket #17830

comment:6 in reply to: ↑ 5 @monperrus4 years ago

  • Cc monperrus added
  • Summary changed from The extension mechanisms related to hashing and storing passwords could be improved to [patch] The extension mechanisms related to hashing and storing passwords could be improved

http://code.trac.wordpress.org/browser/mod_auth_mysql

unless I miss something, this only supports HTTP Basic and not HTTP Digest (unless AuthMySQLPasswordField is in digest format which is equivalent to what I am discussing here).

You are able to replace wp_hash_password().

That's right, but this is not sufficient for the digest format, because the HTTP digest hash takes into account the username and wp_hash_password() just receives the password.

I attach a tentative patch to wp-includes/user.php to explain what I mean. Basically, instead of relying on wp_hash_password for creating and updating users, it relies on the encapsulation provided by wp_set_password.

Regards,

--Martin

comment:7 @monperrus4 years ago

Any opinion on the patch?
Regards, --Martin

Note: See TracTickets for help on using tickets.