passwordmaker / chrome-passwordmaker

A browser extension based on passwordmaker.org. Create unique passwords for every webpage using a cryptographic hash algorithm.
https://passwordmaker.org
GNU Lesser General Public License v3.0
93 stars 53 forks source link

Removed from Chrome Webstore? #180

Closed polm closed 3 years ago

polm commented 3 years ago

This plugin vanished from my browser today, and the Chrome Webstore link gives a 404, though it still shows up in Google search results. What's up?

https://chrome.google.com/webstore/detail/passwordmaker-pro/lnhofcfhehhcbccpmdmdpjncdoihmkkh

IlgazC commented 3 years ago

I see the following on MS Edge (chrome based one). I didn't buy it though. Screenshot (2)

alexbevi commented 3 years ago

Chrome disabled this extension for me (see chrome://extensions/) image

iSWORD commented 3 years ago

Something is going on. I received an email from Google that someone used my password (generated by Password Maker Pro) to login to my account. I'm kinda freaking out.

IlgazC commented 3 years ago

Something is going on. I received an email from Google that someone used my password (generated by Password Maker Pro) to login to my account. I'm kinda freaking out.

for the time being you better enable 2FA (2 factor auth) for all sites that support it. It would make password theft worthless.

77 commented 3 years ago

Does Google report what they detected anywhere? Edge popped up a window saying this extension has malware but the extensions windows simply shows "unsafe" or "violates the terms". The source code in this repo hasn't been updated in years and I cannot tell when the extension was updated in the store as it was removed. Any idea where this violation came from? Did someone upload a patch to the extension recently?

GitTom commented 3 years ago

You can still use it - go to the extensions settings page and re-enable it - and chances are that since it wasn't being maintained it fell foul of some new policy eg. having a privacy policy, so it got booted from the store.

But did it actually get compromised and some bad actor inserted some malware and pushed a new version? I think they would have had to bump the version number and the version number hasn't changed. Did anyone notice getting a new version recently? Also, wouldn't all of us be getting all sorts of warnings if someone managed to steal our master passwords?

So I think it was just a policy violation. If @heavensrevenge doesn't reappear by the weekend I'll grab the code from here (v0.8.9), figure out what needs to be changed so it conforms to new requirements, publish that to a new repo, and upload it to a new listing, and make note of it here in this issue.

I still think that Password Maker is a great way to do passwords so I want to keep it alive.

polm commented 3 years ago

Thanks @GitTom, it'd be great to have this alive again somewhere.

For those of you concerned about the extension being compromised in the future, installing the extension from source is really easy - just check out the repo and tell Chrome to load it from the extensions panel. I did that yesterday and was running again in no time.

ericjung commented 3 years ago

Hi, I’m the creator of the original PasswordMaker and owner of https://passwordmaker.org (original site at http://passwordmaker.sourceforge.net because git did not exit back then)

I’m glad to see so much activity and use of this version of PasswordMaker. I wish I had the time to contribute, help, and do the fork that @GitTom suggests.

good luck and know you have my blessing!

ericjung commented 3 years ago

p.s. I always intended PasswordMaker to be usable from anywhere by anything. So do not feel locked out of your passwords because this chrome extension is blocked. You should still be able to use the self-contained web page even if it is not the most convenient.

i may have written this chrome edition, too, I’d have to check the source code to be sure. @heavensrevenge may have simply uploaded it to GitHub for me. I just don’t remember because it’s been so long.

anyway, good luck

tasermonkey commented 3 years ago

you may want to export your old passwordmaker settings by manually renabling the plugin. Especially if someone else is going to re-deploy it to the chrome store. Though in doing so, I noticed that if you use the cloud sync stuff for the settings, it doesn't seem to export correctly.

GitTom commented 3 years ago

I made another attempt to contact @heavensrevenge, via email this time, but got no response.

So, I submitted it to the Chrome Web Store as a new listing. I'm pretty sure that the reason the old one got taken down was that there are now more stringent privacy policy declarations required.

I got a message saying that it will require manual review and that can take 30 days. Hopefully that is just a safe outer bound for them and it will be much faster. I didn't even realize they did manual reviews.

I will post here again with the link once it is published.

In the meantime you can quite simply (but temporarily) re-enable it from the extension settings page, or go into developer mode and load it manually as if you were testing it.

I will also write another comment about the (minimal) changes I made and upload it to a new GitHub repo.

lahwran commented 3 years ago

Anyone up for doing a security review of the version that was on the chrome web store? If it got removed, there's a significant chance it's because of an attack. maybe it's just a change in policy, but unfortunately with the web store listing not providing more information, it's risky to assume that.

miquelfire commented 3 years ago

Without @heavensrevenge around, we can't know the reason it was removed.

@GitTom Maybe make a pull request of the changes you make so we can see them? We'll talk about adding you to this repo once it's approved.

lahwran commented 3 years ago

I can likely retrieve the extension from backups. I'll check in a couple of hours, unless someone else has a copy. That will allow reviewing it to ensure it actually matches the github version.

GitTom commented 3 years ago

The new repo is https://github.com/GitTom/chrome-passwordmaker

I did a PR against this repo, as requested.

The new version I submitted to CWS is v0.9.0 and includes changes committed by @heavensrevenge back in 2018 but after v0.8.9.

The only significant change I made was to change the name to Password Maker org (instead of "pro"). This links it more closely with the PM.org site and constellation of apps for other platforms. (And I didn't like the "Pro" as it implies that there is a non Pro version too and that maybe this is the one you pay for with more features.)

GitTom commented 3 years ago

It got rejected due to declaring 'clipboardRead' optional permission and not using it. IMO 'clipboardRead' is a very important permission and I did feel confused about why it was needed. I'm a bit embarrassed to have submitted it that way so want to note that I did not add it.

Looking now I see that it was added here: https://github.com/passwordmaker/chrome-passwordmaker/commit/dba1c6cef152958df7ac2357e8db51acce1d16d8
Not sure why he added it, but I would suggest that it was nothing nefarious.

The CWS submission process is clearly much stricter now and this is a good thing. I have re-submitted it (along with a few other cosmetic changes), now as v0.9.1, and will issue a pull request.

ericjung commented 3 years ago

It got rejected due to declaring 'clipboardRead' optional permission and not using it... Looking now I see that it was added here: dba1c6c

That is clipboardWrite, not read. The purpose is to write generated passwords into the clipboard so the user can paste it into a password field manually if needed.

How are you writing to the clipboard now?

GitTom commented 3 years ago

? no, I see that commit as adding clipboardRead. After that commit the manifest was requesting clipboardRead as well as clipboardWrite (which got moved to optional).

clipboardWrite is needed as you have noted. clipboardRead was not used and should not have been added, particularly since it is the more important one from a security perspective.

miquelfire commented 3 years ago

I wonder if his plan was to check if the password was still in the clipboard and clear it if it was still there after a certain amount of time?

GitTom commented 3 years ago

But I think of the password 'expire' function (if that is what you are referring to) as expiring the master password, not the generated password (which gets put into the clipboard).

Also, if I knew an extension was using a timer and then (seemingly out of the blue) checking what was in my clipboard, I would be very concerned.

miquelfire commented 3 years ago

The point would be that without the read check, the extension would clear the clipboard even if you change the contents. If there's an event that would let you know if what you put in the clipboard was replaced (but not with what replaced it), that might be enough for this feature to work without needing to read it.

shaneonabike commented 3 years ago

...or an event that can notify you if the clipboard was changed!

GitTom commented 3 years ago

Yes, I can see how it would be used to further the expunging of the password, but I'm saying that the expunging that happens based on the timer should not include the clipboard because the feature in question pertains to the user's master password, not the password in the clipboard.

Unless I'm misunderstanding that feature, I think what you are describing would be a change in its meaning, which could be justified as long as it was explained to the user. But IMO it would not justify adding back such a sensitive permission.

But none of this matters unless someone is proposing to add that functionality in which case I would be happy to engage more fully. As thing are though, I had to remove the permission to get the extension accepted.

miquelfire commented 3 years ago

I never once said it was related to the master password.

It's something that makes me wish Android had a clear clipboard feature because the keyboard I'm using has an option to display the contents of the clipboard for easy entry (not in password fields however...) and when I copy a password, and since I rarely copy anything, a password can be in my clipboard for days if not weeks.

If a user doesn't use their clipboard often, that might happen with a PasswordMaker password on the platforms this is used on.

GitTom commented 3 years ago

But I thought you were talking about the password 'expire' functionality, and my understanding of that feature is that it pertains to the master password, not the generated password. I've repeated that a few times without being corrected - that's how I got to the master password.

But now I think you are referring to some brand new functionality to clear the generated password? That makes more sense, but if you are proposing that then let's take it to a new issue.

There are lots of changes I would like to see happen to improve the security of this extension, but for now I'm trying to get it back into the store as quickly as possible, and I think we should focus on that in this issue, and that's why clipboardRead had to be removed.

ericjung commented 3 years ago

@GitTom The original PasswordMaker, and some other implementations since then, had this feature:

  1. PasswordMaker generates a password
  2. User clicks a button to copy it to clipboard.

There is now sensitive data in the clipboard that can be read by other extensions and also by non-Chrome (native) applications

  1. So there was an option in these PasswordMaker implementations to clear the password after a certain interval, say 60 seconds. @miquelfire I do not remember PasswordMaker with this feature reading the clipboard first to see if the contents were a generated PasswordMaker password, but it is possible -- my memory just doesn't go back that far.

In any case, if you want to reimplement this, I would skip the clipboardRead. Just add a settings checkbox "clear clipboard after 60 seconds" with a warning that it's going to clear anything there -- password or not. Another option is to just leave it as-is -- drop this clear clipboard feature entirely.

IlgazC commented 3 years ago

I don't think reading clipboard or even having permission to read it will do anything good. It will create massive paranoia. On Linux, people are coding millions of lines of code for a new graphics protocol (wayland) and one of the reasons is clipboard security. Every app can read clipboard on X11. Just by being there, of course with good intentions, it will create needless paranoia both for users and Google.

ha-zzz-lborn commented 3 years ago

"... since I rarely copy anything, a password can be in my clipboard for days if not weeks."

Tasker can solve this problem by over-writing the clipboard as often as wished.

GitTom commented 3 years ago

Yes, I understand this concern, I just thought he was talking about something else.

Again, I wonder if this shouldn't get its own issue, but while we are at it...

I agree with @ericjung and @IlgazC that if this feature is implemented it should be without clipboardRead.

miquelfire commented 3 years ago

If people want it, we should open a new issue. The first step is getting back in the web store.

IlgazC commented 3 years ago

I never once said it was related to the master password.

It's something that makes me wish Android had a clear clipboard feature because the keyboard I'm using has an option to display the contents of the clipboard for easy entry (not in password fields however...) and when I copy a password, and since I rarely copy anything, a password can be in my clipboard for days if not weeks.

There exists "Clipboard Cleaner" which I used for the same reasons.

https://play.google.com/store/apps/details?id=org.bitbucket.caffee.clipboardcleaner

On the release notes it says " Support Android Q limited access to clipboard data", interesting.

GitTom commented 3 years ago

Ok, here is the new Chrome Web Store listing https://chrome.google.com/webstore/detail/passwordmaker-org/fckpmekmkjglpmdcbfkchimdelcjiipd

Please give it some good ratings (if you think it is warranted), so it can show up in searches. As things stand you pretty much have to search on the exact name to get it at all. Ideally, it would show up above the other, abandoned, PM extensions.

mshopf commented 3 years ago

Thank you for your efforts. For what it's worth, Alt-Z to open the popup doesn't work with this extension. I remember this was already the case with another fork, but I never had the time to check what's wrong.

jhoenicke commented 3 years ago

@mshopf Try to remove all passwordmaker extensions (not just deactivate) and install again. I had the same problem and this worked for me. After installing it asked to confirm the hotkey.

I guess the problem was that the old extension did take the hot-key away.

mshopf commented 3 years ago

You were right - this did the trick! Thanks!

ericjung commented 3 years ago

@miquelfire I wanted to add this implementation of PasswordMaker to https://passwordmaker.org/Google_Chrome (top of list), but I can't seem to log into the wiki. Is it in maintenance mode?

miquelfire commented 3 years ago

It's always in maintenance mode because of spammers. I made the changes already.

You need to log in with SSH to comment out a line to be able to edit the pages.

nicerobot commented 3 years ago

Here is my concern with the version that was removed

❯ du -h lnhofcfhehhcbccpmdmdpjncdoihmkkh/0.8.9_0/javascript/libs/jquery.min.js
 88K    lnhofcfhehhcbccpmdmdpjncdoihmkkh/0.8.9_0/javascript/libs/jquery.min.js

The version in the new extension

❯ du -h fckpmekmkjglpmdcbfkchimdelcjiipd/0.9.1_0/javascript/libs/jquery.min.js
 72K    fckpmekmkjglpmdcbfkchimdelcjiipd/0.9.1_0/javascript/libs/jquery.min.js

The version in this repo

❯ du -h passwordmaker/chrome-passwordmaker/javascript/libs/jquery.min.js
 72K    passwordmaker/chrome-passwordmaker/javascript/libs/jquery.min.js

The rest of the code looks like it's the same as the repo. I think it would be worthwhile for someone to review the specific differences between lnhofcfhehhcbccpmdmdpjncdoihmkkh and fckpmekmkjglpmdcbfkchimdelcjiipd to determine if we've been using a malicious extension for three years.

GitTom commented 3 years ago

If you look at the commit history you will see that @heavensrevenge made some commits in 2018 after tagging 0.8.9.

I think his tagging is accurate - that those commits never made it into the 0.8.9 in the CWS. I worked off of main, rather than branching from 0.8.9, so the current version and the one that I tagged and submitted as 0.9.1 included both my changes and the commits he made after 0.8.9.

Those commits of his include this one
https://github.com/passwordmaker/chrome-passwordmaker/commit/557b3621e402d88ef6b798b10a5550c361b5c1d9 Use slim version of jQuery 3.3.1

So I think that would be where the change that you have detected originates. Now, it is a bit odd that the size increased given that his commit suggests he was updating from jQuery 3.3.1 to the 'slim' version of same. I guess the next step would be to confirm that the "jQuery 3.3.1 slim" he substituted matches the official one. I could check that out tonight.

Sure would be nice to get rid of that dependency (most of the functionality of jQuery is part of modern js so jQuery is no longer used).

GitTom commented 3 years ago

I compared the included version with the official version here and it appears to me that they are the same. And they are both 68.3 KB. So I don't think we have a problem.

On the other hand, it would be nice to get rid of that dependency.

Given enough time there are some other improvements I may try to make, eg. updating to manifest v3, but I won't be getting rid of jquery myself. I'm a C++/Java developer who has more recently learned JS/TS for backend functions - I haven't done much html or any jquery. I think it would be a task for someone whose done the transition from jquery to modern HTML/JS.

nicerobot commented 3 years ago

Thanks @GitTom. I'm still not 100% convinced but I'm getting closer 😉 It's been a long time since I've looked at jquery. It appears the 3.3.1 version in the removed 0.8.9 extension lnhofcfhehhcbccpmdmdpjncdoihmkkh is a build without some features and I have no idea how to produce one like that:

jQuery v3.3.1 -ajax,-ajax/jsonp,-ajax/load,-ajax/parseXML,-ajax/script,-ajax/var/location,-ajax/var/nonce,-ajax/var/rquery,-ajax/xhr,-manipulation/_evalUrl,-event/ajax,-effects,-effects/Tween,-effects/animatedSelector

My remaining concern is that that version is quite a bit larger than the 3.3.1 slim min version to which you linked. I would have expected that removing features would reduce the size, not increase it. If we can find an official release of that precise 3.3.1 "slim min -ajax" and confirm it is indeed the 88K, or better, has matching SHA, I'll be convinced. As it stands, I'm still suspicious of 0.8.9.

$ openssl sha256 lnhofcfhehhcbccpmdmdpjncdoihmkkh/0.8.9_0/javascript/libs/jquery.min.js
SHA256(lnhofcfhehhcbccpmdmdpjncdoihmkkh/0.8.9_0/javascript/libs/jquery.min.js)= 160a426ff2894252cd7cebbdd6d6b7da8fcd319c65b70468f10b6690c45d02ef
GitTom commented 3 years ago

Yes, I guess I got sidetracked wanting to verify that what I submitted to the CWS was clean - but you were trying to draw our attention to the what was in the old 0.8.9.

It looks like when they moved jquery to its current location in /libs (in 2018-04) they updated from 3.1.1 to the variant of 3.3.1 that you are concerned about. From this repo's git that file is this one:
https://raw.githubusercontent.com/passwordmaker/chrome-passwordmaker/13e7772a5bb04a14a1a8794adba7b711dc89c90d/javascript/libs/jquery.min.js
So, I then do a compare of that file with jquery-3.3.1.min.js (not slim) downloaded from the jquery site, and they match.

Am I misunderstanding or doing anything wrong?

nicerobot commented 3 years ago

Indeed. Thanks for tracking that down. Exactly what I was looking for. It appears to be an official jquery so I'm satisfied that we were never using a malicious extension.

Much appreciated for getting the extension replaced in the webstore so quickly. Mind tagging the current master with 0.9.1? And maybe pushing the extension bundle to the release for safe keeping?

GitTom commented 3 years ago

Okay, created tag & release.

Only thing remaining, I think, is to fix the CWS link in the about - and I don't think I have permission for that. Maybe @miquelfire?

miquelfire commented 3 years ago

I changed the link, and you should have permissions to make these types of changes in the future now.

heavensrevenge commented 10 months ago

BTW @GitTom and @nicerobot I removed the jQuery dependency with 0.10.0, so no need to worry about some odd jQuery. This is the only place I have seen the proposal to remove jQuery but it has been done.