| Message ID | 20200829215726.3910-1-janneke@gnu.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#43106] DRAFT services: childhurd: Support for setting secrets. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
Jan Nieuwenhuizen writes: Hello, As discussed on IRC, version 3 follows. > Ludovic Courtès writes: >> "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis: >>> >>> +@example >>> +/etc/childhurd/etc/guix/signing-key.pub >>> +/etc/childhurd/etc/guix/signing-key.sec >>> +/etc/childhurd/etc/ssh/ssh_host_ed25519_key >>> +/etc/childhurd/etc/ssh/ssh_host_ecdsa_key >>> +/etc/childhurd/etc/ssh/ssh_host_ed25519_key.pub >>> +/etc/childhurd/etc/ssh/ssh_host_ecdsa_key.pub >>> +@end example >> >> Would it make sense to have a list of source/target pairs instead of a >> directory: >> >> (("/etc/childhurd/pubkey" . "/etc/guix/signing-key.pub") >> …) >> >> ? > > We could do that...I'm not opposed to it and in fact I thought about > something like this but then opted for the file system root idea because > I didn't see the need for adding this extra indirection. If you think > it's a good idea, sure. Postponed that for now, though. [this still open] Also, I think 5900 is a bad idea, qemu opens a server there. We could use ports 2222 (forwarded to 12222), as SSH only starts later -- but hmm. As this is all running as root anyway, I opted for 1004 (MI5). Greetings, Janneke Jan (janneke) Nieuwenhuizen (2): services: Add secret-service-type. services: childhurd: Support installing secrets from the host. doc/guix.texi | 21 +++++ gnu/build/secret-service.scm | 138 +++++++++++++++++++++++++++++ gnu/local.mk | 1 + gnu/services/virtualization.scm | 92 ++++++++++++++++--- gnu/system/examples/bare-hurd.tmpl | 20 +++-- 5 files changed, 251 insertions(+), 21 deletions(-) create mode 100644 gnu/build/secret-service.scm
Ludovic Courtès writes: Hello, > "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis: > >> This adds a "secret-service" that can be added to a Childhurd VM to receive >> out-of-band secrets (keys) sent from the host. >> >> Co-authored-by: Ludovic Courtès <ludo@gnu.org> >> >> * gnu/services/virtualization.scm (secret-service-activation): New procedure. >> (secret-service-type): New variable. >> * gnu/build/secret-service.scm: New file. >> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it. > > Very nice! Minor suggestions: Great! >> + (format (current-error-port) "secret-service-send-secrets\n") > > Perhaps write “sending secrets to ~a:~a...~%” or similar. Ok. >> + (let ((sock (socket AF_INET SOCK_STREAM 0)) >> + (addr (make-socket-address AF_INET INADDR_LOOPBACK port))) >> + ;; connect to wait for port >> + (let loop ((retry retry)) >> + (if (zero? retry) >> + (error "connecting to childhurd failed") > > s/childhurd/secret server/ Ah, sure. >> + (catch 'system-error >> + (lambda _ >> + (connect sock addr)) >> + (lambda (key . args) >> + (format (current-error-port) "connect failed: ~a ~s\n" key args) > > Perhaps remove print “retrying connection” (or similar), and re-throw > the exception when RETRY is zero, so that it goes through as is (and > thus you can remove the call to ‘error’ above.) Ah yes, changed it to (catch 'system-error (cut connect sock addr) (lambda (key . args) (when (zero? retry) (apply throw key args)) (format (current-error-port) "retrying connection~%") (sleep 1) (loop (1- retry))))) >> + ;; copy tree >> + (let* ((files (if secret-root (find-files secret-root) '())) >> + (files-sizes-modes (map file->file+size+mode files)) >> + (secrets `(secrets >> + (version 0) >> + (files ,files-sizes-modes)))) >> + (write secrets sock) >> + (for-each (compose (cute display <> sock) >> + (cute with-input-from-file <> read-string)) > > Instead of loading it all in memory, we can use ‘dump-port’ from (guix > build utils) here. Nice, changed to (for-each (compose (cute dump-port <> sock) (cute open-input-file <>)) files)))) > That’s it! Thanks for your suggestions, Janneke
Ludovic Courtès writes: > "Jan (janneke) Nieuwenhuizen" <janneke@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 writes: > Ludovic Courtès writes: > >> "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> skribis: > 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 [..] After aligning on IRC we decided this can be done later; pushed to master as 01cefb7a570d846476ff5cb05d3b1e3511db5d81, closing. Janneke
Hi! Jan Nieuwenhuizen <janneke@gnu.org> skribis: > Ludovic Courtès writes: [...] >> 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? ;-) Ah no, it’s me. :-) > 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? Yes, exactly. We could pass ‘os’ through ‘secret-service-operating-system’, where: (define (secret-service-operating-system os) (operating-system (inherit os) (services (cons (service secret-service-type) (operating-system-user-services os))))) (A similar pattern is found in ‘virtualized-operating-system’ and ‘containerized-operating-system’.) Thanks for these patches! Ludo’.
diff --git a/doc/guix.texi b/doc/guix.texi index 0b79a49814..334ee5e05c 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -25119,6 +25119,7 @@ Return the name of @var{platform}---a string such as @code{"arm"}. @cindex @code{hurd} @cindex the Hurd +@cindex childhurd Service @code{hurd-vm} provides support for running GNU/Hurd in a virtual machine (VM), a so-called ``Childhurd''. The virtual machine is @@ -25200,6 +25201,24 @@ with forwarded ports <vnc-port>: @code{(+ 15900 (* 1000 @var{ID}))} @end example +@item @code{secret-root} (default: @code{#f}) +If set, the root directory with out-of-band secrets to be injected into +the childhurd once it runs. Childhurds are volatile which means that on +every startup, secrets such as the SSH host keys and Guix signing key +are recreated. + +Typical use is setting @code{secret-root} to @code{"/etc/childhurd"} +pointing at a tree of non-volatile secrets like so + +@example +/etc/childhurd/etc/guix/signing-key.pub +/etc/childhurd/etc/guix/signing-key.sec +/etc/childhurd/etc/ssh/ssh_host_ed25519_key +/etc/childhurd/etc/ssh/ssh_host_ecdsa_key +/etc/childhurd/etc/ssh/ssh_host_ed25519_key.pub +/etc/childhurd/etc/ssh/ssh_host_ecdsa_key.pub +@end example + @end table @end deftp diff --git a/gnu/build/childhurd.scm b/gnu/build/childhurd.scm new file mode 100644 index 0000000000..87c5cc0cd0 --- /dev/null +++ b/gnu/build/childhurd.scm @@ -0,0 +1,77 @@ +;;; GNU Guix --- Functional package management for GNU +;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org> +;;; +;;; This file is part of GNU Guix. +;;; +;;; GNU Guix is free software; you can redistribute it and/or modify it +;;; under the terms of the GNU General Public License as published by +;;; the Free Software Foundation; either version 3 of the License, or (at +;;; your option) any later version. +;;; +;;; GNU Guix is distributed in the hope that it will be useful, but +;;; WITHOUT ANY WARRANTY; without even the implied warranty of +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;;; GNU General Public License for more details. +;;; +;;; You should have received a copy of the GNU General Public License +;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>. + +(define-module (gnu build childhurd) + #:use-module (ice-9 rdelim) + #:use-module (guix build utils) + + ;; #:use-module (ssh auth) + ;; #:use-module (ssh channel) + ;; #:use-module (ssh session) + ;; #:use-module (ssh sftp) + + #:autoload (ssh auth) (userauth-password!) + #:autoload (ssh channel) (make-channel + channel-open-session channel-request-exec + channel-get-exit-status) + #:autoload (ssh session) (make-session connect! disconnect!) + #:autoload (ssh sftp) (make-sftp-session + call-with-remote-output-file sftp-chmod) + + #:export (hurd-vm-copy-secrets)) + +;;; Commentary: +;;; +;;; Utility procedures for a childhurd. +;;; +;;; Code: + +(define* (hurd-vm-copy-secrets port secret-root #:key (retry 20)) + "Copy all files under SECRET-ROOT using ssh to childhurd at local PORT." + (format (current-error-port) "hurd-vm-copy-secrets\n") + (let ((session (make-session #:host "127.0.0.1" #:port port + #:user "root"))) + (let loop ((result (connect! session)) (retry retry)) + (unless (equal? result 'ok) + (format (current-error-port) "Waiting for childhurd...\n") + (when (zero? retry) + (error "Could not connect childhurd" session result)) + (sleep 1) + (disconnect! session) + (loop (connect! session) (1- retry)))) + (let ((result (userauth-password! session ""))) + (unless (equal? result 'success) + (error "Could not set userauth-password" session result))) + (let ((sftp-session (make-sftp-session session))) + (define (copy-file source) + (let ((text (with-input-from-file source read-string)) + (mode (stat:mode (stat source))) + (target (substring source (string-length secret-root)))) + (call-with-remote-output-file sftp-session target + ;;(cute display text <>) + (lambda (port) (display text port))) + (sftp-chmod sftp-session target mode))) + (for-each copy-file (find-files secret-root)) + (let ((channel (make-channel session))) + (channel-open-session channel) + (channel-request-exec channel "herd restart sshd") + (unless (zero? (channel-get-exit-status channel)) + (error "Failed to restart sshd")))) + (disconnect! session))) + +;;; childhurd.scm ends here diff --git a/gnu/local.mk b/gnu/local.mk index d956e52d97..f872f1ba77 100644 --- a/gnu/local.mk +++ b/gnu/local.mk @@ -648,6 +648,7 @@ GNU_SYSTEM_MODULES = \ %D%/build/accounts.scm \ %D%/build/activation.scm \ %D%/build/bootloader.scm \ + %D%/build/childhurd.scm \ %D%/build/cross-toolchain.scm \ %D%/build/image.scm \ %D%/build/file-systems.scm \ diff --git a/gnu/services/virtualization.scm b/gnu/services/virtualization.scm index b93ed70099..f496c06764 100644 --- a/gnu/services/virtualization.scm +++ b/gnu/services/virtualization.scm @@ -39,6 +39,7 @@ #:use-module (gnu system) #:use-module (guix derivations) #:use-module (guix gexp) + #:use-module (guix modules) #:use-module (guix monads) #:use-module (guix packages) #:use-module (guix records) @@ -61,7 +62,10 @@ hurd-vm-configuration-options hurd-vm-configuration-id hurd-vm-configuration-net-options + hurd-vm-configuration-secrets + hurd-vm-disk-image + hurd-vm-port hurd-vm-net-options hurd-vm-service-type @@ -849,7 +853,9 @@ functionality of the kernel Linux."))) (default #f)) (net-options hurd-vm-configuration-net-options ;list of string (thunked) - (default (hurd-vm-net-options this-record)))) + (default (hurd-vm-net-options this-record))) + (secret-root hurd-vm-configuration-secret-root ;#f or string + (default #f))) (define (hurd-vm-disk-image config) "Return a disk-image for the Hurd according to CONFIG." @@ -861,15 +867,23 @@ functionality of the kernel Linux."))) (size disk-size) (operating-system os))))) -(define (hurd-vm-net-options config) +(define (hurd-vm-port config base) + "Return the forwarded vm port for this childhurd config." (let ((id (or (hurd-vm-configuration-id config) 0))) - (define (qemu-vm-port base) - (number->string (+ base (* 1000 id)))) - `("--device" "rtl8139,netdev=net0" - "--netdev" ,(string-append - "user,id=net0" - ",hostfwd=tcp:127.0.0.1:" (qemu-vm-port 10022) "-:2222" - ",hostfwd=tcp:127.0.0.1:" (qemu-vm-port 15900) "-:5900")))) + (+ base (* 1000 id)))) +(define %hurd-vm-ssh-port 10022) +(define %hurd-vm-vnc-port 15900) + +(define (hurd-vm-net-options config) + `("--device" "rtl8139,netdev=net0" + "--netdev" + ,(string-append "user,id=net0" + ",hostfwd=tcp:127.0.0.1:" + (number->string (hurd-vm-port config %hurd-vm-ssh-port)) + "-:2222" + ",hostfwd=tcp:127.0.0.1:" + (number->string (hurd-vm-port config %hurd-vm-vnc-port)) + "-:5900"))) (define (hurd-vm-shepherd-service config) "Return a <shepherd-service> for a Hurd in a Virtual Machine with CONFIG." @@ -900,8 +914,24 @@ functionality of the kernel Linux."))) (string->symbol (number->string id))) provisions) provisions)) - (requirement '(networking)) - (start #~(make-forkexec-constructor #$vm-command)) + (requirement '(loopback networking user-processes)) + (start + (with-imported-modules (source-module-closure '((gnu build childhurd) + (guix build utils))) + (with-extensions (list guile-ssh) + #~(let ((spawn (make-forkexec-constructor #$vm-command))) + (use-modules (gnu build childhurd)) + (lambda _ + (let ((pid (spawn)) + (port #$(hurd-vm-port config %hurd-vm-ssh-port)) + (root #$(hurd-vm-configuration-secret-root config))) + (when (and root (directory-exists? root)) + (catch #t + (lambda _ + (hurd-vm-copy-secrets port root)) + (lambda (key . args) + (format (current-error-port) "childhurd: ~a ~s\n" key args)))) + pid)))))) (stop #~(make-kill-destructor)))))) (define hurd-vm-service-type