Skip to content

Conversation

@dscho
Copy link
Member

@dscho dscho commented Sep 18, 2019

Recently, we hardened the ProgramData support by ignoring the config file when it is owned by a "dubious" user, i.e. by an account other than the local administrators group, the system account or the current user.

Turns out, there is at least one scenario where that config file can be legitimately owned by a specific administrator rather than the administrators group, and that still should not be treated as "dubious". Let's implement code to handle this situation gracefully.

This fixes #2304.

Let's refactor the `validate_system_file_ownership()` a bit, not only to
clarify what is done, but also to prepare for maybe adding a slighty
more expensive check: it is possible that the file
`C:\ProgramData\Git\config` is owned by the local administrator (who has
a different SID than the `Administrators` group).

Signed-off-by: Johannes Schindelin <[email protected]>
We recently hardened Git for Windows by ignoring
`C:\ProgramData\Git\config` files unless they are owned by the system
account, by the administrators group, or by the account under which Git
is running.

Turns out that there are situations when that config file is owned by
_an_ administrator, not by the entire administrators group, and that
still is okay. This can happen very easily e.g. in Docker Containers.

Let's add a fall back when the owner is none of the three currently
expected ones, enumerating the members of the administrators group and
comparing them to the file's owner. If a match is found, the owner is
not dubious.

Enumerating groups' members on Windows is not exactly a cheap operation,
and it requires linking to the `netapi32.dll`. Since this is rarely
needed, and since this is done at most a handful of times during any Git
process' life cycle, it is okay that it is a bit expensive. To avoid the
startup cost of linking to yet another DLL, we do this lazily instead:
that way, the vast majority of Git for Windows' users will not feel any
impact by this patch.

This fixes git-for-windows#2304.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho force-pushed the program-data-config-owned-by-specific-administrator branch from 69c9a84 to 8f8ffd9 Compare September 18, 2019 09:32
@dscho dscho merged commit d01cfeb into git-for-windows:master Oct 2, 2019
@dscho dscho deleted the program-data-config-owned-by-specific-administrator branch October 2, 2019 12:38
dscho added a commit that referenced this pull request Oct 2, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
dscho added a commit that referenced this pull request Oct 2, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 3, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 3, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
dscho added a commit that referenced this pull request Oct 4, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
dscho added a commit that referenced this pull request Oct 4, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 6, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 6, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 6, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 7, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 7, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 7, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 7, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 9, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 9, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 9, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 11, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
dscho added a commit that referenced this pull request Oct 11, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
dscho added a commit that referenced this pull request Oct 11, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
dscho added a commit that referenced this pull request Oct 12, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 15, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 15, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
dscho added a commit that referenced this pull request Oct 15, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 16, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
dscho added a commit that referenced this pull request Oct 16, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 17, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
dscho added a commit that referenced this pull request Oct 17, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 18, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
git-for-windows-ci pushed a commit that referenced this pull request Oct 18, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
dscho added a commit that referenced this pull request Oct 21, 2019
…ific-administrator

Do not complain if `C:\ProgramData\Git\config` is owned by a specific administrator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

warning: 'C:\ProgramData/Git/config' has a dubious owner: 'Administrateur'.

1 participant