[Bug 28419] Replace MD5 password hashing with WHIRLPOOL
<bugzilla-daemon <at> wikimedia.org>
2012-04-01 03:14:07 GMT
https://bugzilla.wikimedia.org/show_bug.cgi?id=28419
--- Comment #39 from Tyler Romeo <tylerromeo <at> gmail.com> 2012-04-01 03:14:07 UTC ---
(In reply to comment #38)
> (In reply to comment #37)
> > OK, now I understand the logic. In that case I'll give my support FWIW. The
> > only thing I would recommend is cleanup the structure a little bit. There's no
> > need for a PasswordType interface when there's a base abstract class that's
> > already required to be inherited by the primary Password class.
> The separation between PasswordType and BasePasswordType (I think I used to
> call it AbstractPasswordType or CommonPasswordType, should I rename it back?)
> is what allows the flexible implementation of things like crypt(3).
> An implementation that uses our : pattern extends BasePasswordType and lets
> BasePasswordType abstract away all the storage, crypt, and comparison bits.
> While an implementation that needs to call a 3rd party library or say crypt(3)
> with raw data instead implements PasswordType directly and uses the raw data to
> handle crypt and compare itself.
> I don't believe I referenced BasePasswordType anywhere but in the extends. All
> instanceof checks should be against the PasswordType interface.
>
> Though I do think I could turn the PasswordType interface into an abstract
> class so that isPreferredFormat can have the same `require true;` default as
> BasePasswordClass.
That's my bad. Mixed up the name of the interface and abstract class and
thought you were requiring password types to be children of the base class.
Ignore my previous comment.
> > Also, I would change the cryptParams() function to defaultCryptParams() and
> > replace preferred format with current format.
(Continue reading)