Closed Bug 1631879 Opened 2 months ago Closed 2 months ago

Improve handling of Windows usernames and domain accounts for OS authentication

Categories

(Firefox :: about:logins, defect, P2)

Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
Firefox 77
Tracking Status
firefox-esr68 --- unaffected
firefox75 --- unaffected
firefox76 --- wontfix
firefox77 --- verified
firefox78 --- verified
firefox79 --- verified

People

(Reporter: jaws, Assigned: jaws)

References

(Depends on 1 open bug, Regression)

Details

(Keywords: parity-chrome, regression)

Attachments

(6 files, 1 obsolete file)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

As part of authenticating against the OS when providing access to stored passwords in about:logins, we attempt to check if the user has no password on their account. This will register as a login attempt, and can count against a users allowed number of failed login attempts.

We should cache if the user has no password so we can skip this step in the future. We can use https://docs.microsoft.com/en-us/windows/win32/api/lmaccess/ns-lmaccess-user_info_1 to check the password age and re-attempt if the password has been changed.

I think this should be higher priority than P3 because it can cause the user to get locked out of their whole OS account. It's also not difficult as we can look at Chromium's code.

Mike, what do you think? Do you have any idea what default AD lockout policies are? Does a successful auth. reset the failed attempt count?

Flags: qe-verify+
Flags: needinfo?(mozilla)
Priority: P3 → P2
Keywords: regression
Regressed by: 1622542

Default is apparently 0, but can be set by the administrator, as well as the lockout duration.

I can't find out if a successful auth resets the attempt count, but that's pretty much how every auth works, so I would imagine that is the case.

My bigger concern would be if a company only allowed one failed login attempt and they would be immediately locked out of their account (although one attempt seems low).

Flags: needinfo?(mozilla)
Keywords: parity-chrome
Assignee: nobody → jaws
Status: NEW → ASSIGNED

Importing security.h introduced namespace collisions so I removed the using namespace mozilla; and replaced it with specific names.

Comment on attachment 9143168 [details]
Bug 1631879 - Cache the result of the empty password checks. r?MattN,agashlin

Revision D72426 was moved to bug 1633090. Setting attachment 9143168 [details] to obsolete.

Attachment #9143168 - Attachment is obsolete: true
OS: Unspecified → Windows
Summary: OS accounts with no password should be remembered to limit future invalid authentication attempts → Improve handling of Windows usernames and domain accounts for OS authentication
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/f1184b12aff4
Use GetUserNameEx with NameSamCompatible to make sure that we are retrieving fully qualified usernames. r=MattN,cmartin
https://hg.mozilla.org/integration/autoland/rev/7dde3a44450d
Only check for blank passwords if the OS is not on a domain. r=MattN
https://hg.mozilla.org/integration/autoland/rev/b18f1698ee69
Remove unused 'save' variable. r=MattN
https://hg.mozilla.org/integration/autoland/rev/1332a388f227
Remove the domain portion of the username when testing for a blank password. r=MattN,cmartin
https://hg.mozilla.org/integration/autoland/rev/a92f439d3a01
Update the name of the osKeyStore log preference now that it has moved to toolkit. r=MattN
Depends on: 1633097
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/28e3d4f0d18d
Workaround IsOS/OS_DOMAINMEMBER missing from mingw headers.

@Jared, @Matthew, can you help me with some instructions on how can I verify this fix? I have compered the auth dialog triggered from a build that contains the fix and one that doesn't contain the fix but I haven't observed any differences after testing a few scenarios.

Flags: needinfo?(jaws)
Flags: needinfo?(MattN+bmo)

Given that we're building the 76 RC today, I assume we're planning to let these fixes ride 77 to release.

Unfortunately I don't have great steps to test this. The only thing I can think about is if you are able to have two user accounts on the same machine but with one on a domain and the other without a domain, yet both sharing the same local portion of the username. For example:
"TESTDOMAIN\Jared" and "Jared". In addition you could make the local account, "Jared" in this case, not have a password set.

Flags: needinfo?(jaws)

The other thing you could test (for https://phabricator.services.mozilla.com/D72423) if you have an Active Directory domain setup is ensuring that you don't have a login failure recorded (on the server?) due to the blank password check.

Flags: needinfo?(MattN+bmo)

Unfortunately, I haven't managed to create a user with a domain or to set up an Active Directory domain on my work station. However, I will give it a try on my personal computer.

Unfortunately, I don't have enough information about how to create windows 10 users on a domain. However, I have followed a few tutorials regarding this and I found out that in order to create accounts on a domain you can join an existent domain or create one using Windows Server 2016. I'm not aware of any existing domain that I could join so I need to create a domain using Windows Server 2016 and active directory setup.
I think the easiest way is to install Windows Server 2016 on a Virtual Machine, but I need a .iso file for it.

@Matt, @Jared is there any other way how I could verify this issue?

Also, can you please confirm if I understood correctly what scenario I need to verify?
Scenario 1:

  • Have two user accounts on the same machine that sharing the same local portion of the username but one account is on a domain and the other one it's not on a domain (local). The account with domain has a password and the local account doesn't have a password.
  • Verify that the Windows Security dialog is NOT required when trying to reveal/copy/edit a login on the local account.
  • Verify that the Windows Security dialog is required when trying to reveal/copy/edit a login on the account with a domain.
    Scenario 2:
  • Verify if there are no failures recorded on the Server Manager when trying to reveal/copy/edit a login on the account with a domain.
Flags: needinfo?(jaws)
Flags: needinfo?(MattN+bmo)

(In reply to Cosmin Muntean [:cmuntean], Ecosystem QA from comment #21)

Unfortunately, I don't have enough information about how to create windows 10 users on a domain. However, I have followed a few tutorials regarding this and I found out that in order to create accounts on a domain you can join an existent domain or create one using Windows Server 2016. I'm not aware of any existing domain that I could join so I need to create a domain using Windows Server 2016 and active directory setup.
I think the easiest way is to install Windows Server 2016 on a Virtual Machine, but I need a .iso file for it.

@Matt, @Jared is there any other way how I could verify this issue?

Also, can you please confirm if I understood correctly what scenario I need to verify?
Scenario 1:

  • Have two user accounts on the same machine that sharing the same local portion of the username but one account is on a domain and the other one it's not on a domain (local). The account with domain has a password and the local account doesn't have a password.
  • Verify that the Windows Security dialog is NOT required when trying to reveal/copy/edit a login on the local account.
  • Verify that the Windows Security dialog is required when trying to reveal/copy/edit a login on the account with a domain.
    Scenario 2:
  • Verify if there are no failures recorded on the Server Manager when trying to reveal/copy/edit a login on the account with a domain.

Yes, both scenarios are correct. I will add that when the dialog appears it should show the currently logged on user (so in this case, it should show that this is the domain user and not the local account).

@Mike, are you able to help test this?

Flags: needinfo?(jaws) → needinfo?(mozilla)

I've tried to enable the Password Protection stuff on Azure AD, so unfortunately no. It's not working as I expected.

IT looks like there is additional stuff needed that I don't have.

There does appear to be a way to do this via local policu though?

https://www.isunshare.com/windows-10/account-lockout-after-failed-logon-attempts-how-to-make-it.html

Flags: needinfo?(mozilla)
Attachment #9143168 - Attachment is obsolete: false

Comment on attachment 9143168 [details]
Bug 1631879 - Cache the result of the empty password checks. r?MattN,agashlin

Revision D72426 was moved to bug 1633090. Setting attachment 9143168 [details] to obsolete.

Attachment #9143168 - Attachment is obsolete: true

(In reply to Cosmin Muntean [:cmuntean], Ecosystem QA from comment #21)

  • Verify that the Windows Security dialog is NOT required when trying to reveal/copy/edit a login on the local account.

I'm not sure why you wouldn't expect it in this case but maybe I'm forgetting context? The dialog should appear as long as the password isn't empty.

Flags: needinfo?(MattN+bmo)

(In reply to Matthew N. [:MattN] (PM me if request are blocking you) from comment #25)

(In reply to Cosmin Muntean [:cmuntean], Ecosystem QA from comment #21)

  • Verify that the Windows Security dialog is NOT required when trying to reveal/copy/edit a login on the local account.

I'm not sure why you wouldn't expect it in this case but maybe I'm forgetting context? The dialog should appear as long as the password isn't empty.

The request was that the local account not have a password set on it. From comment 21, "Have two user accounts on the same machine that sharing the same local portion of the username but one account is on a domain and the other one it's not on a domain (local). The account with domain has a password and the local account doesn't have a password."

The IT department helped me and created a virtual environment with Windows 10 x64 version 1909 with two user accounts. One account is on a domain and the other one its local. Both share the same local portion of the username (domain account: DOMAIN\cosmin.muntean and the local account: cosmin.muntean).

I have verified the following scenarios on Firefox 77.0.1 release, Firefox Beta 78.0b7 (Build ID 20200612174529) and on the latest Nightly 79.0a1 (Build ID 20200615092624):

Account domain:

  • Verified that the correct username and details are displayed when the OS auth is triggered from the Firefox browser.
  • Verified that the correct credentials are required when you are logged into the account domain.
  • Verified that the password is shown/copied or the login can be edited after entering the credentials.

Local account:

  • Verified that the correct username and details are displayed when the OS auth is triggered from the Firefox browser.
  • Verified that the correct credentials are required.
  • Verified that the password is shown/copied or the login can be edited after entering the credentials.

Please note that both accounts have an OS password set. I have tried to remove the password of the local account to verify the scenarios without OS password set, but this cannot be done. I have talked with the IT guys and they explained that as long as we have an account domain we cannot remove the password on the other account.

Besides the scenarios described above I have also verified the following:

  • On my work station that is on a domain, I have verified if the OS auth dialog works correctly if I have an OS password set. I have also verified that the Windows credentials are not required if I don't have a password set on this machine.
  • On my home computer, I have created two local accounts and verified that the OS auth dialog works correctly if I have an OS password set and also if I don't have a password set. In this case, the accounts have different names because since both are local accounts cannot share the same name.

During testing, I haven't encountered any new issues on my end. Please let me know if there are any other scenarios that I could verify.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.