Checking an already-hashed password

[TylerRick]

It would be nice if there was a first-class way to check a password that is already SHA-1 hashed. If that's all I have, I obviously can't simply unhash it. Since this is what https://api.pwnedpasswords.com/range/ expects already, it would be easy to support this option as well.

Especially since the data that gets sent is based on hashed_password, which only looks at the plain-text password if @hashed_password is not yet initialized:

    def hashed_password
      @hashed_password ||= Digest::SHA1.hexdigest(password).upcase
    end

So we just need a way to initialize a Password object with a hashed_password. Would you accept a PR that adds that? And any preference on an API to initialize such a password?

I think this might be the nicest interface:

password = Pwned::Password.new(hashed_password: "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8")

as long as you're okay with ad hoc polymorphism being used for password param:

--- a/lib/pwned/password.rb
+++ b/lib/pwned/password.rb
@@ -43,7 +43,7 @@ module Pwned
     # @example Setting the user agent and the read timeout of the request
     #     password = Pwned::Password.new("password", headers: { "User-Agent" => "My user agent" }, read_timout: 10)
     #
-    # @param password [String] The password you want to check against the API.
+    # @param password [String, Hash] The password you want to check against the API.
     # @param [Hash] request_options Options that can be passed to +Net::HTTP.start+ when
     #   calling the API
     # @option request_options [Symbol] :headers ({ "User-Agent" => '"Ruby Pwned::Password #{Pwned::VERSION}" })
@@ -52,6 +52,7 @@ module Pwned
     # @raise [TypeError] if the password is not a string.
     # @since 1.1.0
     def initialize(password, request_options={})
+      password = password[:hashed_password] if password.respond_to?(:[])
       raise TypeError, "password must be of type String" unless password.is_a? String
       @password = password
       @request_options = Hash(request_options).dup

Another option might be:

password = Pwned::Password.new_hashed_password("5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8")

One use case of this is to check how many user accounts have compromised passwords (assuming passwords are stored in SHA-1, which they probably aren't).

If there's not enough interest, I could always just use this workaround...

main > password = Pwned::Password.new("")

main > password.instance_variable_set :@hashed_password, "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8".upcase

[philnash]

I guess my first question for this is what is the use case?

You said:

One use case of this is to check how many user accounts have compromised passwords (assuming passwords are stored in SHA-1, which they probably aren't).

But I wouldn't want to encourage anyone to store passwords as unsalted SHA-1 hashes. And the only use I can think of for this feature would be to enable those who had stored passwords in that manner.

Do you have any other use cases for it? What is the case you want to use it like this for?

[TylerRick]

Well, I soon realized that the original use case that I wanted this for — to check to what extent / how many existing accounts are using compromised passwords, in order to better decide whether it would make sense to send out a mass notification asking all users to change their password, what user flow would make most sense to build, etc. — is unfortunately not possible to solve :disappointed:, for the same reason that #10 is (also unfortunately) not possible, and because the password are encrypted with bcrypt :lock: :relieved:.

I can think of other possible use cases for this feature, but honestly, they may not be very likely and not ones I have currently.

Perhaps the strongest use case is a user of this library being paranoid and not wanting to trust this library with a plain-text password. I can imagine some users, after reading the API docs and seeing that their privacy is protected by a k-Anonymity model that only requires a hash (a partial hash on the remote end, full hash on the local end) to perform the lookup, may wonder why this library even requires the plain-text password at all then. Perhaps they're looping through their own password list in order to do a checkup of their own passwords (since not everyone uses 1Password which has that feature built-in).

Like Troy Hunt said,

if you're really cautious, create a SHA-1 hash of your password locally on your machine then pass the first 5 chars to the API and match the suffix to the response that's returned.

Allowing a hashed password to be passed to this client would safeguard against the possibility that a compromised version of this library (that secretly sent the plain-text password to a nefarious remote endpoint) got published and used by an application. I audited the source code of the current version for security holes, but I probably wouldn't remember to do that every time I did a bundle update.

So I guess my strongest argument for adding this feature is that since the client doesn't technically need to see the plain-text password, then, by the Principle of least privilege, it should not be passed to it — or at least should not require you to pass it to it. Since the remote API only requires a hash of the password, then it would seem reasonable for users of this client to be able to use it in the same way if they wanted.

Maybe that's taking it too far. (I'm not usually that paranoid myself, though maybe I should be. I am paranoid enough that I refuse to trust a closed-source password manager like 1Password to manage my passwords though.) I have nothing to urge. Just something to think about.

[philnash]

Paranoia is a decent argument when talking security. Adding a method to set the hashed_password instead of the plain text would not add a lot of code and would keep the plain text out of the hands of my gem.

If you want to open a PR for that, then I would happily review and accept it.

[philnash]

This has been implemented and released in version 2.1.0. See the updates in #20.