This is a followup to 8f4ffb3fae.
Commit 8f4ffb3fae fell short in two
ways: (1) it didn’t have any effet for fixed-output derivations
performed in a chroot, which is the case for all of them except those
using “builtin:download” and “builtin:git-download”, and (2) it did not
preserve ownership when copying, leading to “suspicious ownership or
permission […] rejecting this build output” errors.
* nix/libstore/build.cc (DerivationGoal::buildDone): Account for
‘chrootRootDir’ when copying ‘drv.outputs’.
* nix/libutil/util.cc (copyFileRecursively): Add ‘fchown’ and ‘fchownat’
calls to preserve file ownership; this is necessary for chrooted
fixed-output derivation builds.
* nix/libutil/util.hh: Update comment.
Change-Id: Ib59f040e98fed59d1af81d724b874b592cbef156
This fixes a security issue (CVE-2024-27297) whereby a fixed-output
derivation build process could open a writable file descriptor to its
output, send it to some outside process for instance over an abstract
AF_UNIX socket, which would then allow said process to modify the file
in the store after it has been marked as “valid”.
Vulnerability discovered by puck <https://github.com/puckipedia>.
Nix security advisory:
https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37
Nix fix:
244f3eee0b
* nix/libutil/util.cc (readDirectory): Add variants that take a DIR* and
a file descriptor. Rewrite the ‘Path’ variant accordingly.
(copyFile, copyFileRecursively): New functions.
* nix/libutil/util.hh (copyFileRecursively): New declaration.
* nix/libstore/build.cc (DerivationGoal::buildDone): When ‘fixedOutput’
is true, call ‘copyFileRecursively’ followed by ‘rename’ on each output.
Change-Id: I7952d41093eed26e123e38c14a4c1424be1ce1c4
Reported-by: Picnoir <picnoir@alternativebit.fr>, Théophane Hufschmitt <theophane.hufschmitt@tweag.io>
Change-Id: Idb5f2757f35af86b032a9851cecb19b70227bd88
* nix/libstore/build.cc (SubstitutionGoal::finished): Don’t show what
the empty status looks like.
Change-Id: Ie898432aeb047aff3d59024de6ed6d18f68903c4
Drop the confusing ‘invalid’ jargon and display a hint like we do
for ‘--fallback’.
* nix/libstore/build.cc (DerivationGoal::outputsSubstituted): Rewrite error message.
DerivationGoal::startBuilder() is waiting for an empty line as a check that
the environment set up is fine.
Fixes <https://issues.guix.gnu.org/55324>.
* nix/libstore/build.cc (DerivationGoal::runChild): Remove 'debug'
statement corresponding to bind mounts.
Signed-off-by: Ludovic Courtès <ludo@gnu.org>
Fixes <https://issues.guix.gnu.org/46212>.
Reported by Christopher Baines <mail@cbaines.net>.
Previously, the nar size returned by 'guix substitute' would be read as
an 'int'; thus, values above 2^31 - 1 would be read and then stored as
negative integers in the database.
Regression introduced in 9dfa20a22a.
* nix/libstore/build.cc (SubstitutionGoal::finished): Use templatized
'string2Int' instead of 'std::atoi' to get an 'unsigned long long',
which is the type of 'hash.second'.
* tests/store.scm ("substitute and large size"): New test.
This avoids the situation where error messages would unintentionally go
to stderr and be wrongfully interpreted as a reply by the daemon.
Fixes <https://bugs.gnu.org/46362>.
This is a followup to ee3226e9d5.
* guix/scripts/substitute.scm (display-narinfo-data): Add 'port'
parameter and honor it.
(process-query): Likewise.
(process-substitution): Likewise.
(%error-to-file-descriptor-4?, with-redirected-error-port): Remove.
(%reply-file-descriptor): New variable.
(guix-substitute): Remove use of 'with-redirected-error-port'. Define
'reply-port' and pass it to 'process-query' and 'process-substitution'.
* nix/libstore/build.cc (SubstitutionGoal::handleChildOutput): Swap
'builderOut' and 'fromAgent'.
* nix/libstore/local-store.cc (LocalStore::getLineFromSubstituter):
Likewise.
* tests/substitute.scm <top level>: Set '%reply-file-descriptor'
rather than '%error-to-file-descriptor-4?'.
Fixes <https://bugs.gnu.org/47229>.
Reported by Nathan Nye of WhiteBeam Security.
* nix/libstore/build.cc (DerivationGoal::startBuilder): When 'useChroot'
is true, add "/top" to 'tmpDir'.
(DerivationGoal::deleteTmpDir): Adjust accordingly. When
'settings.keepFailed' is true, chown in two steps: first the "/top"
sub-directory, and then rename "/top" to its parent.
This removes the main source of latency between subsequent downloads.
* nix/libstore/build.cc (SubstitutionGoal::tryToRun): Add a
"deduplicate" key to ENV.
(SubstitutionGoal::finished): Remove call to 'optimisePath'.
* guix/scripts/substitute.scm (process-substitution)[destination-in-store?]
[dump-file/deduplicate*]: New variables.
Pass #:dump-file to 'restore-file'.
* guix/scripts/substitute.scm (guix-substitute)[deduplicate?]: New
variable.
Pass #:deduplicate? to 'process-substitution'.
* guix/serialization.scm (dump-file): Export and augment 'dump-file'.
'guix substitute' now takes care of it via 'restore-file'.
* nix/libstore/build.cc (SubstitutionGoal::finished): Remove call to
'canonicalisePathMetaData'.
This way, the hash of the store item can be computed as it is restored,
thereby avoiding an additional file tree traversal ('hashPath' call)
later on in the daemon. Consequently, it should reduce latency between
subsequent substitute downloads.
This is a followup to 5ff521452b.
* guix/scripts/substitute.scm (narinfo-hash-algorithm+value): New
procedure.
(process-substitution): Wrap INPUT into a hash input port, 'hashed', and
read from it. Compare the actual and expected hashes, and print a
"hash-mismatch" status line when they differ. When they match, print
not just "success" but also the nar hash and size.
* nix/libstore/build.cc (class SubstitutionGoal)[expectedHashStr]:
Remove.
(SubstitutionGoal::finished): Tokenize 'status'. Parse it and handle
"success" and "hash-mismatch" accordingly. Call 'hashPath' only when
the returned hash is not SHA256.
(SubstitutionGoal::handleChildOutput): Remove 'expectedHashStr'
handling.
* tests/substitute.scm ("substitute, invalid hash"): Rename to...
("substitute, invalid narinfo hash"): ... this.
("substitute, invalid hash"): New test.
It was already impossible in practice for 'expectedHashStr' to be empty
if 'status' == "success".
* nix/libstore/build.cc (SubstitutionGoal::finished): Throw 'SubstError'
when 'expectedHashStr' is empty.
That way, when fetching a series of substitutes from the same server(s),
the connection is reused instead of being closed/opened for each
substitutes, which saves on network round trips and TLS handshakes.
* guix/http-client.scm (http-fetch): Add #:keep-alive? and honor it.
* guix/progress.scm (progress-report-port): Add #:close? parameter and
honor it.
* guix/scripts/substitute.scm (at-most): Return the tail as a second
value.
(fetch): Add #:port and #:keep-alive? and honor them.
(%max-cached-connections): New variable.
(open-connection-for-uri/cached, call-with-cached-connection): New
procedures.
(with-cached-connection): New macro.
(process-substitution): Wrap 'fetch' call in 'with-cached-connection'.
Pass #:close? to 'progress-report-port'.
This avoids spawning one substitute process per substitution.
* nix/libstore/build.cc (class Worker)[substituter]: New field.
[outPipe, logPipe, pid]: Remove.
(class SubstitutionGoal)[expectedHashStr, status, substituter]: New fields.
(SubstitutionGoal::timedOut): Adjust to check 'substituter'.
(SubstitutionGoal::tryToRun): Remove references to 'outPipe' and
'logPipe'. Run "guix substitute --substitute" as an 'Agent'. Send the
request with 'writeLine'.
(SubstitutionGoal::finished): Likewise.
(SubstitutionGoal::handleChildOutput): Change to fill in
'expectedHashStr' and 'status'.
(SubstitutionGoal::handleEOF): Call 'wakeUp' unconditionally.
(SubstitutionGoal::~SubstitutionGoal): Adjust to check 'substituter'.
* guix/scripts/substitute.scm (process-substitution): Write "success\n"
to stdout upon success.
(%error-to-file-descriptor-4?): New variable.
(guix-substitute): Set 'current-error-port' to file descriptor 4
unless (%error-to-file-descriptor-4?) is false.
Remove "--substitute" arguments. Loop reading line from stdin.
* tests/substitute.scm <top level>: Call '%error-to-file-descriptor-4?'.
(request-substitution): New procedure.
("substitute, no signature")
("substitute, invalid hash")
("substitute, unauthorized key")
("substitute, authorized key")
("substitute, unauthorized narinfo comes first")
("substitute, unsigned narinfo comes first")
("substitute, first narinfo is unsigned and has wrong hash")
("substitute, first narinfo is unsigned and has wrong refs")
("substitute, two invalid narinfos")
("substitute, narinfo with several URLs"): Adjust to new "guix
substitute --substitute" calling convention.
* nix/libstore/local-store.hh (RunningSubstituter): Remove.
(LocalStore)[runningSubstituter]: Change to unique_ptr<Agent>.
[setSubstituterEnv, didSetSubstituterEnv]: Remove.
[getLineFromSubstituter, getIntLineFromSubstituter]: Take an 'Agent'.
* nix/libstore/local-store.cc (LocalStore::~LocalStore): Remove
reference to 'runningSubstituter'.
(LocalStore::setSubstituterEnv, LocalStore::startSubstituter): Remove.
(LocalStore::getLineFromSubstituter): Adjust to 'run' being an 'Agent'.
(LocalStore::querySubstitutablePaths): Spawn substituter agent if
needed. Adjust to 'Agent' interface.
(LocalStore::querySubstitutablePathInfos): Likewise.
* nix/libstore/build.cc (SubstitutionGoal::tryToRun): Remove call to
'setSubstituterEnv' and add 'setenv' call for "_NIX_OPTIONS" instead.
(SubstitutionGoal::finished): Remove 'readLine' call for 'dummy'.
* guix/scripts/substitute.scm (%allow-unauthenticated-substitutes?):
Remove second argument to 'make-parameter'.
(process-query): Call 'warn-about-missing-authentication'
when (%allow-unauthenticated-substitutes?) is #t.
(guix-substitute): Wrap body in 'parameterize'. Set 'guix-warning-port'
too. No longer exit when 'substitute-urls' returns the empty list. No
longer print newline initially.
* tests/substitute.scm (test-quit): Parameterize 'current-error-port' to
account for the port changes in 'guix-substitute'.
Fixes <https://issues.guix.gnu.org/40144>.
Until now, the '--rounds' option, when also using '--check', was ignored.
This change makes it possible to use both, so that an item that has already
been built once can be rebuilt as many times as desired.
* nix/libstore/build.cc: Remove the conditionals causing the daemon to
complete a build task early when 'buildMode' is equal to 'nix::bmCheck'.
Reported-by: Brice Waegeneire <brice@waegenei.re>
Fixes <https://bugs.gnu.org/43668>.
Previously, guix-daemon would try to run GNU/Hurd executables on
GNU/Linux. execve(2) would succeed, but the executable would
immediately crash.
This change prevents it from attempting to execute "i586-gnu" code on
"*-linux", while preserving the binfmt_misc-friendly behavior
implemented in commit 7bf2a70a4f.
* nix/libstore/build.cc (sameOperatingSystemKernel): New function.
(DerivationGoal::runChild): Call 'execve' only when
'sameOperatingSystemKernel' returns true.
* nix/libstore/build.cc (DerivationGoal::tryBuildHook): Add "offload" to
'args' and pass settings.guixProgram as the first argument to
Agent::Agent.
(pathNullDevice, commonChildInit, Agent, Agent::Agent)
(Agent::~Agent): Move to...
* nix/libutil/util.cc: ... here.
* nix/libutil/util.hh (struct Agent, commonChildInit): New
declarations.
* nix/libstore/build.cc (HookInstance): Rename to...
(Agent): ... this. Rename 'toHook' and 'fromHook' similarly and update
users. Change constructor to require a command and an argument list.
(DerivationGoal::tryBuildHook): Pass arguments to the 'Agent'
constructor.
Checking for CLONE_NEWNS is only needed for using tha Linux specific clone(2),
otherwise we can use fork(2). Using clone on the Hurd needs some work, only
support LINUX for now. See
https://lists.gnu.org/archive/html/guix-devel/2020-03/msg00190.html
* nix/libstore/build.cc (CHROOT_ENABLED): Break into CHROOT_ENABLED
and CLONE_ENABLED.
(DerivationGoal::startBuilder): Replace CHROOT_ENABLED with __linux__.
(DerivationGoal::runChild): Only define pivot_root() if SYS_pivot_root is
defined.
Co-authored-by: Jan Nieuwenhuizen <janneke@gnu.org>
As of GNU libc 2.29, <sys/mount.h> declares all the constants and
functions we need, so there's no use in including <linux/fs.h> anymore.
This silences annoying warnings like this one:
In file included from nix/libstore/local-store.cc:32:0:
/gnu/store/…-linux-libre-headers-4.19.56/include/linux/fs.h:108:0: warning: "MS_RDONLY" redefined
#define MS_RDONLY 1 /* Mount read-only */
In file included from nix/libstore/local-store.cc:28:0:
/gnu/store/…-glibc-2.29/include/sys/mount.h:36:0: note: this is the location of the previous definition
#define MS_RDONLY MS_RDONLY
* config-daemon.ac: Remove check for <linux/fs.h>.
* nix/libstore/build.cc: Remove conditional inclusion of <linux/fs.h>.
* nix/libstore/local-store.cc: Remove "#if HAVE_LINUX_FS_H" and
inclusion of <linux/fs.h>.
Fixes <https://bugs.gnu.org/38062>.
This is a followup to ada9a19a2d.
* nix/libstore/build.cc (DerivationGoal::killChild): Add conditional
call to 'worker.childTerminated' for 'hook->pid'.
Until now it was up to 'guix offload' to honor timeouts. Unfortunately
it would sometimes fail to do that, for example due to the libssh bug at
<https://bugs.libssh.org/T33>. With this change, 'guix offload' is
automatically killed by the daemon when one of the timeouts expires.
Thus, data transfers performed by 'guix offload' now count as part of
the timeouts, rather than just actual build time.
* nix/libstore/build.cc (DerivationGoal::tryBuildHook): Pass true as the
'respectTimeouts' argument to 'childStarted'.
The daemon had a mechanism that allows it to handle a list of
substituters and try them sequentially; this removes it.
* nix/scripts/substitute.in: Remove.
* nix/local.mk (nodist_pkglibexec_SCRIPTS): Remove.
* config-daemon.ac: Don't output 'nix/scripts/substitute'.
* nix/libstore/build.cc (SubstitutionGoal)[subs, sub, hasSubstitute]:
Remove.
[tryNext]: Make private.
(SubstitutionGoal::SubstitutionGoal, SubstitutionGoal::init): Remove now
unneeded initializers.
(SubstitutionGoal::tryNext): Adjust to assume a single substituter: call
'amDone' upfront when we couldn't find substitutes.
(SubstitutionGoal::tryToRun): Adjust to run 'guix substitute' via
'settings.guixProgram'.
(SubstitutionGoal::finished): Call 'amDone(ecFailed)' upon failure
instead of setting 'state' to 'tryNext'.
* nix/libstore/globals.hh (Settings)[substituters]: Remove.
* nix/libstore/local-store.cc (LocalStore::~LocalStore): Adjust to
handle a single substituter.
(LocalStore::startSubstituter): Remove 'path' parameter. Adjust to
invoke 'settings.guixProgram'. Don't refer to 'run.program', which no
longer exists.
(LocalStore::querySubstitutablePaths): Adjust for 'runningSubstituters'
being a singleton instead of a list.
(LocalStore::querySubstitutablePathInfos): Likewise, and remove
'substituter' parameter.
* nix/libstore/local-store.hh (RunningSubstituter)[program]: Remove.
(LocalStore)[runningSubstituters]: Remove.
[runningSubstituter]: New field.
[querySubstitutablePathInfos]: Remove 'substituter' parameter.
[startSubstituter]: Remove 'substituter' parameter.
* nix/nix-daemon/guix-daemon.cc (main): Remove references to
'settings.substituters'.
* nix/nix-daemon/nix-daemon.cc (performOp): Ignore the user's
"build-use-substitutes" value when 'settings.useSubstitutes' is false.
Until now, something like "guix build sed -v1 --check" would not get a
'build-succeeded' event, which in turn meant that the spinner would not
be erased upon build completion.
* nix/libstore/build.cc (DerivationGoal::registerOutputs): When
'buildMode' is bmCheck and 'settings.printBuildTrace' emit a
"@ build-succeeded" trace upon success.
* tests/store.scm ("build-succeeded trace in check mode"): New test.
This allows clients to tell whether output comes from the daemon or, if
it comes from a builder, from which builder it comes. The latter is
particularly useful when MAX-BUILD-JOBS > 1.
* nix/libstore/build.cc (DerivationGoal::tryBuildHook)
(DerivationGoal::startBuilder): Print the child's PID in "@ build-started"
traces.
(DerivationGoal::handleChildOutput): Define 'prefix', pass it to
'writeToStderr'.
* nix/libstore/globals.cc (Settings:Settings): Initialize
'multiplexedBuildOutput'.
(Settings::update): Likewise.
* nix/libstore/globals.hh (Settings)[multiplexedBuildOutput]: New field.
Update 'printBuildTrace' documentation.
* nix/libstore/worker-protocol.hh (PROTOCOL_VERSION): Bump to 0.163.
* nix/nix-daemon/nix-daemon.cc (performOp) <wopSetOptions>: Special-case
"multiplexed-build-output" and remove "use-ssh-substituter".
* guix/store.scm (set-build-options): Add #:multiplexed-build-output?
and honor it.
(%protocol-version): Bump to #x163.
* tests/store.scm ("multiplexed-build-output"): New test.
fixlet
* nix/libstore/build.cc (canBuildLocally): Allow building armhf-linux
builds on aarch64-linux.
(DerivationGoal::runChild) Throw error if attempting to build for
armhf-linux on an unsupported platform.
* doc/guix.texi (Invoking guix build): Document how to build natively
for armhf-linux on aarch64-linux. Add note that on some aarch64
machines this is unsupported.
* config-daemon.ac: Don't bail out when libbz2 is missing. Define
'HAVE_LIBBZ2' Automake conditional.
* nix/libstore/build.cc: Wrap relevant bits in '#if HAVE_BZLIB_H'.
* nix/libstore/globals.cc (Settings::Settings): 'logCompression'
defaults to COMPRESSION_GZIP when HAVE_BZLIB_H is false.
* nix/libstore/globals.hh (CompressionType): Make 'COMPRESSION_BZIP2'
conditional on HAVE_BZLIB_H.
* nix/local.mk (guix_register_LDADD, guix_daemon_LDADD): Add -lbz2 only
when HAVE_LIBBZ2.
* nix/nix-daemon/guix-daemon.cc (parse_opt): Ignore "bzip2" when not
HAVE_BZLIB_H.
Fixes <http://bugs.gnu.org/25242>.
Reported by Leo Famulari <leo@famulari.name>.
The regression was introduced in 94d92c7796.
* nix/libstore/build.cc (DerivationGoal::startBuilder): Set 'useChroot'
as a function 'of isBuiltin(drv)'.
(DerivationGoal::runChild): Use 'useChroot' instead of
'useChroot && !isBuiltin(drv)'.
Fixes <http://bugs.gnu.org/15890>.
* nix/libstore/globals.hh (Settings) Add clientUid and clientGid.
* nix/nix-daemon/nix-daemon.cc (daemonLoop] Store UID and GID of the
caller in settings.
* nix/libstore/build.cc (_chown): New function.
(DerivationGoal::deleteTmpDir): Use it, change ownership of build
directory if it is kept and the new owner is not root.
Ludovic Courtès
Tobias Geerinckx-Rice
yarl-baudig@mailoo.org
Maxim Cournoyer
Manolis Ragkousis
Efraim Flashner
Jörg Thalheim
Mark H Weaver
Hartmut Goebel