inn.conf: Support for `newsuser', `newsgrp' options (patch is, hopefully, included)
Julien ÉLIE
julien at trigofacile.com
Fri Dec 21 15:17:56 UTC 2007
Hi Ivan,
First of all, thanks for your patches.
I have just taken the time to apply them on my INN 2.5 news server
in order to test your changes but I have a few questions about them.
By the way, my current patch for INN 2.5 is there, in case someone
wishes to test it:
http://iulius.dinauz.org/vrac/newsuser.diff.txt
I had to manually do almost all the changes because lots of syntax
have changed in the files (logging, paths, checks, etc.).
> diff -drHuN inn2-2.4.3+20070806-debian-1/frontends/inews.c inn2-2.4.3+20070806-debian-1-newsuser/frontends/inews.c
> --- inn2-2.4.3+20070806-debian-1/frontends/inews.c 2007-08-06 16:07:29.000000000 +0700
> +++ inn2-2.4.3+20070806-debian-1-newsuser/frontends/inews.c 2007-12-16 23:13:36.000000000 +0600
> /* See if the we're in the right group. */
> - if ((grp = getgrnam(NEWSGRP)) == NULL || (mem = grp->gr_mem) == NULL)
> - /* Silent falure; clients might not have the group. */
> - return false;
> - if (group == grp->gr_gid)
> - return true;
> - while ((p = *mem++) != NULL)
> - if (strcmp(name, p) == 0)
> - return true;
> + /* IS: here, I examine process supplementary groups, rather than the
> + * group(5) file entry; I'm not sure how the security will be
> + * affected by this change.
> + */
Is it finally safe to do that?
> + {
> + int ngroups = getgroups (0, 0);
> + gid_t *groups, *gp;
> + int rv;
> + int rest;
> +
> + groups = (gid_t *)xmalloc (ngroups * sizeof (*groups));
> + if ((rv = getgroups (ngroups, groups)) < 0) {
> + /* Silent falure; client doesn't have the group. */
^^^^^^
Bad copy/paste from the code above :)
> + return false;
> + }
> + for (rest = ngroups, gp = groups; rest > 0; rest--, gp++) {
> + if (*gp == news_gid)
> + return true;
> + }
> + }
> +
> return false;
> }
> diff -drHu inn2-2.4.3+20070806-debian-1/nnrpd/nnrpd.c inn2-2.4.3+20070806-debian-1-newsuser/nnrpd/nnrpd.c
> --- inn2-2.4.3+20070806-debian-1/nnrpd/nnrpd.c 2007-08-06 16:07:29.000000000 +0700
> +++ inn2-2.4.3+20070806-debian-1-newsuser/nnrpd/nnrpd.c 2007-12-16 23:46:52.000000000 +0600
> @@ -12,7 +12,6 @@
> #include "portable/wait.h"
> #include <grp.h>
> #include <netdb.h>
> -#include <pwd.h>
> #include <signal.h>
Can grp.h be removed from that file?
I also did that for nnrpd/nnrpd.c:
--- nnrpd.c (révision 7701)
+++ nnrpd.c (copie de travail)
@@ -687,7 +687,6 @@
int lfd, fd;
pid_t pid = -1;
FILE *pidfile;
- struct passwd *pwd;
int clienttimeout;
char *ConfFile = NULL;
char *path;
@@ -814,12 +813,7 @@
/* If started as root, switch to news uid. Unlike other parts of INN, we
don't die if we can't become the news user. As long as we're not
running as root, everything's fine; the things we write it's okay to
write as a member of the news group. */
if (getuid() == 0) {
- pwd = getpwnam(NEWSUSER);
- if (pwd == NULL)
- die("cant resolve %s to a UID (account doesn't exist?)", NEWSUSER);
- setuid(pwd->pw_uid);
- if (getuid() != pwd->pw_uid)
- die("cant setuid to %s (%d)", NEWSUSER, pwd->pw_uid);
+ ensure_news_user_grp(1, 1);
}
Do you think it is the right thing?
For tdx-util.c, I had to add tests for INN_TESTSUITE as follows:
--- storage/tradindexed/tdx-util.c (révision 7701)
+++ storage/tradindexed/tdx-util.c (copie de travail)
@@ -539,15 +518,18 @@
tdx_index_audit(false);
break;
case 'F':
- setuid_news();
+ if (getenv("INN_TESTSUITE") == NULL)
+ ensure_news_user_grp(1, 1);
tdx_index_audit(true);
break;
case 'R':
- setuid_news();
+ if (getenv("INN_TESTSUITE") == NULL)
+ ensure_news_user_grp(1, 1);
group_rebuild(newsgroup, path);
break;
case 'c':
- setuid_news();
+ if (getenv("INN_TESTSUITE") == NULL)
+ ensure_news_user_grp(1, 1);
group_create(newsgroup, artlow, arthigh, flag);
break;
I also had to patch innd/innd.c; could you please tell me if what
I did sounds good to you?
--- innd/innd.c (révision 7701)
+++ innd/innd.c (copie de travail)
@@ -5,11 +5,11 @@
#include "config.h"
#include "clibrary.h"
-#include <grp.h>
-#include <pwd.h>
#include "inn/innconf.h"
#include "inn/messages.h"
+#include "inn/newsuser.h"
#include "innperl.h"
#define DEFINE_DATA
@@ -278,8 +278,6 @@
const char *name, *p;
char *path;
bool flag;
- struct passwd *pwd;
- struct group *grp;
static char WHEN[] = "PID file";
int i;
char buff[SMBUF];
@@ -314,22 +312,7 @@
/* Make sure innd is running as the correct user. If it is started as
root, switch to running as the news user. */
- grp = getgrnam(NEWSGRP);
- if (grp == NULL)
- die("SERVER cannot get GID for %s", NEWSGRP);
- pwd = getpwnam(NEWSUSER);
- if (pwd == NULL)
- die("SERVER cannot get UID for %s", NEWSUSER);
- if (getuid() == 0 || geteuid() == 0) {
- setgid(grp->gr_gid);
- setuid(pwd->pw_uid);
- }
- if (getuid() != pwd->pw_uid)
- die("SERVER should run as user %s (%lu), not %lu", NEWSUSER,
- (unsigned long) pwd->pw_uid, (unsigned long) getuid());
- if (getgid() != grp->gr_gid)
- die("SERVER should run as group %s (%lu), not %lu", NEWSGRP,
- (unsigned long) grp->gr_gid, (unsigned long) getgid());
+ ensure_news_user_grp(1, 1);
/* Handle malloc debugging. */
#if defined(_DEBUG_MALLOC_INC)
As for scripts/innshellvars* scripts, I kept $newsgroup (= $newsgrp)
for backward compatibility.
Beware that your patch had for instance broken scripts/inncheck.in
and something like that should be added:
--- scripts/inncheck.in (révision 7701)
+++ scripts/inncheck.in (copie de travail)
@@ -653,7 +650,7 @@
sub
checkperm
{
- local ($f, $m, $u, $g) = ( @_, $newsuser, $newsgroup);
+ local ($f, $m, $u, $g) = ( @_, $INN::Config::newsuser, $INN::Config::newsgrp);
local (@sb, $owner, $group, $mode);
die "Internal error, undefined name in perm from ", (caller(0))[2], "\n"
Or of course $newsgrp instead of $INN::Config::newsgrp under INN 2.4.3.
Well, I believe that's all I have to say. Thanks a lot for all your work!!
If my news server is OK during a week and if innbind works, I will commit
your patch to INN 2.5.
Regards,
--
Julien ÉLIE
« -- Vous ne sortirez jamais d'ici étrangers ! Ce tombeau sera votre tombeau !
-- Tout cela ne serait pas arrivé dans un menhir ! » (Astérix)
More information about the inn-workers
mailing list