Update compile flags per security review #4368

Activity

1. eseidelGoogle

   added

   engineflutter/engine related. See also e: labels.flutter/engine related. See also e: labels.

   on Jun 3, 2016
2. eseidelGoogle

   added this to the Flutter 1.0This milestone has been deleted milestone on Jun 3, 2016
3. Hixie

   modified the milestones: 3: Make conferences happyThis milestone has been deleted, 5: Make Hixie proudThis milestone has been deleted on Jan 30, 2017
4. Hixie

   added

   c: contributor-productivityTeam-specific productivity, code health, technical debt.Team-specific productivity, code health, technical debt.

   on Oct 24, 2017
5. Hixie

   modified the milestones: 3: Current Milestone, 4: Next milestoneThis milestone has been deleted on Dec 20, 2017
6. cdotstout

   added a commit that references this issue on Apr 3, 2018

   roll buildroot to 82a50e8 (flutter#4368)

   384e351
7. kf6gpe commented on Oct 2, 2018

   

   kf6gpe

   on Oct 2, 2018

   Contributor

   More actions

   So, the Android changes are in my branch; I'm working on testing the iOS changes now. Some notes about iOS changes:

   • iOS compiles with PIE by default, but I'll add it as explicit flags so it's clear what we're doing.
   • The compiler flags -fstack-protector-all, -Wa,--noexecstack, -Wformat and -Wformat-security appear to have no corresponding flags in the iOS toolchain. I haven't found good counterparts, but admit I need to continue to look.
   • The linker flag -Wl,-z,relro,-z,now has no correspondence either.
8. kf6gpe commented on Oct 4, 2018

   

   kf6gpe

   on Oct 4, 2018

   Contributor

   More actions

   After chatting with @goderbauer, we agree that we don't want -O2 if we can help it either --- the space hit is just too high --- -Oz on helloworld weighs in at 5,805,240B, while -O2 weighs in at 8,024060B in his test. I'm backing that out now.
9. kf6gpe

   self-assigned this

   on Oct 4, 2018
10. goderbauer commented on Oct 4, 2018

    

    goderbauer

    on Oct 4, 2018

    Member

    More actions

    @kf6gpe The sizes you quoted are for the uncompressed libflutter.so, which ships with every app (not just helloworld).
11. kf6gpe commented on Oct 4, 2018

    

    kf6gpe

    on Oct 4, 2018

    Contributor

    More actions

    @goderbauer Good point --- thanks for the clarification.
12. kf6gpe commented on Oct 29, 2018

    

    kf6gpe

    on Oct 29, 2018

    Contributor

    More actions

    Looks like this has caused regressions in disk space size as well as performance for tiles_scroll_perf__timeline_summary/average_frame_rasterizer_time_millis. We've rolled back this CL at present until we pinpoint which flag is the culprit.
13. Letty-Liang

    added a commit that references this issue on Mar 21, 2019

    Add compiler flags suggested by security review (flutter#4368) (flutt…

    ...

    9bbd435
14. 2 remaining items

    Load more

15. kf6gpe commented on Jul 31, 2019

    

    kf6gpe

    on Jul 31, 2019

    Contributor

    More actions

    We've left the reversion in place, preferring the reduction in package size over the footprint increase.
16. kf6gpe

    added

    P2Important issues not at the top of the work listImportant issues not at the top of the work list

    on May 29, 2020
17. kf6gpe

    removed this from the GoalsThis milestone has been deleted milestone on Jun 2, 2020
18. Hixie

    removed this from the GoalsThis milestone has been deleted milestone on Jun 2, 2020
19. Hixie

    removed this from the None.This milestone has been deleted milestone on Aug 17, 2020
20. markusaksli-nc

    added

    a: releaseChallenges faced when attempting to productionize an appChallenges faced when attempting to productionize an app

    passed first triage

    on Sep 28, 2020
21. Seirdy commented on Dec 9, 2021

    

    Seirdy

    on Dec 9, 2021

    More actions

    Many linux distros are trying to get all executables to be PIE-only as part of the road to full ASLR, so this would be appreciated. -fcf-protection=full in CFLAGS and -Wl,--as-needed in LDFLAGS could also help.

    Additionally, enabling LTO could pave the way to eventually supporting Clang's forward-edge CFI with -fsanitize=cfi, and could reduce binary size when paired with -ffunction-sections -fdata-sections. This could help offset the increases in package size, especially if combined with linking some libs statically and dropping the .so files.
22. chinmaygarde commented on Dec 9, 2021

    

    chinmaygarde

    on Dec 9, 2021

    Contributor

    More actions

    Many linux distros are trying to get all executables to be PIE-only as part of the road to full ASLR, so this would be appreciated. -fcf-protection=full in CFLAGS and -Wl,--as-needed in LDFLAGS could also help.

    Flutter does have support for plugging in custom toolchains and some support for passing toolchain specific flags. So distros can do this for the engine dylib if they want to build the engine on their own. Is there a specific binary/distro you are thinking of? This may be a good thing to do everywhere.

    Additionally, enabling LTO could pave the way to eventually supporting...

    Flutter already performs LTO on all optimized targets with at least some of the flags you mentioned already being applied. The last time LTO was tuned was a while ago however so it may be a good time to revisit all LTO flags.

    linking some libs statically and dropping the .so files.

    All engine libraries are already linked statically into a single engine .so file.
23. Seirdy commented on Dec 9, 2021

    

    Seirdy

    on Dec 9, 2021 via email

    More actions

    On Thu, Dec 09, 2021 at 02:29:47PM -0800, Chinmay Garde wrote: > Many linux distros are trying to get all executables to be PIE-only as part of the road to full ASLR, so this would be appreciated. -fcf-protection=full in CFLAGS and -Wl,--as-needed in LDFLAGS could also help. Flutter does have support for plugging in custom toolchains and some support for passing toolchain specific flags. So distros can do this for the engine dylib if they want to build the engine on their own. Is there a specific binary/distro you are thinking of? This may be a good thing to do everywhere.

    Not exactly a distro, but I was thinking about Flatpak. It's common for Flatpak manifests to pull in tarball artifacts from CI. I think that PIE-by-default for release configurations makes sense; there's little reason not to do it (unless there's some regression like massive binary size, which sounds like an important bug).

    Flutter already performs LTO on all optimized targets with at least some of the flags you mentioned [already being applied](https://github.com/flutter/buildroot/blob/430b57c643883e6090b5af09faddd8048efee57c/build/config/compiler/BUILD.gn#L791). The last time LTO was tuned was a while ago however so it may be a good time to revisit all LTO flags.

    I didn't know this, thanks. LTO along with -fvisibility=hidden is a pre-requisite for some of Clang's most advanced hardening flags, like -fsanitize=cfi (<1% perf penalty for huge exploit mitigations). -fcf-protection=full also allows some CFI (esp. for GCC toolchains).

    All engine libraries are already linked statically into a single engine `.so` file.

    I was thinking that perhaps these could be statically linked into the final binary, but I understand that this would be quite a complex undertaking.

    …

    -- /Seirdy
24. Hixie

    unassigned

    kf6gpe

    on Jan 18, 2023
25. Hixie commented on Mar 16, 2023

    

    Hixie

    on Mar 16, 2023

    Contributor

    More actions

    I'm going to close this; we have more methodical work ongoing to harden our toolchain now.
26. Hixie

    closed this as completedon Mar 16, 2023
27. github-actions commented on Mar 30, 2023

    

    github-actionsbot

    on Mar 30, 2023

    More actions

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.
28. github-actions

    locked as resolved and limited conversation to collaborators on Mar 30, 2023