enable Control Flow Guard

[ghost]

Since LLVM 16: Support for mstorsjo/llvm-mingw#301 (-mguard=cf compile and link flags)

May need to migrate x86_64 build to llvm-mingw

[vszakats]

Thanks for the heads up that cfguard is getting ready for use.

After enabling the necessary options, it worked with llvm-mingw out of the box. curl -V works under wine. I didn't test it more, and don't know how to probe if cfguard is indeed active.

Then I bumped LLVM to 16 on Debian, and it also successfully created the binaries (meaning no build-time errors).
These correctly show GUARD_CF, have their .00cfg segment, but they came with this linker warning:

ld.lld: warning: Control Flow Guard is enabled but '_load_config_used' is missing

This is because the mingw-w64 package is built without the --enable-cfguard option, thus missing this feature alongside the load_config_used bits. (The root cause is that this option needs clang and mingw-64 is usually built with GCC, which still lacks cfguard support.)

I'm hesitating making llvm-mingw the default for x64 (and x86), because most users expect curl-for-win libs to work with the llvm + mingw-w64 coming with the popular package managers. I also want the keep curl-for-win itself run correctly with toolchains offered via package managers.

Test build here:
https://ci.appveyor.com/project/curlorg/curl-for-win/builds/47718371

[vszakats]

Enabled cfguard for ARM64 in live builds:
https://ci.appveyor.com/project/curlorg/curl-for-win/builds/47719051

Binaries:
https://curl.se/windows/dl-8.2.1_4/

[vszakats]

The PS script didn't work for me (The Export-ModuleMember cmdlet can only be called from inside a module.), but peeking into the source, it seems to be relying on the CFGUARD flag in the PE header. curl-for-win logs this already, and it's correctly set.

The docs say that this isn't enough and some other bits need to be set in the "load config" structure.

This is what's missing in builds with the warning mentioned above.

I don't have a suitable Windows system to make tests with Windows Defender though.

[vszakats]

Here are the x64 and x86 binaries with cfguard and the mentioned warning (meaning it's most probably inactive, but it'd be interesting to confirm it on a real system):
https://ci.appveyor.com/project/curlorg/curl-for-win/builds/47720448/artifacts

And these all built with llvm-mingw + cfguard:
https://ci.appveyor.com/project/curlorg/curl-for-win/builds/47720885/artifacts

Debian-testing, macOS+Homebrew and MSYS2 are all well-updated distros, but they rely on GCC + binutils, which is slow to pick-up with Windows features (ARM, UCRT, cfguard).

[vszakats]

@arm64-v9a Thanks for your tests.

Could you try running the x64 curl.exe from this build?:
https://ci.appveyor.com/project/curlorg/curl-for-win/builds/47722127/artifacts

This is an attempt to enable cfguard for the static libs, while keeping it disabled in curl.exe so that it runs correctly.

[vszakats]

Thank you! Based on this it seems that cfguard is an all-or-nothing setting, even a cfguard-compiled object triggers the OS, in an executable with unset CFGUARD flag. It means there is no way to ship cfguard-enabled objects and let the linker decide to enable it or not.

[ Also CMake doesn't offer a way to pass an option to the C compiler only, without passing it to the linker, too. This is a problem because the compiler and linker flags have the same name (-mguard=cf). This may be worked around with some tricks I've yet to figure out. ]

Therefore, enabling cfguard forces all curl-for-win static lib consumers to have to enable it for all the their other objects and their executables, while also switching to llvm-mingw. This may work with ARM64 a little better because it required llvm-mingw already. For x64/x86 it reduces choices at the moment. Hard to say if this inflexibility can be improved by the tooling or something inherent to the design of cfguard. [UPDATE: I'd guess the latter.]

I had expected that including a single cfguard-compiled object won't break a final executable, unless cfguard was explicitly enabled at link-time.

[vszakats]

One thing I find odd that this feature seems entirely the function of objects having cfguard enabled, yet it also requires a link-time option to set the GUARD_CF flag in executables. Either I'm missing something or this flag "should be" unnecessary.

To summarize where we are now:

• curl-for-win has cfguard enabled for ARM64 in official binaries v8.2.1_4.
  Marked as EXPERIMENTAL.
• logic was added to enable cfguard automatically when building with llvm-mingw.
  This can be enabled by setting env CW_LLVM_MINGW_ONLY to 1
  in custom or local builds.

[alvinhochun]

Hi, I just noticed this issue by chance and thought I should comment. Unfortunately some context has been lost because it seems most replies by the OP has been deleted, so I don't have the full picture.

Thank you! Based on this it seems that cfguard is an all-or-nothing setting, even a cfguard-compiled object triggers the OS, in an executable with unset CFGUARD flag. It means there is no way to ship cfguard-enabled objects and let the linker decide to enable it or not.

Therefore, enabling cfguard forces all curl-for-win static lib consumers to have to enable it for all the their other objects and their executables, while also switching to llvm-mingw. This may work with ARM64 a little better because it required llvm-mingw already. For x64/x86 it reduces choices at the moment. Hard to say if this inflexibility can be improved by the tooling or something inherent to the design of cfguard. [UPDATE: I'd guess the latter.]

I had expected that including a single cfguard-compiled object won't break a final executable, unless cfguard was explicitly enabled at link-time.

As far as I remember this is not true. You should be able to compile objects with -mguard=cf but not enable it while linking, and still produce fully working executables (without control flow guard active). It has to be designed this way because the CRT itself also has to be compiled with CFG to include the guard checks, but still allows it to be linked with user code without CFG enabled.

You can check with the test case in llvm-mingw:

> clang -mguard=cf cfguard-test.c -c -o cfguard-test.o

> clang cfguard-test.o -o cfguard-test.exe

> llvm-readobj --coff-load-config cfguard-test.exe
[...]
  GuardFlags [ (0x0)
  ]
[,,,[

> cfguard-test check_enabled
Control Flow Guard is _not_ enabled!

> cfguard-test normal_icall
Performing normal indirect call.
Normal function called.

> cfguard-test invalid_icall
Performing invalid indirect call. If CFG is enabled this should crash with exit code 0xc0000409 (-1073740791)...
Pwned!!!

[ Also CMake doesn't offer a way to pass an option to the C compiler only, without passing it to the linker, too. This is a problem because the compiler and linker flags have the same name (-mguard=cf). This may be worked around with some tricks I've yet to figure out. ]

I have not tried it myself, but the docs for add_compile_options and target_compile_options explicitly states they are not used when linking. Do they not work for you?

[vszakats]

It's possible I got it wrong, but after a considerable amount of time spent on testing, I positively could not make it work that way, meaning: by enabling CFG at compile-time, not enabling it at link-time, yet getting a runnable binary.

(I don't have a Windows machine capable of testing this, so this involved feedback from others, which feedback is apparently now missing from this thread's history.)

I'd be happy to be proven wrong as it'd allow switching to llvm-mingw and enabling this without forcing everyone downstream doing the same.

[ Well, it'd still need some convincing to jump to llvm-mingw and drop the standard toolchains shipping via package managers. ]

[vszakats]

Here's a llvm-mingw build with CFLAGS-only:
https://ci.appveyor.com/project/curlorg/curl-for-win/builds/48394235/artifacts
Here's another one with CFLAGS+LDFLAGS:
https://ci.appveyor.com/project/curlorg/curl-for-win/builds/48396839/artifacts

Please report which one is running or failing to run on old and new Windows versions,
and if CFG is or isn't enabled with them.

[alvinhochun]

Tested on my x64 Windows 10 system, both win32 and win64 builds run fine here (can perform an https request). The CFLAGS-only ones don't have CFG enabled, while the CFLAGS+LDFLAGS ones do have CFG enabled (checked in Process Explorer), which matches my expectations.

I don't have an aarch64 Windows system, nor do I have any older Intel Windows systems/VMs, but I can try to ask someone else to help.

[mstorsjo]

Tested on my x64 Windows 10 system, both win32 and win64 builds run fine here (can perform an https request). The CFLAGS-only ones don't have CFG enabled, while the CFLAGS+LDFLAGS ones do have CFG enabled (checked in Process Explorer), which matches my expectations.

I don't have an aarch64 Windows system, nor do I have any older Intel Windows systems/VMs, but I can try to ask someone else to help.

Both of the aarch64 builds do run just fine; I don't have Process Explorer available right now to check that CFG is enabled though.

[vszakats]

@alvinhochun: Thanks for your tests! This is different from previous tests, but it's possible there was an error along the way before.

Both x64 binaries also run fine on a Win7 system.

This tells that we could publish CFG-enabled static libs and those could still be used with non-llvm-mingw toolchains, though this might need actually testing this to confirm.

[vszakats]

Using this script I built an example program with clang, gcc and llvm-mingw (CFG and non-CFG) and the resulting 4 .exe also built fine and then run fine on Windows 7:

#!/bin/sh

opt='-static -s -DCURL_STATICLIB connect-to.c -I../include -L.
  -lbrotlicommon -lbrotlidec -lcrypto -lssl -lnghttp2 -lnghttp3 -lngtcp2 -lngtcp2_crypto_quictls
  -lssh2 -lz -lzstd -lcurl -lws2_32 -lbcrypt -lcrypt32 -lwldap32'

/usr/local/opt/llvm/bin/clang -fuse-ld=lld \
  -target x86_64-w64-mingw32 --sysroot /usr/local/opt/mingw-w64/toolchain-x86_64 \
  -D_UCRT -lucrt ${opt} -o curltest-clang

x86_64-w64-mingw32-gcc -dumpspecs | sed 's/-lmsvcrt/-lucrt/g' > _gcc-specs-ucrt
x86_64-w64-mingw32-gcc -specs=_gcc-specs-ucrt \
  -D_UCRT -lucrt -Wl,--start-group ${opt} -Wl,--end-group -o curltest-gcc
rm -f _gcc-specs-ucrt

~/llvm-mingw/bin/x86_64-w64-mingw32-clang \
  ${opt} -o curltest-llvm-mingw

~/llvm-mingw/bin/x86_64-w64-mingw32-clang \
  ${opt} -o curltest-llvm-mingw-cfg -mguard=cf

~/llvm-mingw/bin/llvm-readobj --coff-load-config ./*.exe

[alvinhochun]

One thing to note, objects compiled with CFG (if they make indirect calls) do need to reference the symbol __guard_check_icall_fptr (__guard_dispatch_icall_fptr on x86_64) which is only provided in mingw-w64 CRT since v11 (unconditionally included even if itself is not built with CFG). If the user is using an older mingw-w64 toolchain, they will get an undefined symbol error when trying to link to the static library. Did you encounter this when testing or have you always tested with the latest mingw-w64 CRT? (It is possible to supply these symbols externally but doing this is messy and inconvenient.)

[vszakats]

Good point. I did these tests with mingw-w64 v11. In general I'm okay requiring a fresh mingw-w64, but in this case v11 is missing from the latest stable Debian/Ubuntu for example, which might be a problem for some:

🚫 https://packages.debian.org/bookworm/mingw-w64-common (since testing/trixie)
✅ https://packages.ubuntu.com/lunar/mingw-w64 (since 23.10)
✅ https://packages.fedoraproject.org/pkgs/mingw-w64-tools/mingw-w64-tools/ (since 39)
✅ https://archlinux.org/packages/?sort=&q=mingw-w64-crt (since 2023-11-27)
✅ https://pkgs.alpinelinux.org/packages?name=mingw-w64-crt&branch=v3.18 (ok)
✅ https://formulae.brew.sh/formula/mingw-w64 (ok)
✅ https://packages.msys2.org/base/mingw-w64-crt-git (ok)

https://repology.org/project/mingw-w64/versions

Probably more users would use MSYS2, which always has the latest mingw-w64.
Or llvm-mingw, with full CFG support.

Though I have no insight how (and how many) people use curl's static libs.

[vszakats]

Another possibly interesting bit is ASM compatibility with CFG. It is reported that ASM does not work with CFG, to which OpenSSL maintainers suggest to disable ASM for CFG builds.

This should already affect curl-for-win ARM64 Windows builds, having both CFG and ASM enabled. It'd be nice to see any feedback on that binary. I haven't had a chance to ever run those myself, nor do they run in any CI.

Ref: openssl/openssl#22554

This might also affect LibreSSL x64 builds in the future.

[alvinhochun]

Ref: openssl/openssl#22554

This issue is too vague to tell anything.

openssl/openssl#1592 (comment) does give an insight into what may be happening. In particular, the CFG check fails with a pointer inside the table OPENSSL_UplinkTable. Looking at https://github.com/openssl/openssl/blob/22fa1602da91af2194997e0576582bb4f0cdd7e0/ms/uplink-x86_64.pl, this table is constructed in assembly and the function pointers used are not directly referenced from C,

(Typically when C/C++ code references a function defined in assembly it would be declared as an extern function and have its address taken in C/C++ code. Any address-taken functions are marked as valid call targets, which normally would be enough to make any function pointers to assembly functions work with CFG.)

One way to fix this would be to construct OPENSSL_UplinkTable in C instead of assembly, which would mark all the functions as address-taken. It's already done this way in https://github.com/openssl/openssl/blob/22fa1602da91af2194997e0576582bb4f0cdd7e0/ms/uplink.c but only for x86 with MSVC. In fact it looks like to me it could be rewritten in simple arch-independent C and be rid of any handwritten assembly. (Just guessing, I don't know the rationale of it being written in assembly but that looks like code to lazily initialize a function table, surely it can't be that performance-critical?) (On second thought, it does need handwritten assembly to not clobber the original function arguments.)

---

There isn't really a good way to find all these cases other than to run a test suite that tests all code paths calling assembly functions. I suppose one can also look for all indirect call sites (i.e. find references to __guard_{check,dispatch}_icall_fptr) in the generated code and audit what they do but that's a lot of work...

[vszakats]

If the issue is limited to uplink (or 'applink'), curl-for-win is not affected because it builds OpenSSL with no-shared which disables applink, too. Also, no forks copied this crazy (IMO) feature, and indeed, on Windows it is enabled by default only on x86.

An interesting tidbit about applink: It was a feature that was causing continuous headaches for many years downstream. While discussing this in 2016 with the OpenSSL team, it turned out that it was not working before 1.1.0 due to a local bug in the assembly code. This received a fix shortly after.

This was a good opportunity IMO to switch to a different solution or to drop it completely, but the feature is still there and continue causing headaches.