Skip to content

Conversation

@justincormack
Copy link
Contributor

Use the same hardening flags as the Debian build to enable RELRO,
stack protector and hardening.

Signed-off-by: Justin Cormack justin.cormack@docker.com

Use the same hardening flags as the Debian build to enable RELRO,
stack protector and hardening.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
@justincormack
Copy link
Contributor Author

ping @tianon

@tianon
Copy link
Member

tianon commented Oct 18, 2019

I get the "what" here loud and clear, but I think we're missing a lot of context on the "why" -- is there a problem that's caused by not enabling these? Some benefit to doing so? (My own awareness of hardening flags like this is that there are a lot of conflicting opinions about them both for and against.)

More directly, is there any recommendation from upstream httpd to enable these? (We do try to stay as faithful to upstream as possible, so explicit "do this" recommendations from upstream usually make it much easier to determine whether we should include something.)

@justincormack
Copy link
Contributor Author

Upstream httpd no longer distribute Linux binaries (just Windows and ahem Netware), so it is hard to get a sense of what they recommend. Their rpm spec file does include -pie https://github.com/apache/httpd/blob/trunk/build/rpm/httpd.spec.in#L146

All Linux distros that ship httpd use hardening; all major distros expect Debian enable hardening by default in the gcc specfiles I believe; Debian enables for all packages (unless specifically disabled eg gcc). So there are no/few users except those of this package who are using the unhardened builds as far as I can tell.

There are not any significant use cases for disabling in production that I am aware of; there are some other C analysis tools that need it disabled for static analysis. The vast majority of all CVEs in all software are C memory errors, and it seems to be a bad idea to disable the few hardening mechanisms that exist at present.

@tianon
Copy link
Member

tianon commented Oct 28, 2019

Ok, that's fair -- I think in that case I'd rather we invoke dpkg-buildflags to query Debian's values and let them maintain what is safe/sufficient rather than fronting the burden of doing so ourselves.

I think there's also some useful bits to link to in https://salsa.debian.org/apache-team/apache2/blob/master/debian/rules to help inform where our choices come from, so I'll be pushing a commit to adjust shortly so we can get this in. 👍

Also, add links to Debian's apache2 configure bits to make it clear why we add extra flags on top of this.
@tianon
Copy link
Member

tianon commented Oct 28, 2019

Ah shoot we've got an Alpine variant here too, and dpkg-buildflags --get CFLAGS in Alpine returns the empty string... Does Alpine's gcc default to enabling these, or do we have the same problem there?

@tianon
Copy link
Member

tianon commented Oct 28, 2019

@justincormack
Copy link
Contributor Author

I am fairly sure that all these flags are defaults in Alpine, including pie.

@tianon
Copy link
Member

tianon commented Oct 29, 2019

Ok, well this is a net-positive for the Debian variants -- maybe we can find some documentation to point to for Alpine's defaults that show this is unnecessary? 😕 😞

@tianon tianon merged commit 338122b into docker-library:master Oct 29, 2019
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Oct 29, 2019
Changes:

- docker-library/httpd@338122b: Merge pull request docker-library/httpd#148 from justincormack/harden
- docker-library/httpd@8da3138: Query hardening flags directly from Debian
@justincormack justincormack deleted the harden branch October 30, 2019 13:09
@justincormack
Copy link
Contributor Author

Yes, Alpine looks file already, with stack canary, pie etc.

tianon added a commit that referenced this pull request Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants