On Wed, Nov 12, 2003 at 02:07:27AM +0100, Javier Fernández-Sanguino Peña wrote: > On Tue, Nov 11, 2003 at 02:02:49PM -0500, Matt Zimmerman wrote: > > On Tue, Nov 11, 2003 at 11:52:00AM -0600, Steve Langasek wrote: > > > The packages at <http://www.tbble.com/freeradius/> will be sponsored into > > > the archive as soon as I've had a chance to review them (this week). > > This thing is packed full of strcpy() and strcat(), which is the sort of > > sloppiness that I don't like to see in a network server. It was a great > Which flawfinder flawlessly points out, but this also appears in the > current radiusd servers we are shipping. In any case, I'm also worried > about these: > ./src/main/mainconfig.c:267 [5] (race) chown: > [shouldn't fchown() be used instead?] > > and > > ./src/modules/rlm_krb5/rlm_krb5.c:201 [3] (tmpfile) tmpnam: > Temporary file race condition. > [tmpnam should be avoided and tempfile() used instead] Also on my TODO list now. Thankyou. I'll have to go look into flawfinder as well. That looks interesting. > > blessing to find that we weren't shipping this in woody when the last batch > > of security problems was discovered. > Also, just another question. Is there any reason why it needs to run as > root? (as I believe it does in the current Debian package) Would it be > unreasonable to ask it to run as a 'radiusd' user? FreeRADIUS hasn't run as root since I got my hands on it in the upstream CVS. It wasn't supposed to run as root in the 0.71 packages that were NMU'd, nor did it intend run as root in the 0.5 packages... It did run as root in the latter one, due to a change in the upstream config (The default upstream config now runs as root, but the debian/rules script un-comments the lines in the config during build.) > Maybe I'm mistaken, but the rpm spec file seems to use a 'radiusd' user > whileas the Debian rules package does not. I would be more confident with > the package if it was built this way. At least a security problem in > its code (if found) would lead to a remote 'radiusd' compromise (but not > 'root') an important difference. I don't know what debian/rules file you're looking at, since the bug report in the DBS relating to this has my patch to fix it, and both the current stable and unstable debian/ filesets do not run as root. It does adduser freerad shadow on first installation, but not after that (on the advice of Steve Langasek) to allow the local authentication code to work, and to give the admin the freedom to disable this for added security if they're not using the local authentication code. > However, this is the way that currently the radiusd packages we provide > (radiusd-cistron and radiusd-livingston) seem to operate. Is this at all > necessary? (after all they use their separate users database) > PS: I'm not particularly worried about freeradius, I'm just raising some > questions. It seems that our radiusd packages suffer from similar (if not > worst) security issues and, furthermore, are not (I believe) that actively > maintained upstream. Livingston begat YardRADIUS. I'd assumed Livingston was dead upstream, although I did see one of the Bugtraq postings mention RADIUS (Formerly livingston) although the version number was the same as the Livingston RADIUS server listed... Cistron begat FreeRADIUS. FreeRADIUS is certainly actively maintained upstream. xtRADIUS is also begat of Cistron. I'd assumed that Cistron is dead upstream too, and xtRADIUS active. There's also gnuradius, which doesn't appear in Debian. This is why so many of them suffer similar security bugs, as soon as one finds one, the others go look in the same place. -- ----------------------------------------------------------- Paul "TBBle" Hampson, MCSE 6th year CompSci/Asian Studies student, ANU The Boss, Bubblesworth Pty Ltd (ABN: 51 095 284 361) Paul.Hampson@Anu.edu.au "No survivors? Then where do the stories come from I wonder?" -- Capt. Jack Sparrow, "Pirates of the Caribbean" This email is licensed to the recipient for non-commercial use, duplication and distribution. -----------------------------------------------------------
Attachment:
signature.asc
Description: Digital signature