[PATCH 0/3] Improve 'import-paths' tests and 'guix authenticate' interface

  • Done
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • zimoun
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
normal
L
L
Ludovic Courtès wrote on 8 Sep 2020 23:58
(address . guix-patches@gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20200908215837.32037-1-ludo@gnu.org
Hi!

These patches are about improving testing around the ‘import-paths’
RPC and cleaning up the interface between ‘guix authenticate’ and
the daemon.

Ludo’.

Ludovic Courtès (3):
store: Test 'import-paths' with unauthorized and unsigned nar bundles.
doc: Distinguish the "nar bundle" format from "nar".
daemon: Simplify interface with 'guix authenticate'.

doc/guix.texi | 12 +++++-
guix/nar.scm | 15 ++++----
guix/scripts/authenticate.scm | 57 +++++++++------------------
nix/libstore/local-store.cc | 24 ++----------
tests/store.scm | 72 +++++++++++++++++++++++++++++++++++
5 files changed, 113 insertions(+), 67 deletions(-)

--
2.28.0
L
L
Ludovic Courtès wrote on 9 Sep 2020 00:16
[PATCH 1/3] store: Test 'import-paths' with unauthorized and unsigned nar bundles.
(address . 43285@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20200908221635.32684-1-ludo@gnu.org
* tests/store.scm ("import not signed")
("import signed by unauthorized key"): New tests.
---
tests/store.scm | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)

Toggle diff (92 lines)
diff --git a/tests/store.scm b/tests/store.scm
index e168d3dcf6..8ff76e8f98 100644
--- a/tests/store.scm
+++ b/tests/store.scm
@@ -23,6 +23,8 @@
#:use-module (guix utils)
#:use-module (guix monads)
#:use-module ((gcrypt hash) #:prefix gcrypt:)
+ #:use-module ((gcrypt pk-crypto) #:prefix gcrypt:)
+ #:use-module (guix pki)
#:use-module (guix base32)
#:use-module (guix packages)
#:use-module (guix derivations)
@@ -966,6 +968,76 @@
(list out1 out2))))
#:guile-for-build (%guile-for-build)))
+
+(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"))))
+ (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' raises an exception.
+ (guard (c ((store-protocol-error? c)
+ (and (not (zero? (store-protocol-error-status (pk 'C c))))
+ (string-contains (store-protocol-error-message c)
+ "lacks a signature"))))
+ (let* ((source (open-bytevector-input-port dump))
+ (imported (import-paths %store source)))
+ (pk 'unsigned-imported imported)
+ #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"))))
+ (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' raises an exception.
+ (guard (c ((store-protocol-error? c)
+ ;; XXX: The daemon-provided error message currently doesn't
+ ;; mention the reason of the failure.
+ (not (zero? (store-protocol-error-status c)))))
+ (let* ((source (open-bytevector-input-port dump))
+ (imported (import-paths %store source)))
+ (pk 'unauthorized-imported imported)
+ #f))))
+
(test-assert "import corrupt path"
(let* ((text (random-text))
(file (add-text-to-store %store "text" text))
--
2.28.0
L
L
Ludovic Courtès wrote on 9 Sep 2020 00:16
[PATCH 2/3] doc: Distinguish the "nar bundle" format from "nar".
(address . 43285@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20200908221635.32684-2-ludo@gnu.org
* doc/guix.texi (Invoking guix archive): Introduce the term "nar bundle"
and clarify what the output of "guix archive --export" really is.
* guix/nar.scm (restore-one-item, restore-file-set): Use the term "nar
bundle" in docstrings.
---
doc/guix.texi | 12 +++++++++++-
guix/nar.scm | 15 ++++++++-------
2 files changed, 19 insertions(+), 8 deletions(-)

Toggle diff (70 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 1d6782e6fa..5cb4fe2dfd 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -4990,7 +4990,13 @@ what you should use in this case (@pxref{Invoking guix copy}).
@cindex nar, archive format
@cindex normalized archive (nar)
-Archives are stored in the ``normalized archive'' or ``nar'' format, which is
+@cindex nar bundle, archive format
+Each store item is written in the @dfn{normalized archive} or @dfn{nar}
+format (described below), and the output of @command{guix archive
+--export} (and input of @command{guix archive --import}) is a @dfn{nar
+bundle}.
+
+The nar format is
comparable in spirit to `tar', but with differences
that make it more appropriate for our purposes. First, rather than
recording all Unix metadata for each file, the nar format only mentions
@@ -5000,6 +5006,10 @@ entries are stored always follows the order of file names according to
the C locale collation order. This makes archive production fully
deterministic.
+That nar bundle format is essentially the concatenation of zero or more
+nars along with metadata for each store item it contains: its file name,
+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
diff --git a/guix/nar.scm b/guix/nar.scm
index 6bb2ea5b96..a23af2e5de 100644
--- a/guix/nar.scm
+++ b/guix/nar.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2014 Mark H Weaver <mhw@netris.org>
;;;
;;; This file is part of GNU Guix.
@@ -156,7 +156,8 @@ protected from GC."
(define* (restore-one-item port
#:key acl (verify-signature? #t) (lock? #t)
(log-port (current-error-port)))
- "Restore one store item from PORT; return its file name on success."
+ "Restore one store item of a nar bundle read from PORT; return its file name
+on success."
(define (assert-valid-signature signature hash file)
;; Bail out if SIGNATURE, which must be a string as produced by
@@ -251,11 +252,11 @@ a signature"))
(define* (restore-file-set port
#:key (verify-signature? #t) (lock? #t)
(log-port (current-error-port)))
- "Restore the file set read from PORT to the store. The format of the data
-on PORT must be as created by 'export-paths'---i.e., a series of Nar-formatted
-archives with interspersed meta-data joining them together, possibly with a
-digital signature at the end. Log progress to LOG-PORT. Return the list of
-files restored.
+ "Restore the file set (\"nar bundle\") read from PORT to the store. The
+format of the data on PORT must be as created by 'export-paths'---i.e., a
+series of Nar-formatted archives with interspersed meta-data joining them
+together, possibly with a digital signature at the end. Log progress to
+LOG-PORT. Return the list of files restored.
When LOCK? is #f, assume locks for the files to be restored are already held.
This is the case when the daemon calls a build hook.
--
2.28.0
L
L
Ludovic Courtès wrote on 9 Sep 2020 00:16
[PATCH 3/3] daemon: Simplify interface with 'guix authenticate'.
(address . 43285@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20200908221635.32684-3-ludo@gnu.org
There's no reason at this point to mimic the calling convention of the
'openssl' command.

* nix/libstore/local-store.cc (LocalStore::exportPath): Add only "sign"
and HASH to ARGS. Remove 'tmpDir' and 'hashFile'.
(LocalStore::importPath): Add only "verify" and SIGNATURE to ARGS.
Remove 'sigFile'.
* guix/scripts/authenticate.scm (guix-authenticate): Adjust
accordingly; remove the OpenSSL-style clauses.
(read-hash-data): Remove.
(sign-with-key): Replace 'port' with 'sha256' and adjust accordingly.
(validate-signature): Export SIGNATURE to be a canonical sexp.
---
guix/scripts/authenticate.scm | 57 +++++++++++------------------------
nix/libstore/local-store.cc | 24 +++------------
2 files changed, 22 insertions(+), 59 deletions(-)

Toggle diff (155 lines)
diff --git a/guix/scripts/authenticate.scm b/guix/scripts/authenticate.scm
index f1fd8ee895..b5f043e6ac 100644
--- a/guix/scripts/authenticate.scm
+++ b/guix/scripts/authenticate.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2020 Ludovic Courtès <ludo@gnu.org>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -17,7 +17,6 @@
;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
(define-module (guix scripts authenticate)
- #:use-module (guix config)
#:use-module (guix base16)
#:use-module (gcrypt pk-crypto)
#:use-module (guix pki)
@@ -39,16 +38,9 @@
;; Read a gcrypt sexp from a port and return it.
(compose string->canonical-sexp read-string))
-(define (read-hash-data port key-type)
- "Read sha256 hash data from PORT and return it as a gcrypt sexp. KEY-TYPE
-is a symbol representing the type of public key algo being used."
- (let* ((hex (read-string port))
- (bv (base16-string->bytevector (string-trim-both hex))))
- (bytevector->hash-data bv #:key-type key-type)))
-
-(define (sign-with-key key-file port)
- "Sign the hash read from PORT with KEY-FILE, and write an sexp that includes
-both the hash and the actual signature."
+(define (sign-with-key key-file sha256)
+ "Sign the hash SHA256 (a bytevector) with KEY-FILE, and write an sexp that
+includes both the hash and the actual signature."
(let* ((secret-key (call-with-input-file key-file read-canonical-sexp))
(public-key (if (string-suffix? ".sec" key-file)
(call-with-input-file
@@ -58,18 +50,18 @@ both the hash and the actual signature."
(leave
(G_ "cannot find public key for secret key '~a'~%")
key-file)))
- (data (read-hash-data port (key-type public-key)))
+ (data (bytevector->hash-data sha256
+ #:key-type (key-type public-key)))
(signature (signature-sexp data secret-key public-key)))
(display (canonical-sexp->string signature))
#t))
-(define (validate-signature port)
- "Read the signature from PORT (which is as produced above), check whether
-its public key is authorized, verify the signature, and print the signed data
-to stdout upon success."
- (let* ((signature (read-canonical-sexp port))
- (subject (signature-subject signature))
- (data (signature-signed-data signature)))
+(define (validate-signature signature)
+ "Validate SIGNATURE, a canonical sexp. Check whether its public key is
+authorized, verify the signature, and print the signed data to stdout upon
+success."
+ (let* ((subject (signature-subject signature))
+ (data (signature-signed-data signature)))
(if (and data subject)
(if (authorized-key? subject)
(if (valid-signature? signature)
@@ -85,9 +77,7 @@ to stdout upon success."
;;;
-;;; Entry point with 'openssl'-compatible interface. We support this
-;;; interface because that's what the daemon expects, and we want to leave it
-;;; unmodified currently.
+;;; Entry point.
;;;
(define (guix-authenticate . args)
@@ -101,22 +91,11 @@ to stdout upon success."
(with-fluids ((%default-port-encoding "ISO-8859-1")
(%default-port-conversion-strategy 'error))
(match args
- ;; As invoked by guix-daemon.
- (("rsautl" "-sign" "-inkey" key "-in" hash-file)
- (call-with-input-file hash-file
- (lambda (port)
- (sign-with-key key port))))
- ;; As invoked by Nix/Crypto.pm (used by Hydra.)
- (("rsautl" "-sign" "-inkey" key)
- (sign-with-key key (current-input-port)))
- ;; As invoked by guix-daemon.
- (("rsautl" "-verify" "-inkey" _ "-pubin" "-in" signature-file)
- (call-with-input-file signature-file
- (lambda (port)
- (validate-signature port))))
- ;; As invoked by Nix/Crypto.pm (used by Hydra.)
- (("rsautl" "-verify" "-inkey" _ "-pubin")
- (validate-signature (current-input-port)))
+ (("sign" key-file hash)
+ (sign-with-key key-file (base16-string->bytevector hash)))
+ (("verify" signature)
+ (validate-signature (string->canonical-sexp signature)))
+
(("--help")
(display (G_ "Usage: guix authenticate OPTION...
Sign or verify the signature on the given file. This tool is meant to
diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc
index 7a520925e5..0534f2a3fc 100644
--- a/nix/libstore/local-store.cc
+++ b/nix/libstore/local-store.cc
@@ -1277,21 +1277,13 @@ void LocalStore::exportPath(const Path & path, bool sign,
writeInt(1, hashAndWriteSink);
- Path tmpDir = createTempDir();
- AutoDelete delTmp(tmpDir);
- Path hashFile = tmpDir + "/hash";
- writeFile(hashFile, printHash(hash));
-
Path secretKey = settings.nixConfDir + "/signing-key.sec";
checkSecrecy(secretKey);
Strings args;
- args.push_back("rsautl");
- args.push_back("-sign");
- args.push_back("-inkey");
+ args.push_back("sign");
args.push_back(secretKey);
- args.push_back("-in");
- args.push_back(hashFile);
+ args.push_back(printHash(hash));
string signature = runAuthenticationProgram(args);
@@ -1372,17 +1364,9 @@ Path LocalStore::importPath(bool requireSignature, Source & source)
string signature = readString(hashAndReadSource);
if (requireSignature) {
- Path sigFile = tmpDir + "/sig";
- writeFile(sigFile, signature);
-
Strings args;
- args.push_back("rsautl");
- args.push_back("-verify");
- args.push_back("-inkey");
- args.push_back(settings.nixConfDir + "/signing-key.pub");
- args.push_back("-pubin");
- args.push_back("-in");
- args.push_back(sigFile);
+ args.push_back("verify");
+ args.push_back(signature);
string hash2 = runAuthenticationProgram(args);
/* Note: runProgram() throws an exception if the signature
--
2.28.0
Z
Z
zimoun wrote on 9 Sep 2020 01:07
Re: [bug#43285] [PATCH 0/3] Improve 'import-paths' tests and 'guix authenticate' interface
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 43285@debbugs.gnu.org)
CAJ3okZ1QhXpJUooRSyMtYKT91PFERjvw+AoBWxsSt=wRZ80ZpA@mail.gmail.com
Hi Ludo,

On Wed, 9 Sep 2020 at 00:16, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (2 lines)
> daemon: Simplify interface with 'guix authenticate'.

I guess that the subcommand is "guix git authenticate". Well, I do
not know if it matters. :-)

Cheers,
simon
L
L
Ludovic Courtès wrote on 9 Sep 2020 09:03
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 43285@debbugs.gnu.org)
87imcno2hs.fsf@gnu.org
Hello!

zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (7 lines)
> On Wed, 9 Sep 2020 at 00:16, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> daemon: Simplify interface with 'guix authenticate'.
>
> I guess that the subcommand is "guix git authenticate". Well, I do
> not know if it matters. :-)

No, it’s really ‘guix authenticate’, an internal command. :-)

Ludo’.
L
L
Ludovic Courtès wrote on 11 Sep 2020 17:59
(address . 43285-done@debbugs.gnu.org)
87mu1ws3sr.fsf@gnu.org
Hi,

Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (11 lines)
> These patches are about improving testing around the ‘import-paths’
> RPC and cleaning up the interface between ‘guix authenticate’ and
> the daemon.
>
> Ludo’.
>
> Ludovic Courtès (3):
> store: Test 'import-paths' with unauthorized and unsigned nar bundles.
> doc: Distinguish the "nar bundle" format from "nar".
> daemon: Simplify interface with 'guix authenticate'.

Pushed as 6dd8ffc57420ee2f6f19e79e41028e78fe9e6a7e.

I realized I hadn’t updated tests/guix-archive.sh, which I did, and
that also prompted me to keep the temporary file for the “verify”
operation so it’s decoded with the right encoding.

Anyway, the real bit will be https://issues.guix.gnu.org/43340.

Ludo’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 43285@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 43285
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch