Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase fortification level #648

Merged
merged 2 commits into from Jan 11, 2024
Merged

Conversation

imwints
Copy link
Contributor

@imwints imwints commented Oct 18, 2023

Newer versions of GCC and Clang support _FORTIFY_SOURCE=3 which increases code safety by providing better size checks

For reference see
https://developers.redhat.com/articles/2023/02/06/how-improve-application-security-using-fortifysource3

@NexAdn
Copy link
Contributor

NexAdn commented Jan 8, 2024

How about just providing a default value and letting users/packagers override the default as desired? Then we can have a sane default for the standard build and allow for setting a custom fortification level e.g. for testing increased fortification levels or just enabling them for hardened builds.

@imwints
Copy link
Contributor Author

imwints commented Jan 8, 2024

Can you make a suggestion what this would look like?

@NexAdn
Copy link
Contributor

NexAdn commented Jan 8, 2024

For the Makefile, just put -D_FORTIFY_SOURCE=3 (or whatever level you choose) into some overridable variable (similar to OPTFLAGS). For CMake, there is the $<IF: generator expression, so an option() to enable/disable default fortification should be possible.

@imwints
Copy link
Contributor Author

imwints commented Jan 8, 2024

I've put the flag behind a build flag. It is enabled by default and can be disabled, described in the readme.

Might be also worth considering that this is usually a type of flag distributions set or unset, like Alpine and Gentoo do with their fortify.h header and we shouldn't worry about it and maybe remove the flag on our side.

@NexAdn is this what you where thinking about?

@aristocratos what do you think?

@NexAdn
Copy link
Contributor

NexAdn commented Jan 9, 2024

@NexAdn is this what you where thinking about?

Looks good to me. Thanks!

@aristocratos aristocratos merged commit c649369 into aristocratos:main Jan 11, 2024
51 checks passed
@imwints imwints deleted the fortification branch February 12, 2024 09:28
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.

None yet

3 participants