Skip to content
Snippets Groups Projects

Draft: doas and PACMAN_AUTH support. fix #105

Closed Gabriel barros requested to merge gcb/devtools:doas-gcb into master
1 unresolved thread

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • I think support for PACMAN_AUTH is good and the best way to normalize this (in case you have both sudo/doas in your system), hence also included here.

    I am not sure of a use case where keepenv is used. So not sure how to test.

    Lastly, someone who actually uses PACMAN_AUTH, please test this patch before I remove the safeguards. It's a draft/WIP till then.

  • Gabriel barros added 2 commits

    added 2 commits

    Compare with previous version

  • Christian Heusel marked this merge request as draft

    marked this merge request as draft

    • This does not address !131 (comment 85795) it just works around it by not supporting it, which ends up creating unreproducible packages if doas is used.

      Secondly, there are still plenty of sudo invocations in devtools:

      src/makechrootpkg.in:   sudo --preserve-env=SOURCE_DATE_EPOCH \
      src/makechrootpkg.in:           --preserve-env=BUILDTOOL \
      src/makechrootpkg.in:           --preserve-env=BUILDTOOLVER \
      src/makechrootpkg.in:   sudo -u "$makepkg_user" --preserve-env=GNUPGHOME,SSH_AUTH_SOCK \
    • indeed. I was mostly trying to get past mkarchroot $CHROOT/root base-devel

      I see now that simply makechrootpkg -c -r $CHROOT is enough to fail here.

      Thanks for the feedback. Leave this around as draft i will get back to it.

    • Used the same "trick" as su was using for the command escape, but added prefixed env vars. doas doesn't support anything but a command, so used sh -c. 0dda1e3a have a prototype.

    • interesting:

      $ PACMAN_AUTH=sudo makechrootpkg -c -r $CHROOT
      ==> Synchronizing chroot copy [/home/gcb/chroot/root] -> [gcb]...done
      ==> Making package: fwupd 1.9.10.r640.g96cc2c6bc-1 (Wed May 22 10:07:23 2024)
      ==> Retrieving sources...
        -> Cloning fwupd-daemonless git repo...
      ...
      $? = 0

      now forcing plain doas and having permitenv in the conf:

      $ PACMAN_AUTH=doas makechrootpkg -c -r $CHROOT
      ==> Synchronizing chroot copy [/home/gcb/chroot/root] -> [copy]...done
      ==> ERROR: Running makepkg as root is not allowed.
      $? = 1

      second case is doing nothing but using doas (with keepenv) in place of sudo --preserve-env

    • There are a few places (like last comment) where business logic is tied to sudo. For now i just added DOAS_USER to the existing logic that uses SUDO_USERS :( Better solutions: make use of existing getopts that already cover the value, or pass via env vars like other things.

      Then the other places sudo is used is to jump back from root to the makepkg_user. For now, i will not do anything since sudo package is required by all dev packages, so we are bound to have it anyway. What i'm trying to avoid is giving build users sudo access. This way the left over sudo calls are just from root->low user, which already work by default by simply installing the package. Better solutions: nspawn/systemd-run/other ways to just drop the uid (and maybe dropcaps) that are already used for other benefits in the build tools.

    • @gcb I managed to do a build in chroot with doas (via pkgctl build). Somehow sudo manages to pass $USER in that exec call, so I added that to keepenv.

      Here's my patch against v1.2.1: doas.patch

      Replacing sudo --preserve-env with $SUID_CMD env VAR1=$VAR1 ... looks like is enough to make this agnostic?

      One more note, there's a sudo call which is run as root to switch back to the user. I had allow root to use doas:

      permit nopass keepenv root
      Edited by Vladyslav Aviedov
    • I should move to pkgctl too... that's a good patch! will take a look. I think we can close this merge request and maybe open a new one going that route.

    • Please register or sign in to reply
  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • Morten Linderud mentioned in merge request !266

    mentioned in merge request !266

Please register or sign in to reply
Loading