unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues
@ 2023-11-11 11:03 Ludovic Courtès
  2023-11-11 11:06 ` [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC Ludovic Courtès
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-11-11 11:03 UTC (permalink / raw)
  To: 67072
  Cc: Emmanuel Agullo, Ludovic Courtès, Simon Tournier,
	Christopher Baines, Josselin Poiret, Ludovic Courtès,
	Mathieu Othacehe, Ricardo Wurmus, Simon Tournier,
	Tobias Geerinckx-Rice

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





^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC.
  2023-11-11 11:03 [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
@ 2023-11-11 11:06 ` Ludovic Courtès
  2023-11-28 12:09   ` Simon Tournier
  2023-11-11 11:06 ` [bug#67072] [PATCH 2/4] challenge: Use the same substitute URLs as guix-daemon Ludovic Courtès
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 21+ messages in thread
From: Ludovic Courtès @ 2023-11-11 11:06 UTC (permalink / raw)
  To: 67072
  Cc: Ludovic Courtès, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

* 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@gnu.org>
+;;; Copyright © 2012-2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2018 Jan Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2019, 2020 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2020 Florian Pelz <pelzflorian@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)))))
+
 \f
 ;;;
 ;;; 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@gnu.org>
+;;; Copyright © 2012-2021, 2023 Ludovic Courtès <ludo@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





^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 2/4] challenge: Use the same substitute URLs as guix-daemon.
  2023-11-11 11:03 [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
  2023-11-11 11:06 ` [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC Ludovic Courtès
@ 2023-11-11 11:06 ` Ludovic Courtès
  2023-11-11 11:06 ` [bug#67072] [PATCH 3/4] weather: " Ludovic Courtès
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-11-11 11:06 UTC (permalink / raw)
  To: 67072
  Cc: Ludovic Courtès, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

* 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@gnu.org>
+;;; Copyright © 2015-2017, 2019-2023 Ludovic Courtès <ludo@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)))
 
 \f
@@ -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





^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 3/4] weather: Use the same substitute URLs as guix-daemon.
  2023-11-11 11:03 [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
  2023-11-11 11:06 ` [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC Ludovic Courtès
  2023-11-11 11:06 ` [bug#67072] [PATCH 2/4] challenge: Use the same substitute URLs as guix-daemon Ludovic Courtès
@ 2023-11-11 11:06 ` Ludovic Courtès
  2023-11-11 11:06 ` [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute servers Ludovic Courtès
  2023-11-27 17:21 ` [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
  4 siblings, 0 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-11-11 11:06 UTC (permalink / raw)
  To: 67072
  Cc: Ludovic Courtès, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

* 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@gnu.org>
+;;; Copyright © 2017-2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2017 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2018 Kyle Meyer <kyle@kyleam.com>
 ;;; Copyright © 2020 Simon Tournier <zimon.toutoune@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





^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute servers.
  2023-11-11 11:03 [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
                   ` (2 preceding siblings ...)
  2023-11-11 11:06 ` [bug#67072] [PATCH 3/4] weather: " Ludovic Courtès
@ 2023-11-11 11:06 ` Ludovic Courtès
  2023-11-28 13:14   ` Simon Tournier
  2023-11-27 17:21 ` [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
  4 siblings, 1 reply; 21+ messages in thread
From: Ludovic Courtès @ 2023-11-11 11:06 UTC (permalink / raw)
  To: 67072
  Cc: Ludovic Courtès, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

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





^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues
  2023-11-11 11:03 [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
                   ` (3 preceding siblings ...)
  2023-11-11 11:06 ` [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute servers Ludovic Courtès
@ 2023-11-27 17:21 ` Ludovic Courtès
  2023-11-28 13:17   ` Simon Tournier
  2023-11-30 10:11   ` Emmanuel Agullo
  4 siblings, 2 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-11-27 17:21 UTC (permalink / raw)
  To: 67072
  Cc: Josselin Poiret, Simon Tournier, Mathieu Othacehe,
	Tobias Geerinckx-Rice, Ricardo Wurmus, Emmanuel Agullo,
	Christopher Baines

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@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




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC.
  2023-11-11 11:06 ` [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC Ludovic Courtès
@ 2023-11-28 12:09   ` Simon Tournier
  2023-12-02 10:13     ` Ludovic Courtès
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Tournier @ 2023-11-28 12:09 UTC (permalink / raw)
  To: Ludovic Courtès, 67072
  Cc: Josselin Poiret, Mathieu Othacehe, Ludovic Courtès,
	Tobias Geerinckx-Rice, Ricardo Wurmus, Christopher Baines

Hi,

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

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

Out of curiosity, why 18?

Cheers,
simon




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute servers.
  2023-11-11 11:06 ` [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute servers Ludovic Courtès
@ 2023-11-28 13:14   ` Simon Tournier
  2023-12-02 10:20     ` Ludovic Courtès
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Tournier @ 2023-11-28 13:14 UTC (permalink / raw)
  To: Ludovic Courtès, 67072
  Cc: Josselin Poiret, Mathieu Othacehe, Ludovic Courtès,
	Tobias Geerinckx-Rice, Ricardo Wurmus, Christopher Baines

Hi,

On Sat, 11 Nov 2023 at 12:06, Ludovic Courtès <ludo@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






^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues
  2023-11-27 17:21 ` [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
@ 2023-11-28 13:17   ` Simon Tournier
  2023-11-30 10:11   ` Emmanuel Agullo
  1 sibling, 0 replies; 21+ messages in thread
From: Simon Tournier @ 2023-11-28 13:17 UTC (permalink / raw)
  To: Ludovic Courtès, 67072
  Cc: Josselin Poiret, Mathieu Othacehe, Tobias Geerinckx-Rice,
	Ricardo Wurmus, Emmanuel Agullo, Christopher Baines

Hi Ludo,

Sorry for the delay.

On Mon, 27 Nov 2023 at 18:21, Ludovic Courtès <ludovic.courtes@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




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues
  2023-11-27 17:21 ` [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
  2023-11-28 13:17   ` Simon Tournier
@ 2023-11-30 10:11   ` Emmanuel Agullo
  2023-11-30 10:28     ` Ludovic Courtès
  1 sibling, 1 reply; 21+ messages in thread
From: Emmanuel Agullo @ 2023-11-30 10:11 UTC (permalink / raw)
  To: Ludovic Courtes
  Cc: Josselin Poiret, 67072, zimoun, Mathieu Othacehe,
	Tobias Geerinckx-Rice, Ricardo Wurmus, Christopher Baines

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




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues
  2023-11-30 10:11   ` Emmanuel Agullo
@ 2023-11-30 10:28     ` Ludovic Courtès
  0 siblings, 0 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-11-30 10:28 UTC (permalink / raw)
  To: Emmanuel Agullo
  Cc: Josselin Poiret, 67072, zimoun, Mathieu Othacehe,
	Tobias Geerinckx-Rice, Ricardo Wurmus, Christopher Baines

Hi Emmanuel,

Emmanuel Agullo <emmanuel.agullo@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’.




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC.
  2023-11-28 12:09   ` Simon Tournier
@ 2023-12-02 10:13     ` Ludovic Courtès
  2023-12-02 13:16       ` Simon Tournier
  0 siblings, 1 reply; 21+ messages in thread
From: Ludovic Courtès @ 2023-12-02 10:13 UTC (permalink / raw)
  To: Simon Tournier
  Cc: Josselin Poiret, 67072, Mathieu Othacehe, Tobias Geerinckx-Rice,
	Ricardo Wurmus, Christopher Baines

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

> On Sat, 11 Nov 2023 at 12:06, Ludovic Courtès <ludo@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’.




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute servers.
  2023-11-28 13:14   ` Simon Tournier
@ 2023-12-02 10:20     ` Ludovic Courtès
  2023-12-02 13:31       ` Simon Tournier
                         ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-12-02 10:20 UTC (permalink / raw)
  To: Simon Tournier
  Cc: Josselin Poiret, 67072, Mathieu Othacehe, Tobias Geerinckx-Rice,
	Ricardo Wurmus, Christopher Baines

Hi Simon,

Simon Tournier <zimon.toutoune@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’.




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC.
  2023-12-02 10:13     ` Ludovic Courtès
@ 2023-12-02 13:16       ` Simon Tournier
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Tournier @ 2023-12-02 13:16 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Josselin Poiret, 67072, Mathieu Othacehe, Tobias Geerinckx-Rice,
	Ricardo Wurmus, Christopher Baines

Hi,

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

> Because we’re adding 3 tests.

Aah!  Thanks for explaining.

Cheers,
simon




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute servers.
  2023-12-02 10:20     ` Ludovic Courtès
@ 2023-12-02 13:31       ` Simon Tournier
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 0/4] Helping diagnose substitute setup issues Ludovic Courtès
                         ` (4 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Simon Tournier @ 2023-12-02 13:31 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Josselin Poiret, 67072, Mathieu Othacehe, Tobias Geerinckx-Rice,
	Ricardo Wurmus, Christopher Baines

Hi Ludo,

On sam., 02 déc. 2023 at 11:20, Ludovic Courtès <ludo@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




^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH v2 0/4] Helping diagnose substitute setup issues
  2023-12-02 10:20     ` Ludovic Courtès
  2023-12-02 13:31       ` Simon Tournier
@ 2023-12-04 14:15       ` Ludovic Courtès
  2023-12-11 22:52         ` bug#67072: " Ludovic Courtès
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 1/4] daemon: Implement ‘substitute-urls’ RPC Ludovic Courtès
                         ` (3 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Ludovic Courtès @ 2023-12-04 14:15 UTC (permalink / raw)
  To: 67072
  Cc: Ludovic Courtès, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

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





^ permalink raw reply	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH v2 1/4] daemon: Implement ‘substitute-urls’ RPC.
  2023-12-02 10:20     ` Ludovic Courtès
  2023-12-02 13:31       ` Simon Tournier
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 0/4] Helping diagnose substitute setup issues Ludovic Courtès
@ 2023-12-04 14:15       ` Ludovic Courtès
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 2/4] challenge: Use the same substitute URLs as guix-daemon Ludovic Courtès
                         ` (2 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-12-04 14:15 UTC (permalink / raw)
  To: 67072
  Cc: Ludovic Courtès, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

* 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@gnu.org>
+;;; Copyright © 2012-2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2018 Jan Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2019, 2020 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2020 Florian Pelz <pelzflorian@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)))))
+
 \f
 ;;;
 ;;; 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@gnu.org>
+;;; Copyright © 2012-2021, 2023 Ludovic Courtès <ludo@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





^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH v2 2/4] challenge: Use the same substitute URLs as guix-daemon.
  2023-12-02 10:20     ` Ludovic Courtès
                         ` (2 preceding siblings ...)
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 1/4] daemon: Implement ‘substitute-urls’ RPC Ludovic Courtès
@ 2023-12-04 14:15       ` Ludovic Courtès
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 3/4] weather: " Ludovic Courtès
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 4/4] weather: Report unauthorized substitute servers Ludovic Courtès
  5 siblings, 0 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-12-04 14:15 UTC (permalink / raw)
  To: 67072
  Cc: Ludovic Courtès, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

* 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@gnu.org>
+;;; Copyright © 2015-2017, 2019-2023 Ludovic Courtès <ludo@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)))
 
 \f
@@ -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





^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH v2 3/4] weather: Use the same substitute URLs as guix-daemon.
  2023-12-02 10:20     ` Ludovic Courtès
                         ` (3 preceding siblings ...)
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 2/4] challenge: Use the same substitute URLs as guix-daemon Ludovic Courtès
@ 2023-12-04 14:15       ` Ludovic Courtès
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 4/4] weather: Report unauthorized substitute servers Ludovic Courtès
  5 siblings, 0 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-12-04 14:15 UTC (permalink / raw)
  To: 67072
  Cc: Ludovic Courtès, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

* 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@gnu.org>
+;;; Copyright © 2017-2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2017 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2018 Kyle Meyer <kyle@kyleam.com>
 ;;; Copyright © 2020 Simon Tournier <zimon.toutoune@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





^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [bug#67072] [PATCH v2 4/4] weather: Report unauthorized substitute servers.
  2023-12-02 10:20     ` Ludovic Courtès
                         ` (4 preceding siblings ...)
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 3/4] weather: " Ludovic Courtès
@ 2023-12-04 14:15       ` Ludovic Courtès
  5 siblings, 0 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-12-04 14:15 UTC (permalink / raw)
  To: 67072
  Cc: Ludovic Courtès, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice

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





^ permalink raw reply related	[flat|nested] 21+ messages in thread

* bug#67072: [PATCH v2 0/4] Helping diagnose substitute setup issues
  2023-12-04 14:15       ` [bug#67072] [PATCH v2 0/4] Helping diagnose substitute setup issues Ludovic Courtès
@ 2023-12-11 22:52         ` Ludovic Courtès
  0 siblings, 0 replies; 21+ messages in thread
From: Ludovic Courtès @ 2023-12-11 22:52 UTC (permalink / raw)
  To: 67072-done
  Cc: Josselin Poiret, Simon Tournier, Mathieu Othacehe,
	Tobias Geerinckx-Rice, Ricardo Wurmus, Christopher Baines

Hi,

Ludovic Courtès <ludo@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’.




^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-12-11 22:52 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-11 11:03 [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
2023-11-11 11:06 ` [bug#67072] [PATCH 1/4] daemon: Implement ‘substitute-urls’ RPC Ludovic Courtès
2023-11-28 12:09   ` Simon Tournier
2023-12-02 10:13     ` Ludovic Courtès
2023-12-02 13:16       ` Simon Tournier
2023-11-11 11:06 ` [bug#67072] [PATCH 2/4] challenge: Use the same substitute URLs as guix-daemon Ludovic Courtès
2023-11-11 11:06 ` [bug#67072] [PATCH 3/4] weather: " Ludovic Courtès
2023-11-11 11:06 ` [bug#67072] [PATCH 4/4] weather: Report unauthorized substitute servers Ludovic Courtès
2023-11-28 13:14   ` Simon Tournier
2023-12-02 10:20     ` Ludovic Courtès
2023-12-02 13:31       ` Simon Tournier
2023-12-04 14:15       ` [bug#67072] [PATCH v2 0/4] Helping diagnose substitute setup issues Ludovic Courtès
2023-12-11 22:52         ` bug#67072: " Ludovic Courtès
2023-12-04 14:15       ` [bug#67072] [PATCH v2 1/4] daemon: Implement ‘substitute-urls’ RPC Ludovic Courtès
2023-12-04 14:15       ` [bug#67072] [PATCH v2 2/4] challenge: Use the same substitute URLs as guix-daemon Ludovic Courtès
2023-12-04 14:15       ` [bug#67072] [PATCH v2 3/4] weather: " Ludovic Courtès
2023-12-04 14:15       ` [bug#67072] [PATCH v2 4/4] weather: Report unauthorized substitute servers Ludovic Courtès
2023-11-27 17:21 ` [bug#67072] [PATCH 0/4] Helping diagnose substitute setup issues Ludovic Courtès
2023-11-28 13:17   ` Simon Tournier
2023-11-30 10:11   ` Emmanuel Agullo
2023-11-30 10:28     ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).