From 64d4de2a920445e5992f020e56490f5fcbdbba7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= Date: Sat, 30 May 2026 23:54:05 +0200 Subject: daemon: Bypass authentication when importing content-addressed store items. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This puts ‘importPaths’ on par with ‘addToStore’ and ‘addTextToStore’: since the two latter RPCs let anyone add content-addressed items in the store, there’s no reason for ‘importPaths’ to require signatures by authorized keys on these content-addressed items. This will allow for things like ‘guix copy’ of .drv items without authorization, or ‘guix deploy’ with (build-locally? #f) without authorization. * nix/libstore/store-api.hh (isContentAddressedPath): New prototype. * nix/libstore/store-api.cc (isContentAddressedPath): New function. * nix/libutil/util.hh (isPlainFile): New prototype. * nix/libutil/util.cc (isPlainFile): New function. * nix/libstore/local-store.cc (LocalStore::importPath): Define ‘narHash’ and ‘contentAddressed’. Allow unsigned imports when ‘contentAddressed’ is true; bypass signature verification when ‘contentAddressed’ is true. * tests/store.scm ("import not signed"): Rewrite to not use a content-addressed store item. ("import signed by unauthorized key"): Likewise. ("import not signed but content-addressed tree"): New test. ("import not signed but content-addressed regular file"): New test. ("import signed by unauthorized key but content-addressed"): New test. ("import with corrupt signature"): New test. ("import signed by authorized key but hash doesn't match"): New test. ("import with corrupt signature but content-addressed"): New test. ("import signed by authorized key, hash doesn't match, but content-addressed"): New test. * doc/guix.texi (Invoking guix archive): Document the exception for content-addressed store items. Add anchor for ‘--authorize’. (Invoking guix deploy): Document the benefit of (build-locally? #f). Add cross-reference for ‘authorize?’. Co-authored-by: Reepca Russelstein Signed-off-by: Ludovic Courtès Merges: #8979 --- doc/guix.texi | 12 +- nix/libstore/local-store.cc | 45 ++++--- nix/libstore/store-api.cc | 23 ++++ nix/libstore/store-api.hh | 6 + nix/libutil/util.cc | 7 ++ nix/libutil/util.hh | 3 + tests/store.scm | 295 +++++++++++++++++++++++++++++++++++++++++--- 7 files changed, 360 insertions(+), 31 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index c5fed97da08..e7f1826542d 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -5674,7 +5674,11 @@ references, corresponding derivation, and a digital signature. When exporting, the daemon digitally signs the contents of the archive, and that digital signature is appended. When importing, the daemon verifies the signature and rejects the import in case of an invalid -signature or if the signing key is not authorized. +signature or if the signing key is not authorized; as an exception, +store items that are @dfn{content-addressed}---source files and +@file{.drv} files (@pxref{Derivations})---can be imported even if they +lack an authorized signature because adding these to the store is not +creating any new risk. @c FIXME: Add xref to daemon doc about signatures. The main options are: @@ -5724,6 +5728,7 @@ Alternatively, @var{parameters} can specify public-key related Functions, @code{gcry_pk_genkey},, gcrypt, The Libgcrypt Reference Manual}). +@anchor{archive-authorization} @item --authorize @cindex authorizing, archives Authorize imports signed by the public key passed on standard input. @@ -51032,12 +51037,15 @@ with an @code{environment} of @code{managed-host-environment-type}. @item @code{host-name} @item @code{build-locally?} (default: @code{#t}) If false, system derivations will be built on the machine being deployed to. +As a bonus, when false, one does not need the target machine to have the +key of the deployment machine among its authorized keys and thus the +@code{authorize?} field below can remain @code{#f}. @item @code{system} The system type describing the architecture of the machine being deployed to---e.g., @code{"x86_64-linux"}. @item @code{authorize?} (default: @code{#t}) If true, the coordinator's signing key will be added to the remote's ACL -keyring. +keyring (@pxref{archive-authorization, authorizing imports}). @item @code{port} (default: @code{22}) @item @code{user} (default: @code{"root"}) @item @code{identity} (default: @code{#f}) diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc index 6b5c388efc9..c38dd2072d4 100644 --- a/nix/libstore/local-store.cc +++ b/nix/libstore/local-store.cc @@ -1336,6 +1336,8 @@ Path LocalStore::importPath(Source & source) restorePath(unpacked, hashAndReadSource); + Hash narHash = hashAndReadSource.hashSink.currentHash().first; + unsigned int magic = readInt(hashAndReadSource); if (magic != EXPORT_MAGIC) throw Error("normalized archive cannot be imported; wrong format"); @@ -1355,21 +1357,36 @@ Path LocalStore::importPath(Source & source) hashAndReadSource.hashing = false; bool haveSignature = readInt(hashAndReadSource) == 1; + string signature; + if (haveSignature) + signature = readString(hashAndReadSource); - if (!haveSignature) - throw Error(std::format("imported archive of `{}' lacks a signature", dstPath)); - - string signature = readString(hashAndReadSource); - string hash2 = verifySignature(signature); - - /* Note: runProgram() throws an exception if the signature - is invalid. */ - - if (printHash(hash) != hash2) - throw Error( - "signed hash doesn't match actual contents of imported " - "archive; archive could be corrupt, or someone is trying " - "to import a Trojan horse"); + try { + /* First, check whether there is a valid, authorized, matching + signature. */ + if (haveSignature) { + string hash2 = verifySignature(signature); + if (printHash(hash) != hash2) + throw AuthenticationError( + "signed hash doesn't match actual contents of imported " + "archive; archive could be corrupt, or someone is trying " + "to import a Trojan horse"); + } + else + throw AuthenticationError(std::format("imported archive of `{}' lacks a signature", dstPath)); + } + catch (AuthenticationError &e) { + /* Second, check whether 'dstPath' is content-addressed--e.g., a store + item created by 'addToStore' or 'addTextToStore'. XXX: This can + yield an extra 'hashFile' call and is limited to SHA256. */ + if (!(deriver == "" + && (isContentAddressedPath(dstPath, narHash, references, true) + || (isPlainFile(unpacked) + && isContentAddressedPath(dstPath, hashFile(htSHA256, unpacked), + references, false))))) + /* Rethrow. */ + throw; + } /* Do the actual import. */ diff --git a/nix/libstore/store-api.cc b/nix/libstore/store-api.cc index feabc821678..43f46176b5e 100644 --- a/nix/libstore/store-api.cc +++ b/nix/libstore/store-api.cc @@ -258,6 +258,29 @@ Path computeStorePathForText(const string & name, const string & s, return makeStorePath(type, hash, name); } +bool isContentAddressedPath(const Path & path, const Hash & hash, + const PathSet & references, bool recursive) +{ + /* Check whether PATH corresponds to something introduced by 'addToStore' + or by 'addTextToStore'. For simplicity, anything with a hash other + than SHA256 is omitted: this returns false even though they are + content-addressed as well. */ + string name = storePathToName(path); + if (recursive) { + /* HASH is interpreted as the nar hash. This can only come from + 'addToStore'. */ + return references.empty() && + path == makeFixedOutputPath(true, htSHA256, hash, name); + } else { + /* HASH is interpreted as the content hash. This can come from + 'addTextToStore' ("text" type, possibly with references) or from + 'addToStore' ("output:out" type). */ + string type = textTypeWithReferences(references); + return path == makeStorePath(type, hash, name) + || (references.empty() && + path == makeFixedOutputPath(false, htSHA256, hash, name)); + } +} /* Return a string accepted by decodeValidPathInfo() that registers the specified paths as valid. Note: it's the diff --git a/nix/libstore/store-api.hh b/nix/libstore/store-api.hh index 29b33c0be9c..398ccd260ea 100644 --- a/nix/libstore/store-api.hh +++ b/nix/libstore/store-api.hh @@ -362,6 +362,12 @@ Path makeFixedOutputPath(bool recursive, Path computeStorePathForText(const string & name, const string & s, const PathSet & references); +/* Return true if PATH, whose content has the given HASH, refers to a + content-addressed file as added by 'addTextToStore' or 'addToStore'. HASH + is interpreted as a SHA256 nar hash when RECURSIVE is true, and as a SHA256 + file content hash when RECURSIVE is false. */ +bool isContentAddressedPath(const Path & path, const Hash & hash, + const PathSet & references, bool recursive); /* Remove the temporary roots file for this process. Any temporary root becomes garbage after this point unless it has been registered diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc index 95f293ff10f..72c8d38bd50 100644 --- a/nix/libutil/util.cc +++ b/nix/libutil/util.cc @@ -287,6 +287,13 @@ unsigned char getFileType(const Path & path) return DT_UNKNOWN; } +bool isPlainFile(const Path & path) +{ + struct stat st = lstat(path); + return S_ISREG(st.st_mode) + && ((st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0); +} + string readFile(int fd) { diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh index 44d579f3d6a..ee8e5461b42 100644 --- a/nix/libutil/util.hh +++ b/nix/libutil/util.hh @@ -65,6 +65,9 @@ Path readLink(const Path & path); bool isLink(const Path & path); +/* Return true if the file at PATH is a regular, non-executable file. */ +bool isPlainFile(const Path & path); + /* Read the contents of a directory. The entries `.' and `..' are removed. */ struct DirEntry diff --git a/tests/store.scm b/tests/store.scm index e7ba7469047..ba634ce2205 100644 --- a/tests/store.scm +++ b/tests/store.scm @@ -30,7 +30,8 @@ #:use-module (guix derivations) #:use-module ((guix modules) #:select (source-module-closure)) - #:use-module (guix serialization) + #:use-module ((guix serialization) + #:hide (store-path read-string)) #:use-module (guix build utils) #:use-module ((gnu build linux-container) #:select (unprivileged-user-namespace-supported?)) @@ -1456,18 +1457,25 @@ System: x86_64-linux~%" (test-assert "import not signed" - (let* ((text (random-text)) - (file (add-file-tree-to-store %store - `("tree" directory - ("text" regular (data ,text)) - ("link" symlink "text")))) + (let* ((content (random-text)) + (name "fake-derivation") + (store-item (output-path "out" + (gcrypt:sha256 #vu8()) + name)) (dump (call-with-bytevector-output-port (lambda (port) (write-int 1 port) ;start - (write-file file port) ;contents + (write-file-tree name port ;contents + #:file-type+size + (lambda (_) + (values 'regular + (string-length content))) + #:file-port + (lambda (_) + (open-input-string content))) (write-int #x4558494e port) ;%export-magic - (write-string file port) ;store item + (write-string store-item port) ;store item (write-string-list '() port) ;references (write-string "" port) ;deriver (write-int 0 port) ;not signed @@ -1485,11 +1493,11 @@ System: x86_64-linux~%" #f)))) (test-assert "import signed by unauthorized key" - (let* ((text (random-text)) - (file (add-file-tree-to-store %store - `("tree" directory - ("text" regular (data ,text)) - ("link" symlink "text")))) + (let* ((content (random-text)) + (name "fake-derivation") + (store-item (output-path "out" + (gcrypt:sha256 #vu8()) + name)) (key (gcrypt:generate-key (gcrypt:string->canonical-sexp "(genkey (ecdsa (curve Ed25519) (flags rfc6979)))"))) @@ -1497,9 +1505,16 @@ System: x86_64-linux~%" (lambda (port) (write-int 1 port) ;start - (write-file file port) ;contents + (write-file-tree name port ;contents + #:file-type+size + (lambda (_) + (values 'regular + (string-length content))) + #:file-port + (lambda (_) + (open-input-string content))) (write-int #x4558494e port) ;%export-magic - (write-string file port) ;store item + (write-string store-item port) ;store item (write-string-list '() port) ;references (write-string "" port) ;deriver (write-int 1 port) ;signed @@ -1524,6 +1539,256 @@ System: x86_64-linux~%" (pk 'unauthorized-imported imported) #f)))) +(test-assert "import with corrupt signature" + (let* ((content (random-text)) + (name "fake-derivation") + (store-item (output-path "out" + (gcrypt:sha256 #vu8()) + name)) + (dump (call-with-bytevector-output-port + (lambda (port) + (write-int 1 port) ;start + + (write-file-tree name port ;contents + #:file-type+size + (lambda (_) + (values 'regular + (string-length content))) + #:file-port + (lambda (_) + (open-input-string content))) + (write-int #x4558494e port) ;%export-magic + (write-string store-item port) ;store item + (write-string-list '() port) ;references + (write-string "" port) ;deriver + (write-int 1 port) ;signed + (write-string (object->string '(signature broken)) + port) + + (write-int 0 port))))) ;done + + ;; Ensure 'import-paths' raises an exception. + (guard (c ((store-protocol-error? c) + (and (not (zero? (store-protocol-error-status c))) + (string-contains (store-protocol-error-message c) + "corrupt signature")))) + (let* ((source (open-bytevector-input-port dump)) + (imported (import-paths %store source))) + (pk 'corrupt-signature-imported imported) + #f)))) + +(test-assert "import signed by authorized key but hash doesn't match" + (let* ((content (random-text)) + (name "fake-derivation") + (store-item (output-path "out" + (gcrypt:sha256 #vu8()) + name)) + ;; This key is known to be in the ACL by default. + (public-key + (call-with-input-file (string-append %config-directory "/signing-key.pub") + (compose gcrypt:string->canonical-sexp get-string-all))) + (private-key + (call-with-input-file (string-append %config-directory "/signing-key.sec") + (compose gcrypt:string->canonical-sexp get-string-all))) + + (dump (call-with-bytevector-output-port + (lambda (port) + (write-int 1 port) ;start + + (write-file-tree name port ;contents + #:file-type+size + (lambda (_) + (values 'regular + (string-length content))) + #:file-port + (lambda (_) + (open-input-string content))) + (write-int #x4558494e port) ;%export-magic + (write-string store-item port) ;store item + (write-string-list '() port) ;references + (write-string "" port) ;deriver + (write-int 1 port) ;signed + (write-string (gcrypt:canonical-sexp->string + (signature-sexp + (gcrypt:bytevector->hash-data + (gcrypt:sha256 #vu8(0 1 2)) + #:key-type 'rsa) + private-key + public-key)) + port) + + (write-int 0 port))))) ;done + + ;; Ensure 'import-paths' raises an exception. + (guard (c ((store-protocol-error? c) + (and (not (zero? (store-protocol-error-status c))) + (string-contains (store-protocol-error-message c) + "hash doesn't match")))) + (let* ((source (open-bytevector-input-port dump)) + (imported (import-paths %store source))) + (pk 'hash-mismatch-imported imported) + #f)))) + +(test-assert "import not signed but content-addressed tree" + (let* ((text (random-text)) + (file (add-file-tree-to-store %store + `("tree" directory + ("text" regular (data ,text)) + ("link" symlink "text")))) + (dump (call-with-bytevector-output-port + (lambda (port) + (write-int 1 port) ;start + + (write-file file port) ;contents + (write-int #x4558494e port) ;%export-magic + (write-string file port) ;store item + (write-string-list '() port) ;references + (write-string "" port) ;deriver + (write-int 0 port) ;not signed + + (write-int 0 port))))) ;done + + ;; Ensure 'import-paths' completes despite the lack of signature. + (let ((source (open-bytevector-input-port dump))) + (match (import-paths %store source) + ((imported) + (string=? (pk 'imported imported) file)))))) + +(test-assert "import not signed but content-addressed regular file" + (let* ((name "content-addressed") + (content (random-text)) + (store-item (store-path "text" + (gcrypt:sha256 (string->utf8 content)) + name)) + (dump (call-with-bytevector-output-port + (lambda (port) + (write-int 1 port) ;start + (write-file-tree name port ;contents + #:file-type+size + (lambda (_) + (values 'regular + (string-length content))) + #:file-port + (lambda (_) + (open-input-string content))) + (write-int #x4558494e port) ;%export-magic + (write-string store-item port) + (write-string-list '() port) ;references + (write-string "" port) ;deriver + (write-int 0 port) ;not signed + (write-int 0 port))))) ;done + + ;; Ensure 'import-paths' completes despite the lack of signature. + (let ((source (open-bytevector-input-port dump))) + (match (import-paths %store source) + ((imported) + (and (string=? (pk 'imported imported) store-item) + (string=? (call-with-input-file imported get-string-all) + content))))))) + +(test-assert "import signed by unauthorized key but content-addressed" + (let* ((text (random-text)) + (file (add-file-tree-to-store %store + `("tree" directory + ("text" regular (data ,text)) + ("link" symlink "text")))) + (key (gcrypt:generate-key + (gcrypt:string->canonical-sexp + "(genkey (ecdsa (curve Ed25519) (flags rfc6979)))"))) + (dump (call-with-bytevector-output-port + (lambda (port) + (write-int 1 port) ;start + + (write-file file port) ;contents + (write-int #x4558494e port) ;%export-magic + (write-string file port) ;store item + (write-string-list '() port) ;references + (write-string "" port) ;deriver + (write-int 1 port) ;signed + (write-string (gcrypt:canonical-sexp->string + (signature-sexp + (gcrypt:bytevector->hash-data + (gcrypt:sha256 #vu8(0 1 2)) + #:key-type 'ecc) + (gcrypt:find-sexp-token key 'private-key) + (gcrypt:find-sexp-token key 'public-key))) + port) + + (write-int 0 port))))) ;done + + ;; Ensure 'import-paths' succeeds despite the unauthorized signature. + (let ((source (open-bytevector-input-port dump))) + (match (import-paths %store source) + ((imported) + (string=? imported file)))))) + +(test-assert "import with corrupt signature but content-addressed" + (let* ((text (random-text)) + (file (add-file-tree-to-store %store + `("tree" directory + ("text" regular (data ,text)) + ("link" symlink "text")))) + (dump (call-with-bytevector-output-port + (lambda (port) + (write-int 1 port) ;start + + (write-file file port) ;contents + (write-int #x4558494e port) ;%export-magic + (write-string file port) ;store item + (write-string-list '() port) ;references + (write-string "" port) ;deriver + (write-int 1 port) ;signed + (write-string (object->string '(signature broken)) + port) + + (write-int 0 port))))) ;done + + ;; Ensure 'import-paths' succeeds despite the corrupt signature. + (let ((source (open-bytevector-input-port dump))) + (match (import-paths %store source) + ((imported) + (string=? imported file)))))) + +(test-assert "import signed by authorized key, hash doesn't match, but content-addressed" + (let* ((text (random-text)) + (file (add-file-tree-to-store %store + `("tree" directory + ("text" regular (data ,text)) + ("link" symlink "text")))) + ;; This key is known to be in the ACL by default. + (public-key + (call-with-input-file (string-append %config-directory "/signing-key.pub") + (compose gcrypt:string->canonical-sexp get-string-all))) + (private-key + (call-with-input-file (string-append %config-directory "/signing-key.sec") + (compose gcrypt:string->canonical-sexp get-string-all))) + (dump (call-with-bytevector-output-port + (lambda (port) + (write-int 1 port) ;start + + (write-file file port) ;contents + (write-int #x4558494e port) ;%export-magic + (write-string file port) ;store item + (write-string-list '() port) ;references + (write-string "" port) ;deriver + (write-int 1 port) ;signed + (write-string (gcrypt:canonical-sexp->string + (signature-sexp + (gcrypt:bytevector->hash-data + (gcrypt:sha256 #vu8(0 1 2)) + #:key-type 'rsa) + private-key + public-key)) + port) + + (write-int 0 port))))) ;done + + ;; Ensure 'import-paths' succeeds despite the hash mismatch. + (let ((source (open-bytevector-input-port dump))) + (match (import-paths %store source) + ((imported) + (string=? imported file)))))) + (test-assert "import corrupt path" (let* ((text (random-text)) (file (add-text-to-store %store "text" text)) -- cgit v1.2.3