Skip to content

Conversation

@risicle
Copy link
Contributor

@risicle risicle commented Feb 21, 2023

Description of changes

This uses debian-devscripts' hardening-check for most checks, which only works on ELF systems and can only detect a limited subset of flags.

Tests mentioning "fortify3" depend on the fortify3 support currently in staging (#212498, #217394), but I've directed this at master to make it easier for people to test. Expect eval errors for those tests until this staging cycle makes it to master.

For me, this seems to work correctly on linux x86_64, with pkgsMusl, pkgsStatic and pkgsCross.aarch64-multiplatform behaving as expected.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Feb 21, 2023
@ofborg ofborg bot requested a review from 7c6f434c February 21, 2023 00:34
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Feb 21, 2023
@risicle risicle added the 8.has: tests This PR has tests label Feb 25, 2023
@risicle risicle force-pushed the ris-hardening-tests branch 4 times, most recently from 7b35404 to b618a47 Compare March 3, 2023 00:20
@risicle risicle added the 6.topic: testing Tooling for automated testing of packages and modules label Mar 3, 2023
@risicle
Copy link
Contributor Author

risicle commented Mar 29, 2023

@ofborg eval

@risicle risicle marked this pull request as ready for review March 29, 2023 19:03
@ofborg ofborg bot added 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. and removed 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Mar 29, 2023
@risicle
Copy link
Contributor Author

risicle commented Mar 29, 2023

Interesting things about this if reviewed now:

  1. I'm not sure why nixpkgs-review is ignoring the broken=true flag for these cases. Does it ignore brokenness for newly-added packages?
  2. On x86_64 linux at least, tests.hardeningFlags-clang.fortifyExplicitEnabledExecTest is failing - almost suggesting that clang only actually has FORTIFY_SOURCE=1 support on linux (!).

@bobby285271
Copy link
Member

bobby285271 commented Mar 30, 2023

why nixpkgs-review is ignoring the broken=true flag

I just ran into similar issue in #223671 (comment) and wonder if --extra-nixpkgs-config '{ allowBroken = false; }' will help here.

@risicle risicle force-pushed the ris-hardening-tests branch from b618a47 to 9e81849 Compare April 5, 2023 14:17
@risicle risicle force-pushed the ris-hardening-tests branch from 9e81849 to 0ad3ea5 Compare June 25, 2023 11:07
@risicle risicle force-pushed the ris-hardening-tests branch from 0ad3ea5 to fde4ba4 Compare July 29, 2023 10:49
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Jul 29, 2023
@risicle risicle force-pushed the ris-hardening-tests branch from fde4ba4 to faa4350 Compare July 30, 2023 17:38
@risicle
Copy link
Contributor Author

risicle commented Jul 30, 2023

(not actually going to merge this until we have a better idea about what the desired behaviour is...)

most tests use debian-devscripts' hardening-check, so only work on
ELF systems and can only detect a limited subset of flags.

some extra tests actually execute fortify-protected programs and
should be slightly more universally applicable.
@risicle risicle force-pushed the ris-hardening-tests branch from faa4350 to e0f6367 Compare September 2, 2023 14:02
@ofborg ofborg bot requested a review from 7c6f434c September 2, 2023 14:24
@risicle
Copy link
Contributor Author

risicle commented Sep 2, 2023

(rebased, adjusted expected results for current nixpkgs)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: testing Tooling for automated testing of packages and modules 8.has: package (new) This PR adds a new package 8.has: tests This PR has tests 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants