[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: ITS#98 'user' patch for BSD systems



I've been wondering a bit more about this patch...


Pat Lashley writes:

>>> NOTE that if <new-user> is numeric, then at least one <new-group> must
>>> be specified.
>> 
>> Can't we get the user name from getpwuid(run_uid)->pw_name?
> 
> What if there are aliases for that uid?  Admitedly, this is most
> commonly only done to provide for different shells or root directories;
> but there is nothing to prevent it from also affecting group access
> rights.
> 
> Of course, we could just say "Don't do that".  Or "If you specify
> a uid number with no groups, we'll use getpwuid() to get the user
> name.  This may cause unexpected, and possibly even non-deterministic
> results if there is more than one passwd entry with that uid."

Until I read this, I always thought multiple users with the same uid
meant a misconfigured system...

Can't we just mimic what the documentation of commands like 'ps' or 'ls'
say about aliased users on your host?  They too translate uids to names.


> It turns out that initgroups(3) didn't seem to do quite what we want.
> (Since it is based on the current uid, we'd have to change the uid
> first.  And then we might not have all of the right permissions...)

Not here.  initgroups() on Solaris and Alpha is
     int initgroups(const char *name, gid_t basegid);
It sets the supplementary group access list to that users' groups.
As you pointed out, it's getgroups() which is based on the current uid.
So I don't see why you need getgrouplist() when you have initgroups();
I think this should work, plus error checking --

vars:
    uid_t       run_uid = 0;
    char *      run_username = NULL;
    int         ngids = 0;
    gid_t       run_gids[NGROUPS_MAX];

config:
    if ( isdigit( (unsigned char) *cargv[1] )) {
        struct passwd *user = getpwuid( run_uid = atoi( cargv[1] ) );
        run_username = user->pw_name;
    } else {
        struct passwd *user = getpwnam( run_username = cargv[1] );
        run_uid = user->pw_uid;
    }
    run_username = ch_strdup( run_username );

    if (cargc > 2) {
        ngids = 0 ;
        for (i = 2  ;  i < cargc  ;  i++) {
            if ( isdigit( (unsigned char) *cargv[i] )) {
                run_gids[ngids++] = atoi( cargv[i] );
            } else {
                struct group* grp = getgrnam( cargv[i] );
                run_gids[ngids++] = grp->gr_gid ;
            }
        }
    } else {
        run_gids[0] = user->pw_gid;
        ngids = -1;    /* mark that initgroups() should be used */
    }

init:
    if ( ngids != 0 ) {
        if ( ngids > 0 )
            setgroups( ngids, run_gids);
        else
            initgroups( run_username, run_gids[0] );
        setgid( run_gids[0] );
        setegid( run_gids[0] );
    }
    if ( run_uid != 0 ) {
        setuid( run_uid );
        seteuid( run_uid );
    }


> (It is only by a herculean effort of will that I'm avoiding starting a
> rant about isdigit(), et. al., not being able to handle whatever char
> type is native to that compiler.

That's unfixable on hosts where char is signed and EOF == -1, since
isXXX() are specified to recognize EOF.  I'm sure it was nice to be able
to feed the output of getchar() directly to isXXX(), and nobody had
heard about 8-bit character sets...

> Or the extreme brain damage that defined 'char' without specifying
> whether it was to be treated as a signed value or not.)

Nah.  C was designed so signedness and sizes could be chosen as whatever
was most efficient on the architecture it was running on.  The brain
damage is that we are _still_ using C (and C++) as The Language Which
Everything Supports.  It should have been possible to demote it to be a
kind of "super-assember" by now.

-- 
Hallvard