Windows build: Control flow guard feature (/guard:cf)

[matbech]

I was wondering if you have already considered to enable the control flow guard feature [1] for MSVC (Visual Studio 2015) Windows builds?

1. MSDN - Best Practice: https://msdn.microsoft.com/en-us/library/windows/desktop/mt637065(v=vs.85).aspx

Compiler switch: /guard:cf

[richsalz]

We have not considered it. Can you add the flag by hand on the config line and see how it works?

[dot-asm]

Can you add the flag by hand on the config line and see how it works?

There are two ambiguities in suggestion. One is that attempt to pass option starting with slash will be rejected by Configure. Fortunately MSVC accepts both slash and dash, i.e. /guard:cf and -guard:cf are interchangeable. Secondly, and more importantly, passing [dash-form of] this option at config-time won't work all the way, because it will be used when generating .obj-s, but not when linking .dll-s and .exe-s. And latter is as essential. This is because we call link.exe directly and it needs own flag. Fortunately there is a way to slip in command line arguments, and even without passing additional flags at config-time. One can set CL=/guard:cf and set LINK=/guard:cf to achieve the goal.

[mattcaswell]

@dot-asm provided a workaround for this above - so closing.

[matbech]

I'm reopening this issue because the proposed workaround no longer works.
set CL=/guard:cf
set LINK=/guard:cf
...

statem_srvr.c
        cl  /I "." /I "include" -DOPENSSL_USE_APPLINK -DDSO_WIN32 -DOPENSSL_THREADS -DOPENSSL_NO_STATIC_ENGINE -DOPENSSL_PIC -DOPENSSL_BN_ASM_PART_WORDS -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DRC4_ASM -DMD5_ASM -DRMD160_ASM -DAES_ASM -DVPAES_ASM -DGHASH_ASM -DECP_NISTZ256_ASM -DPADLOCK_ASM -DPOLY1305_ASM -DOPENSSL_API_COMPAT=0x10100000L -DUNICODE -D_UNICODE -DWINVER=0x0601 -D_WIN32_WINNT=0x0601 -DOPENSSL_NO_ERR "-DENGINESDIR=\"C:\\Program Files (x86)\\OpenSSL\\lib\\engines-1_1\"" "-DOPENSSLDIR=\"C:\\Program Files (x86)\\Common Files\\SSL\"" -W3 -wd4090 -Gs0 -GF -Gy -nologo -DOPENSSL_SYS_WIN32 -DWIN32_LEAN_AND_MEAN -DL_ENDIAN -D_CRT_SECURE_NO_DEPRECATE -DUNICODE -D_UNICODE /MDd /Od -DDEBUG -D_DEBUG /Zi /Fdossl_static -c /Fossl\t1_enc.obj "ssl\t1_enc.c"
t1_enc.c

/guard:cf is also missing from the linker command

link /nologo /debug /dll /implib:libssl.lib /out:libssl-1_1.dll /def:libssl-1_1.def @C:\Users\mb\AppData\Local\Temp\nm3FE1.tmp || (DEL /Q libssl.* libssl-1_1.* && EXIT 1)

I think it would probably be a good idea to enable CFG by default. All Windows system dlls have CFG enabled.

[mattcaswell]

Ping @dot-asm

[matbech]

any chance that this issue can be reopened? Thanks.

[zhoumgh]

We added option /guard:cf in MSVC 14.12.25827 (from VS 2017) compiler and linker when building openssl 1.0.2p, we saw exception:

   Security check failure or stack buffer overrun - code c0000409 (!!! second chance !!!) 
exception stack:
ntdll!RtlFailFast2
ntdll!RtlpHandleInvalidUserCallTarget+0x5c
ntdll!LdrpHandleInvalidUserCallTarget+0x38  <=== resulted by LIBEAY32!__guard_check_icall_fptr
LIBEAY32!file_ctrl+0x16a [\build\release\win64_vc140\openssl\build\crypto\bio\bss_file.c @ 334]
LIBEAY32!BIO_ctrl+0xab [\build\release\win64_vc140\openssl\build\crypto\bio\bio_lib.c @ 359]
LIBEAY32!BIO_new_fp+0x61 [\build\release\win64_vc140\openssl\build\crypto\bio\bss_file.c @ 209]
LIBEAY32!PEM_write_PrivateKey+0x48 [\build\release\win64_vc140\openssl\build\crypto\pem\pem_pkey.c @ 238]

#  ifdef UP_fsetmod
        if (b->flags & BIO_FLAGS_UPLINK)
            UP_fsetmod(b->ptr, (char)((num & BIO_FP_TEXT) ? 't' : 'b'));  <===
        else
#  endif

/openssl/src/ms/uplink.h:#define UP_fsetmod (*(int (*)(void *,char))OPENSSL_UplinkTable[APPLINK_FSETMOD])

OPENSSL_UplinkTable[APPLINK_FSETMOD] is in assembly code generated by /openssl/src/ms/uplink-x86_64.pl, the assembly code is parsed by nasm. Checked latest nasm and ml64, they don't have equivalent option for C compiler/Linker option /guard:cf, this might be the reason of failure.

Checked
https://www.openssl.org/source/openssl-1.1.1.tar.gz
https://www.openssl.org/source/openssl-fips-2.0.16.tar.gz
they are still using uplink-x86_64.pl, not sure if the issue is fixed.

Thanks!

[noahdav]

Is there any update on this issue? We have an organizational mandate to enable the /guard:cf. Once we do we run into this issue running openssl.exe genrsa -out foo.pem 2048. If we do not add the -out param then this does not occur. We need to build 1.1.1 as we have many services with a dependency on this version. Currently we are using msvc (2017 compilers) although the same issue occurs with 2019 compilers as well as on the latest master 3.0 build of openssl.

[bonafideduck]

@noahdav , I've been looking into this too. I've been able to track down the cause of the crash. It is related to using the Netwide nasm assembler instead of the Microsoft masm assembler. I suspect the masm puts something in the headers to for CFG to use.

The good news for the 64 bit image is that this configuration works great: perl Configure VC-WIN64A-masm LDFLAGS=/guard:cf CFLAGS=/guard:cf LIB_CFLAGS=/guard:cf.

If you are using the 32 bit code, the perlasm will take a lot of conversion to build with masm, so it isn't a slam dunk like above. I've found that directly referencing the asm functions in C gets the linker to mark the functions as CFG compatible, but suspect there is a better solution for that.

I'm probably going to create a specific issue for the 32-bit CFG and put everything I've found in there.

[noahdav]

@bonafideduck Thanks for the information. I will give this a shot for the 64 bit build but we still have a few uses of 32 bit as well. Can you please add the link to whatever issue you add.

[meklund-zoom]

@noahdav , The issue is #16147 and the PR that works so-far is #16148.