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

Hardening compiler flags #578

Merged
merged 1 commit into from
Jan 30, 2020
Merged

Hardening compiler flags #578

merged 1 commit into from
Jan 30, 2020

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Jan 27, 2020

Enable more compile-time and run-time protections which are recommended and used by Debian and Red Hat when building their package bases.

These are quite old compilation flags, only -fstack-protector-strong is supported since GCC 4.9. We might need to support earlier versions for distros like RHEL and CentOS so replace it with -fstack-protector when unavailable. They are usually even enabled by default, but let’s make sure that the compiler uses them. Stack canaries are run-time checks for buffer overflows in local variables which are the main source of ROP attacks.

By the way, you saw that PR with passphrase API for Secure Cell (#577)? It introduces a textbook example of a buffer overflow bug which is caught by a stack canary like this. Can you spot the bug? Because I certainly could not – not without a dead canary in my face.

FORTIFY_SOURCE is not enabled by default usually. It replaces various standard library functions (strcpy(), memcpy(), etc.) with their buffer-length-aware alternatives where possible. They will abort the program (or compilation) if they detect an obvious buffer overflow for arrays of statically-known size.

Immediate binding and read-only relocations prevent some exploits which may redirect functions imported by Themis somewhere different. They also somewhat limit possible effects of accidental memory corruption.

macOS/iOS uses different linker flags for relro (-read_only_relocs) but relocations have to be writeable on some platforms (e.g., ARM64) therefore we limit it to Linux only.

Another recommended option is using position-independent executable code but since we're a shared library we already build with -fPIC.

Checklist

  • Change is covered by automated tests
  • The coding guidelines are followed
  • Public API has proper documentation
  • Changelog is updated (maybe later)

Enable more compile-time and run-time protections which are recommended
and used by Debian [1] and Red Hat [2] when building their package bases.

[1]: https://wiki.debian.org/Hardening
[2]: https://developers.redhat.com/blog/2018/03/21/compiler-and-linker-flags-gcc/

These are quite old compilation flags, only -fstack-protector-strong is
supported since GCC 4.9. We might need to support earlier versions for
distros like RHEL and CentOS so replace it with -fstack-protector when
unavailable. They are usually even enabled by default, but let's make
sure that the compiler uses them. Stack canaries are run-time checks for
buffer overflows in local variables which are the main source of ROP
attacks.

FORTIFY_SOURCE is not enabled by default usually. It replaces various
standard library functions (strcpy(), memcpy(), etc.) with their
buffer-length-aware alternatives where possible. They will abort the
program (or compilation) if they detect an obvious buffer overflow for
arrays of statically-known size.

Immediate binding and read-only relocations prevent some exploits which
may redirect functions imported by Themis somewhere different. They also
somewhat limit possible effects of accidental memory corruption.

macOS/iOS uses different linker flags for relro (-read_only_relocs)
but relocations have to be writeable on some platforms (e.g., ARM64)
therefore we limit it to Linux only.

Another recommended option is using position-independed executable code
but since we're a shared library we already build with -fPIC.
@ilammy ilammy added core Themis Core written in C, its packages O-Linux 🐧 Operating system: Linux labels Jan 27, 2020
Copy link
Contributor

@vixentael vixentael left a comment

Choose a reason for hiding this comment

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

Do all these flags work when building on Mac using clang? We suppose they should work, right?

@ilammy
Copy link
Collaborator Author

ilammy commented Jan 27, 2020

Do all these flags work when building on Mac using clang?

Yep, all these compiler flags are supported by Clang and work on Apple platforms too.

-z relro and -z now are not supported by Apple linker, but they are enabled only for Linux.

@vixentael
Copy link
Contributor

Sounds awesome, thank you for checking and taking care of our mac users :)

@vixentael vixentael added the installation Installation of Themis core and wrapper packages label Jan 27, 2020
@ilammy
Copy link
Collaborator Author

ilammy commented Jan 27, 2020

Come to think of it... Cartage and CocoaPods do not use the Makefile, so they have to be adjusted separately. This change currently affects only Homebrew builds of Themis installed into the system.

@ilammy
Copy link
Collaborator Author

ilammy commented Jan 27, 2020

I've poked around in Xcode and it looks like we can avoid making any changes there.

Modern Xcode already highlights buffer-related issues prevented by FORTIFY_SOURCE even without it being specified. Carthage and CocoaPods do not print out warnings so our users (and us) will not see them. -fstack-protector-strong in enabled by default in Apple's clang.

If there are any issues we will have to catch them on other platforms and they will be fixed for Carthage/CocoaPods users as well.

@vixentael
Copy link
Contributor

I've poked around in Xcode and it looks like we can avoid making any changes there.

Totally agree. For me, Xcode-based projects are the other world with different rules, and their own caveats :D

@ilammy ilammy merged commit 0814764 into cossacklabs:master Jan 30, 2020
@ilammy ilammy deleted the hardening branch January 30, 2020 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Themis Core written in C, its packages installation Installation of Themis core and wrapper packages O-Linux 🐧 Operating system: Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants