Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test
Back on December 27, 1999, Garance A Drosihn wrote: At 8:55 AM -0500 12/24/99, Robert Watson wrote: For example, imagine that the user has a number of hard links to the file in question. Okay, here's my newer version of the code, which takes into account multiple hard links, and also makes it so the spooled data file is owned by daemon instead of the user. This works fine. Has anyone had a chance to look at this? It does work for me, and I think it addressed all security concerns, so if no one else has any comments I'll send it in as an update to the original PR. (I hate to resend the whole patch in a second message to this list, but if anyone missed my earlier message then let me know and I'll send the patch to you to review). --- Garance Alistair Drosehn = [EMAIL PROTECTED] Senior Systems Programmer or [EMAIL PROTECTED] Rensselaer Polytechnic Institute To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test
At 8:55 AM -0500 12/24/99, Robert Watson wrote: ... keep in mind that this optimization does not produce behavior behavior in some cases. For example, imagine that the user has a number of hard links to the file in question. If the file is copied and then deleted, then the link count is decremented by one, and the data used to print is independent of the original file. However, if a single file system rename is used, the link count remains the same until the end of the print job, and if any of the other references to the same inode are modified, the print job will see inconsistent versions of the file (i.e., it being changed out from under it). Hmm. This was a very good point to bring up, even if people don't use it to modify the file (or do anything else nefarious). The real problem here is that lpr changes the ownership and permissions of the file after moving it. That means that if the user does give us a file with multiple hard links, lpr will repermit *all* copies of the file, not just the version it moved into the spool directory. Clearly we also need to check the inode count. Thanks for that observation! I do not know whether the current lpr waits until it finishes copying before returning, but assume so, meaning that once lpr returns, normally, you are guaranteed that unless using the symlink option, you can continue to modify the inode in question with impunity. With this optimization makes that assumption not true. A caveat is already present for the symlink, "-s", option to this effect. I believe this also allows users to present files that start out being within the maximum file size when that is checked, but later add more, allowing users to avoid the file size limit on print jobs. This is presumably already present with "-s", and it's possible that lpd checks for this. Well, for some reason lpr is dumber than that. The spooled datafile ends up being owned by the user who sent the job, and thus the user can go in any time and make changes. That's already true, without this update or '-s' being used. Personally I think that is bad, because it does allow the user to circumvent any checks done at job-submission time. I think I'll write a separate update for that (maybe next year... :-). --- Garance Alistair Drosehn = [EMAIL PROTECTED] Senior Systems Programmer or [EMAIL PROTECTED] Rensselaer Polytechnic Institute To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test
At 8:55 AM -0500 12/24/99, Robert Watson wrote:
For example, imagine that the user has a
number of hard links to the file in question.
Okay, here's my newer version of the code, which takes into
account multiple hard links, and also makes it so the spooled
data file is owned by daemon instead of the user. This works
fine. At some later point I'll write an update so the spooled
data file is owned by daemon in other cases, too.
Further comments or observations welcome. If this looks okay
I'll submit this as a new PR.
Index: lpr/lpr.c
===
RCS file: /Users/cvsdepot/lpr-fbsd/lpr/lpr.c,v
retrieving revision 1.1.1.1
diff -U5 -r1.1.1.1 lpr.c
--- lpr.c 1999/11/30 16:15:22 1.1.1.1
+++ lpr.c 1999/12/27 21:58:39
@@ -382,10 +382,69 @@
nact++;
continue;
}
if (sflag)
printf("%s: %s: not linked, copying
instead\n", name, arg);
+
+ if (f) {
+ /*
+* The user wants the file removed after it is copied,
+* so see if it can be mv'ed instead of copy/unlink'ed.
+* This will be much faster and better than copying the
+* file, especially for larger files. Can be very
+* useful for services like samba, pcnfs, CAP, et al.
+*/
+ int ret, didlink;
+ struct stat statb2;
+ seteuid(euid);
+ didlink = 0;
+ /* don't do this if the user's file is a symlink */
+ if (lstat(arg, statb) 0) goto nohardlink;
+ if (S_ISLNK(statb.st_mode)) goto nohardlink;
+ /* if the attempt to link fails, abandon the move */
+ if (link(arg, dfname) != 0) goto nohardlink;
+ didlink = 1;
+ /* make sure the user hasn't tried to trick us via
+* any race conditions */
+ if (lstat(dfname, statb2) 0)goto nohardlink;
+ if (statb.st_dev != statb2.st_dev) goto nohardlink;
+ if (statb.st_ino != statb2.st_ino) goto nohardlink;
+ /* skip if the file already had multiple hard links,
+* because changing the owner and access-bits would
+* change ALL versions of the file */
+ if (statb2.st_nlink 2) goto nohardlink;
+
+ /* if we can access and remove the given file without
+* special setuid-ness then this method is safe. */
+ seteuid(uid);
+ ret = access(dfname, R_OK);
+ if (ret == 0)
+ ret = unlink(arg);
+ seteuid(euid);
+ /* the user does not have access to read or remove
+* this file, so abandon the move and fall back to
+* the usual (copy) methods. */
+ if (ret != 0) goto nohardlink;
+
+ /* unlink of user file was successful. fixup perms,
+* add entries to control file, and skip copy step */
+ chown(dfname, pp-daemon_user, getegid());
+ chmod(dfname, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+ seteuid(uid);
+ if (format == 'p')
+ card('T', title ? title : arg);
+ for (i = 0; i ncopies; i++)
+ card(format, dfname[inchar-2]);
+ card('U', dfname[inchar-2]);
+ card('N', arg);
+ nact++;
+ continue;
+nohardlink:
+ if (didlink) unlink(dfname);
+ seteuid(uid); /* restore old uid */
+ } /* end: if (f) */
+
if ((i = open(arg, O_RDONLY)) 0) {
printf("%s: cannot open %s\n", name, arg);
} else {
copy(pp, i, arg);
(void) close(i);
===
---
Garance Alistair Drosehn = [EMAIL PROTECTED]
Senior Systems Programmer or [EMAIL PROTECTED]
Rensselaer Polytechnic Institute
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test
This is not a comment on your code, which I have not inspected yet, but
instead on the idea of the optimization. This is probably not a serious
objection, but keep in mind that this optimization does not produce
identical behavior in some case. For example, imagine that the user has a
number of hard links to the file in question. If the file is copied and
then deleted, then the link count is decremented by one, and the data used
to print is independent of the original file. However, if a single file
system rename is used, the link count remains the same until the end of
the print job, and if any of the other references to the same inode are
modified, the print job will see inconsistent versions of the file (i.e.,
it being changed out from under it). I do not know whether the current
lpr waits until it finishes copying before returning, but assume so,
meaning that once lpr returns, normally, you are guaranteed that unless
using the symlink option, you can continue to modify the inode in question
with impunity. With this optimization makes that assumption not true. A
caveat is already present for the symlink, "-s", option to this effect. I
believe this also allows users to present files that start out being
within the maximum file size when that is checked, but later add more,
allowing users to avoid the file size limit on print jobs. This is
presumably already present with "-s", and it's possible that lpd checks
for this.
That said, I doubt anyone relies on this assumption--printing hard linked
files and doing simultaneous modification is probably uncommon. Exceeding
the file size limit may already be possible if the check is performed only
once (given -s), and if it is not possible in the -s usage, it isn't
possible in this situation either. On the other hand, it might be worth a
flag to disable this behavior, or at least to document it in the man page,
as it isn't quite the same and could cause a nightmare in debugging
complex shell scripts if anyone found it the hard way :-).
Robert
On Thu, 23 Dec 1999, Garance A Drosihn wrote:
At 2:33 AM -0800 12/10/99, Alfred Perlstein wrote:
Can someone take a look at this?
Basically, it makes the link to the file, if it can unlink the original
it will then chown the spool file if it can't delete or read the original
then the user didn't have permission and it backs out.
Okay, I've finally gotten back to this and come up with an alternate
version, which I think should be pretty safe. It includes some checks
which are probably redundant for freebsd, but which would be necessary
on some other platforms I use this on.
It's basically Alfred's code, with a few more checks for symlinks
and to make sure race conditions can't trick us. If no problems
are found with this, I'll submit it as a PR follow-on to #bin/11997
Pardon the difference in version-numbering, I probably don't have
CVS setup exactly right for version numbers to match freebsd's...
Index: lpr.c
===
RCS file: /Users/cvsdepot/lpr-fbsd/lpr/lpr.c,v
retrieving revision 1.1.1.1
diff -U5 -r1.1.1.1 lpr.c
--- lpr.c 1999/11/30 16:15:22 1.1.1.1
+++ lpr.c 1999/12/23 23:31:43
@@ -382,10 +382,65 @@
nact++;
continue;
}
if (sflag)
printf("%s: %s: not linked, copying
instead\n", name, arg);
+
+ if (f) {
+ /*
+ * The user wants the file removed after it is copied,
+ * so see if it can be mv'ed instead of copy/unlink'ed.
+ * This will be much faster and better than copying the
+ * file, especially for larger files. Can be very
+ * useful for services like samba, pcnfs, CAP, et al.
+ */
+ int ret, didlink;
+ struct stat statb2;
+ seteuid(euid);
+ didlink = 0;
+ /* don't do this if the user's file is a symlink */
+ if (lstat(arg, statb) 0) goto nohardlink;
+ if (S_ISLNK(statb.st_mode)) goto nohardlink;
+ /* if the attempt to link fails, abandon the move */
+ if (link(arg, dfname) != 0) goto nohardlink;
+ didlink = 1;
+ /* make sure the user hasn't tried to trick us via
+ * any race conditions */
+ if (lstat(dfname, statb2) 0)goto nohardlink;
+ if (statb.st_dev != statb2.st_dev) goto nohardlink;
+ if (statb.st_ino != statb2.st_ino) goto nohardlink;
+
+ /* if we can access and remove the given file without
+ * special setuid-ness then this method is safe. */
+
Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test
At 2:33 AM -0800 12/10/99, Alfred Perlstein wrote:
Can someone take a look at this?
Basically, it makes the link to the file, if it can unlink the original
it will then chown the spool file if it can't delete or read the original
then the user didn't have permission and it backs out.
Okay, I've finally gotten back to this and come up with an alternate
version, which I think should be pretty safe. It includes some checks
which are probably redundant for freebsd, but which would be necessary
on some other platforms I use this on.
It's basically Alfred's code, with a few more checks for symlinks
and to make sure race conditions can't trick us. If no problems
are found with this, I'll submit it as a PR follow-on to #bin/11997
Pardon the difference in version-numbering, I probably don't have
CVS setup exactly right for version numbers to match freebsd's...
Index: lpr.c
===
RCS file: /Users/cvsdepot/lpr-fbsd/lpr/lpr.c,v
retrieving revision 1.1.1.1
diff -U5 -r1.1.1.1 lpr.c
--- lpr.c 1999/11/30 16:15:22 1.1.1.1
+++ lpr.c 1999/12/23 23:31:43
@@ -382,10 +382,65 @@
nact++;
continue;
}
if (sflag)
printf("%s: %s: not linked, copying
instead\n", name, arg);
+
+ if (f) {
+ /*
+* The user wants the file removed after it is copied,
+* so see if it can be mv'ed instead of copy/unlink'ed.
+* This will be much faster and better than copying the
+* file, especially for larger files. Can be very
+* useful for services like samba, pcnfs, CAP, et al.
+*/
+ int ret, didlink;
+ struct stat statb2;
+ seteuid(euid);
+ didlink = 0;
+ /* don't do this if the user's file is a symlink */
+ if (lstat(arg, statb) 0) goto nohardlink;
+ if (S_ISLNK(statb.st_mode)) goto nohardlink;
+ /* if the attempt to link fails, abandon the move */
+ if (link(arg, dfname) != 0) goto nohardlink;
+ didlink = 1;
+ /* make sure the user hasn't tried to trick us via
+* any race conditions */
+ if (lstat(dfname, statb2) 0)goto nohardlink;
+ if (statb.st_dev != statb2.st_dev) goto nohardlink;
+ if (statb.st_ino != statb2.st_ino) goto nohardlink;
+
+ /* if we can access and remove the given file without
+* special setuid-ness then this method is safe. */
+ seteuid(uid);
+ ret = access(dfname, R_OK);
+ if (ret == 0)
+ ret = unlink(arg);
+ seteuid(euid);
+ /* the user does not have access to read or remove
+* this file, so abandon the move and fall back to
+* the usual (copy) methods. */
+ if (ret != 0) goto nohardlink;
+
+ /* unlink of user file was successful. fixup perms,
+* add entries to control file, and skip copy step */
+ chown(dfname, userid, getegid());
+ chmod(dfname, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+ seteuid(uid);
+ if (format == 'p')
+ card('T', title ? title : arg);
+ for (i = 0; i ncopies; i++)
+ card(format, dfname[inchar-2]);
+ card('U', dfname[inchar-2]);
+ card('N', arg);
+ nact++;
+ continue;
+nohardlink:
+ if (didlink) unlink(dfname);
+ seteuid(uid); /* restore old uid */
+ } /* end: if (f) */
+
if ((i = open(arg, O_RDONLY)) 0) {
printf("%s: cannot open %s\n", name, arg);
} else {
copy(pp, i, arg);
(void) close(i);
===
---
Garance Alistair Drosehn = [EMAIL PROTECTED]
Senior Systems Programmer or [EMAIL PROTECTED]
Rensselaer Polytechnic Institute
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test
On Fri, 10 Dec 1999, Andre Albsmeier wrote:
On Thu, 09-Dec-1999 at 15:02:41 -0800, Alfred Perlstein wrote:
On Thu, 9 Dec 1999, Andre Albsmeier wrote:
...
For better reference, here is the current patch:
I don't have too much time to think about this, argue me this:
Sure, please tell me if you don't want to get CC'ed on this anymore.
I'm sorry if I sounded like that, I didn't mean to. :)
why should I allow a user to print any file on the system?
the race condition is still there.
Right :-(. The file won't be given to the user anymore but he can
print everything. However, there must be a solution for this...
Can someone take a look at this?
Basically, it makes the link to the file, if it can unlink the original
it will then chown the spool file if it can't delete or read the original
then the user didn't have permission and it backs out.
Index: lpr.c
===
RCS file: /home/ncvs/src/usr.sbin/lpr/lpr/lpr.c,v
retrieving revision 1.31
diff -u -r1.31 lpr.c
--- lpr.c 1999/11/30 16:15:22 1.31
+++ lpr.c 1999/12/10 14:09:08
@@ -384,6 +384,46 @@
}
if (sflag)
printf("%s: %s: not linked, copying instead\n", name, arg);
+ if (f) {
+ seteuid(euid);
+ if (link(arg, dfname) == 0) {
+ int ret;
+
+ seteuid(uid);
+ /*
+* if we can access and remove the file without
+* special setuid-ness then allow it.
+*/
+ ret = access(dfname, R_OK);
+ if (ret == 0)
+ ret = unlink(arg);
+ seteuid(euid);
+ if (ret == 0) {
+ /* unlink was successful fixup perms */
+ chown(dfname, userid, getegid());
+ chmod(dfname, S_IRUSR | S_IWUSR | S_IRGRP |
+S_IWGRP);
+ } else {
+ /*
+* the user handed me a file the don't have
+access to,
+* remove it from the spooldir and try other
+methods
+*/
+ unlink(dfname);
+ seteuid(uid);
+ goto nohardlink;
+ }
+ seteuid(uid);
+ if (format == 'p')
+ card('T', title ? title : arg);
+ for (i = 0; i ncopies; i++)
+ card(format, dfname[inchar-2]);
+ card('U', dfname[inchar-2]);
+ card('N', arg);
+ nact++;
+ continue;
+ }
+ seteuid(uid); /* restore old uid */
+ }
+nohardlink:
if ((i = open(arg, O_RDONLY)) 0) {
printf("%s: cannot open %s\n", name, arg);
} else {
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test
At 2:33 AM -0800 12/10/99, Alfred Perlstein wrote: Can someone take a look at this? Basically, it makes the link to the file, if it can unlink the original it will then chown the spool file if it can't delete or read the original then the user didn't have permission and it backs out. I'm thinking you'd what to add an lstat call after creating the hardlink. Check the new file to see if it's a symlink, and if it is, then delete the new file and go to nohardlink. Then proceed with the rest of your code (which looks fine to me, but remember I'm the guy who wrote a message explicitly for one mailing list, and then sent it to the other one...). I'm not sure on that, and haven't had time to look at the code yet. I'm just wondering about the case where a user might do a lpr -r -s somesymlink and want to be sure this does the right thing. I suspect that currently (without this patch) lpr will copy the REAL file, and then remove the symlink. I don't think we want to hard-link to the symlink, and then remove the original symlink. Remember, my mind is tired enough that I could easily be making things up here... It may be that the situation I'm wondering about is already covered by other checks in the code. --- Garance Alistair Drosehn = [EMAIL PROTECTED] Senior Systems Programmer or [EMAIL PROTECTED] Rensselaer Polytechnic Institute To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test
On Fri, 10 Dec 1999, Garance A Drosihn wrote: At 2:33 AM -0800 12/10/99, Alfred Perlstein wrote: Can someone take a look at this? Basically, it makes the link to the file, if it can unlink the original it will then chown the spool file if it can't delete or read the original then the user didn't have permission and it backs out. I'm thinking you'd what to add an lstat call after creating the hardlink. Check the new file to see if it's a symlink, and if it is, then delete the new file and go to nohardlink. Then proceed with the rest of your code (which looks fine to me, but remember I'm the guy who wrote a message explicitly for one mailing list, and then sent it to the other one...). I'm not sure on that, and haven't had time to look at the code yet. I'm just wondering about the case where a user might do a lpr -r -s somesymlink and want to be sure this does the right thing. I suspect that currently (without this patch) lpr will copy the REAL file, and then remove the symlink. I don't think we want to hard-link to the symlink, and then remove the original symlink. Remember, my mind is tired enough that I could easily be making things up here... It may be that the situation I'm wondering about is already covered by other checks in the code. ugh, good call. i'll look into it. -Alfred To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test
On Thu, 09-Dec-1999 at 15:02:41 -0800, Alfred Perlstein wrote:
On Thu, 9 Dec 1999, Andre Albsmeier wrote:
...
For better reference, here is the current patch:
*** lpr.c.ORI Thu Dec 9 15:30:18 1999
--- lpr.c Thu Dec 9 15:30:35 1999
***
*** 370,375
--- 370,405
}
if (sflag)
printf("%s: %s: not linked, copying instead\n", name, arg);
+ /*
+* If lpr was invoked with -r we try to move the file to
+* be printed instead of copying and deleting it later.
+* This works if the file and lpd's spool directory are
+* on the same filesystem as it is often the case for files
+* printed by samba or pcnfsd. In this case, a lot of I/O
+* and temporary disk space can be avoided. Otherwise, we
+* will continue normally.
+*/
+ if (f) {/* file should be deleted */
+ seteuid(euid); /* needed for rename() */
+ if (!rename(arg, dfname)) {
+ int i;
+ #if 0
+ chown(dfname, userid, getegid());
+ chmod(dfname, S_IRUSR | S_IWUSR |
+ S_IRGRP | S_IWGRP);
+ #endif
+ seteuid(uid); /* restore old uid */
+ if (format == 'p')
+ card('T', title ? title : arg);
+ for (i = 0; i ncopies; i++)
+ card(format, dfname[inchar-2]);
+ card('U', dfname[inchar-2]);
+ card('N', arg);
+ nact++;
+ continue;
+ }
+ seteuid(uid); /* restore old uid */
+ }
if ((i = open(arg, O_RDONLY)) 0) {
printf("%s: cannot open %s\n", name, arg);
} else {
I don't have too much time to think about this, argue me this:
Sure, please tell me if you don't want to get CC'ed on this anymore.
why should I allow a user to print any file on the system?
the race condition is still there.
Right :-(. The file won't be given to the user anymore but he can
print everything. However, there must be a solution for this...
-Alfred
-Andre
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
[PATCHES] Two fixes for lpd/lpc for review and test
Well, here's my third attempt to get someone to at least acknowledge some minor patches for lpd. The patches are also covered in PR's, as noted in the previous attempts. My theory is that my previous attempts had a subject which made it sound like I was looking for someone to *make* some fixes, as opposed to the fact that I already have fixes for these known bugs and I thought someone with commit privs might want to apply them to the official distribution. In my next installment, I'm going to try a subject of "Make Money Fast via Sex With Green Card Lawyers" Although with this crowd I'd probably have better luck with something like: "I have vmware running native on FreeBSD 3.2-release"... While I'm at it, I'd like to note another patch for lpd which looked interesting. This one might be a bit too risky to get into 3.4-release, but it would be nice to at least add it to current. To quote from another message: - - - Date: Mon, 22 Nov 1999 20:08:13 +0100 From: Andre Albsmeier [EMAIL PROTECTED] Subject: Enhancement for lpr (maybe for STABLE-3.4) A while ago I sent a PR containing an enhancement for lpr -r for the case that the file to be printed is on the same filesystem as lpd's spooling directory. The PR is bin/11997 and can be found at: http://www.freebsd.org/cgi/query-pr.cgi?pr=11997 Maybe someone can look at it, review it and eventually commit it to -STABLE (and current, of course). The patch now runs nearly 2 years without problems and saves quite a lot of time when printing large temporay files. - - - Note that the above patch not only saves time, but reduces the amount of disk space needed in a print-spooling partition. That may not seem like much of an issue when you're printing 10-Kbyte files, but when you're throwing around 200-Meg files (for color plotters, for instance), then it can be very nice. The text from my previous attempts follow. - - - - - - - - - - - - - - - - - - - - - - - - - At 5:04 PM -0500 11/20/99, Garance A Drosihn wrote: Back when we were closing in on the release date for 3.3, I sent the following message. As we now close in on the release date for 3.4, I thought I'd send it again, seeing that both of the problem reports are still marked "open", and neither of the two patches have been applied yet. (not even to current) These are both pretty tiny minor lovable cuddly changes... Any chance of getting them in for 3.4? Note that problem report bin/12912 probably depends on what umask is set when lpd starts up. The pr says "the problem doesn't seem to be reproducible", but reproducing the problem may depend on someone making a change elsewhere. Given that lpd uses the actual access-bit settings to govern it's behavior, then it should make sure the exactly-correct bit settings are set up when it creates the file. Also note that the fix for bin/9362, sent in as bin/13549, is also confirmed in bin/14975. Date: Mon, 6 Sep 1999 18:14:55 -0500 To: [EMAIL PROTECTED] From: Garance A Drosihn [EMAIL PROTECTED] Subject: Two fixes for lpd/lpc (printing) Cc: [EMAIL PROTECTED] I noticed problem-report bin/9362, which reported that the 'lpc start' command no longer works. (it claims to start the queue, but it doesn't actually start it). I came up with a two or three line fix for that bug, and sent it in as problem-report bin/13549. This patch should work on both freebsd-current and freebsd-stable. I also noticed problem report bin/12912. This PR includes a patch to make sure that if a lockfile does not already exist, then it will be created such that the queue is both "enabled" and "started". I have not tried this exact patch yet, but here at RPI we've had a similar patch to lpd for a long time. We add printer queues to a master printcap file, which is then copied to a few hundred workstations. We really don't want to have to run around all those workstations to 'enable' and 'start' a queue that we have just added. Perhaps we have a different 'umask' setting when lpd starts up than most freebsd sites do... Could some other people try these simple patches, and see how well they work? I am hoping these could make it into the next official release, although I realize that time is rapidly running out for that! :-) --- Garance Alistair Drosehn = [EMAIL PROTECTED] Senior Systems Programmer or [EMAIL PROTECTED] Rensselaer Polytechnic Institute --- Garance Alistair Drosehn = [EMAIL PROTECTED] Senior Systems Programmer or [EMAIL PROTECTED] Rensselaer Polytechnic Institute To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: [PATCHES] Two fixes for lpd/lpc for review and test
In message v04210102b472f8589876@[128.113.24.47], Garance A Drosihn writes: I've been using the patch from PR 13549 on 4 -stable stable machines for about 3 weeks with no ill effects -- and it fixes the problem. In my next installment, I'm going to try a subject of "Make Money Fast via Sex With Green Card Lawyers" You can get screwed by a lawyer without sex. Regards, Phone: (250)387-8437 Cy Schubert Fax: (250)387-5766 Sun/DEC Team, UNIX GroupInternet: [EMAIL PROTECTED] ITSD [EMAIL PROTECTED] Province of BC "e**(i*pi)+1=0" To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: [PATCHES] Two fixes for lpd/lpc for review and test
I've been reviewing this patch with someone and I think the last version is ready to commit. I'll take a look at my tree to make sure. Warner To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
NO! Re: [PATCHES] Two fixes for lpd/lpc for review and test
On Tue, 7 Dec 1999, Warner Losh wrote:
I've been reviewing this patch with someone and I think the last
version is ready to commit. I'll take a look at my tree to make
sure.
please do not, the patch in PR 11997 introduces a major security flaw.
someone can hardlink to any file and clobber it with a file owned by
them:
try this:
as root:
# cd /var/tmp ; touch rootfile ; chown root:wheel rootfile ; chmod 600 rootfile
as a user:
% cd /var/tmp ; echo foo foo
% lpr -r foo
sleeping
in another session as user:
% rm foo ; ln rootfile foo
wait a second...
# ls -l rootfile
-rw-rw 3 user daemon5 Dec 7 13:38 rootfile
# cat rootfile
foo
#
ouch!
-Alfred
use this patch to make the race condition apparrent:
Index: usr.sbin/lpr/lpr/lpr.c
===
RCS file: /home/ncvs/src/usr.sbin/lpr/lpr/lpr.c,v
retrieving revision 1.27.2.2
diff -u -u -r1.27.2.2 lpr.c
--- lpr.c 1999/08/29 15:43:29 1.27.2.2
+++ lpr.c 1999/12/08 01:47:47
@@ -370,6 +370,27 @@
}
if (sflag)
printf("%s: %s: not linked, copying instead\n", name, arg);
+ if( f ) { /* means that the file should be deleted */
+ printf("sleeping\n");
+ sleep(5);
+ printf("done.\n");
+ seteuid(euid); /* needed for rename() to succeed */
+ if( ! rename( arg, dfname ) ) {
+ register int i;
+ chmod( dfname, S_IRUSR | S_IWUSR | S_IRGRP |
+S_IWGRP );
+ chown( dfname, userid, getgrnam("daemon")-gr_gid
+);
+ seteuid(uid);
+ if (format == 'p')
+ card('T', title ? title : arg);
+ for (i = 0; i ncopies; i++)
+ card(format, dfname[inchar-2]);
+ card('U', dfname[inchar-2]);
+ card('N', arg);
+ nact++;
+ continue;
+ }
+ seteuid(uid);
+ }
if ((i = open(arg, O_RDONLY)) 0) {
printf("%s: cannot open %s\n", name, arg);
} else {
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
