GNU bug report logs - #43106
[PATCH] DRAFT services: childhurd: Support for setting secrets.

Previous Next

Full log

View this message in rfc822 format

From: Jan Nieuwenhuizen <janneke <at> gnu.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 43106 <at> debbugs.gnu.org
Subject: [bug#43106] [PATCH v3 2/2] services: childhurd: Support installing secrets from the host.
Date: Tue, 01 Sep 2020 15:40:38 +0200

Ludovic Courtès writes:

> "Jan (janneke) Nieuwenhuizen" <janneke <at> gnu.org> skribis:
>
>> * gnu/system/examples/bare-hurd.tmpl (%hurd-os)[services]: Add secret-service.
>> * gnu/services/virtualization.scm (%hurd-vm-operating-system): Likewise.
>>  (hurd-vm-shepherd-service): Use it to install secrets.
>> * doc/guix.texi (The Hurd in a Virtual Machine): Document it.
>
> Yay, minor issues, but overall LGTM!

\o/

>>      (services (cons*
>> +               ;; Receive secret keys on port 1004, TCP.
>> +               (service secret-service-type 1004)
>
>
> [...]
>
>> +      (start
>> +       (with-imported-modules
>> +           (source-module-closure '((gnu build secret-service)
>> +                                    (guix build utils)))
>> +         #~(let ((spawn (make-forkexec-constructor #$vm-command)))
>> +           (lambda _
>> +             (let ((pid (spawn))
>> +                   (port #$(hurd-vm-port config %hurd-vm-secrets-port))
>> +                   (root #$(hurd-vm-configuration-secret-root config)))
>> +               (and root (directory-exists? root)
>> +                    (catch #t
>> +                      (lambda _
>> +                        (secret-service-send-secrets port root))

> In any case, we should assume that the VM is always running the secret
> service server, and thus call ‘secret-service-send-secrets’
> unconditionally (‘secret-service-send-secrets’ does (find-files root),
> which returns the empty list when ROOT doesn’t exist,

Yeah I was struggling a bit with this; the hurd-vm-service and the
childhurd must agree on the usage of secret-service.  That's why I came
up with this root-dir #f switch...but it's certainly simpler if we say
that it must always be there.  Let's see if we can get away with that!

So, I removed the root-dir checks and we always call
'secret-service-send-secrets', and changed the default from #f to

  (secret-root hurd-vm-configuration-secret-root        ;string
               (default "/etc/childhurd")))

where "/etc/childhurd" does not need to exist.

> Perhaps ‘hurd-vm-service-type’ should unconditionally extend (via
> ‘service-extension’) ‘secret-service-type’, just to ensure that Hurd VMs
> always include the secret service.

Eh, hurd-vm-service lives in the host, the secret-services lives in the
client; am I missing something?  ;-)

We could add a check for secret-service, possibly here

    (define (hurd-vm-disk-image config)
      "Return a disk-image for the Hurd according to CONFIG."
      (let ((os (hurd-vm-configuration-os config))
            (disk-size (hurd-vm-configuration-disk-size config)))
        (system-image
         (image
          (inherit hurd-disk-image)
          (size disk-size)
          (operating-system os)))))

and/or insert if it it's missing...seems a bit over the top to me?

> I think.)

Yes, it does, but then the default cannot be #f, it must be a string.
I'm picking "/etc/childurd" as a default that need not exist.

>> +                      (lambda (keys . args)
>
> Should be “key” (singular).

Oops :-)

>> +                        (format (current-error-port)
>> +                                "failed to send secrets: ~a ~s\n" key args)
>> +                        (kill pid)
>
> (kill (- pid)) to kill the whole process group (just in case).
>
> I’d remove the ‘format’ call and just re-throw the exception: shepherd
> should report it correctly.

Done!  Changed to unconditionally run

               (catch #t
                 (lambda _
                   (secret-service-send-secrets port root))
                 (lambda (key . args)
                   (kill (- pid) SIGTERM)
                   (apply throw key args)))
               pid)))))

>> +                            (service (@@ (gnu services virtualization)
>> +                                         secret-service-type) 5999))
>
> This is useful for testing but I wouldn’t commit it (in particular
> because the example would no longer work for people who’re just spawning
> the VM and not trying to feed it secrets over TCP).

Right, removed.

> That’s it, thanks a lot!

You too!
Janneke

-- 
Jan Nieuwenhuizen <janneke <at> gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

Previous Next

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