diff mbox series

[bug#40901] gnu: ktsuss: Fix paths.

Message ID 20200427093702.16f9d953.raghavgururajan@disroot.org
State Accepted
Headers show
Series [bug#40901] gnu: ktsuss: Fix paths. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

Raghav Gururajan April 27, 2020, 1:37 p.m. UTC

Comments

Janneke Nieuwenhuizen April 27, 2020, 4:21 p.m. UTC | #1 
Raghav Gururajan writes:

Hello Raghav,

> From ecdb9f73f2aa9854f03ace199fcc4c4547a2be49 Mon Sep 17 00:00:00 2001
> From: Raghav Gururajan <raghavgururajan@disroot.org>
> Date: Mon, 27 Apr 2020 09:32:43 -0400
> Subject: [PATCH] gnu: ktsuss: Fix paths.
>
> * gnu/packages/admin.scm (ktsuss): Fix paths.

I changed the commit message to

--8<---------------cut here---------------start------------->8---
gnu: ktsuss: Use setuid "su" and "sudo" commands.

This has ktsudo actually gain root access.

* gnu/packages/admin.scm (ktsuss)[arguments]: Instead of <sudo>/bin/{su,sudo}
file names, use the setuid' verrsions.
--8<---------------cut here---------------end--------------->8---

When there is a fix being made, a short explanation is nice to have.
What was wrong that the file name were changed, what was fixed?

Also, we are (mainly) fixing the file names of SU and SUDO, not PATHs.

> -         (add-after 'unpack 'patch-sudo-path
> -           (lambda* (#:key inputs #:allow-other-keys)
> +         (add-after 'unpack 'patch-paths

Renamed to 'patch-file-names'

> +           (lambda* (#:key inputs outputs #:allow-other-keys)

Parameters inputs and outputs are both not used; I changed this to

+           (lambda _

> +             (substitute* "src/ktsuss.c"
> +               (("/usr/sbin:/usr/local/sbin:/sbin")
> +                "~/.guix-profile/sbin"))

No way!  :-)

I removed/did not apply this change.

>               (substitute* "configure.ac"
> +               (("supath=`which su 2>/dev/null`")
> +                "supath=/run/setuid-programs/su")
>                 (("sudopath=`which sudo 2>/dev/null`")
> -                (string-append "sudopath="
> -                               (string-append (assoc-ref inputs "sudo")
> -                                              "/bin/sudo"))))
> +                "sudopath=/run/setuid-programs/sudo"))

LGTM!

Can you please remember to test a program before submitting it?  And I
mean not only check if "it runs", but also if it is actually useful?

Pushed to master with these changes.

Greetings,
janneke
Janneke Nieuwenhuizen April 28, 2020, 5:13 p.m. UTC | #2 
Raghav Gururajan writes:

Hello Raghav,

>> Can you please remember to test a program before submitting it?  And I
>> mean not only check if "it runs", but also if it is actually useful?
>
> Sorry. I have attached a patch with this email to make ktsuss work
> correctly.

Hmm, I experienced no problem when I tested it.  What did you observe
(see my question below).

> From a1d5a14bb634fe52201fdde9ffbd3e385b89d234 Mon Sep 17 00:00:00 2001
> From: Raghav Gururajan <raghavgururajan@disroot.org>
> Date: Tue, 28 Apr 2020 03:34:05 -0400
> Subject: [PATCH 2/2] gnu: ktsuss: Disable sudo.
>
> * gnu/packages/admin.scm (ktsuss): Disable sudo.
>
> Disabled sudo-backend as there is an unknown bug in upstream.
> Ktsuss now fall-back to using su-backend, which works correctly.

Having a like this is great.  Two remarks

  * please move it above the "* gnu/packages/admin" line
  * it would be great if you can add link to the ktsuss bug tracer about
    this "unknown" bug

Without any indication about what the bug is or how it can be
reproduced, it becomes hard to remove this workaround.

Other than that: LGTM.

Greetings,
janneke
diff mbox series

Patch

From ecdb9f73f2aa9854f03ace199fcc4c4547a2be49 Mon Sep 17 00:00:00 2001
From: Raghav Gururajan <raghavgururajan@disroot.org>
Date: Mon, 27 Apr 2020 09:32:43 -0400
Subject: [PATCH] gnu: ktsuss: Fix paths.

* gnu/packages/admin.scm (ktsuss): Fix paths.
---
 gnu/packages/admin.scm | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index 236eebe53c..9316e974bc 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -150,24 +150,25 @@ 
        (list "--enable-sudo=yes")
        #:phases
        (modify-phases %standard-phases
-         (add-after 'unpack 'patch-sudo-path
-           (lambda* (#:key inputs #:allow-other-keys)
+         (add-after 'unpack 'patch-paths
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (substitute* "src/ktsuss.c"
+               (("/usr/sbin:/usr/local/sbin:/sbin")
+                "~/.guix-profile/sbin"))
              (substitute* "configure.ac"
+               (("supath=`which su 2>/dev/null`")
+                "supath=/run/setuid-programs/su")
                (("sudopath=`which sudo 2>/dev/null`")
-                (string-append "sudopath="
-                               (string-append (assoc-ref inputs "sudo")
-                                              "/bin/sudo"))))
+                "sudopath=/run/setuid-programs/sudo"))
              #t)))))
     (native-inputs
      `(("autoconf" ,autoconf)
-       ("autogen" ,autogen)
        ("automake" ,automake)
        ("libtool" ,libtool)
        ("pkg-config" ,pkg-config)))
     (inputs
      `(("glib" ,glib)
-       ("gtk+" ,gtk+-2)
-       ("sudo" ,sudo)))
+       ("gtk+" ,gtk+-2)))
     (synopsis "Graphical front end for @command{su}")
     (description
      "Ktsuss stands for ``Keep the @command{su} simple, stupid''.
-- 
2.26.2