holgerBerger / hpc-workspace

Automatically exported from code.google.com/p/hpc-workspace
GNU General Public License v3.0
18 stars 13 forks source link

access problems to user_config #79

Closed URZ-HD closed 3 years ago

URZ-HD commented 3 years ago

Hi, we are using workspaces with setuid options. ws_allocate has is installed correctly with 4755 permission bits. If the user home is limited to 0700 permission (which should be the default for Home directories), the workspace tools are not able to access the path "~/.ws_user.conf":

I've added the following debug code to ws_allocate.cpp (Line 159):

            ifstream infile;
            infile.exceptions(ifstream::failbit | ifstream::badbit);
            try {
                infile.open((Workspace::getuserhome()+"/.ws_user.conf").c_str());
            }
            catch (std::ios_base::failure& e) {
                cerr << "Exception during opening userconfig file: " << e.what() << endl;
                cerr << "Errorno: " << strerror(errno) << endl ;
            }

This shows the error:

[lsven@login2 ~]$ /tmp/ws_allocate test 5 --debug 
Exception during opening userconfig file: basic_ios::clear
**Errorno: Permission denied**
terminate called after throwing an instance of 'std::ios_base::failure'
  what():  basic_ios::clear
Aborted

After changing the HOME permissions to 0755, it works like expected:

[lsven@login2 ~]$ /tmp/ws_allocate test 5 --debug 
Info: Took email address <hmpf> from users config.
Warning: reminder is only sent after workspace expiry!
debug: secondary group users
[...]

I think the problem would be the "drop_cap" function call before the commandline parsing. Is this intended behaviour or can the "drop_cap" call simply moved after the commandline function ?

thanks, Sven

holgerBerger commented 3 years ago

Yes, that is an oversight.

Problem is that dropping privileges is actually a good idea here, as user input is parsed here, and any kind of e.g. buffer overflow would be pretty bad. So the argument parser and the yaml parser could be attacked here if the privileges would not be dropped before. It is a question of trust in those external libraries.

May be it would make sense to read the file unconditionally before dropping privileges into a buffer and parse the buffer later,

holgerBerger commented 3 years ago

should be fixed.

URZ-HD commented 3 years ago

Hi, you are right, parsing the userinput should be down with dropped privileges. Another way to achieve this, might be dropping the permissions for the commandline call to the current user (=owner of the home), instead of the dbuid-user ? Afterwards the privileges can be dropped to dbuid-user.

best, Sven

holgerBerger commented 3 years ago

i moved the reading before the checking in latest version, that should solve the problem. Also added a check if the file is a symlink, as the reading is now with privileges and could be used to get a few bytes from some files.

URZ-HD commented 3 years ago

perfect, thanks.