On 22Feb2016 12:34, Alan Bawden <a...@csail.mit.edu> wrote:
Cameron Simpson <c...@zip.com.au> writes:

On 16Feb2016 19:24, Alan Bawden <a...@csail.mit.edu> wrote:
So in the FIFO case, I might write something like the following:

   def make_temp_fifo(mode=0o600):
       while True:
           path = tempfile.mktemp()
           try:
               os.mkfifo(path, mode=mode)
           except FileExistsError:
               pass
           else:
               return path

So is there something wrong with the above code?  Other than the fact
that the documentation says something scary about mktemp()?

Well, it has a few shortcomings.

It relies on mkfifo reliably failing if the name exists. It shounds like
mkfifo is reliable this way, but I can imagine analogous use cases without
such a convenient core action, and your code only avoids mktemp's security
issue _because_ mkfifo has that fortuitous aspect.

I don't understand your use of the word "fortuitous" here.  mkfifo is
defined to act that way according to POSIX.  I wrote the code that way
precisely because of that property.  I sometimes write code knowing that
adding two even numbers together results in an even answer.  I suppose
you might describe that as "fortuitous", but it's just things behaving
as they are defined to behave!

I mean here that your scheme isn't adaptable to a system call which will reuse an existing name. Of course, mkfifo, mkdir and open(.., O_EXCL) all have this nice feature.

Secondly, why is your example better than::
 os.mkfifo(os.path.join(mkdtemp(), 'myfifo'))

My way is not much better, but I think it is a little better because
your way I have to worry about deleting both the file and the directory
when I am done, and I have to get the permissions right on two
filesystem objects.  (If I can use a TemporaryDirectory() context
manager, the cleaning up part does get easier.)

And it also seems wasteful to me, given that the way mkdtemp() is
implemented is to generate a possible name, try creating it, and loop if
the mkdir() call fails.  (POSIX makes the same guarantee for mkdir() as
it does for mkfifo().)  Why not just let me do an equivalent loop
myself?

Go ahead. But I think Ben's specificly trying to avoid writing his own loop.

On that basis, this example doesn't present a use case what can't be
addressed by mkstemp or mkdtemp.

Yes, if mktemp() were taken away from me, I could work around it.  I'm
just saying that in order to justify taking something like this away, it
has to be both below some threshold of utility and above some threshold
of dangerousness.  In the canonical case of gets() in C, not only is
fgets() almost a perfectly exact replacement for gets(), gets() is
insanely dangerous.  But the case of mktemp() doesn't seem to me to come
close to this combination of redundancy and danger.

You _do_ understand the security issue, yes? I sure looked like you did,
until here.

Well, it's always dangerous to say that you understand all the security
issues of anything.  In part that is why I wrote the code quoted above.
I am open to the possibility that there is a security problem here that
I haven't thought of.  But so far the only problem anybody has with it
is that you think there is something "fortuitous" about the way that it
works.

(As if that would be of any use in the
situation above!)  It looks like anxiety that some people might use
mktemp() in a stupid way has caused an over-reaction.

No, it is anxiety that mktemp's _normal_ use is inherently unsafe.

So are you saying that the way I used mktemp() above is _abnormal_?

In that you're not making a file. I mean "abnormal" in a statistical sense, and also in the "anticipated use case for mktemp's design". I'm not suggestioning you're wrong to use it like this.

[ Here I have removed some perfectly reasonable text describing the
  race condition in question -- yes I really do understand that. ]

This is neither weird nor even unlikely which is why kmtemp is strongly
discouraged - naive (and standard) use is not safe.

That you have contrived a use case where you can _carefully_ use mktemp in
safety in no way makes mktemp recommendable.

OK, so you _do_ seem to be saying that I have used mktemp() in a
"contrived" and "non-standard" (and "non-naive"!) way.  I'm genuinely
surprised.  I though I was just writing straightforward correct code and
demonstrating that this was a useful utility that it was not hard to use
safely.  You seem to think what I did is something that ordinary
programmers can not be expected to do.  Your judgement is definitely
different from mine!

No, I meant only that (a) mktemp is normally used for regular files and (b) that mkdtemp()/mkfifo() present equivalent results without hand making a pick-a-name loop. Of course any programmer should be able to read the mktemp() spec and built from it.

And ultimately this does all boil down to making judgements.  It does
make sense to remove things from libraries that are safety hazards (like
gets() in C), I'm just trying to argue that mktemp() isn't nearly
dangerous enough to deserve more than a warning in its documentation.
You don't agree.  Oh well...

I think mktemp() is like system() (or popen with a shell command string instead of a list of arguments suitable for exec()): it is easy and obvious and there is a much safe tool sitting right beside it that will frequently be overlooked, and that is why I'm on the "discourage its use" side of the discussion, even to the point of deprecating its use.

It isn't that it cannot be carefully used ever, it is that it is too easy to use when there is a more reliable way to do it.

Up until this point, you haven't said anything that I actually think is
flat out wrong, we just disagree about what tools it is reasonable to
take away from _all_ programmers just because _some_ programmers might
use them to make a mess.

In fact your use case isn't safe, because _another_ task using mktemp
in conflict as a plain old temporary file may grab your fifo.

But here in very last sentence I really must disagree.  If the code I
wrote above is "unsafe" because some _other_ process might be using
mktemp() badly and stumble over the same path, then the current
implementation of tempfile.mkdtemp() is also "unsafe" for exactly the
same reason: some other process using mktemp() badly to create its own
directory might accidentally grab the same directory.

When the other taks goes mkdir with the generated name it will fail, so no.

In your mkfifo case, another task using mktemp thus:

 path = mktemp()
 tmpfp = open(path, "w")

can succeed in the circumstance where your fifo gets made first.

Heck, that other process doesn't even need to be using mktemp().  It
might just be using the hardwired pathname "/tmp/tmp_xyzzy" -- that has
_exactly_ the same chance of collision.  Surely you don't think that the
standard tempfile.mkdtemp() is "unsafe" (in whatever sense of the word
you intend) as long as other programmers are allowed to be stupid and
use constant pathnames?

Of course not.

Cheers,
Cameron Simpson <c...@zip.com.au>
--
https://mail.python.org/mailman/listinfo/python-list

Reply via email to