-
Notifications
You must be signed in to change notification settings - Fork 51
OpenSSF silver badge: hardening #1241
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
| set_target_properties( | ||
| power_grid_model_api_tests | ||
| PROPERTIES BUILD_RPATH $<TARGET_FILE_DIR:power_grid_model_c> | ||
| ) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be removed
| "environment": { | ||
| "SANITIZER_FLAGS": "/RTC1" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we move this also to the CMakeLists?
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
power_grid_model_c/power_grid_model/include/power_grid_model/topology.hpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com> Signed-off-by: Martijn Govers <martygovers@hotmail.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
| project(power_grid_model VERSION ${PGM_VERSION}) | ||
|
|
||
| option(PGM_ENABLE_DEV_BUILD "Enable developer build (e.g.: tests)" OFF) | ||
| option(PGM_ENABLE_HARDENING "Enable compile and link time hardening options" ON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think enable it by default is logical. Disable by default and only enable in the our dev preset is logical.
In the Python build we do not enable it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardening is intended to be used in production
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardened code self (so the adjustment to make compilation to pass with hardening check flag) is intended to be used in production. The compilation flags to check hardening are not intended to be used in production.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the GCC command line options (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fhardened)
This option is intended to be used in production builds, not merely in debug builds.
See also https://www.youtube.com/watch?v=GtYD-AIXBHk&list=PLHTh1InhhwT57vblPGsVag5MkTm_Z9-uq&index=2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still has huge doubt for the default. This goes against the other golden rule of open-source cmake project: make as little as needed for extra compile and link flag in the default cmake build.
Also, enabling address sanitizing for production is questionable.
We need more in-depth consideration and discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also let CIBW set the compiler flags for hardening
| before-all = """yum install -y gcc-toolset-14 && \ | ||
| source /opt/rh/gcc-toolset-14/enable""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be needed if we disable build hardening by default.
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <martijn.govers@alliander.com>
|



Cherry-picked from #1228
Investigate the possibilities of hardening
Relates to #1190