Enable Intel CET on Windows by default

[mvorisek]

based on #8339 (comment)

this is a security fix, so targetting PHP 8.0

tested with #8392, build & all tests on x64 & x86 pass see discussion below, it needs to be tested on CPU with actual CET support

the security can be maybe improved even more, see https://airbus-seclab.github.io/c-compiler-security/msvc_compilation.html

[cmb69]

tested with #8392, build & all tests on x64 & x86 pass

Sure, because it is highly unlikely that the hosted GH action runners do support CET.

[mvorisek]

Can you please test it on a non-virtualized system? Also, is PHP_SECURITY_FLAGS = yes by default?

[cmb69]

The problem is that CET is only supported by pretty recent processors (can be checked with CoreInfo), and if it is not supported, adding the /CETCOMPAT flag accomplishes nothing.

Also, is PHP_SECURITY_FLAGS = yes by default?

Yes.

[mvorisek]

yes, citing https://www.offensive-security.com/offsec/intel-cet-in-action/

To make use of Shadow Stack, Windows requires an 11th-generation Intel CPU or an AMD Ryzen 5000-series CPU, which will not be commonplace for several years. For desktops and laptops, Windows 10 update 19H1 or newer is required to enable CET support; for servers, it’s supported on Windows Server 2019.

I do not have access to such CPU either, can someone test test?

[derickr]

We shouldn't merge build configuration changes into released branches. If this is merged, it should be into master only.

[nikic]

Generally, can you please stop submitting pull requests against non-master branches? If something requires a backport into a stable release branch, people will inform you. Your default assumption should be that everything goes into master only -- this is especially true for anything touching the build system or CI, as well as any changes that constitute "cleanup", such as improving tests. Basically, exactly the kind of changes you've been working on. You submitted a lot of PRs that cannot be merged because they currently all target the wrong branch.

[cmb69]

This must not be merged at all into any branch. Claiming that the binaries would be CET compatible, while they are most likely not (see #8339) would cause the binaries to crash on systems actually supporting CET, what appears to be highly undesireable.

[nikic]

Great, let's close this then.

[mvorisek]

can you please stop submitting pull requests against non-master branches

I did a lot of actual fixes and I belive I target them correctly. I do not agree submitting fixes that should go into PHP 8.0 should be targetted into master. Even if you will require it, and there will be any conflicts, CI will not run.

I consider this a security feature so I targetted the lowest possible branch. If (and it seems there is) any BC break, I agree, it must target master. That is what this discussion brought.

Claiming that the binaries would be CET compatible, while they are most likely not (see #8339) would cause the binaries to crash on systems actually supporting CET, what appears to be highly undesireable.

I was not are of this on the submission time, however, the PR and discussion is valueable. I think this PR should be marked as dependent on #8339, which I belive will fix the CET issues, and finished/merged later. If and only if the mentioned PR intends to provide Windows support, this PR is redundant.

[chen-hu-97]

Hi @mvorisek ,
In #8339, I want to enable CET for PHP, mainly on Linux because I have no knowledge on MSVC. To enable it on Windows:

• Shadow stack
  Windows kernel should provide system call for user space to create shadow stack. (this is because fiber creates its own stack, so it needs maintin shadow stack itself). It will not be complex.
• Indirect branch tracking
  Does MSVC have something like attribute((indirect_return)) to indicate the function may not return. Otherwise we need insert assembly inside C. (again this is from fiber)

Because fiber are from boostorg/context, we need modify upstream as well.

Although the community think such features can't be merged now, I think it's still nice to enable CET in PHP and use PR to track it. Maybe some day we can merge them.

BTW, #8339 pass test on full CET environment. "full" means glibc/Linux/CPU have actual CET support.

[cmb69]

Although the community think such features can't be merged now, […]

To clarify: we should support CET, but we need a working implementation. Just linking with /CETCOMPAT while there is no CET compatibility, would be bad.

[mvorisek]

can this be reopened and what changes in code are needed for Windows?