inn.conf: Support for `newsuser', `newsgrp' options (patch is, hopefully, included)
Ivan Shmakov
oneingray at gmail.com
Fri Dec 14 08:48:53 UTC 2007
>>>>> Russ Allbery <rra at stanford.edu> writes:
>> Is there a problem when the first paragraph of the message is
>> indented with TABs?
> Ecartis does all sorts of fascinating things to messages for no
> particularly good reasons.
Oh, now I see, it strips MIME attachments, too!
> Your plan sounds good to me. I wish I had time to write code. :)
I've actually have written some code already. Could you take a
look at it?
There's an issue with get_news_uid_gid () -- it assumes innconf
!= NULL (i. e., `inn.conf' was read.) However, when `inndstart'
is started ``setuid root'', it's way more secure to read
`inn.conf' /after/ dropping privileges (as it's done now.) So,
get_news_uid_gid () should contain a fallback, like:
const char *newsuser = innconf != 0 ? innconf->newsuser : NEWSUSER;
const char *newsgrp = innconf != 0 ? innconf->newsgrp : NEWSGRP;
to be used in `inndstart'.
There may be other issues with the code, so I'd ask not to apply
these patches right now. However, I'll appreciate any comments
on them.
The change to use get_news_uid_gid () instead of NEWSUSER and
NEWSGRP is probably to be done across the whole source at once.
BTW, since setuid_news () was a way to save one from ``messing
up the ownership'', it was better to refer to EUID, and not UID.
Though, it's a non-issue, unless one of the binaries gets
somehow installed setuid.
The first patch (hope Ecartis doesn't strip diffs):
diff -drHu inn2-2.4.3+20070806-debian-1/include/inn/innconf.h inn2-2.4.3+20070806-debian-1-newsuser/include/inn/innconf.h
--- inn2-2.4.3+20070806-debian-1/include/inn/innconf.h 2007-08-06 16:07:29.000000000 +0700
+++ inn2-2.4.3+20070806-debian-1-newsuser/include/inn/innconf.h 2007-12-11 22:23:57.000000000 +0600
@@ -26,6 +26,8 @@
char *mta; /* MTA for mailing to moderators, innmail */
char *pathhost; /* Entry for the Path line */
char *server; /* Default server to connect to */
+ char *newsuser; /* User to run under */
+ char *newsgrp; /* Group to run under */
/* Feed Configuration */
long artcutoff; /* Max accepted article age */
diff -drHu inn2-2.4.3+20070806-debian-1/lib/innconf.c inn2-2.4.3+20070806-debian-1-newsuser/lib/innconf.c
--- inn2-2.4.3+20070806-debian-1/lib/innconf.c 2007-08-06 16:07:29.000000000 +0700
+++ inn2-2.4.3+20070806-debian-1-newsuser/lib/innconf.c 2007-12-11 22:33:17.000000000 +0600
@@ -121,6 +121,9 @@
{ K(sourceaddress6), STRING (NULL) },
{ K(timer), NUMBER (0) },
+ { K(newsuser), STRING (NEWSUSER) },
+ { K(newsgrp), STRING (NEWSGRP) },
+
{ K(patharchive), STRING (NULL) },
{ K(patharticles), STRING (NULL) },
{ K(pathbin), STRING (NULL) },
diff -drHu inn2-2.4.3+20070806-debian-1/lib/Makefile inn2-2.4.3+20070806-debian-1-newsuser/lib/Makefile
--- inn2-2.4.3+20070806-debian-1/lib/Makefile 2007-08-06 16:07:29.000000000 +0700
+++ inn2-2.4.3+20070806-debian-1-newsuser/lib/Makefile 2007-12-11 23:15:46.000000000 +0600
@@ -10,7 +10,8 @@
conffile.c confparse.c daemonize.c date.c dbz.c defdist.c \
fdflags.c fdlimit.c genid.c getfqdn.c getmodaddr.c gettime.c \
hash.c hashtab.c innconf.c inndcomm.c list.c localopen.c \
- lockfile.c makedir.c md5.c messages.c mmap.c parsedate.c \
+ lockfile.c makedir.c md5.c messages.c mmap.c newsuser.c \
+ parsedate.c \
qio.c radix32.c readin.c remopen.c reservedfd.c resource.c \
sendarticle.c sendpass.c sequence.c sockaddr.c timer.c tst.c \
uwildmat.c vector.c version.c wire.c xfopena.c xmalloc.c \
diff -drHuN inn2-2.4.3+20070806-debian-1/include/newsuser.h inn2-2.4.3+20070806-debian-1-newsuser/include/newsuser.h
--- inn2-2.4.3+20070806-debian-1/include/newsuser.h 1970-01-01 07:00:00.000000000 +0700
+++ inn2-2.4.3+20070806-debian-1-newsuser/include/newsuser.h 2007-12-11 22:04:11.000000000 +0600
@@ -0,0 +1,21 @@
+/*** newsuser.h --- Ensure running as `news' user / group: header -*- C -*- */
+
+/*** Ivan Shmakov, 2007 */
+/** This code is in the public domain */
+
+/*** Code: */
+#ifndef NEWSUSER_H
+#define NEWSUSER_H 1
+
+#include "config.h"
+#include "clibrary.h"
+
+int get_news_uid_gid (uid_t *uid, gid_t *gid, int may_die_p);
+
+/** the following functions die () on failure */
+void ensure_news_user (int may_setuid_p);
+void ensure_news_grp (int may_setgid_p);
+void ensure_news_user_grp (int may_setuid_p, int may_setgid_p);
+
+#endif
+/*** newsuser.h ends here */
diff -drHuN inn2-2.4.3+20070806-debian-1/lib/newsuser.c inn2-2.4.3+20070806-debian-1-newsuser/lib/newsuser.c
--- inn2-2.4.3+20070806-debian-1/lib/newsuser.c 1970-01-01 07:00:00.000000000 +0700
+++ inn2-2.4.3+20070806-debian-1-newsuser/lib/newsuser.c 2007-12-12 00:23:37.000000000 +0600
@@ -0,0 +1,99 @@
+/*** newsuser.c --- Ensure running as `news' user / group -*- C -*- */
+
+/*** Ivan Shmakov, 2007 */
+/** This code is in the public domain */
+
+/*** Code: */
+#include "config.h"
+#include "clibrary.h"
+
+#include <pwd.h>
+#include <grp.h>
+
+#include "inn/innconf.h"
+#include "newsuser.h"
+
+/** Resolve newsuser to a UID and newsgrp to a GID */
+int
+get_news_uid_gid (uid_t *uid, gid_t *gid, int may_die_p)
+{
+ int fail_p = 0;
+ struct passwd *pwd;
+ struct group *grp;
+
+ /* resolve newsuser to a UID */
+ if (uid == 0) {
+ /* do nothing */
+ } else if ((pwd = getpwnam (innconf->newsuser)) != 0) {
+ *uid = pwd->pw_uid;
+ } else if (may_die_p) {
+ die (("can't resolve %s to a UID"
+ " (account doesn't exist?)"), innconf->newsuser);
+ } else {
+ fail_p = 1;
+ }
+
+ /* resolve newsgrp to a GID */
+ if (gid == 0) {
+ /* do nothing */
+ } else if ((grp = getgrnam (innconf->newsgrp)) != 0) {
+ *gid = grp->gr_gid;
+ } else if (may_die_p) {
+ die (("can't resolve %s to a GID"
+ " (group doesn't exist?)"), innconf->newsgrp);
+ } else {
+ fail_p = 1;
+ }
+
+ /* . */
+ return fail_p ? -1 : 0;
+}
+
+/** Ensure running as newsuser */
+void
+ensure_news_user (int may_setuid_p)
+{
+ uid_t uid;
+
+ get_news_uid_gid (&uid, 0, 1);
+ if (geteuid () == 0) {
+ if (! may_setuid_p) {
+ /* NB: mustn't be run as root, unless `may_setuid_p' is true */
+ die ("must be run as %s, not as root", innconf->newsuser);
+ }
+ setuid (uid);
+ }
+ if (geteuid () != uid || getuid () != uid) {
+ die ("must be run as %s", innconf->newsuser);
+ }
+
+ /* . */
+}
+
+/** Ensure running as newsgrp group */
+void
+ensure_news_grp (int may_setgid_p)
+{
+ gid_t gid;
+
+ get_news_uid_gid (0, &gid, 1);
+ if (may_setgid_p && geteuid () == 0) {
+ setgid (gid);
+ }
+ if (getegid () != gid || getgid () != gid) {
+ die ("must be run as %s group", innconf->newsgrp);
+ }
+
+ /* . */
+}
+
+/** Ensure running as newsuser and newsgrp group */
+void
+ensure_news_user_grp (int may_setuid_p, int may_setgid_p)
+{
+ /* NB: changing the group first to lose root privileges last */
+ ensure_news_grp (may_setgid_p);
+ ensure_news_user (may_setuid_p);
+}
+
+/*** newsuser.c ends here */
The second one:
diff -drHu inn2-2.4.3+20070806-debian-1/expire/expire.c inn2-2.4.3+20070806-debian-1-newsuser/expire/expire.c
--- inn2-2.4.3+20070806-debian-1/expire/expire.c 2007-08-06 16:07:29.000000000 +0700
+++ inn2-2.4.3+20070806-debian-1-newsuser/expire/expire.c 2007-12-12 00:03:34.000000000 +0600
@@ -7,7 +7,6 @@
#include "clibrary.h"
#include <ctype.h>
#include <errno.h>
-#include <pwd.h>
#include <sys/stat.h>
#include <syslog.h>
#include <time.h>
@@ -17,10 +16,10 @@
#include "inn/messages.h"
#include "inndcomm.h"
#include "libinn.h"
+#include "newsuser.h"
#include "paths.h"
#include "storage.h"
-
typedef struct _EXPIRECLASS {
time_t Keep;
time_t Default;
@@ -488,25 +487,6 @@
}
-/*
-** Change to the news user if possible, and if not, die. Used for operations
-** that may create new database files so as not to mess up the ownership.
-*/
-static void
-setuid_news(void)
-{
- struct passwd *pwd;
-
- pwd = getpwnam(NEWSUSER);
- if (pwd == NULL)
- die("can't resolve %s to a UID (account doesn't exist?)", NEWSUSER);
- if (getuid() == 0)
- setuid(pwd->pw_uid);
- if (getuid() != pwd->pw_uid)
- die("must be run as %s", NEWSUSER);
-}
-
-
int
main(int ac, char *av[])
{
@@ -627,8 +607,8 @@
RealNow = Now;
Now += TimeWarp;
- /* Change users if necessary. */
- setuid_news();
+ /* Change to the newsuser and newsgrp group if necessary. */
+ ensure_news_user_grp (1, 1);
/* Parse the control file. */
if (av[0]) {
diff -drHu inn2-2.4.3+20070806-debian-1/expire/expireover.c inn2-2.4.3+20070806-debian-1-newsuser/expire/expireover.c
--- inn2-2.4.3+20070806-debian-1/expire/expireover.c 2007-08-06 16:07:29.000000000 +0700
+++ inn2-2.4.3+20070806-debian-1-newsuser/expire/expireover.c 2007-12-12 00:03:34.000000000 +0600
@@ -11,7 +11,6 @@
#include "config.h"
#include "clibrary.h"
#include <errno.h>
-#include <pwd.h>
#include <signal.h>
#include <syslog.h>
#include <time.h>
@@ -46,25 +45,6 @@
}
-/*
-** Change to the news user if possible, and if not, die. Used for operations
-** that may create new database files so as not to mess up the ownership.
-*/
-static void
-setuid_news(void)
-{
- struct passwd *pwd;
-
- pwd = getpwnam(NEWSUSER);
- if (pwd == NULL)
- die("can't resolve %s to a UID (account doesn't exist?)", NEWSUSER);
- if (getuid() == 0)
- setuid(pwd->pw_uid);
- if (getuid() != pwd->pw_uid)
- die("must be run as %s", NEWSUSER);
-}
-
-
int
main(int argc, char *argv[])
{
@@ -142,8 +122,8 @@
if (!innconf_read(NULL))
exit(1);
- /* Change to the news user if necessary. */
- setuid_news();
+ /* Change to the newsuser and newsgrp group if necessary. */
+ ensure_news_user_grp (1, 1);
/* Initialize the lowmark file, if one was requested. */
if (lowmark_path != NULL) {
diff -drHu inn2-2.4.3+20070806-debian-1/expire/makedbz.c inn2-2.4.3+20070806-debian-1-newsuser/expire/makedbz.c
--- inn2-2.4.3+20070806-debian-1/expire/makedbz.c 2007-08-06 16:07:29.000000000 +0700
+++ inn2-2.4.3+20070806-debian-1-newsuser/expire/makedbz.c 2007-12-12 00:03:34.000000000 +0600
@@ -6,7 +6,6 @@
#include "config.h"
#include "clibrary.h"
#include <errno.h>
-#include <pwd.h>
#include <syslog.h>
#include "dbz.h"
@@ -14,6 +13,7 @@
#include "inn/messages.h"
#include "inn/qio.h"
#include "libinn.h"
+#include "newsuser.h"
#include "paths.h"
#include "storage.h"
@@ -230,25 +230,6 @@
}
-/*
-** Change to the news user if possible, and if not, die. Used for operations
-** that may create new database files, so as not to mess up the ownership.
-*/
-static void
-setuid_news(void)
-{
- struct passwd *pwd;
-
- pwd = getpwnam(NEWSUSER);
- if (pwd == NULL)
- die("can't resolve %s to a UID (account doesn't exist?)", NEWSUSER);
- if (getuid() == 0)
- setuid(pwd->pw_uid);
- if (getuid() != pwd->pw_uid)
- die("must be run as %s", NEWSUSER);
-}
-
-
int
main(int argc, char **argv)
{
@@ -309,8 +290,8 @@
if (chdir(HistoryDir) < 0)
sysdie("cannot chdir to %s", HistoryDir);
- /* Change users if necessary. */
- setuid_news();
+ /* Change to the newsuser and newsgrp group if necessary. */
+ ensure_news_user_grp (1, 1);
Rebuild(size, IgnoreOld, Overwrite);
closelog();
diff -drHu inn2-2.4.3+20070806-debian-1/expire/makehistory.c inn2-2.4.3+20070806-debian-1-newsuser/expire/makehistory.c
--- inn2-2.4.3+20070806-debian-1/expire/makehistory.c 2007-08-06 16:07:29.000000000 +0700
+++ inn2-2.4.3+20070806-debian-1-newsuser/expire/makehistory.c 2007-12-12 00:03:34.000000000 +0600
@@ -8,7 +8,6 @@
#include "portable/wait.h"
#include <assert.h>
#include <errno.h>
-#include <pwd.h>
#include <syslog.h>
#include "inn/buffer.h"
@@ -18,6 +17,7 @@
#include "inn/qio.h"
#include "inn/wire.h"
#include "libinn.h"
+#include "newsuser.h"
#include "ov.h"
#include "paths.h"
#include "storage.h"
@@ -740,25 +740,6 @@
}
-/*
-** Change to the news user if possible, and if not, die. Used for operations
-** that may create new database files, so as not to mess up the ownership.
-*/
-static void
-setuid_news(void)
-{
- struct passwd *pwd;
-
- pwd = getpwnam(NEWSUSER);
- if (pwd == NULL)
- die("can't resolve %s to a UID (account doesn't exist?)", NEWSUSER);
- if (getuid() == 0)
- setuid(pwd->pw_uid);
- if (getuid() != pwd->pw_uid)
- die("must be run as %s", NEWSUSER);
-}
-
-
int
main(int argc, char **argv)
{
@@ -853,8 +834,8 @@
if (chdir(HistoryDir) < 0)
sysdie("cannot chdir to %s", HistoryDir);
- /* Change users if necessary. */
- setuid_news();
+ /* Change to the newsuser and newsgrp group if necessary. */
+ ensure_news_user_grp (1, 1);
/* Read in the overview schema */
ARTreadschema(DoOverview);
More information about the inn-workers
mailing list