hardening-check: add support for detecting stack clash protected binaries

Bug #1820798 reported by Alex Murray on 2019-03-19
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
devscripts (Ubuntu)
Wishlist
Alex Murray

Bug Description

The security team is in the process of making -fstack-clash-protection enabled by default in gcc-8/9 for 19.10 / 20.04. To support this it is useful to be able to detect binaries which include this new feature via hardening-check. Unlike previous features this can only be detected by looking for the sequence of instructions which perform this feature in the disassembly output via objdump.

The attachment "debdiff against current version in disco to add this feature" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Alex Murray (alexmurray) wrote :

Updated debdiff with some minor improvements to the proposed changes to be a bit more efficient and add some more comments

Alex Murray (alexmurray) wrote :

Update the debdiff again to fix a possible runtime failure in a highly unlikely corner case.

Changed in devscripts (Ubuntu):
importance: Undecided → Wishlist
Alex Murray (alexmurray) wrote :

Will let the foundations team decide on the importance of this but the security team is keen for this to land in 19.10 / EE to support the toolchain hardening updates so I hope this is seen as a higher priority than Wishlist.

Changed in devscripts (Ubuntu):
importance: Wishlist → Undecided
Steve Beattie (sbeattie) wrote :

It looks like the stack-clash detection is getting tripped up on optimization:

ubuntu@stensal-disco-server-amd64:~$ gcc -O2 -o stack-clash -fstack-clash-protection stack-clash.c
ubuntu@stensal-disco-server-amd64:~$ ./hardening-check ./stack-clash
./stack-clash:
 Position Independent Executable: yes
 Stack protected: yes
 Fortify Source functions: no, only unprotected functions found!
 Read-only relocations: yes
 Immediate binding: yes
 Stack clash protection: no, not found!
ubuntu@stensal-disco-server-amd64:~$ gcc -o stack-clash -fstack-clash-protection stack-clash.c
ubuntu@stensal-disco-server-amd64:~$ ./hardening-check ./stack-clash
./stack-clash:
 Position Independent Executable: yes
 Stack protected: yes
 Fortify Source functions: no, only unprotected functions found!
 Read-only relocations: yes
 Immediate binding: yes
 Stack clash protection: yes

Alex Murray (alexmurray) wrote :

The attached should is more robust to optimisation in gcc and is updated against the latest devscripts in disco

Alex Murray (alexmurray) wrote :

Relaxed some of the checks to find additional stack-clash-protected binaries due to more optimisation shenanigans

Simon Quigley (tsimonq2) wrote :

Subscribing Mattia Rizzolo, who co-maintains devscripts in Debian.

Mattia Rizzolo (mapreri) wrote :

Could you please submit this in the form of a MR against https://salsa.debian.org/debian/devscripts ?

I would be happy to review and merge such contribution once an MR is opened there (as a first look the patch doesn't look crazy, but I would need to look deeper - I'm not familiar with that particular script).

The hardening-check script does not have a test suite, but if you could also consider contributing one (since it's perl, just add a test/t/hardening-checks.t using Test::More) it would be really awesome (not required to get this patch merged, though)

Also, I would love if you could refrain from uploading such diffs to Ubuntu, given that I'm open to get such changes in Debian directly (removing ~ubuntu-sponsors as such…)

Alex Murray (alexmurray) wrote :

Sure I'll see what I can do - my understanding was the process was to get it into Ubuntu first and then submit it back to Debian but am happy to go the other way round.

Alex Murray (alexmurray) wrote :

MR submitted in https://salsa.debian.org/debian/devscripts/merge_requests/121

Will still try and work on the tests for it in addition so expect a follow up MR later.

Mattia Rizzolo (mapreri) wrote :

It's committed, will be in the next release.

Changed in devscripts (Ubuntu):
assignee: nobody → Alex Murray (alexmurray)
status: New → Fix Committed
importance: Undecided → Wishlist
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package devscripts - 2.19.6

---------------
devscripts (2.19.6) unstable; urgency=medium

  [ Christoph Berg ]
  * origtargz:
    + Support unpacking tarballs where the files are in /.

  [ Alex Murray ]
  * hardening-check:
    + Add support for detecting binaries compiled with
      -fstack-clash-protection. LP: #1820798; MR: !121
    + Add detection for -fcf-protection. MR: !127

  [ laokz ]
  * uscan:
    + Fix typo in documentation. MR: !125
    + Remove redundant, misleading string in a debug message. MR: !126

  [ Thomas Goirand ]
  * debchange:
    + Target buster-backports with --bpo. Closes: #931614

  [ Paul Wise ]
  * Devscripts::Config:
    + Improve handling (prevent code execution and errors with spaces and
      newlines) of the configuration files. MR: !124
      - Move String::ShellQuote from Recommends to Depends.

  [ Xavier Guimard ]
  * Reformat code following the changes in the new perltify 20181102. MR: !129
  * debi:
    + Replace dpkg + apt-get by "apt-get install" on .change file.
      Closes: #810294; MR: !45
  * salsa:
    + Add "join" command. Closes: #921314; MR: !108
    + Add "push" command. MR: !108
    + Update doc: completion for aliases. MR: !108
  * uscan:
    + Ignore --download-version when component is marked as "ignore". MR: !130
    + Fix download when <base> tag is relative. Closes: #932399; MR: !133

  [ Mattia Rizzolo ]
  * d/control:
    + Bump Standards-Version to 4.4.0, no changes needed.

  [ Nick Gerow ]
  * debchange:
    + Make sure to escape special characters in the maintainer name. MR: !128

  [ Simon McVittie ]
  * uscan:
    + Don't recurse into directories named .git when searching for
      Debian packages. MR: !132

  [ Unit 193 ]
  * dcmd:
    + Consider .asc files as part of the upstream orig files.
    + Also add .zst as an allowed extension for upstream orig files.

  [ Sean Whitton ]
  * git-deborig:
    + New --just-print-tag-names option. Closes: #931180; MR: !131

 -- Mattia Rizzolo <email address hidden> Sat, 20 Jul 2019 10:43:35 +0200

Changed in devscripts (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers