browserpass / browserpass-legacy

Legacy Browserpass repo, development is now happening at:
https://github.com/browserpass/browserpass-extension
MIT License
999 stars 80 forks source link

Match more specific password first #264

Closed jangari closed 5 years ago

jangari commented 6 years ago

I have several passwords for systems which exist as subdomains of another password. E.g., I have work/university.edu and also work/subsystem.university.edu, which are different (of course a lot of the time the password for subsystem2.university.edu will be the same as university.edu, which works fine as it is caught by work/university.edu).

Such is the pattern matching, that if I'm on system.university.edu and invoke browserpass, it will match both the passwords for work/university.edu and work/system.university.edu, with the order being determined by something like order in the filesystem, I guess.

It would be good if the pattern matching could suggest the more specific matching password by default, and then allowing the user to hit backspace to remove the automatic filter and find the more generic password if they want to. Alternatively, order the matching passwords by specificity first, so that one need only invoke browserpass and hit enter.

maximbaz commented 6 years ago

In browserpass v3 (which is not yet available) this scenario is supported in a different way: it sorts the passwords by frequency of use on a particular domain.

So if you have password entries for work/university.edu and work/subsystem.university.edu, when you use browserpass on subsystem.university.edu (or system.university.edu, doesn't matter), the matching candidates will be shown alphabetically sorted. However once you use a password on a particular domain, that password on this particular domain will get a usage counter += 1, and thus will be positioned higher than other passwords with lower usage counters, allowing you to use it next time with just Ctrl+Shift+L, Enter.

Because of this, right now I don't think we should complicate this logic any further.

I'll keep this ticket open and close it when browserpass v2 is released.

Sneak peak:

image

image

jangari commented 6 years ago

Great solution!

jangari commented 6 years ago

Oops, didn't mean to close.

maximbaz commented 5 years ago

I'm closing the tickets that have already been implemented to have a clearer picture of what is left to do. This one is implemented, thus closing. The design of v3 is very different, so you will notice for sure when your extension gets auto-updated and so when this feature becomes available πŸ˜‰

jangari commented 5 years ago

Thanks! Looking forward to the update rolling out through Chrome. Is there an updated native client with v3?

erayd commented 5 years ago

@jangari Yes, there is an updated client with v3. We are hoping to make updates to the native client very rare after this though - much of the logic that lives in the v2 native client is moving to the extension. The simpler we make the client, the less chance there is of needing to alter it.

anarcat commented 5 years ago

i have to say, i don't like the idea of the extension tracking my password usage very much. i would rather have a smarter default ("more specific first") instead, since you still have to solve that problem for the "i haven't used the passwords yet" use case (which happens when you copy a database over to a new desktop, for example).

maximbaz commented 5 years ago

i don't like the idea of the extension tracking my password usage very much

Don't worry, all the data stays local and never travels to any remote locations, so we can make a smarter algorithm and yet not abuse your privacy β€” and what's even more awesome, if you want to check this, the code if fully open source and I'm happy to give you some pointers on where to look at specifically πŸ˜‰

since you still have to solve that problem for the "i haven't used the passwords yet" use case

We won't be solving this because we don't consider it a problem, we can't predict with 100% certainty that a specific entry would be the most preferable, instead we offer to just login once and second time this password entry will be on top of the list.

anarcat commented 5 years ago

Not traveling on the network is of course a minimum. But even storing locally is an extra liability I would like to live without.

erayd commented 5 years ago

@anarcat Can you explain what you see as the security concern here? I really don't see how this is a liability in any practical sense.

All that is stored (for each entry) is when it was last used, and how many times it has been used. And as @maximbaz has mentioned above, this is stored locally; the stats never leave your browser.

anarcat commented 5 years ago

My concern is that we are creating user-tracking data that was not there before.

The simplest example I can think of is someone trying to clear their browsing history. In firefox, they would go in History -> Clear Recent History (Control-Shift-Del) and select what they want to clear out. This will not clear out the "when it was last used" information from the browserpass plugin here, which will reveal when (and, possibly worse, how often) a site is used.

Browser history does not leave the browser either, at least not by default. Just like extension settings, that data can leave the browser if the user enables the "Firefox Sync" (or google login) features of the browser, in which case that data does leave the browser. And that's besides the point: privacy-conscious users still worry about history retention locally for other reasons.Β Just because something is stored locally does not mean it's perfectly safe. Search and seizures, a stolen laptop or security vulnerability can allow an attacker to seize that history locally.

This is why I am worried about how this feature is implemented. I understand why it's done the way it is, and I can see why some people would find it interesting, but it's just a liability to me. At least there should be a way to opt-out of it, otherwise it's certainly something that will make me stop using this plugin altogether (or just keep the older version running as a fork).

The original feature request here is "Match more specific password first": that's a completely different feature request than "Show more frequently used sites first". My programmer's intuition is that it's much simpler to implement more-specific matching (just take the identical string, then the larger length, then whatever), than a whole record-keeping mechanism which needs to deal with a database and so on... And as I said earlier, it would still be useful to implement the "more specific" feature in the case where a user starts with a fresh database, with no "usage" information stored in it: then you still have to pick the most reasonable option and, on ssh.example.com, that's the password ssh.example.com, not example.com.

anarcat commented 5 years ago

Could we consider at least leaving this issue open so that people (like me) interested in the feature know that it's a welcome enhancement, while keeping a way to opt-out for users?

maximbaz commented 5 years ago

It is not impossible to clear localStorage of a browser extension either (this info is stored in localStorage), the fact that browser's feature "Clear recent history" isn't able to clear extension's history (I didn't verify that it is actually true) I would rather consider a bug in the browser itself. For example, Github stores something in localStorage too, and it doesn't provide a way to opt-out of this.

But also, if an attacker has local access to your computer, I think this metadata is the last thing you will be worrying about. Just listing files in ~/.password-store will already give away more information that we will store in localStorage (all your credentials, access time, modification time).

Regarding Firefox Sync and Google Sync, I believe they are not synchronizing data in localStorage to their servers.

To summarize, I honestly don't see any realistic scenario where an attacker would gain something useful from browserpass cache that they can't retrieve otherwise if they have access to your computer, so I'm not planning to add an option to disable this algorithm.

Also keep in mind that it's not among the goals of browserpass to protect you against attackers with local access, it is done better by enabling disk encryption, using stronger password and updating your software in time. Browserpass is first and foremost about protecting you against phishing attacks and making it easier to find the relevant credentials, and having recency information does help with the latter πŸ™‚


I'll reopen to reconsider fresh database scenario, not promising we will change anything, but we will have another look.

anarcat commented 5 years ago

First, thank you for taking the time to explain your position and discussing this issue openly. :)

On 2018-10-24 17:19:55, Maxim Baz wrote:

It is not impossible to clear localStorage of a browser extension either (this info is stored in localStorage), the fact that browser's feature "Clear recent history" isn't able to clear extension's history (I didn't verify that it is actually true) I would rather consider a bug in the browser itself. For example, Github stores something in localStorage too, and it doesn't provide a way to opt-out of this.

Sure. But there's a difference between a site's local storage (which can be explicitely refused through other extensions, for example) and an extensions'. I would expect the latter to survive such purges, but I must admit I didn't explicitely test this scenario.

Besides, that was just an analogy. I think this is a problem in itself, not necessarily related to history purging.

But also, if an attacker has local access to your computer, I think this metadata is the last thing you will be worrying about. Just listing files in ~/.password-store will already give away more information that we will store in localStorage (all your credentials, access time, modification time).

True. But then there are mitigations possible around that. There's the "tomb" plugin which encloses the password store within an encrypted volume, for example.

Just because some of that information exists outside the problem space doesn't mean we must necessarily create more here. ;)

Regarding Firefox Sync and Google Sync, I believe they are not synchronizing data in localStorage to their servers.

I would assume the opposite, actually. I would certainly expect Firefox Sync to synchronize my extensions settings, at the very least.

To summarize, I honestly don't see any realistic scenario where an attacker would gain something useful from browserpass cache that they can't retrieve otherwise if they have access to your computer, so I'm not planning to add an option to disable this algorithm.

Would you accept a pull request that does so?

Also keep in mind that it's not among the goals of browserpass to protect you against attackers with local access, it is done better by enabling disk encryption, using stronger password and updating your software in time.

I do all three things very dilligently but they will not help in this scenario:

  1. Disk encryption does not deter attacks against a running system (apart from the "tomb" technique mentioned above, which does not protect against the issues introduced here).

  2. Not sure what "stronger password" refers to, but it might protect an SSH account, for example. This will not protect a victim from an in-browser attack, for example through XSS.

  3. Updating software, in this case, will introduce the vulnerability I'm concerned about. But I otherwise agree it is good policy - in fact my day job these days is to provide such security updates, for the record. ;)

Browserpass is first and foremost about protecting you against phishing attacks and making it easier to find the relevant credentials, and having recency information does help with the latter πŸ™‚

Sure. As I said earlier, I totally understand where it's coming from. All security requires certain tradeoffs between usability and strength. I happen to think this is an incorrect choice, but I can appreciate why it's being done.

I just want a way to opt out.

I'll reopen to reconsider fresh database scenario, not promising we will change anything, but we will have another look.

Thank you so much for that! :)

A. -- We must learn to live together as brothers or perish together as fools.

maximbaz commented 5 years ago

I would assume the opposite, actually. I would certainly expect Firefox Sync to synchronize my extensions settings, at the very least.

I'm pretty sure it requires writing some extra code to do so, nothing is synced automatically.

I do all three things very dilligently but they will not help in this scenario:

Sure, you are obviously right, but again this is not a goal for browserpass πŸ™‚

Would you accept a pull request that does so?

Sorry, right now I'm still not convinced this is a good idea. I hear your feedback loud and clear, and I'll be listening if others express similar concerns. But at the same time I hope you understand my position as well, redesigning the way we search and order passwords (with recency being very big part of it) is one of the core reasons behind rewriting everything in v3.

I've just remembered one implementation detail, I wonder if it makes you a liiitle happier: we don't store website or password entry name in plain text, instead it's stored as a sha1(hostname +sha1(passwordstore + sha1(entry))), so an attacker would have to bruteforce their way through the cache to decode which credentials you've used on which domains.

You can complicate this bruteforce by a lot by naming your password entries differently than hostnames, e.g. if you password file is called abcde.gpg and you used it on github.com, while browserpass will be showing abcde in the popup, an attacker would have a hard time finding out this information from the string 3ff3871188b064bacf0899cbb665f39e88dc6600.

hyperfekt commented 5 years ago

I'm sure you've put quite a bit of consideration into choosing the proposed frecency algorithm - I'd love if you could share it here with those who have doubts: Why is it preferable over the solution proposed in the original post? My understanding is that implementing specificity (not necessarily instead, but it could also be in addition) would lead to just as correct behavior even without prior data (which is not synced to other browsers?), and would not expose any metadata (which may be specially harmful if the password store is encrypted by default and the browser history is wiped, remaining as the only trace of visited sites if we don't require every user to obfuscate their entries).

maximbaz commented 5 years ago

Because it allows greater flexibility, specificity assumes there is one correct way of ordering thing, while frecency allows you to build any order you like. I'll consider implementing specificity in addition to frecency since it is such a popular demand, my concerns are that the sorting algorithm is already complicated and will become even more complicated, but maybe I'm overthinking this.

At any rate, the v3 release is unfortunately very behind the schedule due to absence of free time, I really hope we can get back to it. Given that frecency is already implemented, considerations for specificity will go to the bottom of the backlog, first priority is to reach feature parity and finally cut off the release, so we all can focus on the new codebase πŸ˜‰

anarcat commented 5 years ago

Thank you so much for being open to the idea! I understand it can be a hard feature to implement, especially if you don't believe it's important, so thank you very much for taking the time to consider it and answer us. It's really appreciated! :)

maximbaz commented 5 years ago

This has been implemented in Browserpass v3, if you can test now, please share your feedback in https://github.com/browserpass/browserpass/issues/331

In particular, the sorting logic is documented here: https://github.com/browserpass/browserpass-extension#password-matching-and-sorting

If you have feedback, do let me know πŸ˜‰