ntsec patch #4: passwd and group

Pierre A. Humblet Pierre.Humblet@ieee.org
Fri Nov 8 07:15:00 GMT 2002


This is a heads up on a coming patch affecting 
security.h, passwd.cc, grp.cc, sec_helper.cc
and uinfo.cc. The ChangeLog of the patch is detailed
below. No diffs are attached for now as some of these
files were affected by a patch that's under review.

This patch has two objectives:

1) Fix the bug reported on Tuesday in 
and evidenced by:

/usr/sbin> strace sshd | fgrep 'Read /etc/passwd'
10761  101398 [main] sshd 272 read_etc_passwd: Read 
    /etc/passwd, 305 lines
after touching /etc/passwd and running ssh, sshd outputs
10682 32079255 [main] sshd 270 read_etc_passwd: Read 
    /etc/passwd, 610 lines

2) Operate better with incomplete passwd and group files.

Each of these two files can be in one of 4 states:
1) missing or unreadable
2) readable but without entry for the current user
3) readable with an entry, but without sid
4) correct

The goal is to be able to start Cygwin in any state
(16 combinations), to have "id" and "ls -l" display 
correct information (as much as possible), 
and to keep doing so consistently even when the 
state changes to any other value.

The main changes are in the passwd/group emulation code,
which has been totally redone to cover the four cases.
Additionally two new functions have been created:
getpwsid and getgrsid.

There is one design decision: 
 when the group entry is missing, the code calls 
 LookupAccountSidA to get the current group name. 
 That will cause delays for domain users starting 
 Cygwin while disconnected from the PDC 
 (but only if /etc/group is incomplete).
 The alternative is to always use "unknown".

I have also detected 4 outstanding issue:
1) memory leak in grp.cc: grp.gr_mem is calloc'ed
   but never freed.

2) I thought that the passwd/group files where only
   read "for the first cygwin process that start up 
   on a given console", to use Chris' words in

   Now that I have my nose in the code, I see that they
   reside in malloc'ed structures and that consequently they
   are reread when accessing passwd/group after execs 
   (but not after forks).

3) Why is pthread_testcancel called after read_etc_passwd
   in some (but not all) passwd.cc functions, but never after
   read_etc_group in grp.cc ?

4) Altough I see some mutual exclusion stuff, I don't see 
   what prevents two threads from simultaneously updating
   the internal copies, or one thread to free them while 
   they are used by another.

   An  alternative would be to only update the passwd/group 
   internal copies on process startup (fork or exec), if needed.  
   That would be simple, safe and fast. Do you know
   applications that really benefit from reading the files
   every time they change?
   In fact applications such as sshd would benefit from 
   rereading the files (if needed) *before* forks or execs,
   so that a single reread can serve all future children,
   but that approach does not help with thread issues.

Fixing 3 and 4 is beyond me for now (pthread learning curve).
Do you think it would be a good idea to improve 2 by 
placing the copies on the cygheap? If so, I would
fix 1 at the same time.


2002/11/XX  Pierre Humblet  <pierre.humblet@ieee.org>

	* security.h: Declare getpwsid and getgrsid. Undeclare 
	* passwd.cc (getpwsid): Create.
	(internal_getpwent): Suppress.
	(read_etc_passwd): Rewrite the code for the completion line. 
	Reset curr_lines after reading /etc/passwd.
	(parse_pwd): Change type to static int. Return 0 for short lines.
	(add_pwd_line): Pay attention to the value of parse_pwd.     
	(search_for): Do not look for nor return the DEFAULT_UID.
	* grp.cc (read_etc_group): Rewrite the code for the completion line.
	Reset curr_lines after reading /etc/group. Always call add_pwd_line.
	(parse_grp): Never NULL gr_passwd. 
	(getgrgid32): Only return the default if ntsec is off and the gid is 
	(parse_grp): If grp.gr_mem is empty, set it to &null_ptr.
	* sec_helper.cc (cygsid::get_id): Use getpwsid and getgrsid.
	(cygsid_getfrompw): Clean up last line.
	(cygsid_getfromgr): Ditto.
	* uinfo.cc (internal_getlogin): Set DEFAULT_GID at start.
	Use getpwsid. Move the read of /etc/group after the second access 
	of /etc/passwd. Change some debug_printf.

More information about the Cygwin-developers mailing list