On Sat, 25 Aug 2001, Solar Designer wrote: > On Fri, Aug 24, 2001 at 06:38:17PM -0400, Adam Slattery wrote: > > This was briefly discussed last week, but no conclusion was reached. > > I suggest that you search the archives from a year ago when I've > raised this question. Strange. These are the messages I was looking at. For some reason in my mind I was thinking they were on the list. Everything seems to have blured together from last week. There's an archive of the thread here if anybody is curious: http://security-archive.merton.ox.ac.uk/security-audit-200008/index.html > > flock(2) - not 100% reliable on some filesystems, but pretty good. > > > > fcntl(2) - looks good. > > Applying file locking to the files themselves is wrong for at least > password files as those are to be replaced with a rename(2). There > should be a separate lock file which you apply an advisory lock to. It is quite possible to do proper inode-based (flock) locking with rename(). It's been explained before, so I won't bother explaining it here. Using such a scheme has the advantage (or disadvantage...) of only locking the file that is needed rather than all files. > > lckpwdf(3) - undocumented, does it protect /etc/shadow? > > There're certain signal races with existing implementations I've seen. As you mention below, that's not really pam's problem. If somebody really dislikes using lckpwdf() they can disable it with ./configure but I personally would rather have semi-broken locking than no locking at all. > > others? > > Yes, there're other locking methods in use, see the discussion from a > year ago. This is a bug. And it "still" hasn't be addressed. > lckpwdf(3) uses signals, too. And it's not sleep(3) but multiple > password changes by different threads that I'm somewhat concerned > about. This is one of the things I'm worried about too. I'll have to download glibc source and see how they implemented lckpwdf. If it isn't thread-safe, the module will then have to implement a locking scheme to make sure only one thread has called lckpwdf(3) at a time. How safe is lckpwdf() on other systems? > > We should come up with a good solutions and document it in the module > > writers guide. I personally don't have any brilliant proposals, nor will > > I ever since I don't do any coding on some of the systems Linux-PAM runs > > on (Ultrix, AIX, HP-UX, OSX, ...) > > I'm not sure it's a good idea to install the Linux-PAM-provided > pam_unix on all systems that are to some extent supported by > Linux-PAM. Native password file locking conventions are very > different on different systems. We could just get a standard "lock.c" file that all the modules could use and it would know about as many systems as possible and handle native locking for them. This would be problematic for modules not distributed in the pam tarball, but it could be maintained for those that are in the tarball as long as the interface stayed the same. In this way, people could pick and choose what modules they wanted to use without locking problems. Then we have to worry about non-pam applications that might use differant locking schemes. For this we would then be relying on other projects to update their code to be compatible with pam's. > > The next problem is read locking vs write locking. To read such files is > > it neccisary to call lckpwdf (if that's what we use). You would have to > > because it's the only way to prevent another process/thread from writing > > to your file. An fcntl() or flock() solution would solve this, but that > > may be impossible depending on compatibility concerns. > > There's no locking needed to read password files. They're to be > replaced atomically. I don't know of a system which defines things > differently, as that would require privileges to do any getpw*(3) > calls. I didn't think NFS did atomic opening, but I looked at some NFSv4 specifications and it does. Even if it didn't though, I suppose we wouldn't really need to worry about NFS :). This topic has been discussed before, but from my understanding the discussion didn't bring any decisions or changes. Something needs to happen because inconsistent file locking is a bad thing. - Adam Slattery
