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

build: Enable PIE when compiling Linux builds, fix #14961. (2-0-x) #15148

Merged
merged 1 commit into from Oct 14, 2018

Conversation

Projects
None yet
2 participants
@biergaizi

biergaizi commented Oct 13, 2018

PIE allows an application to utilize the full benefits of ASLR
to prevent itself from exploitations, but it was disabled for
all released versions of Electron (3.0 and prior).

Currently, PIE is already enabled since 9294fac but enabling it
for all released version is still an ongoing work (#14961). This
patch backports PIE to the 2.0.x branch, which is an especially
important branch, since security is an urgency for many existing
programs including Signal-Desktop. Enabling it can help protect
many high-risk users.

Signed-off-by: Tom Li tomli@tomli.me

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • PR title follows semantic commit guidelines

Release Notes

Notes: Enable PIE when compiling Linux builds.

build: enable PIE when compiling Linux builds, fix #14961.
PIE allows an application to utilize the full benefits of ASLR
to prevent itself from exploitations, but it was disabled for
all released versions of Electron (3.0 and prior).

Currently, PIE is already enabled since 9294fac but enabling it
for all released version is still an ongoing work (#14961). This
patch backports PIE to the 2.0.x branch, which is an especially
important branch, since security is an urgency for many existing
programs including Signal-Desktop. Enabling it can help protect
many high-risk users.

Signed-off-by: Tom Li <tomli@tomli.me>

@biergaizi biergaizi requested a review from electron/reviewers as a code owner Oct 13, 2018

@welcome

This comment has been minimized.

welcome bot commented Oct 13, 2018

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add app.isPackaged() method
  • docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

  • Follow the JavaScript, C++, and Python coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made following the documentation styleguide.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@biergaizi biergaizi changed the title from Enable PIE in the 2.0.x branch when compiling Linux builds, fix #14961. to build: Enable PIE in the 2.0.x branch when compiling Linux builds, fix #14961. Oct 13, 2018

@biergaizi biergaizi changed the title from build: Enable PIE in the 2.0.x branch when compiling Linux builds, fix #14961. to build: Enable PIE in the branch when compiling Linux builds, fix #14961. (2-0-x) Oct 13, 2018

@biergaizi biergaizi changed the title from build: Enable PIE in the branch when compiling Linux builds, fix #14961. (2-0-x) to build: Enable PIE when compiling Linux builds, fix #14961. (2-0-x) Oct 13, 2018

@MarshallOfSound

This comment has been minimized.

Member

MarshallOfSound commented Oct 14, 2018

mac / mas failures are flakes

@MarshallOfSound MarshallOfSound merged commit 2f2761f into electron:2-0-x Oct 14, 2018

12 of 14 checks passed

ci/circleci: electron-mas-x64 Your tests failed on CircleCI
Details
ci/circleci: electron-osx-x64 Your tests failed on CircleCI
Details
Absolute Zero
Semantic Pull Request ready to be squashed
Details
WIP ready for review
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

release-clerk bot commented Oct 14, 2018

Release Notes Persisted

Enable PIE when compiling Linux builds.

@welcome

This comment has been minimized.

welcome bot commented Oct 14, 2018

Congrats on merging your first pull request! 🎉🎉🎉

@MarshallOfSound

This comment has been minimized.

Member

MarshallOfSound commented Oct 18, 2018

@biergaizi Unfortunately this PR was reverted as it caused release build failures, probably due to the mismatch of PIE / BIND_NOW between the Electron build and the libcc static build. If you want to take another shot at fixing this it'll probably require some build changes in libcc as well.

MarshallOfSound added a commit that referenced this pull request Oct 18, 2018

@biergaizi

This comment has been minimized.

biergaizi commented Oct 21, 2018

What are the necessary steps for reproducing the release build failure? Is there any documentation/code about the release build process?

@MarshallOfSound

This comment has been minimized.

Member

MarshallOfSound commented Oct 21, 2018

@biergaizi Yeah the build instructions for 2.0 and 3.0 tell you how to do it. It's basically:

python script/bootstrap.py -v
python script/build.py -c R
@biergaizi

This comment has been minimized.

biergaizi commented Oct 23, 2018

@MarshallOfSound Thanks for the reply. I've indeed read the documentation in the very beginning and it was exactly what I was following for my testing. I've confirmed all the binaries were built successfully and ran without issues on my system, before I submitted the pull request.

The build failure was not caught by myself, possibly because my distribution enabled relevant compiler flags by default. So again, I'm requesting for details. Which Linux distribution and GCC compiler version were used for your test build? I need these information to reproduce the build failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment