Skip to content
New issue

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

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

Already on GitHub? # to your account

_build_ Set noexecstack on snapcraft builds #9868

Merged
merged 1 commit into from
Dec 15, 2022
Merged

Conversation

ianconsolata
Copy link
Contributor

We're currently failing the automated security review on snapcraft because the lotus binary has the execstack value set:
https://linux.die.net/man/8/execstack

This commit passes the appropriate flags to ld to disable the execstack flag when building the binaries for snapcraft:
https://linux.die.net/man/1/ld

We may want to consider disabling this as part of the main build. Research seems to indicate that allow the executable stack can lead to security issues, but I am not enough of a security expert to know for sure what the right call here is:
https://f0rm2l1n.github.io/2022-04-02-What-is-happended-to-execstack/

Related Issues

https://app.circleci.com/pipelines/github/filecoin-project/lotus/24812/workflows/caf64d82-58af-4985-85ad-8a8e838fd174
https://filecoinproject.slack.com/archives/C029MT4PQB1/p1670971371929079

Proposed Changes

Set the noexecstack ld flag on snapcraft builds.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

We're currently failing the auoptmated security review on snapcraft
because the lotus binary has the execstack value set:
  https://linux.die.net/man/8/execstack

This commit passes the appropriate flags to ld to disable the execstack
flag when building the binaries for snapcraft:
  https://linux.die.net/man/1/ld

We may want to consider disabling this as part of the main build.
Research seems to indicate that allow the executable stack can lead to
security issues, but I am not enough of a security expert to know for
sure what the right call here is:
  https://f0rm2l1n.github.io/2022-04-02-What-is-happended-to-execstack/
@ianconsolata ianconsolata requested a review from a team as a code owner December 14, 2022 02:12
Copy link
Contributor

@geoff-vball geoff-vball left a comment

Choose a reason for hiding this comment

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

Worth a shot! Let me know if this resolves the issue!

@geoff-vball geoff-vball merged commit 1dcae9a into master Dec 15, 2022
@geoff-vball geoff-vball deleted the id/fix-snapcraft branch December 15, 2022 20:30
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants