[PATCH PoC] secure password reset implementation

[Mads Kiilerich]

On 05/20/2015 10:38 AM, Andrew Shadura wrote:
> Hello,
>
> On Tue, 19 May 2015 17:10:46 +0200
> Mads Kiilerich <mads at kiilerich.com> wrote:
>>> @@ -296,10 +300,19 @@ class UserModel(BaseModel):
>>>    
>>>            user_email = data['email']
>>>            user = User.get_by_email(user_email)
>>> +        timestamp = int(time.time())
>>>            if user:
>>>                log.debug('password reset user found %s' % user)
>>> +            token = hashlib.sha1('%s%s%s%s%s' % (user.user_id,
>>> +                                               timestamp,
>>> +                                               user.password,
>>> +                                               session.id,
>>> +
>>> config.get('app_instance_uuid'))
>>> +            ).hexdigest()
>> Now we are reinventing crypto. That is dangerous. One thing: One
>> thing is that hashes could give conflicts in the hash but we should
>> make sure that we never got "collisions" in the input string - such
>> collisions could perhaps be used as attacks. Perhaps use \n as
>> delimiter of the fields?
> Well, none of the data we use here is accepted directly from user
> (except timestamp — when verifying the token; I should probably
> validate it before using). From what I saw in similar cases elsewhere
> (for example, OAuth 1.0), sometimes the data is escaped to avoid
> collisions.

I agree ... but adding a separator that can't occur in the inputs would 
make the code more resilient to future errors


By the way, for the record to others: We will need a similar scheme for 
verifying email addresses configured by users.

/Mads