Re: [Nmh-workers] extensions on tmp filenames?

2014-02-03 Thread David Levine
Robert wrote:

   |   If the link named by the new argument exists, it shall be removed
   |   and old renamed to new. In this case, a link named new shall
   |   remain visible to other processes throughout the renaming
   |   operation and refer either to the file referred to by new or old
   |   before the operation began.
   |  
   |   and that's what Linux still uses for its documentation.
 
 I really cannot believe that anyone is confused by any of this.  The
 documentation is all correct, the operation is all correct, and
 (assuming there are no implementation bugs) nothing has any security
 holes.
 
 All the documentation quoted above says, is that if you have two
 files A and B and do rename(A, B) then a file named B is always
 existing and visible (the name A vanishes) through the rename - and
 that as time passes, the name B will initially refer to its old
 contents (nothing surprising there, the rename hasn't happened yet)
 and later will refer to what was in file A, and that anyone that
 opens the file will get one version or the other - depending upon
 exactly when they open.

I agree that that's what we want and it's obvious that all sane
implementations should do that.  But, that contains an ordering
(initially ... later) that does not appear in the second
sentence of the documentation excerpt.  That documentation appears
to explicitly disclaim any ordering, given the word choices of
throughout and refer either to.

I wonder why someone even bothered to write that sentence if the
ordered interpretation is correct.  Maybe it was just poorly
written, as Earl noted.  But as it was written, it allowed the
implementation to disclaim ordering.

Even without that, there is an opportuntity for a particular DoS
attack with mkstemp + rename (in pseudocode):

  while true; do
find /tmp -name 'mhshow??' -exec touch '{}'.html \;
  done

Sure, we can't prevent all DoS attacks on shared resources.  But
there's no need for us to allow a particular one on the platforms
that have a C library call that does exactly what we want.

 I have no idea what mkstemps() is (I assume some linux invention)

mkstemps() first appeared in OpenBSD 2.4.  It's available on
current FreeBSD, OpenBSD, Mac OS X, Solaris 11, Linux, and Cygwin.

David

___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-03 Thread Robert Elz
Date:Mon, 03 Feb 2014 10:36:25 -0500
From:David Levine levin...@acm.org
Message-ID:  19453-1391441785.792...@oxru.r4ek.qgup


  | I wonder why someone even bothered to write that sentence if the
  | ordered interpretation is correct.

Whoever wrote it was just trying to be correct, but not imagining that
anybody could read it as allowing the interpretation you have suggested,
so didn't bother excluding that.   I cannot even imagine how one would
make an implementation that behaved that way, if that was an explicit aim,
rather than just some unfortunate side effect.  To rename(a, b) any old b
must be unlinked. Before that happens is just the before rename case.
After that unlink, figuring out ow to make b temporarily reappear as the old
file (while eventually completing the rename, so a has gone - but its contents
must be in b after the rename has fully completed) would take some
ingenuity.   There are times when simply believing that the simple way
applies is the right thing to do.

  | But as it was written, it allowed the implementation to disclaim ordering.

Even if that were true, what would it matter?   We know there's a race if
some other program is attempting to access b just around the time of the
rename.   Even if some weird come  go ordering applied during the rename,
anyone that sees the old b contets can simply be regarded as having won the
race, and anyone who sees the new b (old a) contents lost the race, and
even if there were two of those processes, and somehow one lost, and
immediately after, the other won, all this happens within the period of the
rename sys call operation, so it is all very quick - there's no way the two
processes could tell which order they executed.

  | Even without that, there is an opportuntity for a particular DoS
  | attack with mkstemp + rename (in pseudocode):

Yes, of course, I mentioned that (without the detail) in the previous
message.   mkstemps() sounds as if it avoids that, but link()/unlink()
cannot (and nor can mkstemp();link() without the unlink() as in your
pseudocode the file created by mkstemp is being used to fuel the attack,
rather than guard against it.)
 
  | Sure, we can't prevent all DoS attacks on shared resources.  But
  | there's no need for us to allow a particular one on the platforms
  | that have a C library call that does exactly what we want.

No, and I wasn't arguing against using mkstemps() where it is available,
just against using link() in preference to rename() - that one makes no
sense.

  | mkstemps() first appeared in OpenBSD 2.4.

Oh, OK, given all those systems, I guess it is about time for it to
migrate to NetBSD as well (it isn't there yet.)

kre


___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-03 Thread Earl Hood
On Mon, Feb 3, 2014 at 12:46 PM, Robert Elz wrote:

 No, and I wasn't arguing against using mkstemps() where it is available,
 just against using link() in preference to rename() - that one makes no
 sense.

   | mkstemps() first appeared in OpenBSD 2.4.

 Oh, OK, given all those systems, I guess it is about time for it to
 migrate to NetBSD as well (it isn't there yet.)

Just a bit of history:

The whole rename of the temp file was introduced by me so the temp file
can have a proper suffix for cases when the data is provided to an
external process since it is common for programs to use filename
extension as how to process the data.  I understood, at the time,
that rename() was atomic and relatively safe.

At the time, I only knew about mkstemp() [Notice there is no 's' at the
end of the function], therefore, there was no system call to make a
temporary file with a specific suffix.  My older linux system I was
using at the time when making edits to nmh (and still have operational)
does not have the call.  A later one I have running (CentOS 5) does have
it, but there is no manpage!  I had to grep the system header files just
now to verify the later system had the call.

The DoS scenario David mentions seems highly unlikely to occur and it
does not concern me.  But I am all in favor of using mkstemps() and I
would have used it if it widely available at the time.  If it is not
available, mkstemp()/rename() should be the fallback.  nmh should
have an abstract function that allows the creation of a temp file with
a suffix so it can hide the method used based on what the system
supports.

--ewh

___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-03 Thread Lyndon Nerenberg

On Feb 3, 2014, at 8:46 AM, Mike O'Dell m...@ccr.org wrote:

 ISO C can demand anything it wants about atomicity,
 but given that It Doesn't Rule The World, portability
 may make other demands. C is still useful even when
 the underlying runtime doesn't belong to that church.

I can't stop a system from not adhering to the standard.  But I can say 'not 
our fault' if that behaviour breaks MH somehow.

Although one might argue that, when it comes to C, the ISO C standards do rule 
the world.  Given C's application as the language to bootstrap nearly all the 
other higher-level languages and runtimes, breaking your C compiler or library 
is going to have potentially dire consequences.

--lyndon



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-02 Thread Lyndon Nerenberg

On Feb 2, 2014, at 6:02, David Levine levin...@acm.org wrote:

 Well, that's the way it's documented.  This is when an existing
 file is in the way of the new name.

Documented where?  SUSv3 calls out the behaviour explicitly, as inherited from 
the ISO C spec.
___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-02 Thread David Levine
Lyndon wrote:

 On Feb 2, 2014, at 6:02, David Levine levin...@acm.org wrote:
 
  Well, that's the way it's documented.  This is when an existing
  file is in the way of the new name.
 
 Documented where?  SUSv3 calls out the behaviour explicitly, as
 inherited from the ISO C spec.

Well, the SUSv2 spec says:

If the link named by the new argument exists, it shall be removed
and old renamed to new. In this case, a link named new shall
remain visible to other processes throughout the renaming
operation and refer either to the file referred to by new or old
before the operation began.

and that's what Linux still uses for its documentation.

This is pretty much moot now:  we use mkstemps if available,
and if not try link before trying rename.

David

___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-02 Thread Earl Hood
On Sun, Feb 2, 2014 at 4:43 PM, David Levine wrote:

 Documented where?  SUSv3 calls out the behaviour explicitly, as
 inherited from the ISO C spec.

 Well, the SUSv2 spec says:

 If the link named by the new argument exists, it shall be removed
 and old renamed to new. In this case, a link named new shall
 remain visible to other processes throughout the renaming
 operation and refer either to the file referred to by new or old
 before the operation began.

 and that's what Linux still uses for its documentation.

For which manpage.  CentOS 5 system for rename(2):

  If newpath already exists it will be atomically replaced (subject to  a
  few  conditions;  see ERRORS below), so that there is no point at which
  another process attempting to access newpath will find it missing.

What you quote is not well written and is not clear on what really may
happen.  My understanding is that rename() is atomic and safe, hence I felt
there was no real security risk when I included the operation years ago for
suffix support.

I know you tried to explain to me via private email why there was still a
security risk, but I still do not understand it.  If there is a risk, it
would be due to old and/or faulty implementations of the underlying OS
compared to the way a rename() operation is supposed to work.

I know it may be all academic now, but the practice of renaming temp files
into persistent files is a common practice, like to avoid symlink attacks.
Rename() is atomic and should overwrite any existing file with the same
name.  If a process had the file open before it is overwritten due to
rename(), that process will only have a handle to the old file and not the
new one.

--ewh

___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-02 Thread David Levine
Earl wrote:

 On Sun, Feb 2, 2014 at 4:43 PM, David Levine wrote:
 
  Documented where?  SUSv3 calls out the behaviour explicitly, as
  inherited from the ISO C spec.
 
  Well, the SUSv2 spec says:
 
  If the link named by the new argument exists, it shall be removed
  and old renamed to new. In this case, a link named new shall
  remain visible to other processes throughout the renaming
  operation and refer either to the file referred to by new or old
  before the operation began.
 
  and that's what Linux still uses for its documentation.
 
 For which manpage.

rename(3p)

 What you quote is not well written and is not clear on what really may
 happen.

It's clear to me that the implementation was not required to
behave as desired.  Given that we support systems that aren't
always up to date, the safe thing to do here is not use rename.

David

___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-02 Thread Robert Elz

  |   If the link named by the new argument exists, it shall be removed
  |   and old renamed to new. In this case, a link named new shall
  |   remain visible to other processes throughout the renaming
  |   operation and refer either to the file referred to by new or old
  |   before the operation began.
  |  
  |   and that's what Linux still uses for its documentation.

I really cannot believe that anyone is confused by any of this.   The
documentation is all correct, the operation is all correct, and (assuming
there are no implementation bugs) nothing has any security holes.

All the documentation quoted above says, is that if you have two files A and B
and do rename(A, B) then a file named B is always existing and visible
(the name A vanishes) through the rename - and that as time passes, the name B
will initially refer to its old contents (nothing surprising there, the rename
hasn't happened yet) and later will refer to what was in file A, and that 
anyone that opens the file will get one version or the other - depending upon
exactly when they open.

What else is possible?  If my process opens B today, it must get the old B
right?

Tomorrow you do the rename.

The day after my process opens B again, now it gets the content that used to
be A, right?

No-one could possibly expect, or want, anything different.   All you need
then is to compress the intervals until everything is happening very close
together.  As long as the sequence stays the same, the results remain the
same.

Of course, these two processes (the one renaming, and the one opening B)
have a race - what results the second one (the opening one) gets depends on
whether it opens before or after the rename.  That's not rename's fault,
the same happens (exactly) using link/unlink - or any other way of getting
the data in the file named A accessible using the name B ... just that
all ways of accomplishing this are inferior (in other ways) to rename().

Lastly, if rename(2) exists, there is absolutely no reason to prefer a
link();unlink() sequence over it.   Rename is precisely that - except
guaranteed as an atomic operation (it is impossible from outside to observe
the filesystem in the period between the link and unlink, unlike when the
same thing is accomplished using the 2 sys call variant).   Using link()/
unlink() makes sense only on systems that don't have rename() - and good luck
finding one of those that you'd really want to use these days.

I have no idea what mkstemps() is (I assume some linux invention) or how it
is defined to work, so I can't say if it would be better to use that than
mkstemp()/rename() (possibly, mkstemp() guarantees a new file name is
created, not destroying any other file, the mkstemp()/rename() doesn't
promise that on the target of the rename, and there's no way to guarantee
it is safe ... I'm guessing that is what mkstemps() accomplishes).

But certainly prefer rename() over link()/unlink().

Of course, another option is mkstemp()/link() (with no unlink) - using
the mkstemp() created filename as a lock, so that a later mkstemp() won't
create the same thing.   That's workable, as long as you can assume that
nothing ever creates a file names like the link() target using any other
mechanism, and that the two files get unlinked in the correct order.

kre


___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


[Nmh-workers] extensions on tmp filenames?

2014-02-01 Thread David Levine
While cleaning up the tmp files, I noticed a potential security
issue.  mhshow, mhn, etc., used to create temporary files using
mkstemp(3) and then rename(3) them in order to add a filename
extension that reflects the content type.  E.g.,
/tmp/mhshowXYZ123.html.  rename allows the new filename to refer
to the old file, even if very briefly.  So I removed that
rename.

But it was there for a reason:  some external display programs
rely on the filename extension.  Users can get around it with
lynx -force_html, w3m -T text/html, etc.  But is that asking too
much?  If so, what's a better way to handle it?  Maybe do the
rename only if the tmp directory is the user's MH Path?  Or,
always rename those tmp files, but always put them in the MH
Path?  Or?

The tmp directory is the first non-null location of
{MHTMPDIR, TMP, MH Path directory}.

David

___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-01 Thread Ralph Corderoy
Hi David,

 E.g., /tmp/mhshowXYZ123.html.  rename allows the new filename to refer
 to the old file, even if very briefly.  So I removed that rename.

Not sure I understand the issue, but would link(2) avoid it?

Cheers, Ralph.

___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-01 Thread Oliver Kiddle
David Levine wrote:
 While cleaning up the tmp files, I noticed a potential security
 issue.  mhshow, mhn, etc., used to create temporary files using
 mkstemp(3) and then rename(3) them in order to add a filename
 extension that reflects the content type.  E.g.,
 /tmp/mhshowXYZ123.html.  rename allows the new filename to refer
 to the old file, even if very briefly.  So I removed that
 rename.
 
 But it was there for a reason:  some external display programs
 rely on the filename extension.  Users can get around it with

You could use mkstemps to create the temporary file directly with a
suffix. The only problem is that it'd need a configure test for
mkstemps because at least Solaris 10 (but not 11) lacks it. Where
mkstemps is lacking, I'd just do the rename.

Oliver

___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers


Re: [Nmh-workers] extensions on tmp filenames?

2014-02-01 Thread David Levine
Oliver wrote:

 You could use mkstemps to create the temporary file directly with a
 suffix. The only problem is that it'd need a configure test for
 mkstemps because at least Solaris 10 (but not 11) lacks it. Where
 mkstemps is lacking, I'd just do the rename.

Perfect!  It even works on FAT filesystems.

David

___
Nmh-workers mailing list
Nmh-workers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/nmh-workers