[Bug 28419] Replace MD5 password hashing with WHIRLPOOL
<bugzilla-daemon <at> wikimedia.org>
2012-04-01 03:14:07 GMT
--- 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
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.