Compare commits

..

3 commits

Author SHA1 Message Date
alois31 c2fd44e9d7
libstore/build: block io_uring
Change-Id: I45d3895f95abe1bc103a63969f444c334dbbf50d
2024-07-08 17:50:42 +02:00
alois31 29329d42bc
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-08 17:50:42 +02:00
alois31 2b48f9aa7c
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-08 17:50:42 +02:00
9 changed files with 33 additions and 119 deletions

View file

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

View file

@ -1,29 +0,0 @@
---
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()
std::string DrvInfo::queryName() const
{
if (name == "" && attrs) {
auto i = attrs->find(state->sName);
@ -56,7 +56,7 @@ std::string DrvInfo::queryName()
}
std::string DrvInfo::querySystem()
std::string DrvInfo::querySystem() const
{
if (system == "" && attrs) {
auto i = attrs->find(state->sSystem);
@ -66,7 +66,7 @@ std::string DrvInfo::querySystem()
}
std::optional<StorePath> DrvInfo::queryDrvPath()
std::optional<StorePath> DrvInfo::queryDrvPath() const
{
if (!drvPath && attrs) {
Bindings::iterator i = attrs->find(state->sDrvPath);
@ -80,7 +80,7 @@ std::optional<StorePath> DrvInfo::queryDrvPath()
}
StorePath DrvInfo::requireDrvPath()
StorePath DrvInfo::requireDrvPath() const
{
if (auto drvPath = queryDrvPath())
return *drvPath;
@ -88,7 +88,7 @@ StorePath DrvInfo::requireDrvPath()
}
StorePath DrvInfo::queryOutPath()
StorePath DrvInfo::queryOutPath() const
{
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()
std::string DrvInfo::queryOutputName() const
{
if (outputName == "" && attrs) {
Bindings::iterator i = attrs->find(state->sOutputName);

View file

@ -19,11 +19,11 @@ public:
private:
EvalState * state;
std::string name;
std::string system;
std::optional<std::optional<StorePath>> drvPath;
std::optional<StorePath> outPath;
std::string outputName;
mutable std::string name;
mutable std::string system;
mutable std::optional<std::optional<StorePath>> drvPath;
mutable std::optional<StorePath> outPath;
mutable 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();
std::string querySystem();
std::optional<StorePath> queryDrvPath();
StorePath requireDrvPath();
StorePath queryOutPath();
std::string queryOutputName();
std::string queryName() const;
std::string querySystem() const;
std::optional<StorePath> queryDrvPath() const;
StorePath requireDrvPath() const;
StorePath queryOutPath() const;
std::string queryOutputName() const;
/**
* 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) + ANSI_NORMAL "\n");
writeToStderr(filterANSIEscapes(line, false, width) + "\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) + ANSI_NORMAL "\n");
writeToStderr(filterANSIEscapes(activity_line, false, width) + "\n");
state.lastLines++;
} else moreActivities++;
}

View file

@ -89,21 +89,12 @@ static int main_nix_collect_garbage(int argc, char * * argv)
// Run the actual garbage collector.
if (!dryRun) {
auto store = openStore();
auto & gcStore = require<GcStore>(*store);
options.action = GCOptions::gcDeleteDead;
} else {
options.action = GCOptions::gcReturnDead;
}
auto store = openStore();
auto & gcStore = require<GcStore>(*store);
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);
}
GCResults results;
PrintFreed freed(true, results);
gcStore.collectGarbage(options, results);
}
return 0;

View file

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

View file

@ -73,7 +73,6 @@ 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

@ -1,40 +0,0 @@
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"