Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Trezor passphrase being saved to password list and offered by auto fill without permission. #12563

Closed
lowping23 opened this issue Jan 9, 2018 · 12 comments · Fixed by brave/muon#432

Comments

@lowping23
Copy link

lowping23 commented Jan 9, 2018

Description

Just upgraded to the new version. Turned on strict site isolation. Plug Trezor in and now my passphrase is available via autofill. The Trezor had been added to the saved passwords list. I didn't ok that. I deleted the pass phrase and the Trezor from the saved password list, entered it again into the Trezor and brave saved the passphrase again without asking. I've since shutdown auto fill and password management via brave.
This behavior started after the update. i'd used the Trezor multiple times today without this issue until the update was made.

Steps to Reproduce

Actual result:

Expected result:

Reproduces how often:

Brave Version

about:brave info:

Brave 0.19.131
rev da2f347
Muon 4.5.36
libchromiumcontent 63.0.3239.132
V8 6.3.292.49
Node.js 7.9.0
Update Channel Release
OS Platform Microsoft Windows
OS Release 10.0.16299
OS Architecture x64

Reproducible on current live release:

Additional Information

@srirambv
Copy link
Collaborator

srirambv commented Jan 9, 2018

cc: @darkdh @diracdeltas

@lowping23
Copy link
Author

I'm referring to the passphrase for a particular wallet account not the mnemonic.
If you enable passphrase protection on the trezor you can create separate accounts that are accessed by the specific passphrase.

@diracdeltas
Copy link
Member

probably related to the other password autofill changes that were in the last release, not strict site isolation.

@bsclifton bsclifton added this to the 0.19.x Hotfix 12 milestone Jan 9, 2018
@darkdh
Copy link
Member

darkdh commented Jan 9, 2018

@higzac would you mind turning off strict site isolation and try again? It helps us narrow down that it is another password manager changes in the same release.

@darkdh
Copy link
Member

darkdh commented Jan 9, 2018

There are 2 issues:

  1. Built-in password manager couldn't actually be turned off (Brave fills in some password on sites, even though I explicitly turned off password management. #10566)
  2. chromium password manager will save the following form as bravetest/bravetest
    screen shot 2018-01-09 at 11 12 53 am

These two issues have already been there since we migrate to chromium password manager and #12489 (comment) exaggerates it upon page load because we will popup the USERNAME which about to fill to users
screen shot 2018-01-09 at 11 40 37 am

However, I couldn't reproduce the automatically save because it will ask every time
screen shot 2018-01-09 at 11 36 40 am

@bsclifton we might need #10566 in this milestone

darkdh added a commit to brave/muon that referenced this issue Jan 10, 2018
fix brave/browser-laptop#12563

Auditors: @bridiver, @diracdeltas

Test plan:
1. Make sure built-in password manager is enabled
2. Male sure passphrase is set on trezor wallet
3. Plugin trezor and open wallet
4. Type passphrase and submit
5. Brave shouldn't prompt any messages to save password

1. Make sure built-in password manager is enabled
2. Sign up account for https://trac.torproject.org
3. Brave should ask users to save password, click deny
4. Logout and Login
5. Brave should ask users to save password, click allow
6. Change password
7. Brave should ask users to update password, click allow
8. Logout and use the save credentials to login
9. It should be able to login sucessfully
@lowping23
Copy link
Author

lowping23 commented Jan 10, 2018

Sorry for the delay in getting back to you.
Had a chance to play around to get a better idea of what happened.
I can repeat the problem with (strict site isolation) Enabled or disabeld

Looking at what I can repeat this is what is happening

Connect trezor and log into account.
Option to save password presents itself.
I must have clicked ok to this accidentally
Log out of trezor. Then log back in.
I'm now offered the (use password for option) where the user is my password.
Log out of the trezor
Open preferences in brave and go to password settings.
Delete the entry for the trezor.
Connect trezor.
I'm offered the user name password option again where the user name is the password even though I just deleted it from password settings in brave..
Enter my password again log in.
New entry with my username password added back to password settings in brave.
There is no dialogue to ok adding it, its just added back.
This happens if you don't close brave down after deleting the user name password.
If you stay in the same session you don't get a promt from brave when it adds the password back to its list.
If i close brave after deleting the password then restart all the correct prompts happen to warn me brave is saving the password

@darkdh
Copy link
Member

darkdh commented Jan 11, 2018

Test plan:
a.

  1. Make sure built-in password manager is enabled
  2. Male sure passphrase is set on trezor wallet
  3. Plugin trezor and open wallet
  4. Type passphrase and submit
  5. Brave shouldn't prompt any messages to save password

b.

  1. Make sure built-in password manager is enabled
  2. Sign up account for https://trac.torproject.org
  3. Brave should ask users to save password, click deny
  4. Logout and Login
  5. Brave should ask users to save password, click allow
  6. Change password
  7. Brave should ask users to update password, click allow
  8. Logout and use the save credentials to login
  9. It should be able to login sucessfully

@srirambv
Copy link
Collaborator

Test plan B works on Windows.

@LaurenWags
Copy link
Member

Test Plan B works on MacOS as well.

@jumde
Copy link
Contributor

jumde commented Jan 11, 2018

Test Plan A works on MacOS and Windows with 0.19.132

@kjozwiak
Copy link
Member

kjozwiak commented Jan 11, 2018

Test Plan B works under Ubuntu 17.10 x64 using 0.19.132 rev 0fd9b1d.

Test Plan A works on MacOS and Windows with 0.19.132

Awesome! Thanks @jumde! We just need a Linux check for Test Plan A and we can mark this as checked on all three platforms.

@diracdeltas
Copy link
Member

checked on debian 8 and el capitan

bridiver pushed a commit to brave/muon that referenced this issue Jan 16, 2018
fix brave/browser-laptop#12563

Auditors: @bridiver, @diracdeltas

Test plan:
1. Make sure built-in password manager is enabled
2. Male sure passphrase is set on trezor wallet
3. Plugin trezor and open wallet
4. Type passphrase and submit
5. Brave shouldn't prompt any messages to save password

1. Make sure built-in password manager is enabled
2. Sign up account for https://trac.torproject.org
3. Brave should ask users to save password, click deny
4. Logout and Login
5. Brave should ask users to save password, click allow
6. Change password
7. Brave should ask users to update password, click allow
8. Logout and use the save credentials to login
9. It should be able to login sucessfully
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.