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

cmake: add extra security compiler options #7587

Merged
merged 1 commit into from Sep 15, 2022

Conversation

Gumix
Copy link
Contributor

@Gumix Gumix commented Aug 18, 2022

Introduce cmake option ENABLE_HARDENING, which is TRUE by default for
regular and static builds. It passess compiler flags that harden Tarantool
(including the bundled libraries) against memory corruption attacks.
The following flags are passed:

  • -Wformat - Check calls to printf and scanf, etc., to make sure that
    the arguments supplied have types appropriate to the format string
    specified.

  • -Wformat-security -Werror=format-security - Warn about uses of format
    functions that represent possible security problems. And make the
    warning into an error.

  • -fstack-protector-strong - Emit extra code to check for buffer
    overflows, such as stack smashing attacks.

  • -fPIC - Generate position-independent code (PIC). It allows to take
    advantage of Address Space Layout Randomization (ASLR).

  • -z relro -z now - Resolve all dynamically linked functions at the
    beginning of the execution, and then make the GOT read-only.

Also do not disable hardening for Debian and RPM-based Linux distros.

Closes #5372
Closes #7536

@Gumix Gumix added full-ci Enables all tests for a pull request and removed full-ci Enables all tests for a pull request labels Aug 18, 2022
@tarantool tarantool deleted a comment from coveralls Aug 18, 2022
@Gumix Gumix force-pushed the iverbin/gh-7536-add-extra-security-compiler-options branch 2 times, most recently from e42fe44 to 5acbfce Compare August 19, 2022 12:33
@Gumix Gumix added the full-ci Enables all tests for a pull request label Aug 19, 2022
@@ -37,7 +37,6 @@ DEB_DH_SYSTEMD_START_ARGS_tarantool-common := --no-restart-on-upgrade tarantool

# Needed for proper backtraces in fiber.info()
DEB_DH_STRIP_ARGS := -X/usr/bin/tarantool
export DEB_BUILD_MAINT_OPTIONS = hardening=-stackprotector
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here hardening=-stackprotector means disabling stackprotector, not enabling.

Copy link
Member

@locker locker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't forget to patch tarantool-ee when this one is merged.

static-build/CMakeLists.txt Outdated Show resolved Hide resolved
@locker locker removed their assignment Aug 19, 2022
cmake/compiler.cmake Outdated Show resolved Hide resolved
cmake/compiler.cmake Outdated Show resolved Hide resolved
cmake/compiler.cmake Outdated Show resolved Hide resolved
@Gumix Gumix removed the full-ci Enables all tests for a pull request label Aug 25, 2022
@Gumix Gumix assigned Gumix and unassigned Totktonada Aug 25, 2022
@Gumix Gumix force-pushed the iverbin/gh-7536-add-extra-security-compiler-options branch from 5acbfce to 37ee9d5 Compare August 25, 2022 17:09
@Gumix Gumix added the full-ci Enables all tests for a pull request label Aug 25, 2022
@Gumix Gumix force-pushed the iverbin/gh-7536-add-extra-security-compiler-options branch 3 times, most recently from 7718f29 to 6602b37 Compare August 26, 2022 11:41
@Gumix Gumix assigned Totktonada and locker and unassigned Gumix Aug 26, 2022
@Gumix Gumix force-pushed the iverbin/gh-7536-add-extra-security-compiler-options branch from 1f542b4 to 3d68417 Compare September 1, 2022 16:27
@Gumix Gumix assigned Totktonada and unassigned Gumix Sep 1, 2022
cmake/compiler.cmake Outdated Show resolved Hide resolved
@Gumix Gumix force-pushed the iverbin/gh-7536-add-extra-security-compiler-options branch from 3d68417 to bf491a5 Compare September 9, 2022 17:50
Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the painstaking work!

The overall approach is okay for me. I made several manual tests and some of them are failed (see the comments above). I don't know how important those fails are and I don't know details about building platform/infrastructure of Tarantool Enterprise Edition. Depending of that the fails may be negligible or very important.

I'll approve the general approach and will lean on you regarding reproducing/fixing/postponing/discarding the remaining problems.

(TBH, I'm going to a vacation and just can't participate in a next review iteration.)

@Totktonada Totktonada assigned Gumix and unassigned Totktonada Sep 9, 2022
@Gumix Gumix added do not merge Not ready to be merged and removed full-ci Enables all tests for a pull request labels Sep 9, 2022
@Gumix Gumix force-pushed the iverbin/gh-7536-add-extra-security-compiler-options branch from bf491a5 to f8d128b Compare September 14, 2022 14:02
@Gumix Gumix force-pushed the iverbin/gh-7536-add-extra-security-compiler-options branch from f8d128b to 0cc4278 Compare September 15, 2022 09:47
@Gumix Gumix added the full-ci Enables all tests for a pull request label Sep 15, 2022
@Gumix Gumix force-pushed the iverbin/gh-7536-add-extra-security-compiler-options branch from 0cc4278 to 9a52a35 Compare September 15, 2022 12:26
Introduce cmake option ENABLE_HARDENING, which is TRUE by default for
non-debug regular and static builds, excluding AArch64 and FreeBSD.
It passess compiler flags that harden Tarantool (including the bundled
libraries) against memory corruption attacks. The following flags are
passed:

* -Wformat - Check calls to printf and scanf, etc., to make sure that
  the arguments supplied have types appropriate to the format string
  specified.

* -Wformat-security -Werror=format-security - Warn about uses of format
  functions that represent possible security problems. And make the
  warning into an error.

* -fstack-protector-strong - Emit extra code to check for buffer
  overflows, such as stack smashing attacks.

* -fPIC -pie - Generate position-independent code (PIC). It allows to
  take advantage of the Address Space Layout Randomization (ASLR).

* -z relro -z now - Resolve all dynamically linked functions at the
  beginning of the execution, and then make the GOT read-only.

Also do not disable hardening for Debian and RPM-based Linux distros.

Closes #5372
Closes #7536

NO_DOC=build
NO_TEST=build
@Gumix Gumix force-pushed the iverbin/gh-7536-add-extra-security-compiler-options branch from 9a52a35 to 4b02131 Compare September 15, 2022 13:43
@Gumix Gumix removed the do not merge Not ready to be merged label Sep 15, 2022
@Gumix Gumix assigned locker and unassigned Gumix Sep 15, 2022
@locker locker merged commit e6abe1c into master Sep 15, 2022
@locker locker deleted the iverbin/gh-7536-add-extra-security-compiler-options branch September 15, 2022 15:49
@locker
Copy link
Member

locker commented Sep 15, 2022

Cherry-picked to 2.10.

# Fuzzers are compiled without PIC support,
# LuaJIT in FreeBSD doesn't work with PIC (gh-7640),
# ligomp.a for AArch64 CentOS is compiled without PIC support.
if (ENABLE_FUZZER OR TARGET_OS_FREEBSD OR ${CMAKE_SYSTEM_PROCESSOR} MATCHES "aarch64")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why hardening is disabled on arm64?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some arm64 Linux distros (e.g. CentOS) include system libraries that do not allow to compile Tarantool as position-independent binary, that's why I disabled hardening by default for arm64. There is a task about removing dependency on libgomp: #7689

@amdei
Copy link
Contributor

amdei commented Oct 12, 2022

May I ask, why hardening is disabled for ARM64 builds by default?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full-ci Enables all tests for a pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add extra security compiler options Switch to position independent executable compilation
4 participants