From b0b3f3e339ff834fd973f2f0f0bc7ad9be6ffd04 Mon Sep 17 00:00:00 2001 From: Maxime Devos Date: Fri, 26 Feb 2021 15:30:04 +0100 Subject: [PATCH 4/4] =?UTF-8?q?substitute:=20Unstub=20=E2=80=98verify-hash?= =?UTF-8?q?/unknown=E2=80=99.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This procedure is called if a substitution method returns 'unpacked'. While no method does that yet, it is expected the IPFS and GNUnet substituter will. * guix/scripts/substitute.scm (verify-hash/unknown): Unstub procedure. (process-substitution): When the substituter returns ‘unpacked’, verify whether we got the right substitute and canonicalize permissions and timestamps. * doc/substituters.texi (Defining Substituters): Document the absence of a requirement for substituters to normalize timestamps and file permissions. * tests/substitute.scm (write-string-as-nar): Define procedure, and use in test cases. Also test that the hash is verified when a substituter returns 'unpacked'. --- doc/substituters.texi | 4 +++ guix/scripts/substitute.scm | 43 +++++++++++++++++++---- tests/substitute.scm | 70 ++++++++++++++++++++++++++++++++----- 3 files changed, 102 insertions(+), 15 deletions(-) diff --git a/doc/substituters.texi b/doc/substituters.texi index 516e2c1eea..65b1a929b0 100644 --- a/doc/substituters.texi +++ b/doc/substituters.texi @@ -47,4 +47,8 @@ are correctly signed and have a correct hash; this is handled by @code{(guix scripts substitute)}. @var{nar-downloader} and @var{fetch-narinfos} can be @code{#f} if unimplemented by this substituter. + +Likewise, when returning @code{unpacked}, @var{nar-downloader} +does not need to normalize timestamps and file permissions. + @end deffn diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm index 78345dce8f..90fd7dd021 100755 --- a/guix/scripts/substitute.scm +++ b/guix/scripts/substitute.scm @@ -31,8 +31,10 @@ #:use-module (guix records) #:use-module (guix diagnostics) #:use-module (guix i18n) - #:use-module ((guix serialization) #:select (restore-file dump-file)) + #:use-module ((guix serialization) + #:select (restore-file write-file dump-file)) #:autoload (guix store deduplication) (dump-file/deduplicate) + #:autoload (guix store database) (reset-timestamps) #:autoload (guix scripts discover) (read-substitute-urls) #:autoload (guix scripts substitute http) (open-connection-for-uri/cached) #:use-module (gcrypt hash) @@ -49,6 +51,7 @@ #:use-module ((guix build syscalls) #:select (set-thread-name)) #:use-module (ice-9 rdelim) + #:use-module (ice-9 receive) #:use-module (ice-9 regex) #:use-module (ice-9 match) #:use-module (ice-9 format) @@ -531,11 +534,20 @@ to the narinfo." (bytevector->nix-base32-string expected) (bytevector->nix-base32-string actual)))) -(define (verify-hash/unknown . rest) - ;; Variant of verify-hash where the hash hasn't yet been computed. - ;; TODO: this will be implemented later in the patch series! - ;; (To be used by the IPFS and GNUnet substituter) - (leave (G_ "TODO verify-hash/unknown is unimplemented~%"))) +(define* (verify-hash/unknown file expected algorithm narinfo + #:key thunk) + "Check whether we got the data announced in the narinfo NARINFO. +FILE is the actual file we got and EXPECTED is the hash according +to the narinfo. Call THUNK after FILE was read, but before +the daemon is informed." + ;; Recreate the nar, hash it, and let verify-hash + ;; produce the 'success' or 'hash-mismatch' output. + (receive (hashed get-hash) + (open-hash-port algorithm) + (write-file file hashed) + (close hashed) + (thunk) + (verify-hash (get-hash) expected algorithm narinfo))) (define-syntax-rule (receive* kwargs exp exp* exp** ...) (call-with-values (lambda () exp) @@ -606,7 +618,24 @@ the current output port." (when after-input-close (after-input-close)) ;; Check whether we got the data announced in the NARINFO. - (verify-hash/unknown destination narinfo)) + (receive (algorithm expected) + (narinfo-hash-algorithm+value narinfo) + (verify-hash/unknown + destination expected algorithm narinfo + ;; Make sure the permissions and timestamps are canonical. + ;; + ;; This could theoretically be done somewhat more + ;; cache-friendly if done in the substitution method, + ;; by canonicalising a file right after it has been + ;; downloaded, but let's try for correctness first + ;; before efficiency. + ;; + ;; Also, this must be done *after* verifying the hash, + ;; in order to make the access time is set correctly. + ;; + ;; TODO it would be nice to deduplicate DESTINATION. + #:thunk + (lambda () (reset-timestamps destination))))) (else (format (current-error-port) "~s~%" input) (leave diff --git a/tests/substitute.scm b/tests/substitute.scm index 6c754f774d..1cb1a10402 100644 --- a/tests/substitute.scm +++ b/tests/substitute.scm @@ -324,6 +324,16 @@ System: mips64el-linux\n") (lambda () (guix-substitute "--substitute"))))) +(define (write-string-as-nar port content) + (write-file-tree "foo" port + #:file-type+size + (lambda _ + (values 'regular + (string-length content))) + #:file-port + (lambda _ + (open-input-string content)))) + (test-equal "substitute, invalid hash" (string-append "hash-mismatch sha256 " (bytevector->nix-base32-string (sha256 #vu8())) " " @@ -331,14 +341,7 @@ System: mips64el-linux\n") (open-hash-port (hash-algorithm sha256))) ((content) "Substitutable data.")) - (write-file-tree "foo" port - #:file-type+size - (lambda _ - (values 'regular - (string-length content))) - #:file-port - (lambda _ - (open-input-string content))) + (write-string-as-nar port content) (close-port port) (bytevector->nix-base32-string (get-hash))) "\n") @@ -367,6 +370,57 @@ System: mips64el-linux\n"))) (lambda () (guix-substitute "--substitute")))))))))) +(test-equal "substitute (unpacked), invalid hash" + (string-append "hash-mismatch sha256 " + (bytevector->nix-base32-string (sha256 #vu8())) " " + (let-values (((port get-hash) + (open-hash-port (hash-algorithm sha256))) + ((content) "Wrong data!")) + (write-string-as-nar port content) + (close-port port) + (bytevector->nix-base32-string (get-hash))) + "\n") + (with-output-to-string + (lambda () + ;; Arrange so the actual data hash does not match the 'NarHash' field in the + ;; narinfo. Use a substituter that does not produce a nar, but rather + ;; writes the item to the store by itself. + (define narinfo + ;; Pretend this hash and size actually correspond to + ;; some nar. + (string-append "StorePath: " (%store-prefix) + "/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-wrong-hash +URL: irrelevant +Compression: none +NarHash: sha256:" (bytevector->nix-base32-string (sha256 #vu8())) " +NarSize: 123 +References: +Deriver: " (%store-prefix) "/foo.drv +System: mips64el-linux\n")) + (parameterize ((substituters + (list + (make-substituter + 'test + (lambda (destination . rest) + (call-with-output-file destination + (cut display "Wrong data!" <>)) + 'unpacked) + (const + (list + (string->narinfo + (string-append narinfo "Signature: " + (signature-field narinfo) + "\n") + "test://"))) + '(test)))) + (substitute-urls '("test://"))) + (call-with-temporary-directory + (lambda (directory) + (request-substitution + (string-append (%store-prefix) + "/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-foo") + (string-append directory "/wrong-hash")))))))) + (test-quit "substitute, unauthorized key" "no valid substitute" (with-narinfo (string-append %narinfo "Signature: " -- 2.30.0