GNU bug report logs - #67072
[PATCH 0/4] Helping diagnose substitute setup issues

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 67072 in the body.
You can then email your comments to 67072 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: guix-patches <at> gnu.org
Cc: Emmanuel Agullo <emmanuel.agullo <at> inria.fr>,
 Ludovic Courtès <ludo <at> gnu.org>,
 Simon Tournier <zimon.toutoune <at> gmail.com>
Subject: [PATCH 0/4] Helping diagnose substitute setup issues
Date: Sat, 11 Nov 2023 12:03:06 +0100

Hello Guix!

While discussing at the Reproducible Software Environments Workshop
yesterday, Emmanuel Agullo and Simon Tournier suggested adding
tools to help diagnose substitute setup issues: to see which
substitutes URLs are being used and whether one of them is unauthorized.

This is a step in that direction.  First ‘guix weather’ and ‘guix
challenge’ now default to the same substitute URLs as guix-daemon
(this was not the case until now because there was no way to get
that information from the daemon).  Second ‘guix weather’ reports
about unauthorized servers, like so:

--8<---------------cut here---------------start------------->8---
$ guix weather coreutils
computing 1 package derivations for x86_64-linux...
looking for 2 store items on https://ci.guix.gnu.org...
guix weather: warning: substitutes from 'https://ci.guix.gnu.org' are unauthorized
hint: To authorize substitute download from `https://ci.guix.gnu.org', the following command
needs to be run as root:

     guix archive --authorize <<EOF
     (public-key 
      (ecc 
       (curve Ed25519)
       (q #8D156F295D24B0D9A86FA5741A840FF2D24F60F7B6C4134814AD55625971B394#)
       )
      )
     
     EOF

Alternatively, on Guix System, you can add the signing key above to the
`authorized-keys' field of `guix-configuration'.

See "Getting Substitutes from Other Servers" in the manual for more information.

https://ci.guix.gnu.org ☀
  100.0% substitutes available (2 out of 2)
  at least 19.3 MiB of nars (compressed)
  25.3 MiB on disk (uncompressed)
[…]
--8<---------------cut here---------------end--------------->8---

It turned out to be a low-hanging fruit!

Thoughts?

Ludo’.

Ludovic Courtès (4):
  daemon: Implement ‘substitute-urls’ RPC.
  challenge: Use the same substitute URLs as guix-daemon.
  weather: Use the same substitute URLs as guix-daemon.
  weather: Report unauthorized substitute servers.

 doc/guix.texi                   | 26 ++++++++++++++++---
 guix/scripts/challenge.scm      | 11 +++++---
 guix/scripts/weather.scm        | 46 ++++++++++++++++++++++++++++++---
 guix/store.scm                  | 18 ++++++++++---
 nix/libstore/worker-protocol.hh |  5 ++--
 nix/nix-daemon/nix-daemon.cc    | 17 ++++++++++++
 tests/store.scm                 | 25 ++++++++++++++++--
 7 files changed, 132 insertions(+), 16 deletions(-)


base-commit: 08d94fe20eca47b69678b3eced8749dd02c700a4
-- 
2.41.0

Message #8 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 67072 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC.
Date: Sat, 11 Nov 2023 12:06:23 +0100

* nix/libstore/worker-protocol.hh (PROTOCOL_VERSION): Bump.
(WorkerOp): Add ‘wopSubstituteURLs’.
* nix/nix-daemon/nix-daemon.cc (performOp): Implement it.
* guix/store.scm (%protocol-version): Bump.
(operation-id): Add ‘substitute-urls’.
(substitute-urls): New procedure.
* tests/store.scm ("substitute-urls, default")
("substitute-urls, client-specified URLs")
("substitute-urls, disabled"): New tests.

Change-Id: I2c0119500c3a1eecfa5ebf32463ffb0f173161de
---
 guix/store.scm                  | 18 +++++++++++++++---
 nix/libstore/worker-protocol.hh |  5 +++--
 nix/nix-daemon/nix-daemon.cc    | 17 +++++++++++++++++
 tests/store.scm                 | 25 +++++++++++++++++++++++--
 4 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/guix/store.scm b/guix/store.scm
index f8e77b2cd9..97c4f32a5b 100644
--- a/guix/store.scm
+++ b/guix/store.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012-2022 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2012-2023 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2018 Jan Nieuwenhuizen <janneke <at> gnu.org>
 ;;; Copyright © 2019, 2020 Mathieu Othacehe <m.othacehe <at> gmail.com>
 ;;; Copyright © 2020 Florian Pelz <pelzflorian <at> pelzflorian.de>
@@ -145,6 +145,7 @@ (define-module (guix store)
             path-info-nar-size
 
             built-in-builders
+            substitute-urls
             references
             references/cached
             references*
@@ -199,7 +200,7 @@ (define-module (guix store)
             derivation-log-file
             log-file))
 
-(define %protocol-version #x163)
+(define %protocol-version #x164)
 
 (define %worker-magic-1 #x6e697863)               ; "nixc"
 (define %worker-magic-2 #x6478696f)               ; "dxio"
@@ -253,7 +254,8 @@ (define-enumerate-type operation-id
   (query-valid-derivers 33)
   (optimize-store 34)
   (verify-store 35)
-  (built-in-builders 80))
+  (built-in-builders 80)
+  (substitute-urls 81))
 
 (define-enumerate-type hash-algo
   ;; hash.hh
@@ -1780,6 +1782,16 @@ (define-operation (clear-failed-paths (store-path-list items))
 This makes sense only when the daemon was started with '--cache-failures'."
   boolean)
 
+(define substitute-urls
+  (let ((urls (operation (substitute-urls)
+                         #f
+                         string-list)))
+    (lambda (store)
+      "Return the list of currently configured substitutes URLs for STORE, or
+#f if the daemon is too old and does not implement this RPC."
+      (and (>= (store-connection-version store) #x164)
+           (urls store)))))
+
 
 ;;;
 ;;; Per-connection caches.
diff --git a/nix/libstore/worker-protocol.hh b/nix/libstore/worker-protocol.hh
index ea67b10a5b..ef259db2a0 100644
--- a/nix/libstore/worker-protocol.hh
+++ b/nix/libstore/worker-protocol.hh
@@ -6,7 +6,7 @@ namespace nix {
 #define WORKER_MAGIC_1 0x6e697863
 #define WORKER_MAGIC_2 0x6478696f
 
-#define PROTOCOL_VERSION 0x163
+#define PROTOCOL_VERSION 0x164
 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00)
 #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)
 
@@ -44,7 +44,8 @@ typedef enum {
     wopQueryValidDerivers = 33,
     wopOptimiseStore = 34,
     wopVerifyStore = 35,
-    wopBuiltinBuilders = 80
+    wopBuiltinBuilders = 80,
+    wopSubstituteURLs = 81
 } WorkerOp;
 
 
diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
index 497de11a04..4cb05c802e 100644
--- a/nix/nix-daemon/nix-daemon.cc
+++ b/nix/nix-daemon/nix-daemon.cc
@@ -736,6 +736,23 @@ static void performOp(bool trusted, unsigned int clientVersion,
 	break;
     }
 
+    case wopSubstituteURLs: {
+	startWork();
+	Strings urls;
+	if (settings.get("build-use-substitutes", std::string("false")) == "true") {
+	    /* First check the client-provided substitute URLs, then those
+	       passed to the daemon.  */
+	    auto str = settings.get("untrusted-substitute-urls",  std::string(""));
+	    if (str.empty()) {
+		str = settings.get("substitute-urls",  std::string(""));
+	    }
+	    urls = tokenizeString<Strings>(str);
+	}
+	stopWork();
+	writeStrings(urls, to);
+	break;
+    }
+
     default:
         throw Error(format("invalid operation %1%") % op);
     }
diff --git a/tests/store.scm b/tests/store.scm
index 5df28adf0d..45948f4f43 100644
--- a/tests/store.scm
+++ b/tests/store.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2012-2021, 2023 Ludovic Courtès <ludo <at> gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -105,7 +105,28 @@ (define %shell
               "/283gqy39v3g9dxjy26rynl0zls82fmcg-guile-2.0.7/bin/guile")))
        (not (direct-store-path? (%store-prefix)))))
 
-(test-skip (if %store 0 15))
+(test-skip (if %store 0 18))
+
+(test-equal "substitute-urls, default"
+  (list (getenv "GUIX_BINARY_SUBSTITUTE_URL"))
+  (with-store store
+    (set-build-options store #:use-substitutes? #t)
+    (substitute-urls store)))
+
+(test-equal "substitute-urls, client-specified URLs"
+  '("http://substitutes.example.org"
+    "http://other.example.org")
+  (with-store store
+    (set-build-options store #:use-substitutes? #t
+                       #:substitute-urls '("http://substitutes.example.org"
+                                           "http://other.example.org"))
+    (substitute-urls store)))
+
+(test-equal "substitute-urls, disabled"
+  '()
+  (with-store store
+    (set-build-options store #:use-substitutes? #f)
+    (substitute-urls store)))
 
 (test-equal "profiles/per-user exists and is not writable"
   #o755
-- 
2.41.0

Message #11 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 67072 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 2/4] challenge: Use the same substitute URLs as guix-daemon.
Date: Sat, 11 Nov 2023 12:06:24 +0100

* guix/scripts/challenge.scm (%default-options): Remove ‘substitute-urls’.
(guix-challenge): Call ‘substitute-urls’ when OPTS doesn’t have it.  Warn
when ‘substitute-urls’ returns #f.

Change-Id: I49be0e89404c1889970a3430967fbb3498d35d99
---
 guix/scripts/challenge.scm | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/guix/scripts/challenge.scm b/guix/scripts/challenge.scm
index 01e2f9a2b2..d38171b868 100644
--- a/guix/scripts/challenge.scm
+++ b/guix/scripts/challenge.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2015-2017, 2019-2022 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2015-2017, 2019-2023 Ludovic Courtès <ludo <at> gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -504,7 +504,6 @@ (define %options
 
 (define %default-options
   `((system . ,(%current-system))
-    (substitute-urls . ,%default-substitute-urls)
     (difference-report . ,report-differing-files)))
 
 
@@ -539,7 +538,13 @@ (define-command (guix-challenge . args)
                             (G_ "no arguments specified, nothing to do~%"))
                            (exit 0))
                           (x
-                           files))))
+                           files)))
+                 (urls (or urls
+                           (substitute-urls store)
+                           (begin
+                             (warning (G_ "could not determine current \
+substitute URLs; using defaults~%"))
+                             %default-substitute-urls))))
              (set-build-options store
                                 #:use-substitutes? #f)
 
-- 
2.41.0

Message #14 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 67072 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 4/4] weather: Report unauthorized substitute servers.
Date: Sat, 11 Nov 2023 12:06:26 +0100

The goal is to make it easier to diagnose substitute
misconfiguration (where we’re passing a substitute URL whose
corresponding key is not authorized).

Suggested by Emmanuel Agullo.

* guix/scripts/weather.scm (check-narinfo-authorization): New procedure.
(report-server-coverage): Use it.
* doc/guix.texi (Invoking guix weather): Document it.
(Getting Substitutes from Other Servers): Add “Troubleshooting” frame.

Change-Id: I0a049c39eefb10d6a06634c8b16aa86902769791
---
 doc/guix.texi            | 21 ++++++++++++++++++++-
 guix/scripts/weather.scm | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 028c4f3357..45c3b7344f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -4058,6 +4058,7 @@ Substitute Server Authorization
 
 @node Getting Substitutes from Other Servers
 @subsection Getting Substitutes from Other Servers
+@c Note: This section name appears in a hint printed by 'guix weather'.
 
 @cindex substitute servers, adding more
 Guix can look up and fetch substitutes from several servers.  This is
@@ -4157,6 +4158,21 @@ Getting Substitutes from Other Servers
 substitute lookup can be slowed down if too many servers need to be
 contacted.
 
+@quotation Troubleshooting
+To diagnose problems, you can run @command{guix weather}.  For example,
+running:
+
+@example
+guix weather coreutils
+@end example
+
+@noindent
+not only tells you which of the currently-configured servers has
+substitutes for the @code{coreutils} package, it also reports whether
+one of these servers is unauthorized.  @xref{Invoking guix weather}, for
+more information.
+@end quotation
+
 Note that there are also situations where one may want to add the URL of
 a substitute server @emph{without} authorizing its key.
 @xref{Substitute Authentication}, to understand this fine point.
@@ -16395,7 +16411,10 @@ Invoking guix weather
 specified servers so you can have an idea of whether you'll be grumpy
 today.  It can sometimes be useful info as a user, but it is primarily
 useful to people running @command{guix publish} (@pxref{Invoking guix
-publish}).
+publish}).  Sometimes substitutes @emph{are} available but they are not
+authorized on your system; @command{guix weather} reports it so you can
+authorize them if you want (@pxref{Getting Substitutes from Other
+Servers}).
 
 @cindex statistics, for substitutes
 @cindex availability of substitutes
diff --git a/guix/scripts/weather.scm b/guix/scripts/weather.scm
index 7e302fcea7..e7e5a75811 100644
--- a/guix/scripts/weather.scm
+++ b/guix/scripts/weather.scm
@@ -35,6 +35,8 @@ (define-module (guix scripts weather)
   #:use-module ((guix build utils) #:select (every*))
   #:use-module (guix substitutes)
   #:use-module (guix narinfo)
+  #:use-module (guix pki)
+  #:autoload   (gcrypt pk-crypto) (canonical-sexp->string)
   #:use-module (guix http-client)
   #:use-module (guix ci)
   #:use-module (guix sets)
@@ -185,6 +187,32 @@ (define (store-item-system store item)
     (()
      #f)))
 
+(define (check-narinfo-authorization narinfo)
+  "Print a warning when NARINFO is not signed by an authorized key."
+  (unless (valid-narinfo? narinfo)
+    (warning (G_ "substitutes from '~a' are unauthorized~%")
+             (narinfo-uri-base narinfo))
+    ;; The "all substitutes" below reflects the fact that, in reality, it *is*
+    ;; possible to download "unauthorized" substitutes, as long as they match
+    ;; authorized substitutes.
+    (display-hint (G_ "To authorize all substitutes from @uref{~a} to be
+downloaded, the following command needs to be run as root:
+
+@example
+guix archive --authorize <<EOF
+~a
+EOF
+@end example
+
+Alternatively, on Guix System, you can add the signing key above to the
+@code{authorized-keys} field of @code{guix-configuration}.
+
+See \"Getting Substitutes from Other Servers\" in the manual for more
+information.")
+                  (narinfo-uri-base narinfo)
+                  (canonical-sexp->string
+                   (signature-subject (narinfo-signature narinfo))))))
+
 (define* (report-server-coverage server items
                                  #:key display-missing?)
   "Report the subset of ITEMS available as substitutes on SERVER.
@@ -204,6 +232,12 @@ (define* (report-server-coverage server items
                     #:make-progress-reporter
                     (lambda* (total #:key url #:allow-other-keys)
                       (progress-reporter/bar total)))))
+    (match narinfos
+      (() #f)
+      ((narinfo . _)
+       ;; Help diagnose missing substitute authorizations.
+       (check-narinfo-authorization narinfo)))
+
     (let ((obtained  (length narinfos))
           (requested (length items))
           (missing   (lset-difference string=?
-- 
2.41.0

Message #17 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 67072 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 3/4] weather: Use the same substitute URLs as guix-daemon.
Date: Sat, 11 Nov 2023 12:06:25 +0100

* guix/scripts/weather.scm (%default-options): Remove ‘substitute-urls’.
(guix-weather): Call ‘substitute-urls’ when OPTS doesn’t have it.  Warn
when ‘substitute-urls’ returns #f.
* doc/guix.texi (Invoking guix weather): Adjust accordingly.

Change-Id: I3e9100074f2ad559e5c408660db70430d64f2bef
---
 doc/guix.texi            |  5 +++--
 guix/scripts/weather.scm | 12 +++++++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 9f06f1c325..028c4f3357 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -16462,8 +16462,9 @@ Invoking guix weather
 @table @code
 @item --substitute-urls=@var{urls}
 @var{urls} is the space-separated list of substitute server URLs to
-query.  When this option is omitted, the default set of substitute
-servers is queried.
+query.  When this option is omitted, the URLs specified with the
+@option{--substitute-urls} option of @command{guix-daemon} are used or,
+as a last resort, the default set of substitute URLs.
 
 @item --system=@var{system}
 @itemx -s @var{system}
diff --git a/guix/scripts/weather.scm b/guix/scripts/weather.scm
index 140df3435f..7e302fcea7 100644
--- a/guix/scripts/weather.scm
+++ b/guix/scripts/weather.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2017-2022 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2017-2023 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2017 Ricardo Wurmus <rekado <at> elephly.net>
 ;;; Copyright © 2018 Kyle Meyer <kyle <at> kyleam.com>
 ;;; Copyright © 2020 Simon Tournier <zimon.toutoune <at> gmail.com>
@@ -391,7 +391,7 @@ (define %options
          %standard-native-build-options))
 
 (define %default-options
-  `((substitute-urls . ,%default-substitute-urls)))
+  '())
 
 (define (load-manifest file)
   "Load the manifest from FILE and return the list of packages it refers to."
@@ -582,7 +582,13 @@ (define-command (guix-weather . args)
       (let* ((opts     (parse-command-line args %options
                                            (list %default-options)
                                            #:build-options? #f))
-             (urls     (assoc-ref opts 'substitute-urls))
+             (urls     (or (assoc-ref opts 'substitute-urls)
+                           (with-store store
+                             (substitute-urls store))
+                           (begin
+                             (warning (G_ "could not determine current \
+substitute URLs; using defaults~%"))
+                             %default-substitute-urls)))
              (systems  (match (filter-map (match-lambda
                                             (('system . system) system)
                                             (_ #f))
-- 
2.41.0

Message #20 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludovic.courtes <at> inria.fr>
To: 67072 <at> debbugs.gnu.org
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Emmanuel Agullo <emmanuel.agullo <at> inria.fr>,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues
Date: Mon, 27 Nov 2023 18:21:52 +0100

Hello!

Comments or suggestions regarding this change?

  https://issues.guix.gnu.org/67072

If not I’d like to push it soon.

TIA.  :-)

Ludo’.

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

> Hello Guix!
>
> While discussing at the Reproducible Software Environments Workshop
> yesterday, Emmanuel Agullo and Simon Tournier suggested adding
> tools to help diagnose substitute setup issues: to see which
> substitutes URLs are being used and whether one of them is unauthorized.
>
> This is a step in that direction.  First ‘guix weather’ and ‘guix
> challenge’ now default to the same substitute URLs as guix-daemon
> (this was not the case until now because there was no way to get
> that information from the daemon).  Second ‘guix weather’ reports
> about unauthorized servers, like so:
>
> $ guix weather coreutils
> computing 1 package derivations for x86_64-linux...
> looking for 2 store items on https://ci.guix.gnu.org...
> guix weather: warning: substitutes from 'https://ci.guix.gnu.org' are unauthorized
> hint: To authorize substitute download from `https://ci.guix.gnu.org', the following command
> needs to be run as root:
>
>      guix archive --authorize <<EOF
>      (public-key 
>       (ecc 
>        (curve Ed25519)
>        (q #8D156F295D24B0D9A86FA5741A840FF2D24F60F7B6C4134814AD55625971B394#)
>        )
>       )
>      
>      EOF
>
> Alternatively, on Guix System, you can add the signing key above to the
> `authorized-keys' field of `guix-configuration'.
>
> See "Getting Substitutes from Other Servers" in the manual for more information.
>
> https://ci.guix.gnu.org ☀
>   100.0% substitutes available (2 out of 2)
>   at least 19.3 MiB of nars (compressed)
>   25.3 MiB on disk (uncompressed)
> […]
>
> It turned out to be a low-hanging fruit!
>
> Thoughts?
>
> Ludo’.
>
> Ludovic Courtès (4):
>   daemon: Implement ‘substitute-urls’ RPC.
>   challenge: Use the same substitute URLs as guix-daemon.
>   weather: Use the same substitute URLs as guix-daemon.
>   weather: Report unauthorized substitute servers.
>
>  doc/guix.texi                   | 26 ++++++++++++++++---
>  guix/scripts/challenge.scm      | 11 +++++---
>  guix/scripts/weather.scm        | 46 ++++++++++++++++++++++++++++++---
>  guix/store.scm                  | 18 ++++++++++---
>  nix/libstore/worker-protocol.hh |  5 ++--
>  nix/nix-daemon/nix-daemon.cc    | 17 ++++++++++++
>  tests/store.scm                 | 25 ++++++++++++++++--
>  7 files changed, 132 insertions(+), 16 deletions(-)
>
>
> base-commit: 08d94fe20eca47b69678b3eced8749dd02c700a4

Message #23 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>, 67072 <at> debbugs.gnu.org
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Ludovic Courtès <ludo <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC.
Date: Tue, 28 Nov 2023 13:09:31 +0100

Hi,

On Sat, 11 Nov 2023 at 12:06, Ludovic Courtès <ludo <at> gnu.org> wrote:

> -(test-skip (if %store 0 15))
> +(test-skip (if %store 0 18))

Out of curiosity, why 18?

Cheers,
simon

Message #26 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>, 67072 <at> debbugs.gnu.org
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Ludovic Courtès <ludo <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute
 servers.
Date: Tue, 28 Nov 2023 14:14:23 +0100

Hi,

On Sat, 11 Nov 2023 at 12:06, Ludovic Courtès <ludo <at> gnu.org> wrote:

> +  #:use-module (guix pki)

Looking at what it drags, I notice:

--8<---------------cut here---------------start------------->8---
(define* (authorized-key? key #:optional (acl (current-acl)))
  "Return #t if KEY (a canonical sexp) is an authorized public key for archive
imports according to ACL."
  ;; Note: ACL is kept in native sexp form to make 'authorized-key?' faster,
  ;; by not having to convert it with 'canonical-sexp->sexp' on each call.
  ;; TODO: We could use a better data type for ACLs.
  (let ((key (canonical-sexp->sexp key)))
    (match acl
      (('acl
        ('entry subject-keys
                ('tag ('guix 'import)))
        ...)
       (not (not (member key subject-keys))))
      (_
       (error "invalid access-control list" acl)))))
--8<---------------cut here---------------end--------------->8---

I know it is irrelevant with the patch at hand.  Maybe not. :-)

   1. Why this ’(not (not’ ?

   2. When testing the patch, I have not done --sysconfdir=/etc and it
      was not able to find the correct ACL.  Somehow…

> +(define (check-narinfo-authorization narinfo)
> +  "Print a warning when NARINFO is not signed by an authorized key."
> +  (unless (valid-narinfo? narinfo)

…I entered in this part – hence the look up (guix pki) ;-).  Well, my
mistake is hard to reproduce outside of Guix development tree but
’valid-narinfo?’ returns false for more cases than just
unauthorized-key.  Therefore, the hint could be misleading.

Since we are discussing about an helper, I would run ’signature-case’
here in check-narinfo.  For example, if the case is 'unauthorized-key,
then I would check is %acl-file exists.  Maybe display the full
%acl-file explaining that the key is not in, etc.

Moreover, running “guix challenge coreutils” does not warn about
anything; when I was expected the same warning as “guix weather”.


Last, once sysconfig fixed, I get:

--8<---------------cut here---------------start------------->8---
guix weather: warning: could not determine current substitute URLs; using defaults
computing 1 package derivations for x86_64-linux...
looking for 2 store items on https://ci.guix.gnu.org...
guix weather: error: open-file: Permission denied: "/etc/guix/acl"
--8<---------------cut here---------------end--------------->8---

Hum? Maybe I am doing something wrong…  The file /etc/guix/acl has the
permission:

    -rw-------   1 root root   528  acl

Is it incorrect?  Well, if all are allowed to read (chmod a+r) then
there is not error.  And it displays the warning:

--8<---------------cut here---------------start------------->8---
guix weather: warning: could not determine current substitute URLs; using defaults
--8<---------------cut here---------------end--------------->8---

And that’s because the daemon is not supporting the operation.  This
warning appears to me misleading: personally I think that I am
misconfigured something when that’s not the case.  Instead, I would
display:

    warning: using defaults substitute URLs


Cheers,
simon

Message #29 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludovic.courtes <at> inria.fr>,
 67072 <at> debbugs.gnu.org
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Emmanuel Agullo <emmanuel.agullo <at> inria.fr>,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues
Date: Tue, 28 Nov 2023 14:17:22 +0100

Hi Ludo,

Sorry for the delay.

On Mon, 27 Nov 2023 at 18:21, Ludovic Courtès <ludovic.courtes <at> inria.fr> wrote:

> Comments or suggestions regarding this change?
>
>   https://issues.guix.gnu.org/67072

Cool!

Well, I did some minor comments about “guix weather”.  I have not
checked the “guix challenge” counter-part.  Maybe later this week. :-)

Cheers,
simon

Message #32 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Emmanuel Agullo <emmanuel.agullo <at> inria.fr>
To: Ludovic Courtes <ludovic.courtes <at> inria.fr>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, 67072 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues
Date: Thu, 30 Nov 2023 11:11:57 +0100 (CET)

Hello Ludo, hello Simon, hello Guix,

First of all thank you Ludo for the patch.

Indeed, it would be great one can check the substitutes are set up
as expected.

>> First ‘guix weather’ and ‘guix
>> challenge’ now default to the same substitute URLs as guix-daemon
>> (this was not the case until now because there was no way to get
>> that information from the daemon).

This is excellent. I was not aware of the difference.

>>  Second ‘guix weather’ reports about unauthorized servers

I guess it should help a lot!

>> If not I’d like to push it soon.

As best as I can read, green light for me. I'll be pleased to test
it once pushed.

Thanks again!

Best,

Manu

Message #35 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludovic.courtes <at> inria.fr>
To: Emmanuel Agullo <emmanuel.agullo <at> inria.fr>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, 67072 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues
Date: Thu, 30 Nov 2023 11:28:51 +0100

Hi Emmanuel,

Emmanuel Agullo <emmanuel.agullo <at> inria.fr> skribis:

>>> First ‘guix weather’ and ‘guix
>>> challenge’ now default to the same substitute URLs as guix-daemon
>>> (this was not the case until now because there was no way to get
>>> that information from the daemon).
>
> This is excellent. I was not aware of the difference.
>
>>>  Second ‘guix weather’ reports about unauthorized servers
>
> I guess it should help a lot!
>
>>> If not I’d like to push it soon.
>
> As best as I can read, green light for me. I'll be pleased to test
> it once pushed.

Awesome, thanks for your feedback!

Ludo’.

Message #38 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Simon Tournier <zimon.toutoune <at> gmail.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, 67072 <at> debbugs.gnu.org,
 Mathieu Othacehe <othacehe <at> gnu.org>, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 Ricardo Wurmus <rekado <at> elephly.net>, Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC.
Date: Sat, 02 Dec 2023 11:13:18 +0100

Simon Tournier <zimon.toutoune <at> gmail.com> skribis:

> On Sat, 11 Nov 2023 at 12:06, Ludovic Courtès <ludo <at> gnu.org> wrote:
>
>> -(test-skip (if %store 0 15))
>> +(test-skip (if %store 0 18))
>
> Out of curiosity, why 18?

Because we’re adding 3 tests.

(Now, the count might be off, and we’re never running those tests
without a running daemon anyway…)

Ludo’.

Message #41 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Simon Tournier <zimon.toutoune <at> gmail.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, 67072 <at> debbugs.gnu.org,
 Mathieu Othacehe <othacehe <at> gnu.org>, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 Ricardo Wurmus <rekado <at> elephly.net>, Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute
 servers.
Date: Sat, 02 Dec 2023 11:20:21 +0100

Hi Simon,

Simon Tournier <zimon.toutoune <at> gmail.com> skribis:

> I know it is irrelevant with the patch at hand.  Maybe not. :-)
>
>    1. Why this ’(not (not’ ?

This ensures the result is a Boolean.

[...]

>> +(define (check-narinfo-authorization narinfo)
>> +  "Print a warning when NARINFO is not signed by an authorized key."
>> +  (unless (valid-narinfo? narinfo)
>
> …I entered in this part – hence the look up (guix pki) ;-).  Well, my
> mistake is hard to reproduce outside of Guix development tree but
> ’valid-narinfo?’ returns false for more cases than just
> unauthorized-key.  Therefore, the hint could be misleading.

It’s true that ‘valid-narinfo?’ catches more cases, but the other cases
where it returns #f are situations where the substitute server is bogus.
So I chose to favor conciseness here.

> Since we are discussing about an helper, I would run ’signature-case’
> here in check-narinfo.  For example, if the case is 'unauthorized-key,
> then I would check is %acl-file exists.  Maybe display the full
> %acl-file explaining that the key is not in, etc.

Right, checking for ‘%acl-file’ is a good idea; I wouldn’t display its
contents though because that’d be intimidating and unhelpful IMO.

> Moreover, running “guix challenge coreutils” does not warn about
> anything […]

That’s on purpose:

--8<---------------cut here---------------start------------->8---
(define (compare-contents items servers)
  "Challenge the substitute servers whose URLs are listed in SERVERS by
comparing the hash of the substitutes of ITEMS that they serve.  Return the
list of <comparison-report> objects.

This procedure does not authenticate narinfos from SERVERS, nor does it verify
that they are signed by an authorized public keys.  The reason is that, by
definition, we may want to target unknown servers.  Furthermore, no risk is
taken since we do not import the archives."
--8<---------------cut here---------------end--------------->8---

> guix weather: warning: could not determine current substitute URLs; using defaults
> computing 1 package derivations for x86_64-linux...
> looking for 2 store items on https://ci.guix.gnu.org...
> guix weather: error: open-file: Permission denied: "/etc/guix/acl"

Uh, it should be able to deal with it gracefully.

> Hum? Maybe I am doing something wrong…  The file /etc/guix/acl has the
> permission:
>
>     -rw-------   1 root root   528  acl

It’s normally world-readable.

> Is it incorrect?  Well, if all are allowed to read (chmod a+r) then
> there is not error.  And it displays the warning:
>
> guix weather: warning: could not determine current substitute URLs; using defaults
>
> And that’s because the daemon is not supporting the operation.  This
> warning appears to me misleading: personally I think that I am
> misconfigured something when that’s not the case.  Instead, I would
> display:
>
>     warning: using defaults substitute URLs

Yes, good idea.

I’ll send v2 soonish.

Thanks for your feedback!

Ludo’.

Message #44 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, 67072 <at> debbugs.gnu.org,
 Mathieu Othacehe <othacehe <at> gnu.org>, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 Ricardo Wurmus <rekado <at> elephly.net>, Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC.
Date: Sat, 02 Dec 2023 14:16:42 +0100

Hi,

On sam., 02 déc. 2023 at 11:13, Ludovic Courtès <ludo <at> gnu.org> wrote:

> Because we’re adding 3 tests.

Aah!  Thanks for explaining.

Cheers,
simon

Message #47 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, 67072 <at> debbugs.gnu.org,
 Mathieu Othacehe <othacehe <at> gnu.org>, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 Ricardo Wurmus <rekado <at> elephly.net>, Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute
 servers.
Date: Sat, 02 Dec 2023 14:31:40 +0100

Hi Ludo,

On sam., 02 déc. 2023 at 11:20, Ludovic Courtès <ludo <at> gnu.org> wrote:

>> guix weather: warning: could not determine current substitute URLs; using defaults
>> computing 1 package derivations for x86_64-linux...
>> looking for 2 store items on https://ci.guix.gnu.org...
>> guix weather: error: open-file: Permission denied: "/etc/guix/acl"
>
> Uh, it should be able to deal with it gracefully.
>
>> Hum? Maybe I am doing something wrong…  The file /etc/guix/acl has the
>> permission:
>>
>>     -rw-------   1 root root   528  acl
>
> It’s normally world-readable.

On foreign distro, this %acl-file appears by default with ’rw’
permission for root only.  It is not word-readable.

When running guix-install.sh as root, if I read correctly, it runs:

	    local key=~root/.config/guix/current/share/guix/$host.pub
	    [ -f "$key" ] \
		&& guix archive --authorize < "$key" \
		&& _msg "${PAS}Authorized public key for $host"

Therefore, the file %acl-file is written as root by the procedure
’write-acl’.  Hence the permission ’rw’ for root only, no?

Somehow, ’write-acl’ should be tweaked or guix-install.sh, no?

Cheers,
simon

Message #50 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 67072 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH v2 0/4] Helping diagnose substitute setup issues
Date: Mon,  4 Dec 2023 15:15:41 +0100

Hi,

Changes since v1:

  • Gracefully handle unreadable /etc/guix/acl file in ‘guix weather’;

  • Simplify ‘guix weather’ warning message when the daemon does not
    support the ‘substitute-urls’ RPC.

Lemme know what you think!

Ludo’.

Ludovic Courtès (4):
  daemon: Implement ‘substitute-urls’ RPC.
  challenge: Use the same substitute URLs as guix-daemon.
  weather: Use the same substitute URLs as guix-daemon.
  weather: Report unauthorized substitute servers.

 doc/guix.texi                   | 26 ++++++++++++--
 guix/scripts/challenge.scm      | 11 ++++--
 guix/scripts/weather.scm        | 61 +++++++++++++++++++++++++++++++--
 guix/store.scm                  | 18 ++++++++--
 nix/libstore/worker-protocol.hh |  5 +--
 nix/nix-daemon/nix-daemon.cc    | 17 +++++++++
 tests/store.scm                 | 25 ++++++++++++--
 7 files changed, 147 insertions(+), 16 deletions(-)


base-commit: 6e2dd51df5f3f51e9056dd4f2e1b036195ab3caa
-- 
2.41.0

Message #53 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 67072 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH v2 1/4] daemon: Implement ‘substitute-urls’ RPC.
Date: Mon,  4 Dec 2023 15:15:42 +0100

* nix/libstore/worker-protocol.hh (PROTOCOL_VERSION): Bump.
(WorkerOp): Add ‘wopSubstituteURLs’.
* nix/nix-daemon/nix-daemon.cc (performOp): Implement it.
* guix/store.scm (%protocol-version): Bump.
(operation-id): Add ‘substitute-urls’.
(substitute-urls): New procedure.
* tests/store.scm ("substitute-urls, default")
("substitute-urls, client-specified URLs")
("substitute-urls, disabled"): New tests.

Change-Id: I2c0119500c3a1eecfa5ebf32463ffb0f173161de
---
 guix/store.scm                  | 18 +++++++++++++++---
 nix/libstore/worker-protocol.hh |  5 +++--
 nix/nix-daemon/nix-daemon.cc    | 17 +++++++++++++++++
 tests/store.scm                 | 25 +++++++++++++++++++++++--
 4 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/guix/store.scm b/guix/store.scm
index f8e77b2cd9..97c4f32a5b 100644
--- a/guix/store.scm
+++ b/guix/store.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012-2022 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2012-2023 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2018 Jan Nieuwenhuizen <janneke <at> gnu.org>
 ;;; Copyright © 2019, 2020 Mathieu Othacehe <m.othacehe <at> gmail.com>
 ;;; Copyright © 2020 Florian Pelz <pelzflorian <at> pelzflorian.de>
@@ -145,6 +145,7 @@ (define-module (guix store)
             path-info-nar-size
 
             built-in-builders
+            substitute-urls
             references
             references/cached
             references*
@@ -199,7 +200,7 @@ (define-module (guix store)
             derivation-log-file
             log-file))
 
-(define %protocol-version #x163)
+(define %protocol-version #x164)
 
 (define %worker-magic-1 #x6e697863)               ; "nixc"
 (define %worker-magic-2 #x6478696f)               ; "dxio"
@@ -253,7 +254,8 @@ (define-enumerate-type operation-id
   (query-valid-derivers 33)
   (optimize-store 34)
   (verify-store 35)
-  (built-in-builders 80))
+  (built-in-builders 80)
+  (substitute-urls 81))
 
 (define-enumerate-type hash-algo
   ;; hash.hh
@@ -1780,6 +1782,16 @@ (define-operation (clear-failed-paths (store-path-list items))
 This makes sense only when the daemon was started with '--cache-failures'."
   boolean)
 
+(define substitute-urls
+  (let ((urls (operation (substitute-urls)
+                         #f
+                         string-list)))
+    (lambda (store)
+      "Return the list of currently configured substitutes URLs for STORE, or
+#f if the daemon is too old and does not implement this RPC."
+      (and (>= (store-connection-version store) #x164)
+           (urls store)))))
+
 
 ;;;
 ;;; Per-connection caches.
diff --git a/nix/libstore/worker-protocol.hh b/nix/libstore/worker-protocol.hh
index ea67b10a5b..ef259db2a0 100644
--- a/nix/libstore/worker-protocol.hh
+++ b/nix/libstore/worker-protocol.hh
@@ -6,7 +6,7 @@ namespace nix {
 #define WORKER_MAGIC_1 0x6e697863
 #define WORKER_MAGIC_2 0x6478696f
 
-#define PROTOCOL_VERSION 0x163
+#define PROTOCOL_VERSION 0x164
 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00)
 #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)
 
@@ -44,7 +44,8 @@ typedef enum {
     wopQueryValidDerivers = 33,
     wopOptimiseStore = 34,
     wopVerifyStore = 35,
-    wopBuiltinBuilders = 80
+    wopBuiltinBuilders = 80,
+    wopSubstituteURLs = 81
 } WorkerOp;
 
 
diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
index 497de11a04..4cb05c802e 100644
--- a/nix/nix-daemon/nix-daemon.cc
+++ b/nix/nix-daemon/nix-daemon.cc
@@ -736,6 +736,23 @@ static void performOp(bool trusted, unsigned int clientVersion,
 	break;
     }
 
+    case wopSubstituteURLs: {
+	startWork();
+	Strings urls;
+	if (settings.get("build-use-substitutes", std::string("false")) == "true") {
+	    /* First check the client-provided substitute URLs, then those
+	       passed to the daemon.  */
+	    auto str = settings.get("untrusted-substitute-urls",  std::string(""));
+	    if (str.empty()) {
+		str = settings.get("substitute-urls",  std::string(""));
+	    }
+	    urls = tokenizeString<Strings>(str);
+	}
+	stopWork();
+	writeStrings(urls, to);
+	break;
+    }
+
     default:
         throw Error(format("invalid operation %1%") % op);
     }
diff --git a/tests/store.scm b/tests/store.scm
index 5df28adf0d..45948f4f43 100644
--- a/tests/store.scm
+++ b/tests/store.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2012-2021, 2023 Ludovic Courtès <ludo <at> gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -105,7 +105,28 @@ (define %shell
               "/283gqy39v3g9dxjy26rynl0zls82fmcg-guile-2.0.7/bin/guile")))
        (not (direct-store-path? (%store-prefix)))))
 
-(test-skip (if %store 0 15))
+(test-skip (if %store 0 18))
+
+(test-equal "substitute-urls, default"
+  (list (getenv "GUIX_BINARY_SUBSTITUTE_URL"))
+  (with-store store
+    (set-build-options store #:use-substitutes? #t)
+    (substitute-urls store)))
+
+(test-equal "substitute-urls, client-specified URLs"
+  '("http://substitutes.example.org"
+    "http://other.example.org")
+  (with-store store
+    (set-build-options store #:use-substitutes? #t
+                       #:substitute-urls '("http://substitutes.example.org"
+                                           "http://other.example.org"))
+    (substitute-urls store)))
+
+(test-equal "substitute-urls, disabled"
+  '()
+  (with-store store
+    (set-build-options store #:use-substitutes? #f)
+    (substitute-urls store)))
 
 (test-equal "profiles/per-user exists and is not writable"
   #o755
-- 
2.41.0

Message #56 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 67072 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH v2 2/4] challenge: Use the same substitute URLs as guix-daemon.
Date: Mon,  4 Dec 2023 15:15:43 +0100

* guix/scripts/challenge.scm (%default-options): Remove ‘substitute-urls’.
(guix-challenge): Call ‘substitute-urls’ when OPTS doesn’t have it.  Warn
when ‘substitute-urls’ returns #f.

Change-Id: I49be0e89404c1889970a3430967fbb3498d35d99
---
 guix/scripts/challenge.scm | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/guix/scripts/challenge.scm b/guix/scripts/challenge.scm
index 01e2f9a2b2..d38171b868 100644
--- a/guix/scripts/challenge.scm
+++ b/guix/scripts/challenge.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2015-2017, 2019-2022 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2015-2017, 2019-2023 Ludovic Courtès <ludo <at> gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -504,7 +504,6 @@ (define %options
 
 (define %default-options
   `((system . ,(%current-system))
-    (substitute-urls . ,%default-substitute-urls)
     (difference-report . ,report-differing-files)))
 
 
@@ -539,7 +538,13 @@ (define-command (guix-challenge . args)
                             (G_ "no arguments specified, nothing to do~%"))
                            (exit 0))
                           (x
-                           files))))
+                           files)))
+                 (urls (or urls
+                           (substitute-urls store)
+                           (begin
+                             (warning (G_ "could not determine current \
+substitute URLs; using defaults~%"))
+                             %default-substitute-urls))))
              (set-build-options store
                                 #:use-substitutes? #f)
 
-- 
2.41.0

Message #59 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 67072 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH v2 3/4] weather: Use the same substitute URLs as guix-daemon.
Date: Mon,  4 Dec 2023 15:15:44 +0100

* guix/scripts/weather.scm (%default-options): Remove ‘substitute-urls’.
(guix-weather): Call ‘substitute-urls’ when OPTS doesn’t have it.  Warn
when ‘substitute-urls’ returns #f.
* doc/guix.texi (Invoking guix weather): Adjust accordingly.

Change-Id: I3e9100074f2ad559e5c408660db70430d64f2bef
---
 doc/guix.texi            |  5 +++--
 guix/scripts/weather.scm | 12 +++++++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 1fd2e21608..74739c5392 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -16504,8 +16504,9 @@ Invoking guix weather
 @table @code
 @item --substitute-urls=@var{urls}
 @var{urls} is the space-separated list of substitute server URLs to
-query.  When this option is omitted, the default set of substitute
-servers is queried.
+query.  When this option is omitted, the URLs specified with the
+@option{--substitute-urls} option of @command{guix-daemon} are used or,
+as a last resort, the default set of substitute URLs.
 
 @item --system=@var{system}
 @itemx -s @var{system}
diff --git a/guix/scripts/weather.scm b/guix/scripts/weather.scm
index 140df3435f..7e302fcea7 100644
--- a/guix/scripts/weather.scm
+++ b/guix/scripts/weather.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2017-2022 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2017-2023 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2017 Ricardo Wurmus <rekado <at> elephly.net>
 ;;; Copyright © 2018 Kyle Meyer <kyle <at> kyleam.com>
 ;;; Copyright © 2020 Simon Tournier <zimon.toutoune <at> gmail.com>
@@ -391,7 +391,7 @@ (define %options
          %standard-native-build-options))
 
 (define %default-options
-  `((substitute-urls . ,%default-substitute-urls)))
+  '())
 
 (define (load-manifest file)
   "Load the manifest from FILE and return the list of packages it refers to."
@@ -582,7 +582,13 @@ (define-command (guix-weather . args)
       (let* ((opts     (parse-command-line args %options
                                            (list %default-options)
                                            #:build-options? #f))
-             (urls     (assoc-ref opts 'substitute-urls))
+             (urls     (or (assoc-ref opts 'substitute-urls)
+                           (with-store store
+                             (substitute-urls store))
+                           (begin
+                             (warning (G_ "could not determine current \
+substitute URLs; using defaults~%"))
+                             %default-substitute-urls)))
              (systems  (match (filter-map (match-lambda
                                             (('system . system) system)
                                             (_ #f))
-- 
2.41.0

Message #62 received at 67072 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 67072 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH v2 4/4] weather: Report unauthorized substitute servers.
Date: Mon,  4 Dec 2023 15:15:45 +0100

The goal is to make it easier to diagnose substitute
misconfiguration (where we’re passing a substitute URL whose
corresponding key is not authorized).

Suggested by Emmanuel Agullo.

* guix/scripts/weather.scm (check-narinfo-authorization): New procedure.
(report-server-coverage): Use it.
* doc/guix.texi (Invoking guix weather): Document it.
(Getting Substitutes from Other Servers): Add “Troubleshooting” frame.

Change-Id: I0a049c39eefb10d6a06634c8b16aa86902769791
---
 doc/guix.texi            | 21 +++++++++++++++-
 guix/scripts/weather.scm | 53 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 74739c5392..f1780df5a1 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -4058,6 +4058,7 @@ Substitute Server Authorization
 
 @node Getting Substitutes from Other Servers
 @subsection Getting Substitutes from Other Servers
+@c Note: This section name appears in a hint printed by 'guix weather'.
 
 @cindex substitute servers, adding more
 Guix can look up and fetch substitutes from several servers.  This is
@@ -4157,6 +4158,21 @@ Getting Substitutes from Other Servers
 substitute lookup can be slowed down if too many servers need to be
 contacted.
 
+@quotation Troubleshooting
+To diagnose problems, you can run @command{guix weather}.  For example,
+running:
+
+@example
+guix weather coreutils
+@end example
+
+@noindent
+not only tells you which of the currently-configured servers has
+substitutes for the @code{coreutils} package, it also reports whether
+one of these servers is unauthorized.  @xref{Invoking guix weather}, for
+more information.
+@end quotation
+
 Note that there are also situations where one may want to add the URL of
 a substitute server @emph{without} authorizing its key.
 @xref{Substitute Authentication}, to understand this fine point.
@@ -16437,7 +16453,10 @@ Invoking guix weather
 specified servers so you can have an idea of whether you'll be grumpy
 today.  It can sometimes be useful info as a user, but it is primarily
 useful to people running @command{guix publish} (@pxref{Invoking guix
-publish}).
+publish}).  Sometimes substitutes @emph{are} available but they are not
+authorized on your system; @command{guix weather} reports it so you can
+authorize them if you want (@pxref{Getting Substitutes from Other
+Servers}).
 
 @cindex statistics, for substitutes
 @cindex availability of substitutes
diff --git a/guix/scripts/weather.scm b/guix/scripts/weather.scm
index 7e302fcea7..2f8985593d 100644
--- a/guix/scripts/weather.scm
+++ b/guix/scripts/weather.scm
@@ -35,6 +35,8 @@ (define-module (guix scripts weather)
   #:use-module ((guix build utils) #:select (every*))
   #:use-module (guix substitutes)
   #:use-module (guix narinfo)
+  #:use-module (guix pki)
+  #:autoload   (gcrypt pk-crypto) (canonical-sexp->string)
   #:use-module (guix http-client)
   #:use-module (guix ci)
   #:use-module (guix sets)
@@ -185,6 +187,44 @@ (define (store-item-system store item)
     (()
      #f)))
 
+(define (check-narinfo-authorization narinfo)
+  "Print a warning when NARINFO is not signed by an authorized key."
+  (define acl
+    (catch 'system-error
+      (lambda ()
+        (current-acl))
+      (lambda args
+        (warning (G_ "could not read '~a': ~a~%")
+                 %acl-file (strerror (system-error-errno args)))
+        (warning (G_ "'~a' is unreadable, cannot determine whether \
+substitutes are authorized~%")
+                 %acl-file)
+        #f)))
+
+  (unless (or (not acl) (valid-narinfo? narinfo acl))
+    (warning (G_ "substitutes from '~a' are unauthorized~%")
+             (narinfo-uri-base narinfo))
+    ;; The "all substitutes" below reflects the fact that, in reality, it *is*
+    ;; possible to download "unauthorized" substitutes, as long as they match
+    ;; authorized substitutes.
+    (display-hint (G_ "To authorize all substitutes from @uref{~a} to be
+downloaded, the following command needs to be run as root:
+
+@example
+guix archive --authorize <<EOF
+~a
+EOF
+@end example
+
+Alternatively, on Guix System, you can add the signing key above to the
+@code{authorized-keys} field of @code{guix-configuration}.
+
+See \"Getting Substitutes from Other Servers\" in the manual for more
+information.")
+                  (narinfo-uri-base narinfo)
+                  (canonical-sexp->string
+                   (signature-subject (narinfo-signature narinfo))))))
+
 (define* (report-server-coverage server items
                                  #:key display-missing?)
   "Report the subset of ITEMS available as substitutes on SERVER.
@@ -204,6 +244,12 @@ (define* (report-server-coverage server items
                     #:make-progress-reporter
                     (lambda* (total #:key url #:allow-other-keys)
                       (progress-reporter/bar total)))))
+    (match narinfos
+      (() #f)
+      ((narinfo . _)
+       ;; Help diagnose missing substitute authorizations.
+       (check-narinfo-authorization narinfo)))
+
     (let ((obtained  (length narinfos))
           (requested (length items))
           (missing   (lset-difference string=?
@@ -586,8 +632,11 @@ (define-command (guix-weather . args)
                            (with-store store
                              (substitute-urls store))
                            (begin
-                             (warning (G_ "could not determine current \
-substitute URLs; using defaults~%"))
+                             ;; Could not determine the daemon's current
+                             ;; substitute URLs, presumably because it's too
+                             ;; old.
+                             (warning (G_ "using default \
+substitute URLs~%"))
                              %default-substitute-urls)))
              (systems  (match (filter-map (match-lambda
                                             (('system . system) system)
-- 
2.41.0

Message #67 received at 67072-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 67072-done <at> debbugs.gnu.org
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#67072] [PATCH v2 0/4] Helping diagnose substitute setup
 issues
Date: Mon, 11 Dec 2023 23:52:10 +0100

Hi,

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

> Changes since v1:
>
>   • Gracefully handle unreadable /etc/guix/acl file in ‘guix weather’;
>
>   • Simplify ‘guix weather’ warning message when the daemon does not
>     support the ‘substitute-urls’ RPC.

I went ahead and pushed these changes:

  4348947c74 weather: Report unauthorized substitute servers.
  7e11369586 weather: Use the same substitute URLs as guix-daemon.
  f63a8c5ca2 challenge: Use the same substitute URLs as guix-daemon.
  1e47148f46 daemon: Implement ‘substitute-urls’ RPC.

Thanks again for your feedback!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.