aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-07-25libstore/build: use an allowlist approach to syscall filteringAlois Wohlschlager
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. To the contrary, it has been measured that the the actual filter constructed here has approximately the same overhead as a very simple filter blocking only one system call. This commit tries to keep the behavior as close to unchanged as possible. The system call list is in line with libseccomp 2.5.5 and glibc 2.39, which are the latest versions at the point of writing. Since libseccomp 2.5.5 is already a requirement and the distributions shipping this together with older versions of glibc are mostly not a thing any more, this should not lead to more build failures any more. [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-25libstore/build: always treat seccomp setup failures as fatalAlois Wohlschlager
In f047e4357b4f7ad66c2e476506bf35cab82e441e, 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 (ff6becafa8efc2f7e6f2b9b889ba4adf20b8d524). 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-24fix building with Musl, fixing static buildsQyriad
Musl stdout macro expands¹ to something that isn't a valid identifier, so we get syntax errors when compiling usage of a method called stdout with Musl's stdio.h. [1]: https://git.musl-libc.org/cgit/musl/tree/include/stdio.h?id=ab31e9d6a0fa7c5c408856c89df2dfb12c344039#n67 Change-Id: I10e6f6a49504399bf8edd59c5d9e4e62449469e8
2024-07-23libstore: Add FreeBSD findPlatformRootsArtemis Tosini
Use libprocstat to find garbage collector roots on FreeBSD. Tested working on a FreeBSD machine, although there is no CI yet Change-Id: Id36bac8c3de6cc4de94e2d76e9663dd4b76068a9
2024-07-22Merge "libexpr/gc-alloc: fix compilation with !HAVE_BOEHMGC" into mainPierre Bourdon
2024-07-23libexpr/gc-alloc: fix compilation with !HAVE_BOEHMGCPierre Bourdon
Fixes: 72ee25b4025257fdaab7b8e8d5d1ccc83858fdab Change-Id: Ib59386af1415a8ed4b53af24ec22a4ffa5e5877d
2024-07-22libstore: keep Goal errors as unique_ptrseldritch horrors
Error is pretty large, and most goals do not fail. this alone more than halves the size of Goal on x86_64-linux, from 720 bytes down to 344. in derived classes the difference is not as dramatic, but even the largest derived class (`LocalDerivationGoal`) loses almost 20% of its footprint Change-Id: Ifda8f94c81b6566eeb3e52d55d9796ec40c7bce8
2024-07-22libstore: remove an always-defaulted argumenteldritch horrors
Change-Id: I3c7f17d5492a16bb54480fa1aa384b96fba72d61
2024-07-22libstore: use std::async instead of Goal threadseldritch horrors
the goals are either already using std::async and merely forgot to remove std::thread vestiges or they emulate async with threads and promises. we can simply use async directly everywhere for clarity. Change-Id: I3f05098310a25984f10fff1e68c573329002b500
2024-07-22libstore: remove addToWeakGoalseldritch horrors
under owner_less it's equivalent to insert(), only sometimes a little bit faster because it does not construct a weak_ptr if the goal is in the set already. this small difference in performance does not matter here and c++23 will make insert transparent anyway, so we can drop it Change-Id: I7cbd7d6e0daa95d67145ec58183162f6c4743b15
2024-07-22libstore: remove Goal::ecBusyeldritch horrors
this should be an optional. "busy" is not an *exit* code! Change-Id: Ic231cb27b022312b1a7a7b9602f32845b7a9c934
2024-07-22libstore: remove unused Worker::waitForAnyGoaleldritch horrors
Change-Id: Ia3ebd434b17052b6760ce74d8e20025a72148613
2024-07-22De-vendor nixfmtvigress8
Change-Id: I1a051be495318a507d07f6d0a6b157616e26774c
2024-07-22enable -Werror=suggest-overrideeldritch horrors
*accidentally* overriding a function is almost guaranteed to be an error. overriding a function without labeling it as such is merely bad style, but bad style that makes the code harder to understand. Change-Id: Ic0594f3d1604ab6b3c1a75cb5facc246effe45f0
2024-07-22libexpr: fix -Wunused-const-variable warningeldritch horrors
Change-Id: Ib986ece0ab2eff83e7abd7f1f915cd8f761827ad
2024-07-21libutil/logging: fix build without precompiled headerAlois Wohlschlager
Commit 0109368c3faf5516aeddde45e8dc3c33e7163838 missed to include a required header, which is not noticed when the precompiled header is enabled because it's included in that. Also include it in the file so that the build without precompiled header works too. Change-Id: Id7a7979684b64f937f7f8191612952d73c113015
2024-07-21Merge "libstore/binary-cache-store: use correct buffer size for NAR ↵alois31
decompression" into main
2024-07-21Merge "gc: refactor the gc server thread out into a class without changing ↵jade
it" into main
2024-07-21libstore/binary-cache-store: use correct buffer size for NAR decompressionAlois Wohlschlager
Due to a leftover from a previous version where the buffer was allocated on the stack, the change introduced in commit 4ec87742a196d8ed8f41b41ef039706ce791448d accidentally passes the size of a pointer as the size of the buffer to the decompressor. Since the former is much smaller (usually 8 bytes instead of 64 kilobytes), this is safe, but leads to considerable overhead; most notably, due to excessive progress reports, which happen for each chunk. Pass the proper buffer size instead. Change-Id: If4bf472d33e21587acb5235a2d99e3cb10914633
2024-07-20Merge "Fix namespace warning being emitted if sandbox is disabled" into mainWinter Cute
2024-07-20libexpr: add a strongly typed version of gcAllocBytes()Qyriad
This commit adds a new helper template function to gc-alloc.hh (which is probably where you want to look at first, O great reviewer [custom file ordering in review diffs when]), which uses a type argument to determine the size to allocate, rather than making the caller use sizeof(). Change-Id: Ib5d138d91a28bdda304a80db24ea9fb08669ad22
2024-07-20libexpr: rename confusing makeImmutableString -> gcCopyStringIfNeededQyriad
The purpose of this function has little to do with immutability. Value's strings are never mutated, and the point of this function is to singleton empty strings. Change-Id: Ifd41dd952409d54e4d3de9ab59064e6928b0e480
2024-07-20libexpr: refactor gc-agnostic helpers into one placeQyriad
Change-Id: Icc4b367e4f670d47256f62a3a002cd248a5c2d3b
2024-07-20libutil: make basic loggers thread-safeeldritch horrors
SimpleLogger is not fully thread-safe, and all loggers that wrap it are also not safe accordingly. this does not affect much, but in rare cases it can cause interleaving of messages on stderr when used with the json or raw log formats. the fix applied here is a bit of a hack, but fixing this properly requires rearchitecting the logger infrastructure. nested loggers are not the most natural abstraction here, and it is biting us. Change-Id: Ifbf34fe1e85c60e73b59faee50e7411c7b5e7c12
2024-07-20libutil: remove warnOnce macroeldritch horrors
it's only used once, and even that one use is highly questionable. more instances of warnOnce should be much more principled than this has been Change-Id: I5856570c99cb44462e700d753d0c706a5db03c4b
2024-07-19Fix namespace warning being emitted if sandbox is disabledWinter
If useChroot = false, and user namespaces aren't available for some reason (e.g. within a Docker container), this fixes a pointless warning being emitted, as we would never attempt to use them even if they were available. Change-Id: Ibcee91c088edd2cd19e70218d5a5802bff8f537b
2024-07-19gc: refactor the gc server thread out into a class without changing itJade Lovelace
This removes a *whole load* of variables from scope and enforces thread boundaries with the type system. There is not much change of significance in here, so the things to watch out for while reviewing it are primarily that the destructor ordering may have changed inadvertently, I think. Change-Id: I3cd87e6d5a08dfcf368637407251db22a8906316
2024-07-19Merge changes Id8b3d289,Ib75ab5b8,I3792eeb3 into mainjade
* changes: Fixup a bunch of references to nixos.org manuals Add release notes for removing overflow from Nix language expr: fix a compiler warning about different signs in comparison
2024-07-19Merge changes I829581a3,I0016970d,I5dac8e77,Ib7560fe5 into mainalois31
* changes: doc/release-notes: add for pretty printing improvements libexpr/print: do not show elided nested items when there are none libexpr/print: never show empty attrsets or derivations as «repeated» libexpr/print: pretty-print idempotently
2024-07-18Fixup a bunch of references to nixos.org manualsJade Lovelace
(plus one reference to CppNix github) Change-Id: Id8b3d2897f3b54e286861805cfd421adc4d5de47
2024-07-18Add release notes for removing overflow from Nix languageJade Lovelace
Change-Id: Ib75ab5b8b4d879035d7ee7678f9cd0c491a39c0a
2024-07-18expr: fix a compiler warning about different signs in comparisonJade Lovelace
We know that variable is >=0, so we can just cast it to unsigned. Change-Id: I3792eeb3ca43e6a507cc44c1a70584d42b2acd7b
2024-07-18Merge changes Ib20e9aa0,I178a038b,I29c7de04 into mainjade
* changes: docs: document the actual comparison rules instead of lies daemon: remove workaround for macOS kernel bug that seems fixed daemon: fix a crash bug "FATAL: exception not rethrown"
2024-07-18doc/release-notes: add for pretty printing improvementsAlois Wohlschlager
Change-Id: I829581a3f5b8b742e6c866dcdbbc635f91afceb5
2024-07-18libexpr/print: do not show elided nested items when there are noneAlois Wohlschlager
When the configured maximum depth has been reached, attribute sets and lists are printed with ellipsis to indicate the elision of nested items. Previously, this happened even in case the structure being printed is empty, so that such items do not in fact exist. This is confusing, so stop doing it. Change-Id: I0016970dad3e42625e085dc896e6f476b21226c9
2024-07-18libexpr/print: never show empty attrsets or derivations as «repeated»Alois Wohlschlager
The repeated value detection logic exists so that the occurrence of large common substructures does not fill up the screen or the computer's memory. However, empty attribute sets and derivations (when their detection is enabled) are always cheap to print, and in practice I have observed them to make up a significant majority of the cases where I was annoyed by the repeated value detection kicking in. Furthermore, `nix-instantiate --eval` already disables this logic for empty attribute sets, and empty lists are already exempted everywhere. For these reasons, always print empty attribute sets and derivations as what they are. Change-Id: I5dac8e7739f9d726b76fd0521ec46f38af94463f
2024-07-18libexpr/print: pretty-print idempotentlyAlois Wohlschlager
When pretty-printing is enabled, previously an unforced thunk would trigger indentation, even when it subsequently does not evaluate to a nested structure. The resulting output looked inconsistent, and furthermore pretty-printing was not idempotent (since pretty-printing the same value again, which is now fully evaluated, will not trigger indentation). When strict evaluation is enabled, force the item before inspecting its type, so that it is properly known whether it contains a nested structure. Furthermore, there is no need to cause indentation for unforced thunks, since the very next operation will be printing them as `«thunk»`. This is mostly a port of https://github.com/NixOS/nix/pull/11100 , but we only force the item when it's going to be forced anyway due to strict pretty-printing, and a new test was written since the REPL testing framework in Lix is different. Co-Authored-By: Robert Hensing <robert@roberthensing.nl> Change-Id: Ib7560fe531d09e05ca6b2037a523fe21a26d9d58
2024-07-18Merge "doc/manual: clarify documentation related to the `$$` parser bug" ↵alois31
into main
2024-07-17doc/release-notes: link the upcoming release notes againAlois Wohlschlager
The insertion marker comment broke the list into two parts, the first containing only the link to the upcoming release notes and the second the past releases. This confused the generator, leading to the first part being discarded. Indent the marker comment so that it's syntactically part of the preceding item, and in particular doesn't split the list any more. Change-Id: I357c51bb03e4e0d79a76d30158615fd9eda95ea8
2024-07-17doc/release-notes: add date for major releaseRaito Bezarius
Change-Id: I93aab93c069bb3989c3f8d17e0862899e6f76865 Signed-off-by: Raito Bezarius <raito@lix.systems>
2024-07-17doc/manual: clarify documentation related to the `$$` parser bugAlois Wohlschlager
Due to a mistake in the grammar, a dollar character implicitly escapes a second dollar character that immediately follows, so that it cannot start an interpolation. Unfortunately, this behaviour has since come to be relied upon, so it cannot be fixed. Furthermore, the documentation on regular strings did not mention this behaviour at all, while in the case of indented strings it was rather implicit. Mention it explicitly in both cases, and describe how an interpolation can follow a dollar character (namely, by escaping that). Since we have to touch that section anyway, state that any character (other than n, r, and t; but notably including `$` even if not succeeded by `{`) can be escaped using a backslash in regular strings. Change-Id: I7e5d68a9a4130eec98ce8218b485168f4b31a677
2024-07-17Merge "tests/functional/repl.sh: actually fail test on wrong stdout" into mainMax Siling
2024-07-17tests/functional/repl.sh: actually fail test on wrong stdoutGoldstein
Previous test implementation assumed that grep supports newlines in patterns. It doesn't, so tests spuriously passed, even though some tests outputs were broken. This patches output (and expected output) before grepping, so there're no newlines in pattern. Change-Id: Ie6561f9f2e18b83d976f162269d20136e2595141
2024-07-16remove boost coroutine referenceseldritch horrors
we no longer need these since sinkToSource and sourceToSink are gone. Change-Id: Ibbf440e2cf71bf3e9f3b833af2d78a21fb1b3193
2024-07-16remove sourceToSink, sinkToSource, and boehm patcheldritch horrors
Change-Id: I1379841299713175d0225b82a67f50660f9eb5e2
2024-07-16libstore: remove remaining sinkToSource useseldritch horrors
Change-Id: Id1ee0d2ad4a3774f4bbb960d76f0f76ac4f3eff9
2024-07-16libstore: remove WriteConn::sink fieldseldritch horrors
we no longer need these since we're no longer using sinks to serialize things. Change-Id: Iffb1a3eab33c83f611c88fa4e8beaa8d5ffa079b
2024-07-16libstore: generatorize protocol serializerseldritch horrors
this is cursed. deeply and profoundly cursed. under NO CIRCUMSTANCES must protocol serializer helpers be applied to temporaries! doing so will inevitably cause dangling references and cause the entire thing to crash. we need to do this even so to get rid of boost coroutines, and likewise to encapsulate the serializers we suffer today at least a little bit to allow a gradual migration to an actual IPC protocol. (this isn't a problem that's unique to generators. c++ coroutines in general cannot safely take references to arbitrary temporaries since c++ does not have a lifetime system that can make this safe. -sigh-) Change-Id: I2921ba451e04d86798752d140885d3c5cc08e146
2024-07-16libstore: remove a sinkToSouce from old daemon protocoleldritch horrors
this doesn't have a test because this code path is only reached by clients that predate 2.4, and we really should not be caring about those any more right now. even the test suite doesn't, and the few tests that might care are disabled because they will not even work Change-Id: Id9eb190065138fedb2c7d90c328ff9eb9d97385b
2024-07-16libstore: rewrite the nar parser as a contents generatoreldritch horrors
this is not completely necessary at this point because the parser right now already returns a generator to pass through all input data it read, but the nar parser *was* very lax and would accept nars that weren't in canonical form (defined as the form dumpPath would return). nar hashing depends on these things, and as such rewriting the parser now allows us to reject non-canonical nars that extract to the same store contents as their canonical counterpart but have different nar hashes despite that. Change-Id: Iccd319e3bd5912d8297014c84c495edc59019bb7