On 09/02/2011 03:40 PM, Jan Provazník wrote:
On 09/02/2011 01:53 PM, Tomas Sedovic wrote:
> On 08/31/2011 01:54 PM, jprovazn(a)redhat.com wrote:
>> This patchset replaces authlogic with warden which is used
>> also in katello. This is first step which allows us to
>> add more auth strategies (ldap, oauth) or use devise on top
>> of warden, this also synchronizes us with katello auth.
>> What's not very good in this solution is lack of password
>> handling/validations, which authlogic handled so now we have
>> to handle this ourselves. This can be solved in some next
>> step by using devise which is based on warden.
>> Password reset (or db recreation) is required with this patch
>> because encrypted password has different format.
>> aeolus-devel mailing list
> Looking good, ACK.
> 3 notes:
Hi, thanks for the review,
> First, the Pool Family permissions patch was pushed after you sent this
> and it doesn't have the tests fixed for the Warden switch, so some of
> the tests are failing (both rspec and cucumber).
> Could you please make sure all tests pass before pushing this?
> Second, the migrations here invalidate all the existing users -- they
> will not be able to log in again.
> I'm not sure if we need to worry about that now, especially since the
> fix probably won't be very easy. But it's good to keep it in mind.
I don't think it is worth trying to do some conversion. Moving notice
into commit log should be sufficient (currently this notice in in first
> Lastly, we shouldn't be implementing our own password crypto (the
> util/password.rb file). It looks okay on glance (at least we're
> stretching the hashes), but it's a slippery slope.
Actually it's not our own lib - it's taken from katello (as other chunks
of code), which took it from third party (maybe:
Yeah, I realize that you didn't write this. The "we" was meant as Aeolus
> If Warden doesn't provide us with default secure way to encrypt
> password, we should just go ahead and use bcrypt:
> It's packaged in Fedora, the technology has been vetted by security
> researchers over the years and we will not be bitten in the arse if we
> go with it. Can't say the same about a homebrew solution that had right
> about zero cryptographers banging at it.
> We don't have to implement bcrypt in this patchset -- go ahead and push
> this. But we should do it soon. Definitely before any
> okay-for-public-consumption releases. Note that Devise uses brcypt by
> default so if we switch to that, this problem is solved.
> I created a Redmine task so we can track it:
Well, sure - we can consider using bcrypt, though this solves only
password encrytion problem and diverges us from katello. I would prefer
to use devise in conductor and katello, which can handle password
encryption too (among other things).
Devise does all the right things by default. But if we do not go down
that road, we (and Katello) should really switch to to brcypt instead of
carrying and maintaining potentially insecure code.
aeolus-devel mailing list
No trees were killed to send this message, but a large number of
electrons were terribly inconvenienced.