diff mbox series

[bug#36957,v2] machine: Allow non-root users to deploy.

Message ID 87pnlgjymv.fsf_-_@sdf.lonestar.org
State Accepted
Headers show
Series [bug#36957,v2] machine: Allow non-root users to deploy. | expand

Commit Message

Jakob L. Kreuze Aug. 8, 2019, 12:20 a.m. UTC
* doc/guix.texi (Invoking guix deploy): Add section describe
prerequisites for deploying as a non-root user.
* guix/remote.scm (remote-pipe-for-gexp): New optional 'become-command'
argument.
(%remote-eval): New optional 'become-command' argument.
(remote-eval): New 'become-command' keyword argument.
* guix/ssh.scm (remote-inferior): New optional 'become-command'
argument.
(inferior-remote-eval): New optional 'become-command' argument.
(remote-authorize-signing-key): New optional 'become-command' argument.
* gnu/machine/ssh.scm (machine-become-command): New variable.
(managed-host-remote-eval): Invoke 'remote-eval' with the
'#:become-command' keyword.
(deploy-managed-host): Invoke 'remote-authorize-signing-key' with the
'#:become-command' keyword.
---
 doc/guix.texi       | 10 ++++++++
 gnu/machine/ssh.scm | 12 ++++++++-
 guix/remote.scm     | 60 ++++++++++++++++++++++++++++-----------------
 guix/ssh.scm        | 25 +++++++++++++------
 4 files changed, 77 insertions(+), 30 deletions(-)

Comments

Ricardo Wurmus Aug. 8, 2019, 8:33 a.m. UTC | #1
Hi Jakob,

> +@code{user}.  That is: the line in @code{sudoers} granting @code{user} the
> +ability to use @code{sudo} must contain the NOPASSWD tag.

Perhaps also wrap “NOPASSWD” in @code{…}.

> +(define (machine-become-command machine)
> +  "Return as a list of strings the program and arguments necessary to run a
> +shell command with escalated privileges for MACHINE's configuration."
> +  (if (string= "root" (machine-ssh-configuration-user
> +                       (machine-configuration machine)))
> +      '()
> +      '("/run/setuid-programs/sudo" "-n" "--")))
> +

This is a comment for future changes only: currently, we can assume that
the remote machine already runs Guix System.  In the future “guix
deploy” should probably also be able to initialize a system.  In that
case “sudo” may have to be searched on the target or otherwise be
provided.

(What happens if /run/setuid-programs/sudo is not available on the
target machine?)

> +(define* (%remote-eval lowered session #:optional become-command)
>    "Evaluate LOWERED, a lowered gexp, in SESSION.  This assumes that all the
> -prerequisites of EXP are already available on the host at SESSION."
> -  (let* ((pipe   (remote-pipe-for-gexp lowered session))
> +prerequisites of EXP are already available on the host at SESSION.  If
> +BECOME-COMMAND is given, use that to invoke the remote Guile REPL."
> +  (let* ((pipe   (remote-pipe-for-gexp lowered session become-command))
>           (result (read-repl-response pipe)))
>      (close-port pipe)
>      result))
> @@ -90,12 +104,14 @@ result to the current output port using the (guix repl) protocol."
>                        #:key
>                        (build-locally? #t)
>                        (module-path %load-path)
> -                      (socket-name "/var/guix/daemon-socket/socket"))
> +                      (socket-name "/var/guix/daemon-socket/socket")
> +                      (become-command #f))

I’m just stumbling upon “socket-name”.  “/var/guix” is not guaranteed to
be the localstatedir.  It would be better to use (guix config) to
determine the configured value.

This doesn’t block this patch, of course, but it would be good to change
this in the future.

--
Ricardo
Jakob L. Kreuze Aug. 8, 2019, 8:24 p.m. UTC | #2
Hey Ricardo,

Ricardo Wurmus <rekado@elephly.net> writes:

> Perhaps also wrap “NOPASSWD” in @code{…}.

Got it, thanks!

> This is a comment for future changes only: currently, we can assume that
> the remote machine already runs Guix System.  In the future “guix
> deploy” should probably also be able to initialize a system.  In that
> case “sudo” may have to be searched on the target or otherwise be
> provided.

Ah, that's a good point. I'd imagine that would involve changing a few
other things with how the REPL is spawned, too.

> (What happens if /run/setuid-programs/sudo is not available on the
> target machine?)

I'm a bit short on time before boarding this flight, so I can't test it
out at the moment, but I'm pretty sure the "failed to run..." message
condition would be thrown. I'll check and get back to you.

> I’m just stumbling upon “socket-name”.  “/var/guix” is not guaranteed to
> be the localstatedir.  It would be better to use (guix config) to
> determine the configured value.
>
> This doesn’t block this patch, of course, but it would be good to change
> this in the future.

Right, yeah. I may submit a separate patch for it shortly since it
should be a simple change.

Regards,
Jakob
Christine Lemmer-Webber Aug. 14, 2019, 8:52 p.m. UTC | #3
Jakob L. Kreuze writes:

>> (What happens if /run/setuid-programs/sudo is not available on the
>> target machine?)
>
> I'm a bit short on time before boarding this flight, so I can't test it
> out at the moment, but I'm pretty sure the "failed to run..." message
> condition would be thrown. I'll check and get back to you.

Check, and if that's good, let's merge this in the patch series tomorrow.

>> I’m just stumbling upon “socket-name”.  “/var/guix” is not guaranteed to
>> be the localstatedir.  It would be better to use (guix config) to
>> determine the configured value.
>>
>> This doesn’t block this patch, of course, but it would be good to change
>> this in the future.
>
> Right, yeah. I may submit a separate patch for it shortly since it
> should be a simple change.

See if you can get it in the patch series (as the last patch)!  If you
can't, file a bug, and let's not block on it.
Jakob L. Kreuze Aug. 15, 2019, 8:03 a.m. UTC | #4
Christopher Lemmer Webber <cwebber@dustycloud.org> writes:

> Check, and if that's good, let's merge this in the patch series
> tomorrow.

As predicted, it handles a missing 'sudo' just fine :]

> See if you can get it in the patch series (as the last patch)! If you
> can't, file a bug, and let's not block on it.

Got it in! Pretty simple change.

Jakob L. Kreuze (5):
  machine: Allow non-root users to deploy.
  machine: Implement 'roll-back-machine'.
  machine: Automatically authorize the coordinator's signing key.
  doc: Add description of 'build-locally?'.
  remote: Use (%daemon-socket-uri) rather than hard-coded path.

 doc/guix.texi           |  15 ++++++
 gnu/machine.scm         |  27 +++++++++-
 gnu/machine/ssh.scm     | 113 ++++++++++++++++++++++++++++++++++++----
 guix/remote.scm         |  57 ++++++++++++--------
 guix/scripts/deploy.scm |  17 +++++-
 guix/ssh.scm            |  48 ++++++++++++++---
 6 files changed, 236 insertions(+), 41 deletions(-)
diff mbox series

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index 734206a4b2..1f0750255d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -25514,6 +25514,7 @@  evaluates to.  As an example, @var{file} might contain a definition like this:
        (environment managed-host-environment-type)
        (configuration (machine-ssh-configuration
                        (host-name "localhost")
+                       (user "alice")
                        (identity "./id_rsa")
                        (port 2222)))))
 @end example
@@ -25546,6 +25547,15 @@  accepts store items it receives from the coordinator:
 # guix archive --authorize < coordinator-public-key.txt
 @end example
 
+@code{user}, in this example, specifies the name of the user account to log in
+as to perform the deployment.  Its default value is @code{root}, but root
+login over SSH may be forbidden in some cases.  To work around this,
+@command{guix deploy} can log in as an unprivileged user and employ
+@code{sudo} to escalate privileges.  This will only work if @code{sudo} is
+currently installed on the remote and can be invoked non-interactively as
+@code{user}.  That is: the line in @code{sudoers} granting @code{user} the
+ability to use @code{sudo} must contain the NOPASSWD tag.
+
 @deftp {Data Type} machine
 This is the data type representing a single machine in a heterogeneous Guix
 deployment.
diff --git a/gnu/machine/ssh.scm b/gnu/machine/ssh.scm
index ba3e33c922..aba98f8de5 100644
--- a/gnu/machine/ssh.scm
+++ b/gnu/machine/ssh.scm
@@ -99,6 +99,14 @@  one from the configuration's parameters if one was not provided."
 ;;; Remote evaluation.
 ;;;
 
+(define (machine-become-command machine)
+  "Return as a list of strings the program and arguments necessary to run a
+shell command with escalated privileges for MACHINE's configuration."
+  (if (string= "root" (machine-ssh-configuration-user
+                       (machine-configuration machine)))
+      '()
+      '("/run/setuid-programs/sudo" "-n" "--")))
+
 (define (managed-host-remote-eval machine exp)
   "Internal implementation of 'machine-remote-eval' for MACHINE instances with
 an environment type of 'managed-host."
@@ -106,7 +114,9 @@  an environment type of 'managed-host."
   (remote-eval exp (machine-ssh-session machine)
                #:build-locally?
                (machine-ssh-configuration-build-locally?
-                (machine-configuration machine))))
+                (machine-configuration machine))
+               #:become-command
+               (machine-become-command machine)))
 
 
 ;;;
diff --git a/guix/remote.scm b/guix/remote.scm
index 5fecd954e9..b0b6afba93 100644
--- a/guix/remote.scm
+++ b/guix/remote.scm
@@ -26,6 +26,8 @@ 
   #:use-module (guix derivations)
   #:use-module (ssh popen)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-35)
   #:use-module (ice-9 match)
   #:export (remote-eval))
 
@@ -40,29 +42,41 @@ 
 ;;;
 ;;; Code:
 
-(define (remote-pipe-for-gexp lowered session)
-  "Return a remote pipe for the given SESSION to evaluate LOWERED."
+(define* (remote-pipe-for-gexp lowered session #:optional become-command)
+  "Return a remote pipe for the given SESSION to evaluate LOWERED.  If
+BECOME-COMMAND is given, use that to invoke the remote Guile REPL."
   (define shell-quote
     (compose object->string object->string))
 
-  (apply open-remote-pipe* session OPEN_READ
-         (string-append (derivation-input-output-path
-                         (lowered-gexp-guile lowered))
-                        "/bin/guile")
-         "--no-auto-compile"
-         (append (append-map (lambda (directory)
-                               `("-L" ,directory))
-                             (lowered-gexp-load-path lowered))
-                 (append-map (lambda (directory)
-                               `("-C" ,directory))
-                             (lowered-gexp-load-path lowered))
-                 `("-c"
-                   ,(shell-quote (lowered-gexp-sexp lowered))))))
+  (define repl-command
+    (append (or become-command '())
+            (list
+             (string-append (derivation-input-output-path
+                             (lowered-gexp-guile lowered))
+                            "/bin/guile")
+             "--no-auto-compile")
+            (append-map (lambda (directory)
+                          `("-L" ,directory))
+                        (lowered-gexp-load-path lowered))
+            (append-map (lambda (directory)
+                          `("-C" ,directory))
+                        (lowered-gexp-load-path lowered))
+            `("-c"
+              ,(shell-quote (lowered-gexp-sexp lowered)))))
 
-(define (%remote-eval lowered session)
+  (let ((pipe (apply open-remote-pipe* session OPEN_READ repl-command)))
+    (when (eof-object? (peek-char pipe))
+      (raise (condition
+              (&message
+               (message (format #f (G_ "failed to run '~{~a~^ ~}'")
+                                repl-command))))))
+    pipe))
+
+(define* (%remote-eval lowered session #:optional become-command)
   "Evaluate LOWERED, a lowered gexp, in SESSION.  This assumes that all the
-prerequisites of EXP are already available on the host at SESSION."
-  (let* ((pipe   (remote-pipe-for-gexp lowered session))
+prerequisites of EXP are already available on the host at SESSION.  If
+BECOME-COMMAND is given, use that to invoke the remote Guile REPL."
+  (let* ((pipe   (remote-pipe-for-gexp lowered session become-command))
          (result (read-repl-response pipe)))
     (close-port pipe)
     result))
@@ -90,12 +104,14 @@  result to the current output port using the (guix repl) protocol."
                       #:key
                       (build-locally? #t)
                       (module-path %load-path)
-                      (socket-name "/var/guix/daemon-socket/socket"))
+                      (socket-name "/var/guix/daemon-socket/socket")
+                      (become-command #f))
   "Evaluate EXP, a gexp, on the host at SESSION, an SSH session.  Ensure that
 all the elements EXP refers to are built and deployed to SESSION beforehand.
 When BUILD-LOCALLY? is true, said dependencies are built locally and sent to
 the remote store afterwards; otherwise, dependencies are built directly on the
-remote store."
+remote store.  If BECOME-COMMAND is given, use that to invoke the remote Guile
+REPL."
   (mlet %store-monad ((lowered (lower-gexp (trampoline exp)
                                            #:module-path %load-path))
                       (remote -> (connect-to-remote-daemon session
@@ -115,7 +131,7 @@  remote store."
             (built-derivations inputs)
             ((store-lift send-files) to-send remote #:recursive? #t)
             (return (close-connection remote))
-            (return (%remote-eval lowered session))))
+            (return (%remote-eval lowered session become-command))))
         (let ((to-send (append (map (compose derivation-file-name
                                              derivation-input-derivation)
                                     inputs)
@@ -124,4 +140,4 @@  remote store."
             ((store-lift send-files) to-send remote #:recursive? #t)
             (return (build-derivations remote inputs))
             (return (close-connection remote))
-            (return (%remote-eval lowered session)))))))
+            (return (%remote-eval lowered session become-command)))))))
diff --git a/guix/ssh.scm b/guix/ssh.scm
index ede00133c8..0f65f9e65b 100644
--- a/guix/ssh.scm
+++ b/guix/ssh.scm
@@ -97,16 +97,27 @@  specifies; otherwise use them.  Throw an error on failure."
                 (message (format #f (G_ "SSH connection to '~a' failed: ~a~%")
                                  host (get-error session))))))))))
 
-(define (remote-inferior session)
-  "Return a remote inferior for the given SESSION."
-  (let ((pipe (open-remote-pipe* session OPEN_BOTH
-                                 "guix" "repl" "-t" "machine")))
+(define* (remote-inferior session #:optional become-command)
+  "Return a remote inferior for the given SESSION.  If BECOME-COMMAND is
+given, use that to invoke the remote Guile REPL."
+  (let* ((repl-command (append (or become-command '())
+                               '("guix" "repl" "-t" "machine")))
+         (pipe (apply open-remote-pipe* session OPEN_BOTH repl-command)))
+    ;; XXX: 'channel-get-exit-status' would be better here, but hangs if the
+    ;; process does succeed. This doesn't reflect the documentation, so it's
+    ;; possible that it's a bug in guile-ssh.
+    (when (eof-object? (peek-char pipe))
+      (raise (condition
+              (&message
+               (message (format #f (G_ "failed to run '~{~a~^ ~}'")
+                                repl-command))))))
     (port->inferior pipe)))
 
-(define (inferior-remote-eval exp session)
+(define* (inferior-remote-eval exp session #:optional become-command)
   "Evaluate EXP in a new inferior running in SESSION, and close the inferior
-right away."
-  (let ((inferior (remote-inferior session)))
+right away.  If BECOME-COMMAND is given, use that to invoke the remote Guile
+REPL."
+  (let ((inferior (remote-inferior session become-command)))
     (dynamic-wind
       (const #t)
       (lambda ()