inn.conf: Support for `newsuser', `newsgrp' options (patch is, hopefully, included)

Ivan Shmakov oneingray at gmail.com
Fri Dec 14 14:49:19 UTC 2007


>>>>> Julien ÉLIE <julien at trigofacile.com> writes:

 >>> Ecartis does all sorts of fascinating things to messages for no
 >>> particularly good reasons.

 >> Oh, now I see, it strips MIME attachments, too!

 > Yes, that's true.  I think it is the ml inn-patches [A.T.] isc.org
 > which does not strip such attachments.

	Is it available from Gmane?

 >> I've actually have written some code already.  Could you take a look
 >> at it?

 > That looks like a very good start.

 > But shouldn't ensure_news_user_grp() also be factorised in
 > frontends/rnews.c, innd/innd.c, nnrpd/nnrpd.c, storage/ovdb/ovdb.c
 > and storage/tradindexed/tdx-util.c?  I see the same patterns in these
 > files.

	Surely.

 >> 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'.

 > It is a bit different in INN 2.5 since inndstart is no longer used:
 > backends/innbind is now starting everything and it will not change
 > users.  So perhaps get_news_uid_gid() should not be used there and
 > the code kept intact (?)

 > /* If we're running privileged (effective and real UIDs are different),
 >    convert NEWSUSER to a UID and exit if run by another user.  Don't do
 >    this if we're not running privileged to make installations that don't
 >    need privileged ports easier and to make testing easier. */
 > real_uid = getuid();
 > if (real_uid != geteuid()) {

	Looks like `ensure_news_user (0)' will fit there, like:

->     pwd = getpwnam(NEWSUSER);
->     if (pwd == NULL)
->         die("cannot get UID for %s", NEWSUSER);
->     if (real_uid != pwd->pw_uid)
->     if (real_uid != news_uid)
->         die("must be run by user %s (%lu), not %lu", NEWSUSER,
->             (unsigned long) pwd->pw_uid, (unsigned long) real_uid);
+      ensure_news_user (0);
 > }

	Provided that get_news_uid_gid () (on top of which
	ensure_news_user () is implemented) is corrected as suggested
	above.

 >> 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.

 > Well, I am not an expert so I do not have many comments on that (I
 > believe other people here will provide you with wise remarks).

	Thanks for reading the code, anyway.

[...]

 > +    char *newsuser; /* User to run under */
 > +    char *newsgrp; /* Group to run under */

 > It should be detabify for INN's coding style ;-) But it does not
 > matter much.

	Yes, I've spotted that, too.

 > Well, here are a few remaining and easy patches (and
 > doc/pod/inn.conf.pod should also be updated):

[...]

	Thanks!



More information about the inn-workers mailing list