1704764 - mach bootstrap warns about Defender source dir exclusions even when they exist

Status: RESOLVED FIXED

(Firefox Build System :: Bootstrap Configuration, defect)

Description

[:Gijs (he/him)]

Attachment: Exclusions

My Windows Defender exclusions are set up as per the screenshot. My objdir is under D:\builds, and my srcdir is at D:\mozilla-central (well, one of them). Yet if I run ./mach bootstrap from that srcdir, I still get warned by the code at https://searchfox.org/mozilla-central/rev/3de2db87f3c9001ae478318d47a2ca3427574382/python/mozboot/mozboot/mozillabuild.py#120-129 . Perhaps the lack of trailing backslashes is an issue?

Comment 1

[Mitchell Hentges [:mhentges] 🦀]

Can you chuck a print("exclusion_path={} srcdir={}".format(exclusion_path, srcdir)) in is_windefender_affecting_srcdir and let me know what those values are?

My first guess is that this is something "non-C-drive-related", but os.path.commonpath seems to behave well with D: paths.

Comment 2

[:Gijs (he/him)]

(In reply to Mitchell Hentges [:mhentges] 🦀 from comment #1)

Can you chuck a print("exclusion_path={} srcdir={}".format(exclusion_path, srcdir)) in is_windefender_affecting_srcdir and let me know what those values are?

My first guess is that this is something "non-C-drive-related", but os.path.commonpath seems to behave well with D: paths.

So this didn't log anything, which confused me for a bit. After a bit more printf debugging, it turns out fetching the paths fails entirely with:

Error: [WinError 5] Access is denied

so we hit https://searchfox.org/mozilla-central/rev/3de2db87f3c9001ae478318d47a2ca3427574382/python/mozboot/mozboot/mozillabuild.py#71-72 which means the length of "paths" is 0 and we never check any of them. I guess when fetching the paths fails, the message could be clearer about the fact that we weren't able to check if the exclusion is present?

Comment 3

[Mitchell Hentges [:mhentges] 🦀]

Interesting!
I did some testing locally (e.g.: querying these registry keys as a non-admin), but I'm unable to reproduce the problem.

Can you do a bit of research for me?

• Which line specifically is raising the error? is it OpenKeyEx, QueryInfoKey, or EnumValue (if EnumValue, then with which parameters)?
• Is your user an Administrator on that machine?
• Would you mind doing some digging into regedit and taking a peek at the permissions of the registry key that's causing the error (right click > "Permissions")
  • For me, for both Exclusions and Paths, I'm seeing that Everyone has Read permissions (for Paths, Read is unchecked, but clicking Advanced, and then I can see that it's inheriting Read permissions from Exclusions.
• Have you done any permission-related tweaking for Windows Defender, or installed any AntiVirus software?

I'm hoping that this information will be enough to be able to provide a useful warning when WinError 5 is encountered here.

Comment 4

[:Gijs (he/him)]

Attachment: Permissions on Paths

(In reply to Mitchell Hentges [:mhentges] 🦀 from comment #3)

Interesting!
I did some testing locally (e.g.: querying these registry keys as a non-admin), but I'm unable to reproduce the problem.

Can you do a bit of research for me?

• Which line specifically is raising the error? is it OpenKeyEx, QueryInfoKey, or EnumValue (if EnumValue, then with which parameters)?

I changed the code to:

    paths = []
    try:
        print("Opening key")
        with winreg.OpenKeyEx(
            winreg.HKEY_LOCAL_MACHINE,
            r"SOFTWARE\Microsoft\Windows Defender\Exclusions\Paths",
        ) as exclusions_key:
            print("Querying info for key")
            _, values_count, __ = winreg.QueryInfoKey(exclusions_key)
            print("Got {} values, iterating".format(values_count))
            for i in range(0, values_count):
                print("Enuming value {}".format(i))
                path, _, __ = winreg.EnumValue(exclusions_key, i)
                paths.append(path)

and only "Opening key" prints. So I think that's what's failing...

• Is your user an Administrator on that machine?

It's a local admin account, yes, though I get a UAC prompt whenever I open regedit.

• Would you mind doing some digging into regedit and taking a peek at the permissions of the registry key that's causing the error (right click > "Permissions")
  • For me, for both Exclusions and Paths, I'm seeing that Everyone has Read permissions (for Paths, Read is unchecked, but clicking Advanced, and then I can see that it's inheriting Read permissions from Exclusions.

I've attached a screenshot of what this looks like for me. It appears I don't have an "everyone" item listed, though administrators have read privileges - the "Windows Defender" key does, but the Exclusions key (and thus everything under it) does not.

• Have you done any permission-related tweaking for Windows Defender, or installed any AntiVirus software?

Definite "no" to the latter. I also don't recall ever doing any Windows registry tweaking for permissions - in fact, I didn't realize the registry had such a granular permissions system...

However, this is a Windows Insider build, and I wonder if this is Tamper Protection, cf. https://docs.microsoft.com/en-us/microsoft-365/security/defender-endpoint/prevent-changes-to-security-settings-with-tamper-protection

I'm hoping that this information will be enough to be able to provide a useful warning when WinError 5 is encountered here.

I hope this helped, lmk if I can check anything else...

Comment 5

[Mitchell Hentges [:mhentges] 🦀]

However, this is a Windows Insider build

Ah haaah, ok. I'll spool one up today and see if I can reproduce this and find an appropriate workaround. Thanks Gijs!

Comment 6

[Mitchell Hentges [:mhentges] 🦀]

Yep, it's reproducible on a "DEV" insider build.
I'll see what options we have for discovering if our directory is excluded, or if we have to remove our check.

Comment 7

[Mitchell Hentges [:mhentges] 🦀]

Hmm, JetBrains has seen this too.
It looks like viewing Windows Defender exclusions is not allowed for non-elevated processes anymore:

PS > (Get-MpPreference).ExclusionPath
N/A: Must be admin to view exclusions

We could do some shenanigans around running an elevated python process that checks the exclusions list, but we'd probably need pywin32 which is wheel-only (which can't fit nicely with our current vendoring strategy). I'm not sure it's worth digging into this too much yet, especially since this only affects insiders at the moment.

So, I'll just politely catch the permission error and avoid printing an incorrect warning.
Thanks for your help diagnosing this issue, Gijs 👍

Comment 8

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1704764: Remove OSError catch for fetching nonexistent registry key

Since Python 3, attempting to fetch a registry key that doesn't exist
throws FileNotFoundError, not WindowsError: [Error 2].

Comment 9

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1704764: Don't warn about Defender exclusions if missing permissions

Newer versions of Windows don't allow un-elevated processes to access
the list of Windows Defender exclusions.
Rather than incorrectly falling back to assuming that there's no
exclusions, we now instead skip the exclusion check.

Depends on D112647

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1da658a5571f
Remove OSError catch for fetching nonexistent registry key r=sheehan
https://hg.mozilla.org/integration/autoland/rev/255461fcffe4
Don't warn about Defender exclusions if missing permissions r=firefox-build-system-reviewers,glandium

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/1da658a5571f
https://hg.mozilla.org/mozilla-central/rev/255461fcffe4