[RFC] Password hashing in own class

[Jhonne]

Replaced by [RFC]More secure password hashing


phpbb_hash and related functions is found in functions.php which is included on all pages. Yet password hashing is only needed for a few pages mainly those involving authentication and registration.

Feature: Put all functions from the PHP password hashing framework in an own class and use it only when needed.

Goal: Encapsulation and to remove some functions from functions.php which is not really needed there.

[AmigoJack]

+1 for the idea.
-1 for the RFC in general.

Too similar to [RFC] Auth Plugin Refactoring & User Integration.

[Pony99CA]

If the function is only used on a few pages (included in a few files), it makes some sense to remove it from a huge include file and put it in its own include file. I don't think this is about "refactoring and user integration" as much as making the include files a bit more reasonable.

I suppose it depends on performance issues, though -- will removing it save significant RAM for those files that don't need it, and will an extra include cause significant slow-downs for files that do use it? if the answers are YES and NO (or even NO and NO), why not do it?

Steve

[AmigoJack]

It's too similar because password checking (and hence hashing) should become a part of the authentication plugin anyway. Currently it's hardcoded to the phpBB core and custom auth plugins can't reach/realize everything they should. In a perfect world all of the default authentication stuff resides is a plugin instead of being bound to the core - which also makes sure custom plugins can really serve/support all needed functionality. For example: currently a custom plugin can't catch the event of changing a password.

The overall performance increase should be minimal to zero, as right now most hosters already use caches like eAccelerator which keep nearly all PHP files in memory anyway, so moving functions based on their usage is virtually only an optimization to the concept/design. Though, I might be wrong

[nickvergessen]

Well this one can be done easily, while auth refactor is quite time intensive. Also if auth is refactored, the functions still should be available in general, f.e. they are used for forum passwords and maybe modifications aswell.

[Oleg]

phpbb_hash and related functions is found in functions.php which is included on all pages. Yet password hashing is only needed for a few pages mainly those involving authentication and registration.

If the goal is to take hashing out of functions.php for the sake of improving phpbb's performance, this is a pointless undertaking.

If the goal is to include a smaller file in third-party applications that need to authenticate against phpbb, the difference will be obvious and if this change does not conflict with outstanding auth refactoring pr(s) I would support it.

If it does conflict, I don't think it's worth complicating the auth refactoring pr(s) for this optimization.

[bantu]

I think this should be combined with upgrading password hashing to bcrypt. As such, I'd reject the RFC as is.

[naderman]

We are replacing password hashing with a patch based on the proposal [RFC]More secure password hashing, which also refactors the related code, so I'm moving this one to rejected RFCs.