Workaround for Mandatory ASLR

[conioh]

The Git for Windows binaries don't work with ASLR because of the way cygwin/Mingw/MSYS/whatever implements fork. This is unfortunate, but ain't gonna change anytime soon (as per #1196 etc.).

However, running with Mandatory ASLR enabled may be useful or desired as part of computer hardening.

A (sad) workaround is to add exceptions to the system policy and exempt the Git binaries from ASLR.

Doing that through the GUI is crazy as there are 444 such EXEs. The PowerShell Set-ProcessMitigation cmdlet doesn't support using full paths; it would exempt any process with a given name. The two options left are preparing an XML of an undocumented but easily understandable schema and importing it using Set-ProcessMitigation -PolicyFilePath or editing the Registry manually.

The following is a PowerShell script that directly manipulates the Registry and adds exceptions for all the EXE files inside the Git directory except the 3 found at the root: https://gitlab.com/conio.h/GfW_ASLR_Exceptions

It expects a path to the Git directory (e.g. "C:\Program Files\Git") and looks for the git install directory in the Registry if no path is provided. The path parameter can be used for "portable git" installs.

There's virtually no error checking there. I have no idea what happens if you gives a bad path, for example. What I can say is that "it works on my computer" (RS3 x64, if anybody cares).

[dscho]

Interesting.

Are you sure that you have to exclude all .exe files, not just the ones under usr/?

[conioh]

No. I only have to exclude those who have relocations and aren't marked ASLR-ready. If they're marked ASLR-ready there's no reason to exempt them from ASLR and if they don't have relocations ASLR is irrelevant anyway.

Maybe such binaries are only found under usr/, but as I wrote here - I got a few error messages when I launched git-bash and added exceptions for the binaries there, and then used another git command and got a bunch more, so I went on with a broad strike.

I just didn't get to it, and it wasn't that important at the moment. I just needed Git to work so I did the minimum that worked for me at the moment.

(I also wanted something that works on a clean machine, so I didn't have Python and pefile to use. I later found some PowerShell packages on GitHub that parse PE files, but didn't get to it yet. Of course, if you incorporate something like this into the GfW installer you won't have those limitations. Or you can know in advance which binaries require exemptions and which don't and generate a list of files to exempt as part of the GfW build process.)

[dscho]

Or you can know in advance which binaries require exemptions

Indeed. I know that pretty precisely because the fork() emulation is provided by usr\bin\msys-2.0.dll, and by convention all .exe files relying explicitly or implicitly on that emulation are in the usr directory tree.

In fact, it is even safe to assume that the opposite is true, too: all .exe files found in the usr directory tree link to msys-2.0.dll. That is by design, because the usr directory tree holds so-called "MSYS programs" which link to msys-2.0.dll implicitly (read: you do not have to say so explicitly when linking with usr\bin\gcc.exe).

This means that all of the .exe files in the mingw64 directory tree (or on 32-bit systems, the mingw32 one) do not link to msys-2.0.dll, and since that is the only .dll in Git for Windows (as far as I know) that is not ASLR-ready, they should not be exempt.

In fact, Git's own executables (mingw64\bin\git*.exe and mingw64\libexec\git-core\*.exe) are marked specifically to use ASLR, even without Mandatory ASLR.

And yes, I think the best place for doing essentially what your Powershell script does would be the installer, as opt-in.

Interested? It should be a fun ride.

Step 1: install Git for Windows' SDK

https://git-for-windows.github.io/#download-sdk

It will run for a while, download ~200MB of MSYS2's packages, and build a current Git.

Step 2: build an installer

This should be as easy as calling /usr/src/build-extra/installer/release.sh 0-test in Git for Windows' SDK's Bash (the git-bash.exe at the top-level directory where you installed your SDK).

Step 3: change the installer

Probably the best place would be to add it as a "component". See e.g. how the "autoupdate" feature works: https://github.com/git-for-windows/build-extra/blob/af9cff50050b15520a8a3885ccfb6c9b4b65611b/installer/install.iss#L105

You will want to add the exceptions around the same place the autoupdater is installed: https://github.com/git-for-windows/build-extra/blob/af9cff50050b15520a8a3885ccfb6c9b4b65611b/installer/install.iss#L2101-L2106

And remove those exceptions around the same place the autoupdater is uninstalled: https://github.com/git-for-windows/build-extra/blob/af9cff50050b15520a8a3885ccfb6c9b4b65611b/installer/install.iss#L2347-L2352

You can set registry values by imitating how Git for Windows' installer registers the Explorer integration: https://github.com/git-for-windows/build-extra/blob/af9cff50050b15520a8a3885ccfb6c9b4b65611b/installer/install.iss#L2040-L2054

You can enumerate files in a directory using FindFirst()/FindNext()/FindClose() just like we do when trying to hard-link the .dll files into libexec: https://github.com/git-for-windows/build-extra/blob/af9cff50050b15520a8a3885ccfb6c9b4b65611b/installer/install.iss#L1671-L1682

And you can extend this to a recursive search by handing FILE_ATTRIBUTE_DIRECTORY: https://github.com/git-for-windows/build-extra/blob/af9cff50050b15520a8a3885ccfb6c9b4b65611b/installer/install.iss#L1674

Finally, the top-level directory with which to start is the usr directory of the installed Git for Windows: ExpandConstant('{app}\usr').

Step 4: build and test a custom installer

This is really important, to verify that it worked: /usr/src/build-extra/installer/release.sh 0-test. This will overwrite the file in your home directory which was generated in step 2, and that's okay.

Step 5: open a Pull Request

... and I'll merge it as soon as possible. And then I'll celebrate!

[conioh]

... and by convention all .exe files relying explicitly or implicitly on that emulation are in the usr directory tree.

That's good to know. I'll take a look at implementing this when I could spare a little bit more time.

[dori4n]

Hello,

Please, and I must reiterate, do not automatically deactivate, disable, or otherwise circumvent security options users or administrators have set up.

Also @conioh, please be aware, that the proper method to enable system-wide ASLR is to set the following registry options:

Windows Registry Editor Version 5.00

[HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\kernel]
"MitigationOptions"=hex:11,11,11,00,00,00,00,00,00,00,00,00,00,00,00,00

To properly enforce DEP, you would run bcdedit /set {current} nx alwayson at an elevated command prompt. You will need to reboot the computer for both settings to be correctly applied.

If you have done that, using exclusion rules is not possible, and applications cannot opt-out of image relocation or execute disable bit protections. As mentioned in https://github.com/git-for-windows/git/issues/1374#issuecomment-348019162 the --dynamicbase compile option is broken in MinGW. Relevant issues: https://sourceforge.net/p/mingw-w64/mailman/message/31034877/, https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=836365, bitcoin/bitcoin#8248, and this StackOverflow question: https://stackoverflow.com/questions/24283918/

[dscho]

@NightmareJoker2 you would have a ton more credibility if you would not have claimed to be able to fix Perl with regards to ASLR "in less than 5 minutes", without ever backing up such a strong claim with anything substantial.

I am not saying that your statements are incorrect. I am saying that I doubt them, based on how past claims held up in bright daylight.

[dori4n]

@dscho I did not claim "fixing it" would take me five minutes. I said in https://github.com/git-for-windows/git/issues/1196#issuecomment-348006411, and I quote: "[I] could send in a pull request [of what I already have] in less than 5 minutes, if I wanted to". That said, you can read the material I have referenced yourself, verify it, and do not need to put any trust in my words. In fact, I don't want you to. If you think carefully about this, you will understand why I take that position. What I have referenced in the last paragraph will help you with your existing toolchain, but I would still strongly advocate, since you are targeting Windows, and considering the project name, are focusing on it, that you switch to one with better platform support. It will help you long-term. If you do however for some reason need cross-platform build support, I'd suggest to look into CMake or similar abstractions. Based on how much effort you are convinced this would take, I'd prefer you do it yourself and learn from it, rather than have it done for you. Even if you don't like me saying that, it will only make you a better programmer in the end.

[dscho]

Yes, all of this makes me trust those claims even less. Now you are suggesting to switch to CMake. Seriously.

[dscho]

@conioh let's get back on track and just ignore the distraction, okay? Please feel free to reach out if anything in my write-up is unclear (or does not work as advertised).

And thank you for offering to contribute!

[dori4n]

Yes, all of this makes me trust those claims even less.

You are not supposed to trust me, you are supposed to verify them for yourself. That is the whole point of the exercise. I want you to learn something rather than take what I say at face value.

Now you are suggesting to switch to CMake. Seriously.

What is the problem with that? Using CMake or a similar build system abstraction layer gets you binaries built with the native tools and is easy to integrate with a continuous integration system that tells you about build errors on any targeted platform for each commit and pull request, if you so choose. You can even automate creation and tear-down of the build VM or container. Of course you can also tackle the problem of fixing the MinGW compiler, but I'd wager that to be a challenging endeavor, if nobody considered it in over 7 years. Quite possibly, also, because switching the toolset is so much easier, especially thanks to standardization of the programming language.

For the last time: Compile with -Wl,--nxcompat,--dynamicbase,--export-all-symbols,--high-entropy-va and -Wl,--image-base,0x140000000 for dynamic libraries and -Wl,--image-base,0x180000000 for executables. Please stop saying this is difficult. Doing so only does two things, neither of them favorable to you.

[dscho]

1. Writing about the benefits of CMake without any consideration of the costs is just uninformed. Please stop suggesting it without even converting a single project (you would not tout that horn if you had done that; I have).
2. The MinGW compiler has no problem. If you want to be taken seriously about your claims, you have to get the facts right.
3. Passing compiler flags is not difficult. That's child's play. But it is not as easy as you say because it is not a matter of simply passing compiler flags. Again, you are just proving even further how uninformed you are about the matter you claim to understand.

And the worst part is: this here ticket is not in need of distractions like the one provided by you. It is just wasting time and good will. So let's stop here.

[dori4n]

Writing about the benefits of CMake without any consideration of the costs is just uninformed. Please stop suggesting it without even converting a single project (you would not tout that horn if you had done that; I have).

You are pretending to know something you cannot possibly have any knowledge about. You know how the saying goes: "If you assume..." (I'll stop there)

The MinGW compiler has no problem. If you want to be taken seriously about your claims, you have to get the facts right.

It does have several problems. The people from the VideoLAN, Rust, or Bitcoin projects can sing you songs about it. If you feel what I say is incorrect, point it out, detail your issue and render proof of error and I will correct it for future use. You are more likely than not, reading something into my words, that is simply not there, or focusing on an irrelevant detail that has next to no relation to the problem at hand.

Passing compiler flags is not difficult. That's child's play. But it is not as easy as you say because it is not a matter of simply passing compiler flags.

Well, do it then, because you currently aren't. Your makefile does not mention them, and the manner in which you are compiling right now causes MinGW to not emit or to emit an incorrect relocation table.

Microsoft (R) COFF/PE Dumper Version 14.11.25508.2
Copyright (C) Microsoft Corporation.  All rights reserved.

Dump of file C:\Program Files\Git\bin\git.exe

PE signature found

File Type: EXECUTABLE IMAGE

FILE HEADER VALUES
            8664 machine (x64)
               C number of sections
               0 time date stamp
               0 file pointer to symbol table
               0 number of symbols
              F0 size of optional header
             22E characteristics
                   Executable
                   Line numbers stripped
                   Symbols stripped
                   Application can handle large (>2GB) addresses
                   Debug information stripped
OPTIONAL HEADER VALUES
             20B magic # (PE32+)
            2.28 linker version
            3800 size of code
            7600 size of initialized data
           60C00 size of uninitialized data
            1510 entry point (0000000000401510)
            1000 base of code
          400000 image base (0000000000400000 to 0000000000470FFF)
            1000 section alignment
             200 file alignment
            4.00 operating system version
            0.00 image version
            5.02 subsystem version
               0 Win32 version
           71000 size of image
             400 size of headers
           14B04 checksum
               3 subsystem (Windows CUI)
             140 DLL characteristics
                   Dynamic base
                   NX compatible

This is clearly not a binary that was compiled and linked with the flags I mentioned. It is probable, that the Dynamic base and NX compatible have been added manually later or simply added by the dumpbin tool, because it was a 64-bit binary and it is assumed that these options are present, because they are mandatory (the NX compatible is at least).

Again, you are just proving even further how uninformed you are about the matter you claim to understand.

Much rather, you have proven to me, how little you care, how you can't read the information given to you or choose not to, and how you appear to rather turn off security so developers can work with the tool, and go on their merry way of ignoring it in their own applications, too, because they don't notice it missing. This is not okay, Sir.

And the worst part is: this here ticket is not in need of distractions like the one provided by you. It is just wasting time and good will. So let's stop here.

No, the worst part is, you don't even understand yourself what the actual issue is. You have started to propose, that someone create security exception rules for your application and install those by default for all users of your application, when that is precisely what the users or system administrators who manage the systems your application gets installed onto do not want, and you are able, with relative ease, to make most parts (short of those relying on Perl or external Cygwin bits) of the applications work by adjusting linker flags during compilation, which aren't set by default. So, call this a "distraction" all you want, but for as long as you are planning on compromising security or suggest to your users they do it for you, especially that of others, even though that is more work than not to, "distract you" I must. Anything else would be feigning ignorance and utterly irresponsible. I really want to give you the benefit of the doubt here, but I am starting to get the impression you are doing this out of malice now.

[conioh]

@dscho:

I hope to take a look at it the coming week. I understand from the instructions you wrote that the SDK you prepared includes everything necessary to build GfW from source. (That is, I can install it on an empty Windows 10 VM and everything will work.) Correct me if I'm wrong.

The joker did have one valid point that got lost inside all the nonsense he wrote. I intended to raise it after I see that I manage to make reasonable changes to the InnoSetup scripts etc., but since he kind-of mentioned it: I think that adding a screen and a checkbox during the install to enable setting the exceptions might be better.

Sure, I think adding the exceptions is the only reasonable approach if you have Mandatory ASLR enabled, but someone else might think otherwise. He might think that he prefers not to use GfW at all in that case. Maybe he prefers to use git on WSL or write a bunch of his own tools based on libgit2, or God know what else. I think this should be his choice.

[conioh]

@NightmareJoker2:

You haven't got the faintest idea what you're talking about. Please stop wasting everybody's time. The amount of nonsense you wrote is literally incredible.

Also @conioh, please be aware, that the proper method to enable system-wide ASLR is to set the following registry options:

Windows Registry Editor Version 5.00

[HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\kernel]
"MitigationOptions"=hex:11,11,11,00,00,00,00,00,00,00,00,00,00,00,00,00

This is doubly- (or even triply-) wrong.

The flags you enabled here are (documented on here): 0x000001 - PROCESS_CREATION_MITIGATION_POLICY_DEP_ENABLE (0x00000001) 0x000010 - WTF? 0x000100 - PROCESS_CREATION_MITIGATION_POLICY_FORCE_RELOCATE_IMAGES_ALWAYS_ON (0x00000001 << 8) 0x001000 - PROCESS_CREATION_MITIGATION_POLICY_HEAP_TERMINATE_ALWAYS_ON (0x00000001 << 12) 0x010000 - PROCESS_CREATION_MITIGATION_POLICY_BOTTOM_UP_ASLR_ALWAYS_ON (0x00000001 << 16) 0x100000 - PROCESS_CREATION_MITIGATION_POLICY_HIGH_ENTROPY_ASLR_ALWAYS_ON (0x00000001 << 20)

0x1000 in unrelated while 0x10 is... what exactly?

That's strike one.

Even if the nonsense I just quote wasn't wrong, it's simply irrelevant. The question isn't how to enable Mandatory ASLR. The question is how to make "Git Bash" along with all its MSYS utilities to work when Mandatory ASLR is enabled. In other words, the question is how to exempt the MSYS binaries from Mandatory ASLR.

You don't even understand what the issue at hand is.

That's strike two.

If you have done that, using exclusion rules is not possible, and applications cannot opt-out of image relocation or execute disable bit protections.

Again, this is not only completely false, but also utterly stupid. The Microsoft Docs page on customizing Windows Defender Exploit Guard protections explicitly states it is possible and gives the exact steps required to do so.

You have to pick the leave the System settings section and go into the Program settings where you choose or add a binary and select Override system settings for whatever setting you're interested in. Including ASLR.

If you weren't a complete moron - something dscho already knew but I wasn't sure of - and actually read something before blabbering on here you would know that. Do you even understand what "override system settings" means?

That's strike three.

You're out.

But you're too stupid to know it so you continue with your nonsense and post another comment, as if you haven't embarrassed yourself enough by that point.

For the last time: Compile with -Wl,--nxcompat,--dynamicbase,--export-all-symbols,--high-entropy-va and -Wl,--image-base,0x140000000 for dynamic libraries and -Wl,--image-base,0x180000000 for executables. Please stop saying this is difficult. Doing so only does two things, neither of them favorable to you.

But the thing is that regardless of the MinGW GCC not adding relocation information without --export-all-symbols etc. as your linkes suggest, IT'S IMPOSSIBLE TO RELOCATE cygwin1.dll (or msys-2.0.dll which is apparently what the Git toolchain uses) and have fork() work, since its implementation relies on certain addresses being the same in the parent and the child. This is a known issue with fork() in Cygwin. There's even a page in the GfW wiki that talks about it, albeit in a different context: https://github.com/git-for-windows/git/wiki/32-bit-issues

Again, if you had just read more and wrote less...

If it were up to me you'd be banned from the Internet. Luckily for you and unfortunately for the rest of humanity it's not up to me.

Don't mention me again unless it's to apologize for all the nonsense you just wrote.

[dori4n]

0x1000 in unrelated while 0x10 is... what exactly?

Undocumented, obviously. And since that's the case, I can't say at this time.

Even if the nonsense I just quote wasn't wrong, it's simply irrelevant. The question isn't how to enable Mandatory ASLR. The question is how to make "Git Bash" along with all its MSYS utilities to work when Mandatory ASLR is enabled.

The solution to which would be fixing the binaries so they can run with the protections enabled.

In other words, the question is how to exempt the MSYS binaries from Mandatory ASLR.

Wrong.

Again, this is not only completely false, but also utterly stupid. The Microsoft Docs page on customizing Windows Defender Exploit Guard protections explicitly states it is possible and gives the exact steps required to do so.

The registry options are not set by the new Exploit Guard protection options, they used to be set by EMET, which is no longer supported. Setting the registry option to enforce the options, will cause the exclusion rules to be ignored. Please do try it.

IT'S IMPOSSIBLE TO RELOCATE cygwin1.dll (or msys-2.0.dll which is apparently what the Git toolchain uses) and have fork() work, since its implementation relies on certain addresses being the same in the parent and the child.

You can relocate anything, but the finer details of that are too far removed from the topic at hand. Windows does not support fork() and the only manner to achieve a similar result is to use spawn() in places where it is followed by exec() instead, as appropriate, or to emulate the behavior by spawning a thread in the process, or cloning the process with identical register context like WSL does (requires kernel support).

[conioh]

0x1000 in unrelated while 0x10 is... what exactly?

Undocumented, obviously. And since that's the case, I can't say at this time.

That why did you specify it? Because you're an idiot? Because you're an idiot.

Even if the nonsense I just quote wasn't wrong, it's simply irrelevant. The question isn't how to enable Mandatory ASLR. The question is how to make "Git Bash" along with all its MSYS utilities to work when Mandatory ASLR is enabled.

The solution to which would be fixing the binaries so they can run with the protections enabled.

Fine. Then you go ahead and do it if you're so smart. The sane people will continue working with the approach that gave millions of Windows users the ability to use Git.

In other words, the question is how to exempt the MSYS binaries from Mandatory ASLR.

Wrong.

Wrong.

Again, this is not only completely false, but also utterly stupid. The Microsoft Docs page on customizing Windows Defender Exploit Guard protections explicitly states it is possible and gives the exact steps required to do so.

The registry options are not set by the new Exploit Guard protection options, they used to be set by EMET, which is no longer supported. Setting the registry option to enforce the options, will cause the exclusion rules to be ignored. Please do try it.

Not only wrong, but also incredibly dumb. Like the rest of your nonsense. First, WDEG is the supported replacement to EMET on Windows 10 FCU, as written on Microsoft sources multiple times. For example:

Windows Defender Exploit Guard is a new set of intrusion prevention capabilities that ships with the Windows 10 Fall Creators Update. ... The four components of Windows Defender Exploit Guard are:

• Attack Surface Reduction (ASR): A set of controls that enterprises can enable to prevent malware from getting on the machine by blocking Office-, script-, and email-based threats
• Network protection: Protects the endpoint against web-based threats by blocking any outbound process on the device to untrusted hosts/IP through Windows Defender SmartScreen
• Controlled folder access: Protects sensitive data from ransomware by blocking untrusted processes from accessing your protected folders
• Exploit protection: A set of exploit mitigations (replacing EMET) that can be easily configured to protect your system and applications ...

  Rest In Peace (RIP) EMET

  Users of the Enhanced Mitigation Experience Toolkit (EMET) will notice that it was automatically uninstalled from your machine during the upgrade. This is because WDEG includes the best of EMET built directly into Windows 10, so it’s now just part of the platform. You can the find previous user experiences for configuring EMET vulnerability mitigation capabilities in Windows Defender Security Center.

Windows Defender Exploit Guard: Reduce the attack surface against next-generation malware

This is also said in Moving Beyond EMET II – Windows Defender Exploit Guard, etc.

Second, I have tried it. I published my script after seeing it works and fixed my problem. It is you, dumbass, who should try things before saying they don't work. Why won't you take 5 minutes and spend them on trying and learning something instead of spreading BS all over the Internet?

You have no idea what you're talking about and your trolling is incredible disruptive. Please stop it. @dscho, please make him stop.

[dscho]

Dump of file C:\Program Files\Git\bin\git.exe

That's the wrong binary for starters.

And then some.

@conioh I would like to implore you to avoid getting emotional about this. And please, please do not get lured into name calling and insults. I really want this project to be a nice place. I actually do more than that: I require it to be a nice place, because I have to work in it.

I sadly agree with the problematic nature of the claims and the really unfortunate turn the discussion took, and in order to prevent even more toxicity, I (reluctantly) blocked NightmareJoker2.

So can we please set this unhappy distraction aside?

I still think that this ticket is important enough that it deserves not to be derailed. And I still hope that you are up to this:

I hope to take a look at it the coming week.

Thanks!

[sylveon]

Might be at least a good idea to use GetProcessMitigationPolicy during the use of the affected tools and emit a warning/error if force relocation of images is enabled.

This'll at least be much less confusing to the user, and allow him to take appropriate action to fix the issue.

[dscho]

@sylveon I do not know about this feature. How would we use it?

[sylveon]

PROCESS_MITIGATION_ASLR_POLICY aslr_policy;
// GetProcessMitigationPolicy returns non-zero on success
if (GetProcessMitigationPolicy(GetCurrentProcess(), ProcessASLRPolicy, &aslr_policy, sizeof(aslr_policy)))
{
    if (aslr_policy.EnableForceRelocateImages)
    {
        // fatal: image force relocation is enabled.
    }
}
else
{
    // GetProcessMitigationPolicy failed, relevant info in GetLastError()
}

Additionally, if this check is performed before the msys dll is loaded (unfortunately disabling force relocation won't magically move back the images to their preffered base address, that would be too easy), we can try to turn it off:

aslr_policy.EnableForceRelocateImages = false;
if (!SetProcessMitigationPolicy(ProcessASLRPolicy, &aslr_policy, sizeof(aslr_policy)))
{
    // fatal: image force relocation is enabled and couldn't be turned off.
}

After a bit of looking around, it seems that UpdateProcThreadAttribute can also disable it. This one is particularly interesting because we can create the process directly with those attributes using STARTUPINFOEX's lpAttributeList field (the presence of STARTUPINFOEX being signaled to CreateProcess by bitwise ORing EXTENDED_STARTUPINFO_PRESENT to dwCreationFlags) and it overrides the system and user settings.

PROC_THREAD_ATTRIBUTE_LIST *attribute_list;
InitializeProcThreadAttributeList(attribute_list, ???);
DWORD mitigation_policy = PROCESS_CREATION_MITIGATION_POLICY_FORCE_RELOCATE_IMAGES_ALWAYS_OFF;
UpdateProcThreadAttribute(attribute_list, NULL, PROC_THREAD_ATTRIBUTE_MITIGATION_POLICY, &mitigation_policy, sizeof(mitigation_policy), NULL, NULL);
CreateProcess(...);
DeleteProcThreadAttributeList(attribute_list);

This snippet is probably incorrect, as I've never used the ProcThreadAttribute functions before. This MSDN blog article might help: https://blogs.msdn.microsoft.com/oldnewthing/20130426-00/?p=4543/

[dscho]

Is there anybody who is still interested in working on this?

[sylveon]

Well I'd be interested (and I feel more confident about my Win32 programming abilities so it doesn't seems as bad as when I submitted my message) but this would probably require digging in the cygwin source (and maybe the ASLR-compatible git.exe entry point), which I've got no clue how is organized, for all calls which can spawn a new process (so that it applies the exception in case that subprocess forks) as well as the implementation of fork itself (which honestly sounds spooky).

I've also never had much luck compiling anything that is cygwin-based to begin with (they always fail with the most cryptic of errors) so I try to stay away.

Additionally, I'm not sure if git for windows uses a custom cygwin, but if it doesn't it's additional work to get this merged upstream.

Of course, work could be done instead to implement all git commands natively without using fork, but that will most likely be a ton of work that I don't have the time to do nor confidence to get right and cause issues when updating from upstream (unless that work is submitted upstream too)

[dscho]

Well I'd be interested (and I feel more confident about my Win32 programming abilities so it doesn't seems as bad as when I submitted my message) but this would probably require digging in the cygwin source (and maybe the ASLR-compatible git.exe entry point), which I've got no clue how is organized, for all calls which can spawn a new process (so that it applies the exception in case that subprocess forks) as well as the implementation of fork itself (which honestly sounds spooky).

The fork() implementation is pretty involved, I agree.

As to spawning other processes: the fork() and spawn() family of functions are implemented in https://github.com/git-for-windows/msys2-runtime/blob/master/winsup/cygwin/fork.cc and in https://github.com/git-for-windows/msys2-runtime/blob/master/winsup/cygwin/spawn.cc, respectively.

The git.exe program itself emulates a version of spawn(), and the "Git wrapper" (which underlies e.g. the Git Bash or /cmd/git.exe) also spawns processes.

I've also never had much luck compiling anything that is cygwin-based to begin with (they always fail with the most cryptic of errors) so I try to stay away.

The idea here would be to install the Git for Windows SDK and then run sdk build msys2-runtime. That will build the MSYS2 runtime (which is the fork of the Cygwin runtime that we're using).

Additionally, I'm not sure if git for windows uses a custom cygwin, but if it doesn't it's additional work to get this merged upstream.

We do have a fork at https://github.com/git-for-windows/msys2-runtime, and no, you do not need to get this merged upstream, we ship with a custom MSYS2 runtime.

Of course, work could be done instead to implement all git commands natively without using fork, but that will most likely be a ton of work that I don't have the time to do nor confidence to get right and cause issues when updating from upstream (unless that work is submitted upstream too)

Well, it definitely is a ton of work, but I am happy to report that most of the important commands are already converted. So I should actually say it was a ton of work.

The only major commands that still need to be converted are git bisect, git submodule and that's it. There is an Outreachy project under way to take care of git bisect, so there is even less of a big project left to work on.

Having said that, Git aliases will still be executed via the MSYS2 Bash, and I think also the editor and the hooks (unless they are .exe hooks).

[sylveon]

This is extremely helpful, thank you. I'll try getting something working later this month.

[dscho]

@sylveon that would be awesome!

[sylveon]

Alright this is a bit late, but when compiling msys2-runtime by following https://github.com/git-for-windows/git/wiki/Building-msys2-runtime, makepkg fails with the following

make[5]: Entering directory '/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib/libm'
rm -f libm.a
rm -rf tmp
mkdir tmp
cd tmp; \
  for i in math/lib.a common/lib.a complex/lib.a fenv/lib.a machine/lib.a; do \
    ar x ../$i; \
done; \
ar rc ../libm.a *.o
ar: ../machine/lib.a: No such file or directory
ranlib libm.a
rm -rf tmp
make[5]: Leaving directory '/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib/libm'
make[4]: *** [Makefile:549: all-recursive] Error 1
make[4]: Leaving directory '/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib/libm'
make[3]: *** [Makefile:641: all-recursive] Error 1
make[3]: Leaving directory '/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib'
make[2]: *** [Makefile:452: all] Error 2
make[2]: Leaving directory '/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib'
make[1]: *** [Makefile:8496: all-target-newlib] Error 2
make[1]: Leaving directory '/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys'
make: *** [Makefile:883: all] Error 2
==> ERROR: A failure occurred in build().
    Aborting...

[dscho]

@sylveon hmm. I seem to not have that problem. Maybe make -C /usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib/machine works?

[sylveon]

Should have looked a bit before that output, this seems to be the error:

gcc -L/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/winsup/cygwin -isystem /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/cygwin/include -B/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib/ -isystem /usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib/targ-include -isystem /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libc/include    -I/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/cygwin/include -DPACKAGE_NAME=\"newlib\" -DPACKAGE_TARNAME=\"newlib\" -DPACKAGE_VERSION=\"3.1.0\" -DPACKAGE_STRING=\"newlib\ 3.1.0\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -I. -I/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64 -I /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64/../../../../newlib/libm/common -fno-builtin -DHAVE_OPENDIR -DHAVE_RENAME -DGETREENT_PROVIDED -DSIGNAL_PROVIDED -D_COMPILING_NEWLIB -DHAVE_BLKSIZE -DHAVE_FCNTL -DMALLOC_PROVIDED -DHAVE_INIT_FINI      -g -O2 -pipe -ggdb -c -o lib_a-feholdexcept.o `test -f 'feholdexcept.c' || echo '/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64/'`feholdexcept.c
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64/feclearexcept.c:1:1: error: expected identifier or '(' before '.' token
    1 | ../../fenv/fenv_stub.c
      | ^
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64/fegetenv.c:1:1: error: expected identifier or '(' before '.' token
    1 | ../../fenv/fenv_stub.c
      | ^
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64/fegetexceptflag.c:1:1: error: expected identifier or '(' before '.' token
    1 | ../../fenv/fenv_stub.c
      | ^
make[6]: *** [Makefile:347: lib_a-feclearexcept.o] Error 1
make[6]: *** Waiting for unfinished jobs....
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64/fegetround.c:1:1: error: expected identifier or '(' before '.' token
    1 | ../../fenv/fenv_stub.c
      | ^
make[6]: *** [Makefile:353: lib_a-fegetenv.o] Error 1
make[6]: *** [Makefile:365: lib_a-fegetround.o] Error 1
make[6]: *** [Makefile:359: lib_a-fegetexceptflag.o] Error 1
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64/feholdexcept.c:1:1: error: expected identifier or '(' before '.' token
    1 | ../../fenv/fenv_stub.c
      | ^
make[6]: *** [Makefile:371: lib_a-feholdexcept.o] Error 1
make[6]: Leaving directory '/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib/libm/machine/x86_64'
Making all in .
make[6]: Entering directory '/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib/libm/machine'
rm -f lib.a
ln x86_64/lib.a lib.a >/dev/null 2>/dev/null || \
 cp x86_64/lib.a lib.a
cp: cannot stat 'x86_64/lib.a': No such file or directory
make[6]: *** [Makefile:575: lib.a] Error 1

Looks like symlinks are not properly created. I enabled developer mode and gave my user permission to create symlinks in secpol.msc, rebooted, set core.symlinks to true both global and the MSYS2-packages repo level, then ran makepkg -C to no avail.

[sylveon]

Set core.symlinks to true at /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime level, ran git checkout and it built.

[sylveon]

spoke too fast

c++wrap -pipe -MMD -D__OUTSIDE_CYGWIN__ -DSYSCONFDIR="\"/etc\"" -O2 -g -ggdb -fno-rtti -fno-exceptions -fno-use-cxa-atexit -Wall -Wstrict-aliasing -Wwrite-strings -fno-common -pipe -fbuiltin -fmessage-length=0 -c -o setpwd.o /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/cygserver/setpwd.cc
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/dumper.cc: In function 'void print_section_name(bfd*, asection*, void*)':
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/dumper.cc:134:22: error: 'bfd_get_section_name' was not declared in this scope; did you mean 'bfd_get_section_by_name'?
  134 |   deb_printf (" %s", bfd_get_section_name (abfd, sect));
      |                      ^~~~~~~~~~~~~~~~~~~~
      |                      bfd_get_section_by_name
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/dumper.cc: In member function 'int dumper::prepare_core_dump()':
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/dumper.cc:717:11: error: 'bfd_get_section_size' was not declared in this scope; did you mean 'bfd_set_section_size'?
  717 |          (bfd_get_section_size (status_section)
      |           ^~~~~~~~~~~~~~~~~~~~
      |           bfd_set_section_size
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/dumper.cc:741:35: error: cannot convert 'bfd*' to 'asection*' {aka 'bfd_section*'}
  741 |       if (!bfd_set_section_flags (core_bfd, new_section, sect_flags) ||
      |                                   ^~~~~~~~
      |                                   |
      |                                   bfd*
In file included from /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/dumper.cc:23:
/usr/include/bfd.h:1410:46: note:   initializing argument 1 of 'bfd_boolean bfd_set_section_flags(asection*, flagword)'
 1410 | bfd_boolean bfd_set_section_flags (asection *sec, flagword flags);
      |                                    ~~~~~~~~~~^~~
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/dumper.cc:742:27: error: cannot convert 'bfd*' to 'asection*' {aka 'bfd_section*'}
  742 |    !bfd_set_section_size (core_bfd, new_section, sect_size))
      |                           ^~~~~~~~
      |                           |
      |                           bfd*
In file included from /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/dumper.cc:23:
/usr/include/bfd.h:1425:45: note:   initializing argument 1 of 'bfd_boolean bfd_set_section_size(asection*, bfd_size_type)'
 1425 | bfd_boolean bfd_set_section_size (asection *sec, bfd_size_type val);
      |                                   ~~~~~~~~~~^~~
c++wrap -c -o bloda.o -pipe -fno-exceptions -fno-rtti -O2 -g -ggdb  /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/bloda.cc
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/dumper.cc: In member function 'int dumper::write_core_dump()':
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/dumper.cc:826:5: error: 'bfd_get_section_size' was not declared in this scope; did you mean 'bfd_set_section_size'?
  826 |     bfd_get_section_size (p->section),
      |     ^~~~~~~~~~~~~~~~~~~~
      |     bfd_set_section_size
make[3]: *** [/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/../Makefile.common:41: dumper.o] Error 1
make[3]: *** Waiting for unfinished jobs....
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/parse_pe.cc: In function 'void select_data_section(bfd*, asection*, void*)':
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/utils/parse_pe.cc:76:20: error: 'bfd_get_section_size' was not declared in this scope; did you mean 'bfd_set_section_size'?
   76 |       sect->vma && bfd_get_section_size (sect))
      |                    ^~~~~~~~~~~~~~~~~~~~
      |                    bfd_set_section_size

[sylveon]

@dscho any idea about the error above and/or other communication channels you prefer to get help about those issues?

[dscho]

gcc -L/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/winsup/cygwin -isystem /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/cygwin/include -B/usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib/ -isystem /usr/src/MSYS2-packages/msys2-runtime/src/build-x86_64-pc-msys/x86_64-pc-msys/newlib/targ-include -isystem /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libc/include    -I/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/winsup/cygwin/include -DPACKAGE_NAME=\"newlib\" -DPACKAGE_TARNAME=\"newlib\" -DPACKAGE_VERSION=\"3.1.0\" -DPACKAGE_STRING=\"newlib\ 3.1.0\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -I. -I/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64 -I /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64/../../../../newlib/libm/common -fno-builtin -DHAVE_OPENDIR -DHAVE_RENAME -DGETREENT_PROVIDED -DSIGNAL_PROVIDED -D_COMPILING_NEWLIB -DHAVE_BLKSIZE -DHAVE_FCNTL -DMALLOC_PROVIDED -DHAVE_INIT_FINI      -g -O2 -pipe -ggdb -c -o lib_a-feholdexcept.o `test -f 'feholdexcept.c' || echo '/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64/'`feholdexcept.c
/usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime/newlib/libm/machine/x86_64/feclearexcept.c:1:1: error: expected identifier or '(' before '.' token
    1 | ../../fenv/fenv_stub.c
      | ^

That rings a bell. I think it had something to do with Cygwin's source code all of a sudden wanting to create symbolic links (and MSYS2's git.exe not checking them out correctly).

Let me look.

[sylveon]

I resolved that by setting core.symlinks to true for the makepkg clone of the repo (so at /usr/src/MSYS2-packages/msys2-runtime/src/msys2-runtime, globally wouldn't work), and that part of the build runs fine.

The issue is those bfd_get_section_name errors now.

[dscho]

I don't think that core.symlinks works in the MSYS version of Git.

IOW what should be the symlinks are instead files (containing the symlink target in plain text), and obviously that cannot work.

[dscho]

The bfd_get_section_name error seems related to https://github.com/msys2/MSYS2-packages/issues/1906. I don't think the solution will be to downgrade binutils, but to see what the new name of that function is.

[dscho]

I don't think the solution will be to downgrade binutils, but to see what the new name of that function is.

The answer might lie here: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=fd3619828e94a24a92cddec42cbc0ab33352eeb4

[sylveon]

Was able to build and dogfood the DLL successfully after downgrading binutils to 2.30-2, will be able to work on this now. Thanks.

[dscho]

For the record, I just built MSYS2 runtime v3.1.4, with the fixes I had hinted at earlier.

[kotx]

It seems the repo linked in the initial post doesn't exist anymore. I found a copy here https://github.com/Wurzelmann/GfW_ASLR_Exceptions for anybody who needs it now.

However now I'm getting this error:

❯ git submodule add ...
C:/Users/toast/scoop/apps/git/2.31.1.windows.1/mingw64/libexec/git-core\git-submodule: line 954: cmd_: command not found

This probably is not related, but I thought it would be wise to check this in.

Edit: seems to be related to #2448.

[conioh]

It seems the repo linked in the initial post doesn't exist anymore. I found a copy here https://github.com/Wurzelmann/GfW_ASLR_Exceptions for anybody who needs it now.

It's also available at: https://gitlab.com/conio.h/GfW_ASLR_Exceptions

I edited the top post with the updated URL.

[dscho]

In the end, it seems unlikely that we can have a sensible workaround for Mandatory ASLR. The MSYS2 runtime will simply not work with it.

[kotx]

In the end, it seems unlikely that we can have a sensible workaround for Mandatory ASLR. The MSYS2 runtime will simply not work with it.

Would it be possible to detect it and/or update the errors to be more helpful though? Or maybe add a warning during installation.

[dscho]

@kotx if you can figure out how to detect Mandatory ASLR, then implement that in Git for Windows' installer and open a PR, I will gladly work with you to get it merged!

[rimrul]

There is potential detection code in https://github.com/git-for-windows/git/issues/1412#issuecomment-380820412.

It would require dynamically importing GetProcessMitigationPolicy() from kernel32.dll. Since I assume this would also cause issues with PortableGit and MinGit, would git-wrapper potentially be a better place to detect this?

[dscho]

I don't think that Git wrapper is the best place for this. Well, maybe it's a good fall-back when no installer was involved, but it is unclear what's the least annoying way to surface that warning/error.

In my mind, step number one really is to implement this check in the installer.

And the responsibility for doing the actual work lies with the people who are concerned about ASLR. It's really not okay to just talk and try to leave the hard work to others.

[conioh]

And the responsibility for doing the actual work lies with the people who are concerned about ASLR. It's really not okay to just talk and try to leave the hard work to others.

Is that supposed to refer to me?

[dscho]

And the responsibility for doing the actual work lies with the people who are concerned about ASLR. It's really not okay to just talk and try to leave the hard work to others.

Is that supposed to refer to me?

If you truly care about trying to get Git for Windows to work with Mandatory ASLR, then yes, this includes you. I, for one, don't, so it does not include me.

[conioh]

And the responsibility for doing the actual work lies with the people who are concerned about ASLR. It's really not okay to just talk and try to leave the hard work to others.

Is that supposed to refer to me?

If you truly care about trying to get Git for Windows to work with Mandatory ASLR, then yes, this includes you. I, for one, don't, so it does not include me.

Then I must inform you that you are gravely mistaken. Git for Windows already works with ASLR for me.

Perhaps you did not understand what transpired here. I resolved my issue long ago. I just run my script and everything works for me. The work that needed to be done to solve my issue was done. By me. And it solved my issue.

I tried to do the nice thing and save others the time of finding the correct bits to set and writing the script on their own, so I provided it here, so that anyone who would look for "ASLR" in the issues for this repo could find the script and use it. It is the responsibility of all people, and especially project maintainers, to understand what is being said (or written) before trying to "respond" or participate in a discussion. The actual work that needed to be done was done by the people who were concerned with the issue. There's no more work to be done by them.

Now for the issue of other people, I looked at the install scripts in this repo a while back, after you provided some instructions on setting up a build environment etc., and while getting a working build environment wasn't too hard, I came to the conclusion that I am unable to contribute to these scripts. For not reporting this conclusion I apologize, and for that only.

I emphasize once more, since somehow it wasn't clear by now: My issue is resolved. On my machine Git for Windows works with ForceASLR.

Another thing that should have been clear but somehow wasn't is that despite what you may think you are not entitled for my work. I may choose to provide you with some work, and indeed I have, but the fact that I gave you something doesn't mean you're entitled to get everything. Therefore I suggest you adjust your tone. You're owed nothing.

Despite your accusatory and entitled message, for the benefit of the users, I'm still willing to to share the aforementioned script. If you or someone else will integrate it with the installer properly I'm willing to make some necessary improvements (remove the keys on uninstall, for example; something I didn't need so I didn't bother to write for myself) and license it to the project.

If you want it I can give it to you. If you don't that's fine too. I don't mind. As I've said, it works for me.