fluent-bit icon indicating copy to clipboard operation
fluent-bit copied to clipboard

build: Set security flags for release builds

Open Garfield96 opened this issue 3 years ago • 20 comments

Signed-off-by: Christian Menges [email protected]

Set security flags for release builds using gcc or clang:

  • -Wl,-z,relro,-z,now Set Global Offset Table (GOT) to read-only. In theory, this increases startup times, but I couldn't observe a performance degradation.
  • -fstack-protector Protect against stack manipulation.
  • -D_FORTIFY_SOURCE=1 Replace certain functions with more secure alternatives. With level 1, most of the added security checks are optimized away.

Fixes #7315


Enter [N/A] in the box, if an item is not applicable to your change.

Testing Before we can approve your change; please submit the following in a comment:

  • [N/A] Example configuration file for the change
  • [N/A] Debug log output from testing the change
  • [N/A] Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [ ] Attached local packaging test output showing all targets (including any new ones) build.

Documentation

  • [N/A] Documentation required for this feature

Backporting

  • [N/A] Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Garfield96 avatar Sep 23 '22 09:09 Garfield96

We probably need a build of all targets to confirm this works ok on everything.

patrick-stephens avatar Oct 13 '22 12:10 patrick-stephens

@patrick-stephens I executed the package tests in my fork. Unfortunately, the timeout was reached before the tests completed. However, all executed tests were successful. Build log: fluent-bit-build-logs.txt

Garfield96 avatar Dec 30 '22 09:12 Garfield96

@niedbalski Could you please review this PR? Thanks

Garfield96 avatar Jan 05 '23 08:01 Garfield96

@Garfield96 thanks, we're both on PTO at the moment but I'll catch up with @niedbalski once he's back.

patrick-stephens avatar Jan 09 '23 11:01 patrick-stephens

Hi @patrick-stephens and @niedbalski, this PR is already open for some time. Shall I change something or do you require additional information to review the PR?

Garfield96 avatar Feb 27 '23 16:02 Garfield96

@Garfield96 I appreciate your time - unfortunately @niedbalski is off for an extended period.

patrick-stephens avatar Feb 28 '23 15:02 patrick-stephens

Hi @niedbalski, I saw you are back. Could you please have a look at this PR? Thanks

Garfield96 avatar Apr 18 '23 21:04 Garfield96

@Garfield96 can you update the merge commit title (and ideally rebase)?

patrick-stephens avatar Apr 24 '23 09:04 patrick-stephens

@patrick-stephens I rebased the PR. The test failures are unrelated to the change and are also present in several other recently opened PRs.

Garfield96 avatar May 04 '23 06:05 Garfield96

@leonardo-albertovich Can this PR get merged?

Garfield96 avatar Aug 01 '23 11:08 Garfield96

Is this the cause of the following in my dmesg output?

[  365.250531] process 'opt/fluent-bit/bin/fluent-bit' started with executable stack

rossigee avatar Oct 02 '23 06:10 rossigee

Is this the cause of the following in my dmesg output?

[  365.250531] process 'opt/fluent-bit/bin/fluent-bit' started with executable stack

@rossigee I'm not sure what you're asking there - this PR is not merged so any changes in it are not in official releases. As to why you're seeing that message, this really should be captured on an issue (if it is a problem) along with full details on the version of FB, OS, config, etc.?

patrick-stephens avatar Oct 02 '23 10:10 patrick-stephens

@patrick-stephens - sorry I wasn't clear. I saw this message while reviewing the logs on one of my hosts, and was alarmed. I wasn't sure if my 'fluent-bit' binary (official release build v2.1.9, deployed as a K8S DS) had somehow been tampered with, but on further investigation I came across this PR which addresses certain 'official release build' compile-time issues, one of which appears to be related to preventing the executable stack problem.

I wasn't able to find anyone else reporting the same issue by Googling the error message, and I was curious as to whether or not this PR would solve my problem (once it's merged and released). Apologies for the noise.

rossigee avatar Oct 02 '23 13:10 rossigee

@rossigee I assume that this PR will remove the warning from your logs.

Garfield96 avatar Oct 18 '23 14:10 Garfield96

@patrick-stephens I'm sorry for asking again, but can this PR be merged soon? I think a lot of users would benefit from it and as mentioned by @rossigee and https://github.com/fluent/fluent-bit/issues/7315, some platforms even issue warnings for the current fluent-bit binary.

Garfield96 avatar Oct 18 '23 14:10 Garfield96

@Garfield96 as I understand it I think we're happy with this - I'm not the best person to review the compilation flags themselves however @leonardo-albertovich indicated he was so I'll ping @edsiper to see if we can get it merged.

patrick-stephens avatar Oct 19 '23 11:10 patrick-stephens

Hi @leonardo-albertovich and @edsiper, I'd highly appreciate if you could review and, ideally, merge this PR. Thanks

Garfield96 avatar Nov 12 '23 15:11 Garfield96

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions[bot] avatar Feb 12 '24 01:02 github-actions[bot]

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions[bot] avatar May 14 '24 01:05 github-actions[bot]

added to Milestone v3.1.0

edsiper avatar May 14 '24 16:05 edsiper