NetBSD Problem Report #4891

Received: (qmail 4086 invoked from network); 26 Jan 1998 00:21:04 -0000
Message-Id: <199801260020.RAA00745@clydesdale.math.montana.edu>
Date: Sun, 25 Jan 1998 17:20:02 -0700 (MST)
From: Chris Jones <cjones@clydesdale.math.montana.edu>
Reply-To: cjones@clydesdale.math.montana.edu
To: gnats-bugs@gnats.netbsd.org
Subject: NITPICK: skeyverify should do getpriority before setpriority
X-Send-Pr-Version: 3.95

>Number:         4891
>Category:       lib
>Synopsis:       NITPICK: skeyverify should do getpriority before setpriority
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jan 25 16:35:01 +0000 1998
>Closed-Date:    Thu Apr 13 13:30:49 +0000 2000
>Last-Modified:  Fri Jan 03 09:12:07 +0000 2014
>Originator:     Chris Jones
>Release:        <NetBSD-current source date>1.3
>Organization:
-------------------------------------------------------------------------------
Chris Jones                                    cjones@rupert.honors.montana.edu
           Mad scientist in training...
"Is this going to be a stand-up programming session, sir, or another bug hunt?"
>Environment:

System: NetBSD clydesdale.math.montana.edu 1.3 NetBSD 1.3 (CLYDESDALE) #0: Wed Jan 7 17:06:26 MST 1998 cjones@clydesdale.math.montana.edu:/usr/newsrc/sys/arch/i386/compile/CLYDESDALE i386


>Description:
skeyverify() in lib/libskey/skeylogin.c does a setpriority to -4 before it
writes its files, in order to decrease the window of opportunity for a would-be
hacker.  (The skeykeys file should be updated quickly, so a single password can
only be used once, not twice.)  When it's done, this function does a
setpriority back to 0.

Theoretically, a sysadmin could have their users log in with a priority other
than 0 by changing something in gettytab.  However, a knowledgeable user could
defeat this by logging in via S/key instead of other means, thus getting his
priority reset to 0 when skeyverify() is done with its work.

>How-To-Repeat:

>Fix:
*** skeylogin.c.old	Mon Jan 19 13:59:19 1998
--- skeylogin.c	Sun Jan 25 17:10:19 1998
***************
*** 184,189 ****
--- 184,190 ----
  	struct tm *tm;
  	char tbuf[27];
  	char *cp;
+ 	int prevprio;

  	time(&now);
  	tm = localtime(&now);
***************
*** 214,226 ****
  	 * to the system
  	 */

! 	setpriority(PRIO_PROCESS, 0, -4);

  	/* reread the file record NOW*/

  	fseek(mp->keyfile,mp->recstart,0);
  	if (fgets(mp->buf,sizeof(mp->buf),mp->keyfile) != mp->buf){
! 		setpriority(PRIO_PROCESS, 0, 0);
  		fclose(mp->keyfile);
  		return -1;
  	}
--- 215,237 ----
  	 * to the system
  	 */

! 	/* Save the priority for later use. */
! 	errno = 0;
! 	prevprio = getpriority(PRIO_PROCESS, 0);
! 	if(errno) {
! 		/* Don't report the error; just don't use it later. */
! 		prevprio = PRIO_MAX + 1;
! 	} else {
! 		setpriority(PRIO_PROCESS, 0, -4);
! 	}

  	/* reread the file record NOW*/

  	fseek(mp->keyfile,mp->recstart,0);
  	if (fgets(mp->buf,sizeof(mp->buf),mp->keyfile) != mp->buf){
! 		if(prevprio != PRIO_MAX + 1) {
! 			setpriority(PRIO_PROCESS, 0, prevprio);
! 		}
  		fclose(mp->keyfile);
  		return -1;
  	}
***************
*** 237,243 ****

  	if (memcmp(filekey,fkey,8) != 0){
  		/* Wrong response */
! 		setpriority(PRIO_PROCESS, 0, 0);
  		fclose(mp->keyfile);
  		return 1;
  	}
--- 248,256 ----

  	if (memcmp(filekey,fkey,8) != 0){
  		/* Wrong response */
! 		if(prevprio != PRIO_MAX + 1) {
! 			setpriority(PRIO_PROCESS, 0, prevprio);
! 		}
  		fclose(mp->keyfile);
  		return 1;
  	}
***************
*** 255,261 ****

  	fclose(mp->keyfile);

! 	setpriority(PRIO_PROCESS, 0, 0);
  	return 0;
  }

--- 268,276 ----

  	fclose(mp->keyfile);

! 	if(prevprio != PRIO_MAX + 1) {
! 		setpriority(PRIO_PROCESS, 0, prevprio);
! 	}
  	return 0;
  }

>Release-Note:
>Audit-Trail:
State-Changed-From-To: open->closed 
State-Changed-By: is 
State-Changed-When: Thu Apr 13 06:28:11 PDT 2000 
State-Changed-Why:  
Patch applied manually. 
XXX I'm not convinced skeyverify() should touch priorities at all, insead of  
using some sort of locking protocol.  

>Unformatted:

Home	PR Database Search