Hi all,
There is a permission problem if the daemon is started given -u and -g parameters (sets up user and group for the process).
The do_suid function (defined in demonize.c) is called after the call to init_modules(), so the mod_init functions of the configured module are loaded before the call to do_suid. This wasn't a problem in 1.3 because no module(I am aware off) use the uid and gid of the process to do permission checks.
This has changed in 1.5, module carrierroute, as there is a check to see if the route file in config-file mode (usually /etc/kamailio/carrierroute.conf) has the right permission set on it (Issues an warning if it's worldly writable and error if it's not writable by the process owner). This of course is a problem because kamailio hasn't yet setuid()/setgid() so the checks are done using the wrong uid.
A correct (imho) course of action is to move the call to do_suid function before the call to init_modules()(and before any other calls to initialization functions).
I've attached a small patch that does this (tested).
There are any considerations on why the init_modules() should be called with another uid/gid?
Greetings, Marius
Index: main.c =================================================================== --- main.c (revision 5937) +++ main.c (working copy) @@ -656,10 +656,6 @@ LM_WARN("using only the first listen address (no fork)\n"); }
- /* try to drop privileges */ - if (do_suid(uid, gid)==-1) - goto error; - /* we need another process to act as the timer*/ if (start_timer_processes()!=0) { LM_CRIT("cannot start timer process(es)\n"); @@ -752,7 +748,6 @@
/* all processes should have access to all the sockets (for sending) * so we open all first*/ - if (do_suid(uid, gid)==-1) goto error; /* try to drop privileges */
/* udp processes */ for(si=udp_listen; si; si=si->next){ @@ -1278,6 +1273,12 @@ LM_INFO("no private memory pool configured, processes use system memory\n"); #endif
+ /* Set uid/gid */ + if (do_suid(uid, gid)==-1){ + LM_ERR("failed to drop privileges"); + goto error; /* try to drop privileges */ + } + /* Init statistics */ if (init_stats_collector()<0) { LM_ERR("failed to initialize statistics\n"); @@ -1330,3 +1331,4 @@ cleanup(0); return ret; } +
On Oct 13, 2009 at 12:55, marius zbihlei marius.zbihlei@1and1.ro wrote:
Hi all,
There is a permission problem if the daemon is started given -u and -g parameters (sets up user and group for the process).
The do_suid function (defined in demonize.c) is called after the call to init_modules(), so the mod_init functions of the configured module are loaded before the call to do_suid. This wasn't a problem in 1.3 because no module(I am aware off) use the uid and gid of the process to do permission checks.
This has changed in 1.5, module carrierroute, as there is a check to see if the route file in config-file mode (usually /etc/kamailio/carrierroute.conf) has the right permission set on it (Issues an warning if it's worldly writable and error if it's not writable by the process owner). This of course is a problem because kamailio hasn't yet setuid()/setgid() so the checks are done using the wrong uid.
You should do the check then from child_init(PROC_INIT) (rank==PROC_INIT). It's executed after setuid(), but before any real forking (so you could still exit gracefully).
A correct (imho) course of action is to move the call to do_suid function before the call to init_modules()(and before any other calls to initialization functions).
No, the do_suid() is on purpose _after_ the mod_init() to allow the modules to open sockets/files a.s.o. before the suid part (e.g. this way if started as root a module can open a socket on a port < 1024 from mod_init). All the operations that require special privileges should be done from mod_init().
I've attached a small patch that does this (tested).
There are any considerations on why the init_modules() should be called with another uid/gid?
Yes, see above.
Note that the PROC_INIT stuff will work on sip-router and probably not on old kamailio (> 3.0). You could try using 0 there (PROC_MAIN), but it will be called after forking some of the processes (the exit won't be as graceful).
Andrei
Andrei Pelinescu-Onciul wrote:
On Oct 13, 2009 at 12:55, marius zbihlei marius.zbihlei@1and1.ro wrote:
Hi all,
There is a permission problem if the daemon is started given -u and -g parameters (sets up user and group for the process).
The do_suid function (defined in demonize.c) is called after the call to init_modules(), so the mod_init functions of the configured module are loaded before the call to do_suid. This wasn't a problem in 1.3 because no module(I am aware off) use the uid and gid of the process to do permission checks.
This has changed in 1.5, module carrierroute, as there is a check to see if the route file in config-file mode (usually /etc/kamailio/carrierroute.conf) has the right permission set on it (Issues an warning if it's worldly writable and error if it's not writable by the process owner). This of course is a problem because kamailio hasn't yet setuid()/setgid() so the checks are done using the wrong uid.
You should do the check then from child_init(PROC_INIT) (rank==PROC_INIT). It's executed after setuid(), but before any real forking (so you could still exit gracefully).
A correct (imho) course of action is to move the call to do_suid function before the call to init_modules()(and before any other calls to initialization functions).
No, the do_suid() is on purpose _after_ the mod_init() to allow the modules to open sockets/files a.s.o. before the suid part (e.g. this way if started as root a module can open a socket on a port < 1024 from mod_init). All the operations that require special privileges should be done from mod_init().
Ok, so tests like in carrierroute module's mod_init (testing the permission against what is returned from geteuid() and getegid()) should be moved to a later stage?
I've attached a small patch that does this (tested).
There are any considerations on why the init_modules() should be called with another uid/gid?
Yes, see above.
Note that the PROC_INIT stuff will work on sip-router and probably not on old kamailio (> 3.0). You could try using 0 there (PROC_MAIN), but it will be called after forking some of the processes (the exit won't be as graceful).
Andrei
The bug is present in kamailio 1.5 so i haven't yet looked into the implementation of 3.0 . Looks nice that there is a central way where to do this kind of actions
Greetings Marius
On Oct 13, 2009 at 13:57, marius zbihlei marius.zbihlei@1and1.ro wrote:
Andrei Pelinescu-Onciul wrote:
On Oct 13, 2009 at 12:55, marius zbihlei marius.zbihlei@1and1.ro wrote:
Hi all,
There is a permission problem if the daemon is started given -u and -g parameters (sets up user and group for the process).
The do_suid function (defined in demonize.c) is called after the call to init_modules(), so the mod_init functions of the configured module are loaded before the call to do_suid. This wasn't a problem in 1.3 because no module(I am aware off) use the uid and gid of the process to do permission checks.
This has changed in 1.5, module carrierroute, as there is a check to see if the route file in config-file mode (usually /etc/kamailio/carrierroute.conf) has the right permission set on it (Issues an warning if it's worldly writable and error if it's not writable by the process owner). This of course is a problem because kamailio hasn't yet setuid()/setgid() so the checks are done using the wrong uid.
You should do the check then from child_init(PROC_INIT) (rank==PROC_INIT). It's executed after setuid(), but before any real forking (so you could still exit gracefully).
A correct (imho) course of action is to move the call to do_suid function before the call to init_modules()(and before any other calls to initialization functions).
No, the do_suid() is on purpose _after_ the mod_init() to allow the modules to open sockets/files a.s.o. before the suid part (e.g. this way if started as root a module can open a socket on a port < 1024 from mod_init). All the operations that require special privileges should be done from mod_init().
Ok, so tests like in carrierroute module's mod_init (testing the permission against what is returned from geteuid() and getegid()) should be moved to a later stage?
Yes, it should.
But why do you need it, you open/re-open the file at runtime if changed?
I've attached a small patch that does this (tested).
There are any considerations on why the init_modules() should be called with another uid/gid?
Yes, see above.
Note that the PROC_INIT stuff will work on sip-router and probably not on old kamailio (> 3.0). You could try using 0 there (PROC_MAIN), but it will be called after forking some of the processes (the exit won't be as graceful).
Andrei
The bug is present in kamailio 1.5 so i haven't yet looked into the implementation of 3.0 . Looks nice that there is a central way where to do this kind of actions
Andrei
Andrei Pelinescu-Onciul wrote:
On Oct 13, 2009 at 13:57, marius zbihlei marius.zbihlei@1and1.ro wrote:
Yes, it should.
But why do you need it, you open/re-open the file at runtime if changed?
Changes which are done to the routing table at runtime via kamctl(and aliases) are stored also into this file, so the file must be writable by the user. (forget the last reply i forgot to cc to list)
-
Andrei Pelinescu-Onciul -
marius zbihlei