RFC: chop down fakeidentd?
Thomas Lundquist
lists at zelow.no
Fri Jan 12 09:00:53 UTC 2007
On Thu, Jan 11, 2007 at 10:53:38PM +0100, Denis Vlasenko wrote:
>
> Please take a cursory look as networking/fakeidentd.c.
> It's outright scary.
Believe me, it was worse.
I won't take credit for it being as good as it is, that was the work of
whoever ( I can't remember and he's been smart enough to not include
himself in the .c file :=).
> Why do we write pidfile?
it's nice behavior. you are the one wanting busybox to be run on a
desktop, so good behavior in an environment like that is the right thing
to do.
personally, I agree with you, rip it out but I also se no point
whatsoever in the desktop goal. busybox should be small and dedicated
embedded devices.
> Why do we setuid to "nobody"? What if I don't have "nobody"
> in the first place?
that should be an option. but I'd rather have it try to setuid nobody
while there are no nobody than not have it at all.
> Isn't it lame to rely on openlog() having open fd 1
> (when we closed all fd's except 0)? Yes, it is,
> but what standard guarantees that?
>
> Why we have this MAXCONNS limit in fakeidentd,
> maybe we should do it in inetd instead so that ALL services
> get this protection from one copy of the code?
no. you can make it a libbb function but fakeident should be able to run
without any inetd. it will even without MAXCONNS but it's nice to have.
(MAXCONNS can be implemented via iptables anyway, so I won't cry if it's
gone, I just don't see the point.)
> Why we care to parse the request - we can use it verbatim:
> <- "6191, 23\r\n" - peer asks us "what user connected from your port 6191 to my port 23?"
> -> "6193, 23 : USERID : UNIX : nobody\r\n"
>
> IOW: let's replace it with something like this:
>
> int main(int argc, char **argv)
> {
> char buf[128];
> char *cur = buf;
> int rem = sizeof(buf)-1;
>
> alarm(30);
> while (1) {
> char *p;
> int sz = safe_read(0, cur, rem);
> if (sz < 0) return 1;
> rem -= sz;
> cur += sz;
> *cur = '\0';
> p = strpbrk(buf, "\r\n");
> if (p) {
> *p = '\0';
> break;
> }
> if (!rem || !sz)
> break;
> }
> printf("%s : USERID : UNIX : nobody\r\n", buf);
> return 0;
> }
>
> Objections?
You forgot the string it's supposed to be possible to change.
>From the existing code (fakeident_main):
/* handle optional REPLY STRING */
if (optind < argc)
G.identuser = argv[optind];
else
G.identuser = nobodystr;
That's a feature we definately want to keep.
But I won't comment on the rest of the code, I've always thought it to
be a bit bloated.. but it is the least bloated and I didnt dare writing
my own.
So, if you keep the features, feel free to chop it down.
Thomas.
More information about the busybox
mailing list