7 ms·

Password Hijacking Security Incident and Response

48 points by mazsa 14y ago

codegeek 14y ago
"While both Mr. Sclafani and Heroku endeavoured to use test accounts exclusively, a very small number of customer account passwords were reset during the incident" This is interesting.
- jmillikin 14y ago
  Likely due to "related issue in the password reset flow that could be used to reset the passwords of a certain subset of users at random" I would have liked to see more details about what was going on to cause that sort of problem. I can't think of any reasonable code that would cause password resets for an empty ID to be assigned to a random account.
  - patio11 14y ago
    #Start a password reset @user = User.find_by_email @user.update_attribute :password_reset_nonce, rand(16 ** 16).to_hex #mail them the password reset email #Look up a user for a password reset @user.find_by_password_reset_nonce params[:password_reset_nonce] #if nil, above line returns random user on some databases. Oops. session[:user_id] = @user #Logs in user #We assume if they know the nonce that proves they own email. redirect_to passwordReset_url I've elided additional code which might theoretically be used to make the nonce expire and to prevent re-use for brevity, but it's possible that neither of these measures would fix the issue. Bugs like this one always make me think "There but for the grace of God go I."
    - jmillikin 14y ago
      It's the "if nil, above line returns random user on some databases" I'm curious about. What databases behave thus? It's a very problematic behavior.
      - Xylakant 14y ago
        It may be evaluated as "SELECT * FROM USERS WHERE password_reset_nonce IS NULL". If your password_reset_nonce field may contain null values, this will fetch the first row of those it finds. Some databases show quasi-random behavior when fetching that row.
    - lacerus 14y ago
      to_hex does not seem to be in the standard library, but these will work and are rather elegant to get a short random string. Thanks, Patrick! rand(16**16).to_s(16) rand(16**16).to_s(36)
      - earless1 14y ago
        http://www.ruby-doc.org/stdlib-1.9.3/libdoc/securerandom/rdoc/SecureRandom.html http://www.ruby-doc.org/stdlib-1.9.3/libdoc/securerandom/rdo...
    - t-crayford 14y ago
      Doing params.fetch(:password_reset_nonce) would get you out of that nil issue. Doing fetch is generally good practice, especially on params, as often without the right params you can't do anything anyway and should be bailing out with a 404 or another error code.
vld 14y ago
It is very pleasing to see companies like heroku publicly thanking (we even got a blog link here) the security researcher that initially reported a vulnerability. Hopefully, we'll see more of this in the future.
tptacek 14y ago
Nobody ever gets password resets right. They're #1 in our list of 7 Deadly Web App Features. Even if you think you understand your password reset, and can fit all its workings in your head, I recommend you go look at it closely again.
- codegeek 14y ago
  Any good reference to look at in terms of best practices for password reset ?
  - danielweber 14y ago
    Asked this before, and got this nice summary from 'patio11 http://news.ycombinator.com/item?id=4940102 http://news.ycombinator.com/item?id=4940102
  - tptacek 14y ago
    No; every implementation fails in some new and creative way. The implementation I feel most comfortable with: 1. Generate a 256+ bit cryptographically secure random number and base64 it to create a token. 2. Record that token in your database, timestamped, along with the user account for which the token was requested. 3. Mail the token to the user's email address. 4. When the user returns to the site after recovering the token, use that token to look up their account from the database. 5. Expire tokens within single-digit hours so users don't end up accidentally banking password-equivalents in their email accounts. 6. When a user changes their password or requests another password reset, expire all tokens already associated with their account. I would also recommend: (a) Not having any in-band administration functionality in your application; instead, have a separate admin application, attached to the same databases, available only on a VPN. (b) Require 2-factor authentication (such as Duo Security) for both admin VPN access and admin login. A knock-on benefit of (a): your admin functionality is easier to build, because it doesn't have to do the UI/UX chinups your normal exposed app code has to do; crappy looking admin screens nobody but your employees see are generally fine.
    - codegeek 14y ago
codegeek 14y ago
How is this for a "decently secure" password reset method ? - User enters email address to reset password - A reset link (uniquely hashed,salted etc.) is generated server side tied to that specific email address. This link can only be used once and also has a expiry date if un-used by that time. - If this reset link is accessed (hopefully by the intended user), A form is presented to the user where it asks to enter the email address, new password. If entered email address does not match the original email, user gets an error. Immediately, all sessions are invalidated/reset if any. If entered email address matches, then reset the user's password. Again, invalidate/reset any existing sessions. - After resetting password, never login the user directly. Ask them to login manually again.
- mixedbit 14y ago
  I think the large source of problems for password resets is not how the flow is designed from the user perspective, but how it is implemented to handle corner cases that normally rarely occur. Such corner cases are likely to be not well though-out and poorly tested. For example: A user with a valid session cookie for one account, follows a password reset link for some other account. Or a user generates a password reset link, logs in with an old password, changes the password and follows the reset link. There are many such tricky cases and the challenge is to explicitly design how they should be handled and cover them with tests.
  - tptacek 14y ago
    It is this kind of thing --- the password reset code not expecting ever to be hit from a logged-in session, because it's used in the real world exclusively by people who can't log in --- that create 1/2 the bugs in the real world with password resets. You will find blatant bugs that nobody in the world could possibly miss, except that everyone on the dev team missed them because who tests a password reset from a logged-in account? "What? Every page gets 'current user' from a common header included in every file that it pulls out of the session, and this page also takes 'email address' from a parameter passed in by the user? OOPS."
- skymt 14y ago
  > After resetting password, never login the user directly. Ask them to login manually again. What vulnerability could this close? In order to finish the reset procedure in the previous step, any attacker would need to know the user's email address, and the attacker would obviously know the new password that was just set. I'm probably missing something, but I don't see this extra step adding any security.
laurencei 14y ago
What am I missing here? This is what I do; - User resets password, email link to user - User clicks link, random temporary password emailed to user - User logs in with temp password, system then asks them for new password. This way there is no risk of someone somehow guessing the reset link. Even if they do, all it does is email the user, so they gain nothing...
- tptacek 14y ago
  Random temporary password is a terrible idea. It is both less usable and less secure than sending a secure random token and asking the user to provide a new password. Many users will never change the temporary password. All of them will leave the password in their mail spool. Without even more database signaling, the "resets" will never expire, because they are just passwords. Don't do it this way.
  - laurencei 14y ago
    No - the system first sends the secure random token, then instead of asking them for a new password then, it instead auto-generates a random password and emails it to them. Then the system FORCES them to change the password when they login with the new password. Oh - and the tempoary password only valid for 24 hours. So you get the best of both worlds - without anyone being able to 'guess' anything able to do anything...
    - tptacek 14y ago
      The system forces them to change the password if they actually log in. If they don't, the password just sits there. And some users will just cut/paste it. I'm sure there's a million fiddly things you can do to address the weaknesses of temporary password issuance, but you'd be better off sending a semantically meaningless random token. All the countermeasures you're thinking of here apply identically to the token. I am advocating for the reset scheme that is the hardest to mess up. Yours is not the hardest to mess up. I'm not trying to get you to change yours.
deleted 14y ago
[deleted]
ballard 14y ago
Bloody hell, passwords are st. More often than not, the user has the same 6 character, dictionary-based password. If that's not sad enough, the app rarely uses a hash as good as bcrypt or scrypt. Why the fk aren't we using tokens in 2013? How hard would it be for an co-op NGO to stand up as-simple-as-possible-but-no-simpler PKI solid enough for the benefit of many?

48 points by mazsa 14y ago

"While both Mr. Sclafani and Heroku endeavoured to use test accounts exclusively, a very small number of customer account passwords were reset during the incident" This is interesting.

jmillikin 14y ago
Likely due to "related issue in the password reset flow that could be used to reset the passwords of a certain subset of users at random" I would have liked to see more details about what was going on to cause that sort of problem. I can't think of any reasonable code that would cause password resets for an empty ID to be assigned to a random account.
- patio11 14y ago
  #Start a password reset @user = User.find_by_email @user.update_attribute :password_reset_nonce, rand(16 ** 16).to_hex #mail them the password reset email #Look up a user for a password reset @user.find_by_password_reset_nonce params[:password_reset_nonce] #if nil, above line returns random user on some databases. Oops. session[:user_id] = @user #Logs in user #We assume if they know the nonce that proves they own email. redirect_to passwordReset_url I've elided additional code which might theoretically be used to make the nonce expire and to prevent re-use for brevity, but it's possible that neither of these measures would fix the issue. Bugs like this one always make me think "There but for the grace of God go I."
  - jmillikin 14y ago
    It's the "if nil, above line returns random user on some databases" I'm curious about. What databases behave thus? It's a very problematic behavior.
    - Xylakant 14y ago
      It may be evaluated as "SELECT * FROM USERS WHERE password_reset_nonce IS NULL". If your password_reset_nonce field may contain null values, this will fetch the first row of those it finds. Some databases show quasi-random behavior when fetching that row.
  - lacerus 14y ago
    to_hex does not seem to be in the standard library, but these will work and are rather elegant to get a short random string. Thanks, Patrick! rand(16**16).to_s(16) rand(16**16).to_s(36)
    - earless1 14y ago
      http://www.ruby-doc.org/stdlib-1.9.3/libdoc/securerandom/rdoc/SecureRandom.html http://www.ruby-doc.org/stdlib-1.9.3/libdoc/securerandom/rdo...
  - t-crayford 14y ago
    Doing params.fetch(:password_reset_nonce) would get you out of that nil issue. Doing fetch is generally good practice, especially on params, as often without the right params you can't do anything anyway and should be bailing out with a 404 or another error code.

vld 14y ago

It is very pleasing to see companies like heroku publicly thanking (we even got a blog link here) the security researcher that initially reported a vulnerability. Hopefully, we'll see more of this in the future.

tptacek 14y ago

Nobody ever gets password resets right. They're #1 in our list of 7 Deadly Web App Features. Even if you think you understand your password reset, and can fit all its workings in your head, I recommend you go look at it closely again.

codegeek 14y ago
Any good reference to look at in terms of best practices for password reset ?
- danielweber 14y ago
  Asked this before, and got this nice summary from 'patio11 http://news.ycombinator.com/item?id=4940102 http://news.ycombinator.com/item?id=4940102
- tptacek 14y ago
  No; every implementation fails in some new and creative way. The implementation I feel most comfortable with: 1. Generate a 256+ bit cryptographically secure random number and base64 it to create a token. 2. Record that token in your database, timestamped, along with the user account for which the token was requested. 3. Mail the token to the user's email address. 4. When the user returns to the site after recovering the token, use that token to look up their account from the database. 5. Expire tokens within single-digit hours so users don't end up accidentally banking password-equivalents in their email accounts. 6. When a user changes their password or requests another password reset, expire all tokens already associated with their account. I would also recommend: (a) Not having any in-band administration functionality in your application; instead, have a separate admin application, attached to the same databases, available only on a VPN. (b) Require 2-factor authentication (such as Duo Security) for both admin VPN access and admin login. A knock-on benefit of (a): your admin functionality is easier to build, because it doesn't have to do the UI/UX chinups your normal exposed app code has to do; crappy looking admin screens nobody but your employees see are generally fine.
  - codegeek 14y ago

codegeek 14y ago

How is this for a "decently secure" password reset method ? - User enters email address to reset password - A reset link (uniquely hashed,salted etc.) is generated server side tied to that specific email address. This link can only be used once and also has a expiry date if un-used by that time. - If this reset link is accessed (hopefully by the intended user), A form is presented to the user where it asks to enter the email address, new password. If entered email address does not match the original email, user gets an error. Immediately, all sessions are invalidated/reset if any. If entered email address matches, then reset the user's password. Again, invalidate/reset any existing sessions. - After resetting password, never login the user directly. Ask them to login manually again.

mixedbit 14y ago
I think the large source of problems for password resets is not how the flow is designed from the user perspective, but how it is implemented to handle corner cases that normally rarely occur. Such corner cases are likely to be not well though-out and poorly tested. For example: A user with a valid session cookie for one account, follows a password reset link for some other account. Or a user generates a password reset link, logs in with an old password, changes the password and follows the reset link. There are many such tricky cases and the challenge is to explicitly design how they should be handled and cover them with tests.
- tptacek 14y ago
  It is this kind of thing --- the password reset code not expecting ever to be hit from a logged-in session, because it's used in the real world exclusively by people who can't log in --- that create 1/2 the bugs in the real world with password resets. You will find blatant bugs that nobody in the world could possibly miss, except that everyone on the dev team missed them because who tests a password reset from a logged-in account? "What? Every page gets 'current user' from a common header included in every file that it pulls out of the session, and this page also takes 'email address' from a parameter passed in by the user? OOPS."
skymt 14y ago
> After resetting password, never login the user directly. Ask them to login manually again. What vulnerability could this close? In order to finish the reset procedure in the previous step, any attacker would need to know the user's email address, and the attacker would obviously know the new password that was just set. I'm probably missing something, but I don't see this extra step adding any security.

laurencei 14y ago

What am I missing here? This is what I do; - User resets password, email link to user - User clicks link, random temporary password emailed to user - User logs in with temp password, system then asks them for new password. This way there is no risk of someone somehow guessing the reset link. Even if they do, all it does is email the user, so they gain nothing...

tptacek 14y ago
Random temporary password is a terrible idea. It is both less usable and less secure than sending a secure random token and asking the user to provide a new password. Many users will never change the temporary password. All of them will leave the password in their mail spool. Without even more database signaling, the "resets" will never expire, because they are just passwords. Don't do it this way.
- laurencei 14y ago
  No - the system first sends the secure random token, then instead of asking them for a new password then, it instead auto-generates a random password and emails it to them. Then the system FORCES them to change the password when they login with the new password. Oh - and the tempoary password only valid for 24 hours. So you get the best of both worlds - without anyone being able to 'guess' anything able to do anything...
  - tptacek 14y ago
    The system forces them to change the password if they actually log in. If they don't, the password just sits there. And some users will just cut/paste it. I'm sure there's a million fiddly things you can do to address the weaknesses of temporary password issuance, but you'd be better off sending a semantically meaningless random token. All the countermeasures you're thinking of here apply identically to the token. I am advocating for the reset scheme that is the hardest to mess up. Yours is not the hardest to mess up. I'm not trying to get you to change yours.

deleted 14y ago

[deleted]

ballard 14y ago

Bloody hell, passwords are st. More often than not, the user has the same 6 character, dictionary-based password. If that's not sad enough, the app rarely uses a hash as good as bcrypt or scrypt. Why the fk aren't we using tokens in 2013? How hard would it be for an co-op NGO to stand up as-simple-as-possible-but-no-simpler PKI solid enough for the benefit of many?