Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ejabberdctl: support OpenBSD su #4320

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

catap
Copy link

@catap catap commented Nov 28, 2024

OpenBSD has a different than Linux su:

  1. -c before username is treated as login class;
  2. it doesn't require -- as arguments separator.

Without (1) it complains as:

su: no such login class: exec "$0" "$@"

and without (2):

-: --: not found

Here, I've added detection of OS via uname -s which routes to the right su. I really think that other BSD may need it as well.

OpenBSD has a different than Linux su:
 1. `-c` before username is treated as login class;
 2. it doesn't require `--` as arguments separator.

Without (1) it complains as:

    su: no such login class: exec "$0" "$@"

and without (2):

    -: --: not found

Here, I've added detection of OS via `uname -s` which routes to the
right `su`. I really think that other BSD may need it as well.
@weiss
Copy link
Member

weiss commented Nov 28, 2024

Thanks. Having no way to use su(1) in a portable manner keeps being a PITA.

-c before username is treated as login class;

I think this could be swapped on Linux as well (in which case -c $args is just handed over to the shell directly, rather than being parsed by su first, which seems just fine).

it doesn't require -- as arguments separator.

At first glance I'm unsure why we need the -- "$@" part at all, but I'm probably just overlooking something (I'll test things later).

I really think that other BSD may need it as well.

On other BSDs, there's the unrelated issue that they don't support -s "$SHELL" (unlike OpenBSD and Linux). For eturnal, I just omit that option on other systems and hope for the best (i.e., I hope the user running the service has a usable shell):

https://github.com/processone/eturnal/blob/1.12.1/overlay/eturnalctl#L157-L166

@catap
Copy link
Author

catap commented Nov 28, 2024

I really think that other BSD may need it as well.

On other BSDs, there's the unrelated issue that they don't support -s "$SHELL" (unlike OpenBSD and Linux). For eturnal, I just omit that option on other systems and hope for the best (i.e., I hope the user running the service has a usable shell):

Probably something like this?

exec_cmd()
{
    case $EXEC_CMD,$(uname -s) in
        as_install_user,OpenBSD)
            su -s /bin/sh "$INSTALLUSER" -c 'exec "$0" "$@"' "$@"
            ;;
        as_install_user,Linux)
            su -s /bin/sh -c 'exec "$0" "$@"' "$INSTALLUSER" -- "$@"
            ;;
        as_install_user,*)
            su "$INSTALLUSER" "$@"
            ;;
        as_current_user,*)
            "$@"
            ;;
    esac
}

@coveralls
Copy link

Coverage Status

coverage: 32.9%. remained the same
when pulling 1b77abc on catap:openbsd
into 7d0c20e on processone:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants