Subject: Re: [bug#44800] [PATCH v2 3/3] Use substitute servers on the local network.

From: Ludovic Courtès <[email-masked]>
Date: Fri, 27 Nov 2020 18:37:42 +0100

Mathieu Othacehe <[email-masked]> skribis:

> * guix/scripts/discover.scm: New file.
> * Makefile.am (MODULES): Add it.
> * nix/nix-daemon/guix-daemon.cc (options): Add "use-local-publish" option,
> (parse-opt): parse it,
> (main): start "guix discover" process when the option is set.
> * nix/libstore/globals.hh (Settings): Add "useLocalPublish" public member.
> * nix/libstore/globals.cc (Settings): Initialize it.
> * guix/scripts/substitute.scm (%local-substitute-urls): New variable,
> (substitute-urls): add it.
> * gnu/services/base.scm (<guix-configuration>): Add "use-local-publish?"
> field,
> (guix-shepherd-service): honor it.
> * doc/guix.texi (Invoking guix-daemon): Document "use-local-publish" option,
> (Base Services): ditto.

[...]

> +@item --use-local-publish[=yes|no]
> +Whether to use publish servers discovered a the local network, using
> +Avahi, for substitutution.

How about ‘--discover-substitute-servers’ or ‘--discover-substitutes’ or
even ‘--discover’?

s/publish servers/substitute servers/

I think we need a note about the performance, security, and privacy
implications of this here, namely:

  0. It might be faster/less expensive than fetching from remote
     servers; 

  1. There are no security risks, only genuine substitutes will be used
     (add cross-ref);

  2. An attacker advertising ‘guix publish’ on your LAN cannot serve you
     malicious binaries, but they can learn what software you’re
     installing.

  3. Servers may serve substitute over HTTP, unencrypted, so anyone on
     the LAN can see what software you’re installing.

IWBN to have an action of the Shepherd service to turn it on and off;
you might want to do that depending on how much you trust the LAN you’re
on.  (That can come later though.)

> +++ b/gnu/services/base.scm
> @@ -1529,6 +1529,8 @@ archive' public keys, with GUIX."
>                      (default 0))
>    (log-compression  guix-configuration-log-compression
>                      (default 'bzip2))
> +  (use-local-publish? guix-configuration-use-local-publish?
> +                      (default #f))

Same here.

> +(define %publish-services
> +  ;; Set of discovered publish services.
> +  (make-hash-table))
> +
> +(define (publish-file cache-directory)
> +  "Return the name of the file storing the discovered publish services inside
> +CACHE-DIRECTORY."
> +  (let ((directory (string-append cache-directory "/discover")))
> +    (string-append directory "/publish")))
> +
> +(define %publish-file
> +  (make-parameter (publish-file %state-directory)))
> +
> +(define* (write-publish-file #:key (file (%publish-file)))
> +  "Dump the content of %PUBLISH-SERVICES hash table into FILE.  Use a write
> +lock on FILE to synchronize with any potential readers."

Aren’t we partly duplicating what avahi-daemon’s already doing?
avahi-daemon maintains a list of currently valid advertisements, which
can be seen with:

  avahi-browse --cache _workstation._tcp

However, that cache first needs to be initialized by running the same
command without ‘--cache’.  Hmm, maybe there’s no other choice.  I
wonder how others deal with that.

> +(define-command (guix-discover . args)
> +  (category plumbing)

Should be “internal” IMO.

> +++ b/nix/libstore/globals.cc
> @@ -35,6 +35,7 @@ Settings::Settings()
>      maxSilentTime = 0;
>      buildTimeout = 0;
>      useBuildHook = true;
> +    useLocalPublish = false;
>      printBuildTrace = false;
>      multiplexedBuildOutput = false;
>      reservedSize = 8 * 1024 * 1024;
> diff --git a/nix/libstore/globals.hh b/nix/libstore/globals.hh
> index 27616a2283..43653aef48 100644
> --- a/nix/libstore/globals.hh
> +++ b/nix/libstore/globals.hh
> @@ -116,6 +116,10 @@ struct Settings {
>         users want to disable this from the command-line. */
>      bool useBuildHook;
>  
> +    /* Whether to use publish servers found on the local network for
> +       substitution. */
> +    bool useLocalPublish;

I think you don’t even need to field here since the variable is only
used in guix-daemon.cc.

> +    case GUIX_OPT_USE_LOCAL_PUBLISH:
> +      settings.useLocalPublish = string_to_bool (arg);
> +      settings.set("use-local-publish", arg);
> +      break;

Just set a variable local to this file and that’s enough.  You still
need the second line so that (guix scripts substitute) knows whether it
should read the thing.

> diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
> index ddb885d344..16e8fe6106 100755
> --- a/guix/scripts/substitute.scm
> +++ b/guix/scripts/substitute.scm
> @@ -27,6 +27,7 @@
>    #:use-module (guix config)
>    #:use-module (guix records)
>    #:use-module ((guix serialization) #:select (restore-file))
> +  #:use-module (guix scripts discover)
>    #:use-module (gcrypt hash)
>    #:use-module (guix base32)
>    #:use-module (guix base64)
> @@ -1078,9 +1079,17 @@ found."
>       ;; daemon.
>       '("http://ci.guix.gnu.org"))))
>  
> +(define %local-substitute-urls
> +  ;; If the following option is passed to the daemon, use the substitutes list
> +  ;; provided by "guix discover" process.
> +  (if (find-daemon-option "use-local-publish")
> +      (read-publish-urls)
> +      '()))
> +
>  (define substitute-urls
>    ;; List of substitute URLs.
> -  (make-parameter %default-substitute-urls))
> +  (make-parameter (append %local-substitute-urls
> +                          %default-substitute-urls)))

As discussed on IRC, we should probably need to set an upper limit. on
the number of local substitute URLs.

Imagine: you’re at GuixCon 2021, there are 500 participants all of which
are running ‘guix publish --advertise’; every Guix operation leads to
everyone’s Guix talking to every other person’s Guix, the whole thing
gets slow as hell, 500 people staring at “updating list of substitutes”,
500 people eventually giving up and signing up for CONDACon.

Also, we must make sure ‘guix substitute’ gracefully handles disconnects
and servers still advertised but no longer around (timeouts etc.)

We’ll need real world tests to see how it behaves I think.  In the
meantime, we can describe it as a technology preview™ in the manual.

WDYT?

Ludo’.