MinGW: link as terminal server aware

[rimrul]

Whith Windows 2000, Microsoft introduced a flag to the PE header to mark executables as
"terminal server aware". Windows terminal servers provide a redirected Windows directory and
redirected registry hives when launching legacy applications without this flag set. Since we
do not use any INI files in the Windows directory and don't write to the registry, we don't
need this additional preparation. Telling the OS that we don't need this should provide
slightly improved startup times in terminal server environments.

When building for supported Windows Versions with MSVC the /TSAWARE linker flag is
automatically set, but MinGW requires us to set the --tsaware flag manually.

This partially addresses #3935

[dscho]

@rimrul thank you for providing this PR.

I have to admit that I am not really knowledgeable of the full extent of the impact of this tsaware flag. Is there any documentation? My worry is that in the endeavor to slightly speed up git.exe invocations in Terminal Services (which might even make too negligible a difference to be noticeable) we might introduce a regression in a weird corner case that would make us regret introducing this change.

[dscho]

FWIW I kicked off a git-artifacts build to have an installer and a Portable Git to test.

[dscho]

FWIW I kicked off a git-artifacts build to have an installer and a Portable Git to test.

@ElemenTP since you seem to have access to Windows Terminal Services, could you give this a good testing? I am really unsure of the risk, and that would give me some confidence.

[rimrul]

There is some documentation, but it's not much.
https://docs.microsoft.com/en-us/cpp/build/reference/tsaware-create-terminal-server-aware-application

https://docs.microsoft.com/en-us/previous-versions/aa382957

The older versions of the first document are a little more detailed on when MSVC does it by default:

https://github.com/MicrosoftDocs/cpp-docs/blob/897e3bb83468f06e9f7410aeb783be1b7bea40fa/docs/build/reference/tsaware-create-terminal-server-aware-application.md

I've also found this stackoverflow discussion noticing that it leads to a separate .idata section, presumably to optimise use of memory pages in TS environments.

https://stackoverflow.com/questions/1261291/what-does-the-tsaware-linker-flag-do-to-the-pe-executable

The flag should be set for all cygwin executables, so I'm slightly confused why the original report mentions executables in /usr/bin.

https://cygwin.com/pipermail/cygwin/2012-April/202019.html

https://gcc-patches.gcc.gnu.narkive.com/eaLPK1Ru/patch-cygwin-add-tsaware-linker-flag-to-cygwin-linker-specs

[rimrul]

I have to admit that I am not really knowledgeable of the full extent of the impact of this tsaware flag.

We could ping the documentation author and kindly ask them about this. They're fairly active on GitHub and might be able to tell us (and/or document) all the things that happen with/without this flag.

[ElemenTP]

Thank you for your attention.
Noticed that Terminal Service has been renamed to Remote Desktop Terminal Services has been renamed
These docs also may help:
https://docs.microsoft.com/en-us/windows/win32/termserv/application-compatibility-layer
https://docs.microsoft.com/en-us/troubleshoot/windows/win32/access-violation-bex-appcrash
https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getwindowsdirectoryw

FWIW I kicked off a git-artifacts build to have an installer and a Portable Git to test.

@ElemenTP since you seem to have access to Windows Terminal Services, could you give this a good testing? I am really unsure of the risk, and that would give me some confidence.

I have tested the installer git in git-artifacts with my projects on a Windows 11 PC through Remote Desktop connection, and it works fine. This can't be good enough, though. I am sorry that I can not provide much help since I am not pro with git and git for windows.

[ElemenTP]

I've also found this stackoverflow discussion noticing that it leads to a separate .idata section, presumably to optimise use of memory pages in TS environments.

https://stackoverflow.com/questions/1261291/what-does-the-tsaware-linker-flag-do-to-the-pe-executable

I tested this for both mingw-gcc with linker version 2.38 and MSVC with linker version 14.32, the artifacts with or without tsaware flag are of the same number of sections.

Dump of file .\TestApplication_msvc_tsa.exe

File Type: EXECUTABLE IMAGE

  Summary

        1000 .data
        1000 .pdata
        1000 .rdata
        1000 .reloc
        1000 .rsrc
        1000 .text
Dump of file .\TestApplication_msvc_notsa.exe

File Type: EXECUTABLE IMAGE

  Summary

        1000 .data
        1000 .pdata
        1000 .rdata
        1000 .reloc
        1000 .rsrc
        1000 .text
Dump of file .\test_mingw_tsa.exe

File Type: EXECUTABLE IMAGE

  Summary

        1000 .CRT
        1000 .bss
        1000 .data
        1000 .idata
        1000 .pdata
        1000 .rdata
        1000 .reloc
        3000 .text
        1000 .tls
        1000 .xdata
Dump of file .\test_mingw_notsa.exe

File Type: EXECUTABLE IMAGE

  Summary

        1000 .CRT
        1000 .bss
        1000 .data
        1000 .idata
        1000 .pdata
        1000 .rdata
        1000 .reloc
        3000 .text
        1000 .tls
        1000 .xdata

[dscho]

https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getwindowsdirectoryw

Quoting from there:

Terminal Services: If the application is running in a Terminal Services environment, each user has a private Windows directory. There is also a shared Windows directory for the system. If the application is Terminal-Services-aware (has the IMAGE_DLLCHARACTERISTICS_TERMINAL_SERVER_AWARE flag set in the image header), this function returns the path of the system Windows directory, just as the GetSystemWindowsDirectory function does. Otherwise, it retrieves the path of the private Windows directory for the user.

Wouldn't this be something we want, to safe-guard Git users from other (potentially malicious) users on the same Terminal Services server?

[rimrul]

Wouldn't this be something we want, to safe-guard Git users from other (potentially malicious) users on the same Terminal Services server?

Do we read any config from or write any config to the windows directory?

[dscho]

Wouldn't this be something we want, to safe-guard Git users from other (potentially malicious) users on the same Terminal Services server?

Do we read any config from or write any config to the windows directory?

Not that I am aware of, other than the registry entry to determine whether we're running in a container.

But I am worried about things I might have missed. For example, for years I did not realize that C:\Windows\Temp was word-writable. Or imagine my shock when I realized that some software installs an OpenSSL DLL into C:\Windows\system32 _and then leaves that file not updated for years.

[reflectronic]

safe-guard Git users from other (potentially malicious) users on the same Terminal Services server?

I do not think this flag is a security feature. If an attacker can write malicious payloads into the system's Windows directory, there is no need to exploit Git through some kind of DLL planting -- they already have administrative access. Nearly every app compiled with a 21st century toolchain has this flag set by default -- Microsoft seems to think that there's very little risk.

[dscho]

I do not think this flag is a security feature. If an attacker can write malicious payloads into the system's Windows directory, there is no need to exploit Git through some kind of DLL planting -- they already have administrative access

I guess you're right ;-)

[dscho]

Okay, I made up my mind, thank y'all for your patience convincing me!