This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Fix tempfile.mktemp()
Type: security Stage:
Components: Library (Lib) Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: dlukes, serhiy.storchaka
Priority: normal Keywords:

Created on 2021-03-23 15:26 by dlukes, last changed 2022-04-11 14:59 by admin.

Messages (4)
msg389393 - (view) Author: David Lukeš (dlukes) * Date: 2021-03-23 15:26
I recently came across a non-testing use case for `tempfile.mktemp()` where I struggle to find a viable alternative -- temporary named pipes (FIFOs):

```
import os
import tempfile
import subprocess as sp

fifo_path = tempfile.mktemp()
os.mkfifo(fifo_path, 0o600)
try:
    proc = sp.Popen(["cat", fifo_path], stdout=sp.PIPE, text=True)
    with open(fifo_path, "w") as fifo:
        for c in "Kočka leze dírou, pes oknem.":
            print(c, file=fifo)
    proc.wait()
finally:
    os.unlink(fifo_path)

for l in proc.stdout:
    print(l.strip())
```

(`cat` is obviously just a stand-in for some useful program which needs to read from a file, but you want to send it input from Python.)

`os.mkfifo()` needs a path which doesn't point to an existing file, so it's not possible to use a `tempfile.NamedTemporaryFile(delete=False)`, close it, and pass its `.name` attribute to `mkfifo()`.

I know there has been some discussion regarding `mktemp()` in the relatively recent past (see the Python-Dev thread starting with <https://mail.python.org/pipermail/python-dev/2019-March/156721.html>). There has also been some confusion as to what actually makes it unsafe (see <https://mail.python.org/pipermail/python-dev/2019-March/156778.html>). Before the discussion petered out, it looked like people were reaching a consensus "that mktemp() could be made secure by using a longer name generated by a secure random generator" (quoting from the previous link).

A secure `mktemp` could be as simple as (see <https://mail.python.org/pipermail/python-dev/2019-March/156765.html>):

```
def mktemp(suffix='', prefix='tmp', dir=None):
    if dir is None:
        dir = gettempdir()
    return _os.path.join(dir, prefix + secrets.token_urlsafe(ENTROPY_BYTES) + suffix)
```

There's been some discussion as to what `ENTROPY_BYTES` should be. I like Steven D'Aprano's suggestion (see <https://mail.python.org/pipermail/python-dev/2019-March/156777.html>) of having an overkill default just to be on the safe side, which can be overridden if needed. Of course, the security implications of lowering it should be clearly documented.

Fixing `mktemp` would make it possible to get rid of its hybrid deprecated (in the docs) / not depracated (in code) status, which is somewhat confusing for users. Speaking from experience -- when I realized I needed it, the deprecation notice led me down this rabbit hole of reading mailing list threads and submitting issues :) People could stop losing time worrying about `mktemp` and trying to weed it out whenever they come across it (see e.g. https://bugs.python.org/issue42278).

So I'm wondering whether there would be interest in:

1. A PR which would modify `mktemp` along the lines sketched above, to make it safe in practice. Along with that, it would probably make sense to undeprecate it in the docs, or at least indicate that while users should prefer `mkstemp` when they're fine with the file being created for them, `mktemp` is alright in cases where this is not acceptable.
2. Following that, possibly a PR which would encapsulate the new `mktemp` + `mkfifo` into a `TemporaryNamedPipe` or `TemporaryFifo`:

```
import os
import tempfile
import subprocess as sp

with tempfile.TemporaryNamedPipe() as fifo:
    proc = sp.Popen(["cat", fifo.name], stdout=sp.PIPE, text=True)
    for c in "Kočka leze dírou, pes oknem.":
        print(c, file=fifo)
    proc.wait()

for l in proc.stdout:
    print(l.strip())
```

(Caveat: opening the FIFO for writing cannot happen in `__enter__`, it would have to be delayed until the first call to `fifo.write()` because it hangs if no one is reading from it.)
msg389394 - (view) Author: David Lukeš (dlukes) * Date: 2021-03-23 15:41
> A secure `mktemp` could be as simple as ...

Though in practice, I'd rather be inclined to make the change in `tempfile._RandomNameSequence`, so as to get the same behavior across the entire module, instead of special-casing `mktemp`. As Guido van Rossum points out (see <https://mail.python.org/pipermail/python-dev/2019-March/156746.html>), that would improve the security of all the names generated by the `tempfile` module, not just `mktemp`:

> Hm, the random sequence (implemented in tempfile._RandomNameSequence) is
> currently derived from the random module, which is not cryptographically
> secure. Maybe all we need to do is replace its source of randomness with
> one derived from the secrets module. That seems a one-line change.
msg389402 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-03-23 18:50
You can use TemporaryDirectory.

with tempfile.TemporaryDirectory() as dir:
    fifo_path = os.path.join(dir, "fifo")
    os.mkfifo(fifo_path, 0o600)
    ...
msg389407 - (view) Author: David Lukeš (dlukes) * Date: 2021-03-23 19:35
> You can use TemporaryDirectory.

That was actually the first approach I tried :) I even thought this could be used to make `mktemp` safe -- just create the name in a `TemporaryDirectory`.

However, after reading through the mailing list thread, I realized this just restricts the potential collision/hijacking to misbehaving/malicious processes running under the same user or under the super user. But the core problem with too easily guessable filenames (= not random enough, or not at all, as in your example) remains. Correct me if I'm wrong though.

Sorry, I should probably have mentioned this in OP. I thought about doing so, but then it turned out very long even without it, so I decided it would be better to discuss it only if someone else mentions it.
History
Date User Action Args
2022-04-11 14:59:43adminsetgithub: 87770
2021-03-23 19:35:24dlukessetmessages: + msg389407
2021-03-23 18:50:58serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg389402
2021-03-23 15:41:03dlukessetmessages: + msg389394
2021-03-23 15:26:30dlukescreate