Compare commits

..

7 commits

Author SHA1 Message Date
alois31 3dbfdfcced
libstore/build: block io_uring
Change-Id: I45d3895f95abe1bc103a63969f444c334dbbf50d
2024-07-10 05:58:51 +02:00
alois31 d34c683819
libstore/build: use an allowlist approach to syscall filtering
Previously, system call filtering (to prevent builders from storing files with
setuid/setgid permission bits or extended attributes) was performed using a
blocklist. While this looks simple at first, it actually carries significant
security and maintainability risks: after all, the kernel may add new syscalls
to achieve the same functionality one is trying to block, and it can even be
hard to actually add the syscall to the blocklist when building against a C
library that doesn't know about it yet. For a recent demonstration of this
happening in practice to Nix, see the introduction of fchmodat2 [0] [1].

The allowlist approach does not share the same drawback. While it does require
a rather large list of harmless syscalls to be maintained in the codebase,
failing to update this list (and roll out the update to all users) in time has
rather benign effects; at worst, very recent programs that already rely on new
syscalls will fail with an error the same way they would on a slightly older
kernel that doesn't support them yet. Most importantly, no unintended new ways
of performing dangerous operations will be silently allowed.

Another possible drawback is reduced system call performance due to the larger
filter created by the allowlist requiring more computation [2]. However, this
issue has not convincingly been demonstrated yet in practice, for example in
systemd or various browsers.

This commit tries to keep the behavior as close to unchanged as possible. Only
newer syscalls that are not supported by glibc 2.38 (as found in NixOS 23.11)
are blocked. Since this includes fchmodat2, the compatibility code added for
handling this syscall can be removed too.

[0] https://github.com/NixOS/nixpkgs/issues/300635
[1] https://github.com/NixOS/nix/issues/10424
[2] https://github.com/flatpak/flatpak/pull/4462#issuecomment-1061690607

Change-Id: I541be3ea9b249bcceddfed6a5a13ac10b11e16ad
2024-07-10 05:58:51 +02:00
alois31 4202552cce
libstore/build: always treat seccomp setup failures as fatal
In f047e4357b, I missed the behavior that if
building without a dedicated build user (i.e. in single-user setups), seccomp
setup failures are silently ignored. This was introduced without explanation 7
years ago (ff6becafa8). Hopefully the only
use-case nowadays is causing spurious test suite successes when messing up the
seccomp filter during development. Let's try removing it.

Change-Id: Ibe51416d9c7a6dd635c2282990224861adf1ceab
2024-07-10 05:58:51 +02:00
alois31 fa92f41a18 Merge "libmain: clear display attributes in the multiline progress bar" into main 2024-07-10 03:43:07 +00:00
Quantum Jump 6e0ca02425 Fix dry-run flag for nix-collect-garbage
`nix-collect-garbage --dry-run` previously elided the entire garbage
collection check, meaning that it would just exit the script without
printing anything.

This change makes the dry run flag instead set the GC action to
`gcReturnDead` rather than `gcDeleteDead`, and then continue with the
script. So if you set `--dry-run`, it will print the paths it *would*
have garbage collected, but not actually delete them.

I filed a bug for this: lix-project/lix#432 but then realised I could give fixing it a go myself.

Change-Id: I062dbf1a80bbab192b5fd0b3a453a0b555ad16f2
2024-07-09 13:55:05 +00:00
Qyriad accfd8aa9d libexpr: stop lying about DrvInfo's constness
DrvInfo's query methods all use mutable fields to cache, but like.
that's basically the entire interface for DrvInfo. Not only that, but
these formerly-const-marked functions can even throw due to eval errors!
Changing this only required removing some `const` markers in nix-env,
and changing a single inline `queryInstalled()` call to be an lvalue
instead.

Change-Id: I796807118f3b35b0e93668b5e28210d9e521b2ae
2024-07-08 17:52:02 +00:00
alois31 f5ff70d7f3
libmain: clear display attributes in the multiline progress bar
Activities can set display attributes in their log output using the "Select
Graphics Rendition" functionality. To prevent interfering with subsequent text
displayed, these should be reset after writing the log line. The multiline
progress bar neglected to do this, resulting for example in a colorised
"building …" header in the next line. Reset the attributes properly, like the
standard progress bar already does.

Change-Id: I1dc69f4a1d747a76b83e8721a72d9bb0e5554488
2024-07-08 19:08:23 +02:00
9 changed files with 119 additions and 33 deletions

View file

@ -96,6 +96,10 @@ puck:
forgejo: puck
github: puckipedia
quantumjump:
display_name: Quantum Jump
github: QuantumBJump
r-vdp:
github: r-vdp

View file

@ -0,0 +1,29 @@
---
synopsis: Fix nix-collect-garbage --dry-run
issues: [fj#432]
cls: [1566]
category: Fixes
credits: [quantumjump]
---
`nix-collect-garbage --dry-run` did not previously give any output - it simply
exited without even checking to see what paths would be deleted.
```
$ nix-collect-garbage --dry-run
$
```
We updated the behaviour of the flag such that instead it prints out how many
paths it *would* delete, but doesn't actually delete them.
```
$ nix-collect-garbage --dry-run
finding garbage collector roots...
determining live/dead paths...
...
<nix store paths>
...
2670 store paths deleted, 0.00MiB freed
$
```

View file

@ -45,7 +45,7 @@ DrvInfo::DrvInfo(EvalState & state, ref<Store> store, const std::string & drvPat
}
std::string DrvInfo::queryName() const
std::string DrvInfo::queryName()
{
if (name == "" && attrs) {
auto i = attrs->find(state->sName);
@ -56,7 +56,7 @@ std::string DrvInfo::queryName() const
}
std::string DrvInfo::querySystem() const
std::string DrvInfo::querySystem()
{
if (system == "" && attrs) {
auto i = attrs->find(state->sSystem);
@ -66,7 +66,7 @@ std::string DrvInfo::querySystem() const
}
std::optional<StorePath> DrvInfo::queryDrvPath() const
std::optional<StorePath> DrvInfo::queryDrvPath()
{
if (!drvPath && attrs) {
Bindings::iterator i = attrs->find(state->sDrvPath);
@ -80,7 +80,7 @@ std::optional<StorePath> DrvInfo::queryDrvPath() const
}
StorePath DrvInfo::requireDrvPath() const
StorePath DrvInfo::requireDrvPath()
{
if (auto drvPath = queryDrvPath())
return *drvPath;
@ -88,7 +88,7 @@ StorePath DrvInfo::requireDrvPath() const
}
StorePath DrvInfo::queryOutPath() const
StorePath DrvInfo::queryOutPath()
{
if (!outPath && attrs) {
Bindings::iterator i = attrs->find(state->sOutPath);
@ -164,7 +164,7 @@ DrvInfo::Outputs DrvInfo::queryOutputs(bool withPaths, bool onlyOutputsToInstall
}
std::string DrvInfo::queryOutputName() const
std::string DrvInfo::queryOutputName()
{
if (outputName == "" && attrs) {
Bindings::iterator i = attrs->find(state->sOutputName);

View file

@ -19,11 +19,11 @@ public:
private:
EvalState * state;
mutable std::string name;
mutable std::string system;
mutable std::optional<std::optional<StorePath>> drvPath;
mutable std::optional<StorePath> outPath;
mutable std::string outputName;
std::string name;
std::string system;
std::optional<std::optional<StorePath>> drvPath;
std::optional<StorePath> outPath;
std::string outputName;
Outputs outputs;
/**
@ -47,12 +47,12 @@ public:
DrvInfo(EvalState & state, std::string attrPath, Bindings * attrs);
DrvInfo(EvalState & state, ref<Store> store, const std::string & drvPathWithOutputs);
std::string queryName() const;
std::string querySystem() const;
std::optional<StorePath> queryDrvPath() const;
StorePath requireDrvPath() const;
StorePath queryOutPath() const;
std::string queryOutputName() const;
std::string queryName();
std::string querySystem();
std::optional<StorePath> queryDrvPath();
StorePath requireDrvPath();
StorePath queryOutPath();
std::string queryOutputName();
/**
* Return the unordered map of output names to (optional) output paths.
* The "outputs to install" are determined by `meta.outputsToInstall`.

View file

@ -356,7 +356,7 @@ std::chrono::milliseconds ProgressBar::draw(State & state, const std::optional<s
line += "]";
}
if (printMultiline && !line.empty()) {
writeToStderr(filterANSIEscapes(line, false, width) + "\n");
writeToStderr(filterANSIEscapes(line, false, width) + ANSI_NORMAL "\n");
state.lastLines++;
}
@ -398,7 +398,7 @@ std::chrono::milliseconds ProgressBar::draw(State & state, const std::optional<s
if (printMultiline) {
if (state.lastLines < (height -1)) {
writeToStderr(filterANSIEscapes(activity_line, false, width) + "\n");
writeToStderr(filterANSIEscapes(activity_line, false, width) + ANSI_NORMAL "\n");
state.lastLines++;
} else moreActivities++;
}

View file

@ -89,12 +89,21 @@ static int main_nix_collect_garbage(int argc, char * * argv)
// Run the actual garbage collector.
if (!dryRun) {
options.action = GCOptions::gcDeleteDead;
} else {
options.action = GCOptions::gcReturnDead;
}
auto store = openStore();
auto & gcStore = require<GcStore>(*store);
options.action = GCOptions::gcDeleteDead;
GCResults results;
PrintFreed freed(true, results);
gcStore.collectGarbage(options, results);
if (dryRun) {
// Only print results for dry run; when !dryRun, paths will be printed as they're deleted.
for (auto & i : results.paths) {
printInfo("%s", i);
}
}
return 0;

View file

@ -236,10 +236,10 @@ static void checkSelectorUse(DrvNames & selectors)
namespace {
std::set<std::string> searchByPrefix(const DrvInfos & allElems, std::string_view prefix) {
std::set<std::string> searchByPrefix(DrvInfos & allElems, std::string_view prefix) {
constexpr std::size_t maxResults = 3;
std::set<std::string> result;
for (const auto & drvInfo : allElems) {
for (auto & drvInfo : allElems) {
const auto drvName = DrvName { drvInfo.queryName() };
if (drvName.name.starts_with(prefix)) {
result.emplace(drvName.name);
@ -319,7 +319,7 @@ std::vector<Match> pickNewestOnly(EvalState & state, std::vector<Match> matches)
} // end namespace
static DrvInfos filterBySelector(EvalState & state, const DrvInfos & allElems,
static DrvInfos filterBySelector(EvalState & state, DrvInfos & allElems,
const Strings & args, bool newestOnly)
{
DrvNames selectors = drvNamesFromArgs(args);
@ -331,7 +331,7 @@ static DrvInfos filterBySelector(EvalState & state, const DrvInfos & allElems,
for (auto & selector : selectors) {
std::vector<Match> matches;
for (const auto & [index, drvInfo] : enumerate(allElems)) {
for (auto && [index, drvInfo] : enumerate(allElems)) {
const auto drvName = DrvName { drvInfo.queryName() };
if (selector.matches(drvName)) {
++selector.hits;
@ -457,9 +457,12 @@ static void queryInstSources(EvalState & state,
user environment. These are then filtered as in the
`srcNixExprDrvs' case. */
case srcProfile: {
elems = filterBySelector(state,
queryInstalled(state, instSource.profile),
args, newestOnly);
auto installed = queryInstalled(state, instSource.profile);
elems = filterBySelector(
state,
installed,
args, newestOnly
);
break;
}
@ -838,7 +841,7 @@ static bool cmpChars(char a, char b)
}
static bool cmpElemByName(const DrvInfo & a, const DrvInfo & b)
static bool cmpElemByName(DrvInfo & a, DrvInfo & b)
{
auto a_name = a.queryName();
auto b_name = b.queryName();
@ -891,7 +894,7 @@ void printTable(Table & table)
typedef enum { cvLess, cvEqual, cvGreater, cvUnavail } VersionDiff;
static VersionDiff compareVersionAgainstSet(
const DrvInfo & elem, const DrvInfos & elems, std::string & version)
DrvInfo & elem, DrvInfos & elems, std::string & version)
{
DrvName name(elem.queryName());

View file

@ -73,6 +73,7 @@ functional_tests_scripts = [
'flakes/flake-in-submodule.sh',
'gc.sh',
'nix-collect-garbage-d.sh',
'nix-collect-garbage-dry-run.sh',
'remote-store.sh',
'legacy-ssh-store.sh',
'lang.sh',

View file

@ -0,0 +1,40 @@
source common.sh
clearStore
## Test `nix-collect-garbage --dry-run`
testCollectGarbageDryRun () {
clearProfiles
# Install then uninstall a package
# This should leave packages ready to be garbage collected.
nix-env -f ./user-envs.nix -i foo-1.0
nix-env -f ./user-envs.nix -e foo-1.0
nix-env --delete-generations old
[[ $(nix-store --gc --print-dead | wc -l) -eq 7 ]]
nix-collect-garbage --dry-run
[[ $(nix-store --gc --print-dead | wc -l) -eq 7 ]]
}
testCollectGarbageDryRun
# Run the same test, but forcing the profiles an arbitrary location.
rm ~/.nix-profile
ln -s $TEST_ROOT/blah ~/.nix-profile
testCollectGarbageDryRun
# Run the same test, but forcing the profiles at their legacy location under
# /nix/var/nix.
#
# Note that we *don't* use the default profile; `nix-collect-garbage` will
# need to check the legacy conditional unconditionally not just follow
# `~/.nix-profile` to pass this test.
#
# Regression test for #8294
rm ~/.nix-profile
testCollectGarbageDryRun --profile "$NIX_STATE_DIR/profiles/per-user/me"