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

fix: fix partial check for fortify #248

Merged
merged 1 commit into from
Jun 8, 2024
Merged

Conversation

slimm609
Copy link
Owner

@slimm609 slimm609 commented Jun 2, 2024

  • fix partial check for fortify
  • remove photon tests

@slimm609 slimm609 force-pushed the update_fortify_partial branch 2 times, most recently from 153b6fc to 9946a2c Compare June 2, 2024 14:36
@teoberi
Copy link
Contributor

teoberi commented Jun 2, 2024

In this case, with -D_FORTIFY_SOURCE=1 defined as Partial, wouldn't it be more correct that for -D_FORTIFY_SOURCE=2 or -D_FORTIFY_SOURCE=3 to clearly specify this in order not to mislead, that is, instead of Yes, should have Yes (level 2-3)? At least this way there is a warning that the fortification level may not have been set to maximum during compilation.
I think it would be much more correct that way.

Remove photon tests is welcome, it reduces the time required for checks.

@slimm609
Copy link
Owner Author

slimm609 commented Jun 2, 2024

In this case, with -D_FORTIFY_SOURCE=1 defined as Partial, wouldn't it be more correct that for -D_FORTIFY_SOURCE=2 or -D_FORTIFY_SOURCE=3 to clearly specify this in order not to mislead, that is, instead of Yes, should have Yes (level 2 or 3 )? At least this way there is a warning that the fortification level may not have been set to maximum during compilation. I think it would be much more correct that way.

Remove photon tests is welcome, it reduces the time required for checks.

Partial is only for 1. If we detect that it has ANY fortify and not partial, then its "Yes".

Level 1 = Partial
Level 2/3 = Yes

I could change Yes to Full instead but we don't want to add extra like "(level 2)" or anything because that makes json, xml, etc more complex.

@slimm609 slimm609 force-pushed the update_fortify_partial branch 2 times, most recently from f054298 to 8f08bf5 Compare June 2, 2024 21:51
@teoberi
Copy link
Contributor

teoberi commented Jun 3, 2024

Partial is only for 1. If we detect that it has ANY fortify and not partial, then its "Yes".

Level 1 = Partial
Level 2/3 = Yes

I could change Yes to Full instead but we don't want to add extra like "(level 2)" or anything because that makes json, xml, etc more complex.

Level 2 = Yes is incorrect, in this case the fortification is incomplete. Replacing Yes with Full is even more misleading, I don't think it's a solution. I remain to my opinion from the previous post, level 2-3 should be specified until a clear way to distinguish them is found.
https://developers.redhat.com/articles/2023/02/06/how-improve-application-security-using-fortifysource3#how_to_improve_application_fortification
https://sourceware.org/git/?p=glibc.git;a=commit;h=c43c5796121bc5bcc0867f02e5536874aa8196c1
https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html

@teoberi
Copy link
Contributor

teoberi commented Jun 3, 2024

/checksec --file=/usr/bin/systemd

ELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH	Symbols		FORTIFY		Fortified	Fortifiable	FILE
Full RELRO     Canary found      NX enabled    PIE enabled     No RPATH   RW-RUNPATH   No Symbols	Yes (level 2-3)	10		17		/usr/bin/systemd

./checksec --proc-all

* System-wide ASLR (kernel.randomize_va_space): Full (Setting: 2)
  Description - Make the addresses of mmap base, heap, stack and VDSO page randomized.
  This, among other things, implies that shared libraries will be loaded to random 
  addresses. Also for PIE-linked binaries, the location of code start is randomized.
  See the kernel file 'Documentation/admin-guide/sysctl/kernel.rst' for more details.
* Does the CPU support NX: Yes
* Core-Dumps access to all users: Not Restricted
        COMMAND    PID RELRO           STACK CANARY            SECCOMP          NX/PaX        PIE                     FORTIFY
         systemd   1702 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Yes (level 2-3)
        pipewire   1710 Full RELRO      Canary found            Seccomp-bpf      NX enabled    PIE enabled             Yes (level 2-3)
 pipewire-media-   1711 Full RELRO      Canary found            Seccomp-bpf      NX enabled    PIE enabled             Yes (level 2-3)
      pulseaudio   1712 Full RELRO      Canary found            Seccomp-bpf      NX enabled    PIE enabled             Yes (level 2-3)
 gnome-keyring-d   1720 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Yes (level 2-3)
 gdm-wayland-ses   1734 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Partial
     dbus-daemon   1736 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Yes (level 2-3)
 gnome-session-b   1738 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Partial
 xdg-document-po   1740 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Yes (level 2-3)
 xdg-permission-   1750 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Yes (level 2-3)
 gnome-session-c   1800 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             No
           gvfsd   1810 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             No
      gvfsd-fuse   1817 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Yes (level 2-3)
 gnome-session-b   1827 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Partial
 at-spi-bus-laun   1855 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Partial
     gnome-shell   1859 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             N/A
     dbus-daemon   1867 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             Yes (level 2-3)
 gnome-shell-cal   1905 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             N/A
 evolution-sourc   1911 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             N/A
      goa-daemon   1919 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             N/A
 evolution-calen   1923 Full RELRO      Canary found            No Seccomp       NX enabled    PIE enabled             N/A

It only needs to add a tab in the lines:
https://github.com/slimm609/checksec.sh/blob/cbb1914f57ee06eaf21ca7d5eaf1c17d7acafdd5/src/functions/chk_file.sh#L32
and
https://github.com/slimm609/checksec.sh/blob/cbb1914f57ee06eaf21ca7d5eaf1c17d7acafdd5/src/functions/chk_file.sh#L34
before Fortified.

@slimm609
Copy link
Owner Author

slimm609 commented Jun 3, 2024

Partial is level 1, yes is level 2-3. There have already been several discussions that identifying 2 vs 3 is not possible. Adding “(level 2 or 3)” is not a good formatting structure and better for documentation.

It also depends on the version of gcc to decide if level 3 is even possible.

@teoberi
Copy link
Contributor

teoberi commented Jun 3, 2024

It would be good to at least write this somewhere in the documentation.
Please, if not, everyone will find out about it the hard way, that is, on their own skin.
As it is now, Yes can mean 3 BUT and 2, which is not very good even if it looks good in the picture.

@teoberi
Copy link
Contributor

teoberi commented Jun 8, 2024

https://github.com/slimm609/checksec.sh/blob/9a9fa9c6ac4629eb3a0f0013f744b914467b2ef9/Dockerfile.ubuntu#L12
then pt-get update -> then apt-get update
gcc-multilib is not installed?
ubuntu checksec line 1600
gcc -m32 -o output/all32 test.c -w -D_FORTIFY_SOURCE=3 -fstack-protector-strong -fpie -O2 -z relro -z now -z noexecstack -pie -s

/usr/include/stdio.h:27:10: fatal error: bits/libc-header-start.h: No such file or directory

https://stackoverflow.com/questions/54082459/fatal-error-bits-libc-header-start-h-no-such-file-or-directory-while-compili

@slimm609
Copy link
Owner Author

slimm609 commented Jun 8, 2024

https://github.com/slimm609/checksec.sh/blob/9a9fa9c6ac4629eb3a0f0013f744b914467b2ef9/Dockerfile.ubuntu#L12

then pt-get update -> then apt-get update
gcc-multilib is not installed?
ubuntu checksec line 1600
gcc -m32 -o output/all32 test.c -w -D_FORTIFY_SOURCE=3 -fstack-protector-strong -fpie -O2 -z relro -z now -z noexecstack -pie -s

/usr/include/stdio.h:27:10: fatal error: bits/libc-header-start.h: No such file or directory

https://stackoverflow.com/questions/54082459/fatal-error-bits-libc-header-start-h-no-such-file-or-directory-while-compili

I am fixing this currently. aarch64 (macbook pro) does not include gcc-multilib so adding a condition to make it easier to debug certain things without having to remove and readd it constantly. The tests for the pipeline will continue to be x86_64 so it will be included for pipeline tests.

- remove partial check for fortify due to lacking the ability to detect which version
- remove photon tests
@slimm609 slimm609 merged commit 4ef4258 into main Jun 8, 2024
1 check passed
@slimm609 slimm609 deleted the update_fortify_partial branch June 8, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants