From owner-cvs-all Sun Nov 26 18:22:49 2000 Delivered-To: cvs-all@freebsd.org Received: from fw.wintelcom.net (ns1.wintelcom.net [209.1.153.20]) by hub.freebsd.org (Postfix) with ESMTP id 9D9BC37B479; Sun, 26 Nov 2000 18:22:40 -0800 (PST) Received: (from bright@localhost) by fw.wintelcom.net (8.10.0/8.10.0) id eAR2MeX14406; Sun, 26 Nov 2000 18:22:40 -0800 (PST) Date: Sun, 26 Nov 2000 18:22:40 -0800 From: Alfred Perlstein To: "Brian F. Feldman" Cc: obrien@FreeBSD.org, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/usr.sbin/inetd builtins.c Message-ID: <20001126182240.A8051@fw.wintelcom.net> References: <200011262140.eAQLe2576200@green.dyndns.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <200011262140.eAQLe2576200@green.dyndns.org>; from green@FreeBSD.org on Sun, Nov 26, 2000 at 04:39:59PM -0500 Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG * Brian F. Feldman [001126 13:40] wrote: > Alfred Perlstein wrote: > > Because your "fix" was a gross hack on top of the gross hack already > > in place. > > Here, you can review this, then: > Ok, a bit better. Why do you keep setting errno to 0 and then testing against it? GETPWENT(3) says nothing about errno being set, it might set errno through one of it's routines and not clear it. I've also simplified one of the tricky conditionals you had, particularly the one that begins with 'if (getcredfail != 0)' but you should just go over it one last time to make sure it's correct. Index: builtins.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/inetd/builtins.c,v retrieving revision 1.26 diff -u -u -r1.26 builtins.c --- builtins.c 2000/11/25 04:13:05 1.26 +++ builtins.c 2000/11/26 16:38:35 @@ -453,7 +453,8 @@ */ gettimeofday(&to, NULL); to.tv_sec += tv.tv_sec; - if ((to.tv_usec += tv.tv_usec) >= 1000000) { + to.tv_usec += tv.tv_usec; + if (to.tv_usec >= 1000000) { to.tv_usec -= 1000000; to.tv_sec++; } @@ -519,7 +520,7 @@ * so right here we are only setting the ports. */ if (ss[0].ss_family != ss[1].ss_family) - iderror(lport, fport, s, errno); + iderror(lport, fport, s, EINVAL); size = sizeof(uc); switch (ss[0].ss_family) { case AF_INET: @@ -529,7 +530,7 @@ sin[1].sin_port = htons(fport); if (sysctlbyname("net.inet.tcp.getcred", &uc, &size, sin, sizeof(sin)) == -1) - getcredfail = 1; + getcredfail = errno; break; #ifdef INET6 case AF_INET6: @@ -539,23 +540,30 @@ sin6[1].sin6_port = htons(fport); if (sysctlbyname("net.inet6.tcp6.getcred", &uc, &size, sin6, sizeof(sin6)) == -1) - getcredfail = 1; + getcredfail = errno; break; #endif default: /* should not reach here */ - getcredfail = 1; + getcredfail = EAFNOSUPPORT; break; } + /* + * If we couldn't find the cred because sysctl failed or + * the query is on a non INET/INET6 port then use a default + * if asked to, otherwise bail. + */ if (getcredfail != 0) { - if (fallback == NULL) /* Use a default, if asked to */ - iderror(lport, fport, s, errno); + if (fallback == NULL) + iderror(lport, fport, s, getcredfail); usedfallback = 1; } else { - /* Look up the pw to get the username */ - pw = getpwuid(uc.cr_uid); + /* + * if sysctl returned success and we have the cred then try + * to look the user up, if there's no entry we bail. + */ + if ((pw = getpwuid(uc.cr_uid)) == NULL) + iderror(lport, fport, s, ENOENT); } - if (pw == NULL && !usedfallback) /* No such user... */ - iderror(lport, fport, s, errno); /* * If enabled, we check for a file named ".noident" in the user's * home directory. If found, we return HIDDEN-USER. @@ -589,23 +597,23 @@ iderror(lport, fport, s, errno); seteuid(pw->pw_uid); /* - * If we were to lstat() here, it would do no good, since it - * would introduce a race condition and could be defeated. + * We can't stat() here since that would be a race + * condition. * Therefore, we open the file we have permissions to open * and if it's not a regular file, we close it and end up * returning the user's real username. */ fakeid_fd = open(p, O_RDONLY | O_NONBLOCK); free(p); - if ((fakeid = fdopen(fakeid_fd, "r")) != NULL && - fstat(fileno(fakeid), &sb) != -1 && S_ISREG(sb.st_mode)) { + if (fakeid_fd != -1 && fstat(fakeid_fd, &sb) != -1 && + S_ISREG(sb.st_mode) && + (fakeid = fdopen(fakeid_fd, "r")) != NULL) { buf[sizeof(buf) - 1] = '\0'; if (fgets(buf, sizeof(buf), fakeid) == NULL) { cp = pw->pw_name; fclose(fakeid); goto printit; } - fclose(fakeid); /* * Usually, the file will have the desired identity * in the form "identity\n", so we use strcspn() to @@ -630,12 +638,14 @@ if (!*cp || getpwnam(cp)) { pw = getpwuid(uc.cr_uid); if (pw == NULL) - iderror(lport, fport, s, errno); + iderror(lport, fport, s, ENOENT); cp = pw->pw_name; } } else cp = pw->pw_name; - if (fakeid_fd != -1) + if (fakeid != NULL) + fclose(fakeid); + else if (fakeid_fd != -1) close(fakeid_fd); } else if (!usedfallback) cp = pw->pw_name; To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message