Solaris Autostart service can only work for 1st user in staff group => fixed in SVN

[kburtch]

First: The use of "exec" in smf-vboxautostart.sh breaks the loop through the users as it replaces the current process with the "su" process. It also prevents the immediately following exit status check from functioning.

Second: The smf-vboxautostart.sh script loops over the "staff" Unix group instead of the "vboxuser" Unix group to determine which users to look for to start autostart-enabled VMs. The use of the "vboxuser" group makes more sense than "staff" as not all companies use "staff" for their users, and the "vboxuser" group is a required group for anyone running VirtualBox anyways.

Third: Looping through "logins -g group" includes 5 columns of data (including the GECOS field), so it will be looping over UID, group, GID, first name, last name, etc. as well as the username. At a minimum, it should be piped through "cut -d' ' -f1" to strip out the irrelevant data.

        # Get all users
        for VW_USER in `logins -g staff`
        do
            exec su - "$VW_USER" -c "/opt/VirtualBox/VBoxAutostart --background --start --config \"$VW_CONFIG\" --logrotate \"$VW_ROTATE\" --logsize \"$VW_LOGSIZE\" --loginterval \"$VW_LOGINTERVAL\""

            VW_EXIT=$?
            if [ $VW_EXIT != 0 ]; then
                echo "VBoxAutostart failed with $VW_EXIT."
                VW_EXIT=1
                break
            fi
        done

I suggest changing it to the following (diff attached):

        for VW_USER in `logins -g vboxuser | cut -d' ' -f1`
        do
            su - "$VW_USER" -c "/opt/VirtualBox/VBoxAutostart --background --start --config \"$VW_CONFIG\" --logrotate \"$VW_ROTATE\" --logsize \"$VW_LOGSIZE\" --loginterval \"$VW_LOGINTERVAL\""

            VW_EXIT=$?
            if [ $VW_EXIT != 0 ]; then
                echo "VBoxAutostart failed with $VW_EXIT."
                VW_EXIT=1
                break
            fi
        done

The above fixes all three problems: autostart VMs not starting unless you're 1st in the list; users not in "staff" group not having VMs started); and attempting to su to groups, numbers, etc. (diff attached)

Note the diff also contains a clarification of error output for the top section where it checks if a file is executable, then if it exists. I reversed those and changed the error to make more sense.

--- /opt/VirtualBox/smf-vboxautostart.sh.orig   Fri Apr 12 12:19:08 2013
+++ /opt/VirtualBox/smf-vboxautostart.sh        Mon Apr 15 19:23:31 2013
@@ -24,13 +24,13 @@
 
 case $VW_OPT in
     start)
-        if [ ! -x /opt/VirtualBox/VBoxAutostart ]; then
+        if [ ! -f /opt/VirtualBox/VBoxAutostart ]; then
             echo "ERROR: /opt/VirtualBox/VBoxAutostart does not exist."
             return $SMF_EXIT_ERR_CONFIG
         fi
 
-        if [ ! -f /opt/VirtualBox/VBoxAutostart ]; then
-            echo "ERROR: /opt/VirtualBox/VBoxAutostart does not exist."
+        if [ ! -x /opt/VirtualBox/VBoxAutostart ]; then
+            echo "ERROR: /opt/VirtualBox/VBoxAutostart is not executable."
             return $SMF_EXIT_ERR_CONFIG
         fi
 
@@ -51,9 +51,9 @@
         [ -z "$VW_LOGINTERVAL" ] && VW_LOGINTERVAL=86400
 
         # Get all users
-        for VW_USER in `logins -g staff`
+        for VW_USER in `logins -g vboxuser | cut -d' ' -f1`
         do
-            exec su - "$VW_USER" -c "/opt/VirtualBox/VBoxAutostart --background --start --config \"$VW_CONFIG\" --logrotate \"$VW_ROTATE\" --logsize \"$VW_LOGSIZE\" --loginterval \"$VW_LOGINTERVAL\""
+            su - "$VW_USER" -c "/opt/VirtualBox/VBoxAutostart --background --start --config \"$VW_CONFIG\" --logrotate \"$VW_ROTATE\" --logsize \"$VW_LOGSIZE\" --loginterval \"$VW_LOGINTERVAL\""
 
             VW_EXIT=$?
             if [ $VW_EXIT != 0 ]; then

I started a forum post on this, originally thinking the problem was the use of null arguments to options when calling VBoxAutostart (before I noticed the "exec"), as VBoxAutostart fails under those conditions. ​https://forums.virtualbox.org/viewtopic.php?f=11&t=55006

[kburtch]

diff -u output, patch for smf-vboxautostart.sh

[Ramshankar Venkataraman]

Thanks for the patch.

A few notes:

The vboxuser group is not "required". It is only needed on Solaris 11 hosts for permissions for host USB support. Keep in mind that the installer only creates the "vboxuser" group when these conditions are met.

[kburtch]

You're very welcome for the patch.

Agreed, the vboxuser group is only required for use of USB, but it is the most appropriate group for determining what accounts may have VB VMs defined (if group parsing is used for this purpose). As stated, not all companies use "staff" as a default group.

One way this could be made flexible is by adding a property (config/group?) to the virtualbox/autostart service to specify the group to parse.

A possible alternative may be parsing *all* users (getent passwd | cut -d: -f1), but that could be busy in a large environment with a very large number of users.

[Ramshankar Venkataraman]

I agree that the "vboxuser" group is the most appropriate. I merely wanted to put a reminder in here that the group creation is only done conditionally by the installer, and that when one of our developers gets around to fixing this, they'd notice that they might have to change the installer as well.

[aeichner]

Hi, thanks for the patch again. Can you please put your patch under MIT license so we can include it? Regarding the default group. I think I'll go with your suggestion and make the default group configurable and leave the default to staff. The staff group was the group every user is in with a default Solaris installation as far as I can tell.

[darrenmoffat]

In addition to all of the above you really shouldn't be using logins that way since it will scan through the entire nameservice looking for all users. There is a "-S files" option to logins that you might want to consider using to limit it to just users with local accounts.

An alternative way to do this might be to have separate instances of the SMF for each user that is allowed to have autostarting VMs, that way the hosts admin has to explicitly enable each user (which they would have to do if you are using a special group). The advantage of using a separate instance is that you then don't need to use su but instead set the method_credential for the instance so it runs as the user.

[darrenmoffat]

Incase you need any more justification of why scanning the whole nameservice isn't nice on my desktop "logins -g staff" takes 1 minute 37 seconds.

[kburtch]

My apologies for not responding long ago.

Replying to aeichner:

Hi, thanks for the patch again. Can you please put your patch under MIT license so we can include it?

Absolutely. I release it under the MIT license, or any license you need to be able to include it in VirtualBox (I actually didn't think there was enough to it to require a license).

Thank you very much.

[kburtch]

Patch against smf-vboxautostart.sh from VirtualBox 4.3.4

[kburtch]

I updated the patch since there is a similar routine in the stop section of the script now (in 4.3.4, not sure when it crept in).

I also release this patch under the MIT license.

[aeichner]

Thanks for the updated patch. It will be part of the next maintenance release. I also made the user group for autostart configurable.

@darrenmoffat: I agree that this way might not be the best one for all configurations out there but if we would change it to check only local accounts we will get complaints from other users why it was changed.

[kburtch]

Interesting timing... I just now successfully tested the patch against 4.3.6.

Thank you very much!

[Frank Mehnert]

Could you test ​VBox 4.3.8 RC1?

[Frank Mehnert]

Fix is part of VBox 4.3.8.

[Az]

Hello Sirs,

I'm on Solaris 11.3 (GA aka "release repo") and VirtualBox 5.1.18 and the issue seems to persist although all the previous discussion remains pretty much valid. What I would suggest further investigation is that the "logins -g" command may have a bug as it correctly returns user logins only if those user logins have the value of the "-g" parameter as their primary group. For instance, if the primary group of users user1 and user2 is staff all will work as expected (no bug revealed). But if vboxuser is their secondary group and we select that group on the respective SMF property (implying that we'll run a "logins -g vboxuser") no login will be returned by logins and the script will fail.

The suggestion for a fix perhaps could be to adjst the line that contains "logins -g $VW_VBOXGROUP | cut -d' ' -f1" to the following:

for VW_USER in $(echo $(logins -g $VW_VBOXGROUP | cut -d' ' -f1) $(getent group $VW_VBOXGROUP | cut -d: -f4 | tr , ' ') | xargs -n1 | sort -u)

Regards!

[Az]

In fact I've been studying the feature under Solaris 11.3 release + VirtualBox 5.1.18 and I've got a few more comments about it. I'll remark that the format of the "Autostart configuration file" isn't strictly documented; all we have is an sample that's apparently fair enough but that may cause difficulties if you have to deal with it as it happens to be the case due to an apparent bug in VBoxAutostart binary file. VBoxAutostart doesn't really consider the policy file as it should because it forces us to declare the rules for every user that is part of config/vboxgroup. In fact, if a user from config/vboxgroup doesn't have a rule in the configuration file, the authorization should be inferred from the default_policy declaration, but that's is not current behavior of VBoxAutostart. In fact, another behavioral problem is that if a user's rule lists "deny", it's not the case for VBoxAutostart exit with a non-zero value, because this will cause the SMF method to trigger a maintenance status for the service.

Here is some extra code (plus auxiliary scripts) for the SMF service method script as an idea for a fix:

Excerpt from /opt/VirtualBox/smf-autostart.sh with the suggested fixes

        ...
        # Auxiliary scripts path
        SCRIPT_PATH=${0%/`/usr/bin/basename $0`}

        # Auxiliary temporary cache files
        TMPFILE1=`mktemp`
        TMPFILE2=`mktemp`
        trap 'rm -f -- "$TMPFILE1" "$TMPFILE2"' EXIT

        # Tokenize and cache autostart configuration file 
        ${SCRIPT_PATH}/smf-vboxautostart-tokenize.sh $VW_CONFIG $TMPFILE1

        # Cache auxiliary AWK script for checking user allow rule 
        ${SCRIPT_PATH}/smf-vboxautostart-allow-entries.sh $TMPFILE2

        # Get default policy
        VW_POLICY=`${SCRIPT_PATH}/smf-vboxautostart-policy.sh $TMPFILE1`

        # Get all users
        # for VW_USER in `logins -g $VW_VBOXGROUP | cut -d' ' -f1`
        for VW_USER in $(echo `logins -g $VW_VBOXGROUP | cut -d' ' -f1` `getent group $VW_VBOXGROUP | cut -d: -f4 | tr , ' '` | xargs -n1 | sort -u)
        do
            if [[ `${SCRIPT_PATH}/smf-vboxautostart-user.sh $TMPFILE1 $TMPFILE2 $VW_USER` == "true" ]] || \
             ( [[ "$VW_POLICY" == "allow" ]] && [[ `${SCRIPT_PATH}/smf-vboxautostart-user.sh $TMPFILE1 $TMPFILE2 $VW_USER` != "false" ]] ); then

                su - "$VW_USER" -c "/opt/VirtualBox/VBoxAutostart --stop --config \"$VW_CONFIG\" --logrotate \"$VW_ROTATE\" --logsize \"$VW_LOGSIZE\" --loginterval \"$VW_LOGINTERVAL\""

                VW_EXIT=$?
                if [ $VW_EXIT != 0 ]; then
                    echo "VBoxAutostart failed with $VW_EXIT."
                    VW_EXIT=1
                    break
                fi
            fi
        done
        ...

And here are the additional shell scripts for the fix:

/opt/VirtualBox/smf-vboxautostart-tokenize.sh

#!/bin/sh

#
# Parse and tokenize the VirtualBox Autostart configuration file.
# $1 should be the configuration file.
# $2 should be the output file name to use.
#
if [[ -n "$1" ]] && [[ -n "$2" ]]; then
        cat "$1" |tr -d '[:blank:]' |sed -e 's/#.*//' -e 's/{/{=/' -e 's/}/=}/' |tr '=' '\n' |sed -e '/^$/ d' >"$2"
fi

/opt/VirtualBox/smf-vboxautostart-allow-entries.sh

#!/bin/sh

#
# Report an user's allow rule value in VirtualBox Autostart policy.
# $1 should be the output file name to use.
#

if [[ -n "$1" ]]; then

cat <<"EOF" >$1
BEGIN {
        policy = "unknown"
        users = 0
}

/^[A-Za-z0-9._-]+/ {
#
        if ( $0 == "default_policy" ) {
                getline
                if ( $0 == "allow" || $0 == "deny" ) {
                        policy = $0
                }  else {
                        print "Invalid policy!"
                        exit 1
                }
        } else {
                if ( $0 == "allow" ) {
                        getline
                        allow[u] = $0
                } else if ( $0 == "startup_delay" ) {
                        getline
                        startup_delay[u] = $0
                } else {
                        # Got an user!
                        u = users++
                        user[u] = $0
                }
        }
}

END {
        for (i in user) {
                printf( "%s:%s\n", user[i], allow[i] )
        }
}
EOF

fi

/opt/VirtualBox/smf-vboxautostart-policy.sh

#!/bin/sh

#
# Report the current VirtualBox Autostart policy.
# $1 should the tokenized configuration file.
#
if [[ -n "$1" ]]; then
        cat "$1" |awk '/^default_policy$/ {getline; if ($0=="allow"||$0=="deny") print $0}'
fi

/opt/VirtualBox/smf-vboxautostart-user.sh

#!/bin/sh

#
# Report an user's allow rule value in VirtualBox Autostart policy.
# $1 should be the tokenized configuration file.
# $2 should be the auxiliary awk script.
# $3 should be the user to query.
#

if [[ -n "$1" ]] && [[ -n "$2" ]] && [[ -n "$3" ]]; then
        cat "$1" |awk -f $2 |grep "$3:" |sed "s/$3://"
fi

Regards.