From miles at lubin.us Sat Dec 3 01:21:51 2005 From: miles at lubin.us (Miles Lubin) Date: Sat Dec 3 01:23:25 2005 Subject: [suPHP] mod_userdir patch for 0.6.1 Message-ID: <4390E51F.3080506@lubin.us> Attached is the mod_userdir patch updated to 0.6.1. The patch allows suphp to correctly handle permissions with user sites, generated by mod_suphp; instead of using the permissions of the main site for force and paranoid mode, it will use the permissions of the correct user. Changes from 0.6.0 patch: - handle_userdir is enabled by default, set handle_userdir=false in /etc/suphp.conf to disable it - check the length of the url argument in checkUserDir() (a theoretical security issue, though not exploitable) - made the code more conforming to the suphp's coding style Miles Lubin -------------- next part -------------- diff -ur suphp-0.6.1/doc/CONFIG suphp-0.6.1-userdir/doc/CONFIG --- suphp-0.6.1/doc/CONFIG 2005-11-26 14:45:49.000000000 -0500 +++ suphp-0.6.1-userdir/doc/CONFIG 2005-12-02 15:07:41.000000000 -0500 @@ -95,6 +95,11 @@ Minimum GID allowed to execute scripts. Defaults to compile-time value. +handle_userdir: + Handle sites created by mod_userdir. + Scripts on userdir sites will be executed with the permissions + of the owner of the site. This option only affects force and paranoid mode. + This option is enabled by default. 3. Handlers diff -ur suphp-0.6.1/doc/suphp.conf-example suphp-0.6.1-userdir/doc/suphp.conf-example --- suphp-0.6.1/doc/suphp.conf-example 2005-11-26 14:45:49.000000000 -0500 +++ suphp-0.6.1-userdir/doc/suphp.conf-example 2005-12-02 15:07:41.000000000 -0500 @@ -38,6 +38,8 @@ ; Minimum GID min_gid=100 +; Use correct permissions for mod_userdir sites +handle_userdir=true [handlers] ;Handler for php-scripts diff -ur suphp-0.6.1/src/Application.cpp suphp-0.6.1-userdir/src/Application.cpp --- suphp-0.6.1/src/Application.cpp 2005-11-26 14:45:49.000000000 -0500 +++ suphp-0.6.1-userdir/src/Application.cpp 2005-12-02 17:18:27.000000000 -0500 @@ -19,6 +19,7 @@ */ #include +#include #include "config.h" @@ -300,29 +301,33 @@ // Paranoid and force mode #if (defined(OPT_USERGROUP_PARANOID) || defined(OPT_USERGROUP_FORCE)) - std::string targetUsername, targetGroupname; - try { - targetUsername = environment.getVar("SUPHP_USER"); - targetGroupname = environment.getVar("SUPHP_GROUP"); - } catch (KeyNotFoundException& e) { - throw SecurityException( + if (config.getHandleUserdir() && checkUserDir(environment.getVar("SUPHP_URI"),targetUser)) { + targetGroup = targetUser.getGroupInfo(); + } else { + std::string targetUsername, targetGroupname; + try { + targetUsername = environment.getVar("SUPHP_USER"); + targetGroupname = environment.getVar("SUPHP_GROUP"); + } catch (KeyNotFoundException& e) { + throw SecurityException( "Environment variable SUPHP_USER or SUPHP_GROUP not set", __FILE__, __LINE__); - } + } - if (targetUsername[0] == '#' && targetUsername.find_first_not_of( + if (targetUsername[0] == '#' && targetUsername.find_first_not_of( "0123456789", 1) == std::string::npos) { - targetUser = api.getUserInfo(Util::strToInt(targetUsername.substr(1))); - } else { - targetUser = api.getUserInfo(targetUsername); - } + targetUser = api.getUserInfo(Util::strToInt(targetUsername.substr(1))); + } else { + targetUser = api.getUserInfo(targetUsername); + } - if (targetGroupname[0] == '#' && targetGroupname.find_first_not_of( + if (targetGroupname[0] == '#' && targetGroupname.find_first_not_of( "0123456789", 1) == std::string::npos) { - targetGroup = api.getGroupInfo( + targetGroup = api.getGroupInfo( Util::strToInt(targetGroupname.substr(1))); - } else { - targetGroup = api.getGroupInfo(targetGroupname); + } else { + targetGroup = api.getGroupInfo(targetGroupname); + } } #endif // OPT_USERGROUP_PARANOID || OPT_USERGROUP_FORCE @@ -473,6 +478,28 @@ } } +bool suPHP::Application::checkUserDir(const std::string& url, UserInfo& user) const { + + if (url.length() <= 2 || url[1] != '~') + return false; + + API& api = API_Helper::getSystemAPI(); + std::string topDir; + std::istringstream strm(url); + + for (int i = 0; i < 2; i++) + if (!std::getline(strm, topDir, '/')) + return false; + + std::string userName = topDir.substr(1,topDir.length()); + + try { + user = api.getUserInfo(userName); + return true; + } catch (LookupException& e) { + return false; + } +} int main(int argc, char **argv) { try { diff -ur suphp-0.6.1/src/Application.hpp suphp-0.6.1-userdir/src/Application.hpp --- suphp-0.6.1/src/Application.hpp 2005-11-26 14:45:49.000000000 -0500 +++ suphp-0.6.1-userdir/src/Application.hpp 2005-12-02 15:07:41.000000000 -0500 @@ -39,6 +39,7 @@ #include "SystemException.hpp" #include "SoftException.hpp" #include "SecurityException.hpp" +#include "UserInfo.hpp" namespace suPHP { /** @@ -107,6 +108,12 @@ const Configuration& config) const throw (SoftException); + /** + * Checks if a given URL is a userdir + * associated user is assigned to the user parameter + */ + bool checkUserDir(const std::string& url, + UserInfo& user) const; public: /** diff -ur suphp-0.6.1/src/Configuration.cpp suphp-0.6.1-userdir/src/Configuration.cpp --- suphp-0.6.1/src/Configuration.cpp 2005-11-26 14:45:49.000000000 -0500 +++ suphp-0.6.1-userdir/src/Configuration.cpp 2005-12-02 17:22:46.000000000 -0500 @@ -112,6 +112,7 @@ #endif this->umask = 0077; this->chroot_path = ""; + this->handle_userdir = true; } void suPHP::Configuration::readFromFile(File& file) @@ -157,6 +158,8 @@ this->umask = Util::octalStrToInt(value); else if (key == "chroot") this->chroot_path = value; + else if (key == "handle_userdir") + this->handle_userdir = this->strToBool(value); else throw ParsingException("Unknown option \"" + key + "\" in section [global]", @@ -250,3 +253,7 @@ std::string suPHP::Configuration::getChrootPath() const { return this->chroot_path; } + +bool suPHP::Configuration::getHandleUserdir() const { + return this->handle_userdir; +} diff -ur suphp-0.6.1/src/Configuration.hpp suphp-0.6.1-userdir/src/Configuration.hpp --- suphp-0.6.1/src/Configuration.hpp 2005-11-26 14:45:49.000000000 -0500 +++ suphp-0.6.1-userdir/src/Configuration.hpp 2005-12-02 15:07:41.000000000 -0500 @@ -57,7 +57,8 @@ int min_gid; int umask; std::string chroot_path; - + bool handle_userdir; + /** * Converts string to bool */ @@ -165,6 +166,12 @@ * Return chroot path */ std::string getChrootPath() const; + + /** + * Return whether to correctly handle mod_userdir sites + */ + bool getHandleUserdir() const; + }; }; diff -ur suphp-0.6.1/src/apache/mod_suphp.c suphp-0.6.1-userdir/src/apache/mod_suphp.c --- suphp-0.6.1/src/apache/mod_suphp.c 2005-11-26 14:45:49.000000000 -0500 +++ suphp-0.6.1-userdir/src/apache/mod_suphp.c 2005-12-02 15:07:41.000000000 -0500 @@ -444,7 +444,10 @@ } } } - + + /* for mod_userdir checking */ + apr_table_setn(r->subprocess_env, "SUPHP_URI", apr_pstrdup(p, r->uri)); + if (auth_user && auth_pass) { ap_table_setn(r->subprocess_env, "SUPHP_AUTH_USER", auth_user); ap_table_setn(r->subprocess_env, "SUPHP_AUTH_PW", auth_pass); diff -ur suphp-0.6.1/src/apache2/mod_suphp.c suphp-0.6.1-userdir/src/apache2/mod_suphp.c --- suphp-0.6.1/src/apache2/mod_suphp.c 2005-11-26 14:45:49.000000000 -0500 +++ suphp-0.6.1-userdir/src/apache2/mod_suphp.c 2005-12-02 15:07:41.000000000 -0500 @@ -461,6 +461,10 @@ } } + /* for mod_userdir checking */ + apr_table_setn(r->subprocess_env, "SUPHP_URI", + apr_pstrdup(r->pool, r->uri)); + if (auth_user && auth_pass) { apr_table_setn(r->subprocess_env, "SUPHP_AUTH_USER", auth_user);