Add CFLAGS ENV to Dockerfile

[jrmoserbaltimore]

Please add this to the Dockerfile around line 4 (above or below the first ENV command):

ENV CFLAGS "-fstack-protector-strong -fpic -fpie -O2 -Wl,-O,1 -Wl,--hash-style=both"

These flags:

• Apply stack smash protection to functions using local buffers and alloca();
• Make PHP's main executable position-independent (improves ASLR security mechanism, and has no performance impact on x86_64);
• Enable optimization (-O2 should already be the default);
• Enable linker optimization (this sorts the hash buckets to improve cache locality, and is non-default);
• Adds GNU HASH segments to generated executables (this is used if present, and is much faster than sysv hash; in this configuration, sysv hash is also generated)

These settings should provide added security and improved loading time. Setting these in ENV is harmless and will propagate the changes down to any Dockerfile, scripted, or interactive call to build php additional modules.

Also consider -D_FORTIFY_SOURCE=1, which adds compiler checking to some standard string library functions; this option is incompatible with Alpine's musl libc.

[jrmoserbaltimore]

Any comments on this yet?

The below is perhaps more-complete:

ENV CFLAGS "-fstack-protector-strong -fpic -fpie -Wl,-O1 -O2 -Wl,--hash-style=both"
ENV CPPFLAGS "$CFLAGS"
ENV LDFLAGS "-O1 --hash-style=both"

Again, without the -fstack-protector-strong and position-independent executable, the PHP docker container does not enjoy protection from buffer-overflow-style attacks via address space randomization and stack smash detection. The rest are just performance improvements, mostly in loading libraries and extensions.

[yosifkit]

Sorry for the long tail; this must've been hidden in all my email. 😞

I like adding the these for better protection and faster loading, but I would rather apply them where needed (like here in php) rather than applying to all Debian derived images with tianon/docker-brew-debian#56. I am not sure users would want that forced upon them. But still 👍 to adding them where we can.

Although there is some overhead when stack-protector-strong is on, it is a config option in the kernel and turned on in the Debian kernel for sid (jessie's kernel was only built with gcc 4.8, so it didn't even have support for it).

[jrmoserbaltimore]

I think deploying it to all Debian-derived images might make sense in the long-run; I share concerns about breaking a minority of downstream images, though. Ideally, these would have been slapped down in anticipation of this exact situation when the Debian image was first published; and downstream images could set CFLAGS to exclude the troublesome ones from the get-go, instead of facing sudden, unexpected breakage in what was once working. That's a discussion for elsewhere, and possibly viable for the next tagged release (look, if you FROM somedistro:latest, you deserve what you get), depending on who wants to make the argument.

As far as overhead and PHP goes, there's always overhead. On entry and exit from a function, a word-sized memory entry is set and tested. Each affected function call becomes a few cycles longer. In general, added overhead is unimportant if your CPU isn't pinned to 100%: if your CPU is running at 99% and you experience a 1% added overhead, your CPU will run at 99.99% and will effectively "catch up" on average once per second, and rarely find itself more than 10mS behind a reference load.

Because a function call moves %esp by the span of all local variables, allocating an additional local variable has zero performance impact: you subtract 112 from %esp instead of 108, or such. When entering a function, stack-protector performs a single mov; when exiting, it performs an appropriate comparison and local jne (probably 4-6 cycles, but can consume essentially 1 thanks to pipelining and branch prediction). Simply calling a function in C can be 15-20 instructions, with maybe 50 or so cycles associated; and with libraries, you have to make an indirect jump through the PLT and such. That doesn't even get into cache locality, RAS/CAS and row precharge, and so forth.

This is why we inline small functions: a 5-10 instruction function is actually doing more work getting called and returning than actually performing its function. By contrast, the overhead of function calling conventions on complex functions is vanishingly-small, especially if those functions have loops.

In the context of stack-protector, you have three options:

• stack-protector applies its 3-instruction overhead to the call of only functions with local arrays;
• stack-protector-strong additionally applies its overhead to the call of functions using alloca() to create local arrays (it doesn't proliferate for alloca() calls: the canary is already in-place once you enter the function);
• stack-protector-all applies its overhead everywhere, with no heuristics

stack-protector and stack-protector-strong tend to show up in functions which will consume multiple cycles in loops building array contents, which minimizes their practical overhead. After all, if you have a ten-character array, you have to make either ten byte moves or two word moves and a short word move (glibc has some fancy strcpy() and memcpy() implementations).

This overhead is further diminished in the context of the entire execution by how frequently said functions are called. If stack-protector-strong makes a 30-instruction function 3 instructions longer (10%), with 15 instructions for set-up anyway (45 instructions total, 7% overhead), then that overhead is only made smaller if that instruction is called with as many instructions between: 45 instructions between calls means stack-protector-strong is adding 3.3% overhead, although at this point failing to inline the function has already added 50% overhead--and inlining the function would context the stack-protector logic to the outer function, avoiding its overhead completely.

Between inline functions, the large code-path size of most non-inline functions, and the sparseness of functions candidate for stack-protector-strong heuristics, the actual overhead in practice is vanishingly-small.

I know that's a lot of gobbledygook, but throwing around terms like "overhead" tends to spook people without appropriate context. I once had to argue with Theo de Raadt about whether -fpie was "very expensive", and eventually showed it has a 0.02% overhead in practice--meaning a 24-hour, pinned-to-100%-CPU execution would take 17.28 seconds longer. (PIE only affects the main executable's code, not any library code--which is already PIC.) 17 seconds per CPU-day is why most civilized Linux distributions gained effective address space randomization before OpenBSD finally got rid of the known base address of the main executable.

[andypost]

This issue needs to work again as musl 1.2.4 is removed support LFS64 so both flags can be removed

-D_LARGEFILE_SOURCE and -D_FILE_OFFSET_BITS=64

On the API level, the legacy "LFS64" ("large file support") interfaces, which were provided by macros remapping them to their standard names (#define stat64 stat and similar) have been deprecated and are no longer provided under the _GNU_SOURCE feature profile, only under explicit _LARGEFILE64_SOURCE. The latter will also be removed in a future version. Builds broken by this change can be fixed short-term by adding -D_LARGEFILE64_SOURCE to CFLAGS, but should be fixed to use the standard interfaces.

Filed php/php-src#11678

[yosifkit]

😕 I deleted those two flags and switched to alpine:edge (to get the newest musl) on both PHP 8.1.21 and 8.3.0-alpha3 and they both compiled fine. So, I am unable to reproduce any failure. We should take this to a new issue or PR since this one is very old.

[andypost]

3.18 also has musl 1.2.4 https://pkgs.alpinelinux.org/packages?name=musl&branch=v3.18&repo=&arch=&maintainer=

So I have to patch all 8.1-8.3 versions with https://gitlab.alpinelinux.org/alpine/aports/-/commit/a0fde6c117c0d486340727bb538fe09646c2e4f1