[Python-Dev] Remove tempfile.mktemp()

[Stéphane Wirtel]

Hi,

Context: raise a warning or remove tempfile.mktemp()
BPO: https://bugs.python.org/issue36309

Since 2.3, this function is deprecated in the documentation, just in the
documentation. In the code, there is a commented RuntimeWarning.
Commented by Guido in 2002, because the warning was too annoying (and I
understand ;-)).

So, in this BPO, we start to discuss about the future of this function
and Serhiy proposed to discuss on the Python-dev mailing list.

Question: Should we drop it or add a (Pending)DeprecationWarning?

Suggestion and timeline:

3.8, we raise a PendingDeprecationWarning
* update the code
* update the documentation
* update the tests
(check a PendingDeprecationWarning if sys.version_info == 3.8)

3.9, we change PendingDeprecationWarning to DeprecationWarning
(check DeprecationWarning if sys.version_info == 3.9)

3.9+, we drop tempfile.mktemp()

What do you suggest?

Have a nice day and thank you for your feedback.
_______________________________________________
Python-Dev mailing list
Pytho...@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: https://mail.python.org/mailman/options/python-dev/dev-python%2Bgarchive-30976%40googlegroups.com

[Antoine Pitrou]

-1. Please don't remove tempfile.mktemp(). mktemp() is useful to
create a temporary *name*. All other tempfile functions create an
actual file and impose additional burden, for example by making the
file unaccessible by other processes. But sometimes all I want is a
temporary name that an *other* program will create / act on, not Python.
It's a very common use case when writing scripts.

The only reasonable workaround I can think of is to first create a
temporary directory using mkdtemp(), then use a well-known name inside
that directory. But that has the same security implications AFAICT,
since another process can come and create the file / symlink first.

Regards

Antoine.

[Serhiy Storchaka]

19.03.19 15:03, Stéphane Wirtel пише:

> Suggestion and timeline:
>
> 3.8, we raise a PendingDeprecationWarning
> * update the code
> * update the documentation
> * update the tests
> (check a PendingDeprecationWarning if sys.version_info == 3.8)
>
> 3.9, we change PendingDeprecationWarning to DeprecationWarning
> (check DeprecationWarning if sys.version_info == 3.9)
>
> 3.9+, we drop tempfile.mktemp()

This plan LGTM.

Currently mkdir() is widely used in distutils, Sphinx, pip, setuptools,
virtualenv, and many other third-party projects, so it will take time to
fix all these places. But we should do this, because all this code
likely contains security flaws.

[Chris Angelico]

On Wed, Mar 20, 2019 at 12:25 AM Antoine Pitrou <soli...@pitrou.net> wrote:
>
>
> -1. Please don't remove tempfile.mktemp(). mktemp() is useful to
> create a temporary *name*. All other tempfile functions create an
> actual file and impose additional burden, for example by making the
> file unaccessible by other processes. But sometimes all I want is a
> temporary name that an *other* program will create / act on, not Python.
> It's a very common use case when writing scripts.
>
> The only reasonable workaround I can think of is to first create a
> temporary directory using mkdtemp(), then use a well-known name inside
> that directory. But that has the same security implications AFAICT,
> since another process can come and create the file / symlink first.

Can't you create a NamedTemporaryFile and permit the other program to
use it? I just tried that (with TiMidity, even though it's quite
capable of just writing to stdout) and it worked fine.

>>> f = tempfile.NamedTemporaryFile(suffix=".flac")
>>> subprocess.check_call(["timidity", "-OF", "-o", f.name, "Music/gp_peers.mid"])
... snip ...
Wrote 29645816/55940900 bytes(52.9949% compressed)
>>> data = f.read()
>>> len(data)
29645816

ChrisA

[Antoine Pitrou]

On Tue, 19 Mar 2019 15:32:25 +0200
Serhiy Storchaka <stor...@gmail.com> wrote:
> 19.03.19 15:03, Stéphane Wirtel пише:
> > Suggestion and timeline:
> >
> > 3.8, we raise a PendingDeprecationWarning
> > * update the code
> > * update the documentation
> > * update the tests
> > (check a PendingDeprecationWarning if sys.version_info == 3.8)
> >
> > 3.9, we change PendingDeprecationWarning to DeprecationWarning
> > (check DeprecationWarning if sys.version_info == 3.9)
> >
> > 3.9+, we drop tempfile.mktemp()
>
> This plan LGTM.
>
> Currently mkdir() is widely used in distutils, Sphinx, pip, setuptools,
> virtualenv, and many other third-party projects, so it will take time to
> fix all these places. But we should do this, because all this code
> likely contains security flaws.

The fact that many projects, including well-maintained ones such Sphinx
or pip, use mktemp(), may be a hint that replacing it is not as easy as
the people writing the Python documentation seem to think.

Regards

Antoine.

[Antoine Pitrou]

On Wed, 20 Mar 2019 00:37:56 +1100
Chris Angelico <ros...@gmail.com> wrote:
> On Wed, Mar 20, 2019 at 12:25 AM Antoine Pitrou <soli...@pitrou.net> wrote:
> >
> >
> > -1. Please don't remove tempfile.mktemp(). mktemp() is useful to
> > create a temporary *name*. All other tempfile functions create an
> > actual file and impose additional burden, for example by making the
> > file unaccessible by other processes. But sometimes all I want is a
> > temporary name that an *other* program will create / act on, not Python.
> > It's a very common use case when writing scripts.
> >
> > The only reasonable workaround I can think of is to first create a
> > temporary directory using mkdtemp(), then use a well-known name inside
> > that directory. But that has the same security implications AFAICT,
> > since another process can come and create the file / symlink first.
>
> Can't you create a NamedTemporaryFile and permit the other program to
> use it? I just tried that (with TiMidity, even though it's quite
> capable of just writing to stdout) and it worked fine.

Does it always work? According to the docs, """Whether the name can be
used to open the file a second time, while the named temporary file is
still open, varies across platforms (it can be so used on Unix; it
cannot on Windows NT or later)""".

tempfile.mktemp() is portable.

Regards

Antoine.

[Victor Stinner]

Hi,

I would prefer to keep tempfile.mktemp(), remove the deprecation, but
better explain the risk of race condition affecting security.

Le mar. 19 mars 2019 à 14:41, Chris Angelico <ros...@gmail.com> a écrit :
> Can't you create a NamedTemporaryFile and permit the other program to
> use it? I just tried that (with TiMidity, even though it's quite
> capable of just writing to stdout) and it worked fine.

When I write tests, I don't really care of security, but
NamedTemporaryFile caused me many troubles on Windows: you cannot
delete a file if it's still open in a another program. It's way more
convenient to use tempfile.mktemp().

O_EXCL, open(tmpname, "wx"), os.open(tmpname, os.O_CREAT | os.O_EXCL |
os.O_WRONLY), etc. can be used to get an error if the file already
exists.

I agree that for production code where security matters,
tempfile.mktemp() must be avoided. But I would prefer to keep it for
tests.

"with NamedTemporaryFile() as tmp: name = tmp.name" isn't a great
replacement for tempfile.mktemp(): it creates the file and it opens
it, whereas I only want a file name and be the first file to create
and open it.

Victor

[Stéphane Wirtel]

totally agree with you but this function is deprecated (2002) since 2.3,
with a simle comment about a security issue.

2.3 -> 2.7, 3.0 -> 3.7, 13 releases and 17 years.

Maybe we could remove it with an official PendingDeprecationWarning.

Le 19/03/19 à 14:39, Antoine Pitrou a écrit :

> The fact that many projects, including well-maintained ones such Sphinx
> or pip, use mktemp(), may be a hint that replacing it is not as easy as
> the people writing the Python documentation seem to think.

What's the relation with the people writing the Python documentation?

The suggestion about the deprecation warning was proposed by Brett
Cannon, and Serhiy has proposed to deprecate this function with some
releases.

The final release for 3.8 is scheduled for October 2019
(PendingDeprecationWarning).
Maybe 3.9 will be released 18 months later (DeprecationWarning).
and maybe 3.10 or 4.0 will be released 18 months after 3.9.

so, from today to 3.9+ there are approximatively 43 months -> 3,5 years.
I think it's enough in term of time for the big projects to improve
their code.

Stéphane

[Sebastian Rittau]

Am 19.03.19 um 14:53 schrieb Victor Stinner:

>
> When I write tests, I don't really care of security, but
> NamedTemporaryFile caused me many troubles on Windows: you cannot
> delete a file if it's still open in a another program. It's way more
> convenient to use tempfile.mktemp().
>
> O_EXCL, open(tmpname, "wx"), os.open(tmpname, os.O_CREAT | os.O_EXCL |
> os.O_WRONLY), etc. can be used to get an error if the file already
> exists.
>
> I agree that for production code where security matters,
> tempfile.mktemp() must be avoided. But I would prefer to keep it for
> tests.

If there are valid use cases for mktemp(), I recommend renaming
it to mkname_unsafe() or something equally obvious. Experience
(and the list of packages still using mktemp() posted here) shows
that just adding a warning to documentation is not enough. Users
often discover functions by experimentation or looking at examples
on the internet.

mktemp() is also unfortunately named, as it does not create a temp
file as implied. This can also add to the impression that it is the
proper function to use.

Adding a new function and following the deprecation process for the
old one should only be a minor inconvenience for existing users that
need it, should wake up existing users that should not use it in the
first place, and still allows it to be used for relevant use cases.

I believe for security reasons sometimes inconvenient changes like
this are necessary.

 - Sebastian

[Paul Ganssle]

I'm not sure the relationship with mkdir and mktemp here. I don't see
any uses of tempfile.mktemp in pip or setuptools, though they do use
os.mkdir (which is not deprecated).

Both pip and setuptools use pytest's tmpdir_factory.mktemp() in their
test suites, but I believe that is not the same thing.

> Unsubscribe: https://mail.python.org/mailman/options/python-dev/paul%40ganssle.io

[Antoine Pitrou]

On Tue, 19 Mar 2019 15:10:40 +0100
Stéphane Wirtel <step...@wirtel.be> wrote:
> totally agree with you but this function is deprecated (2002) since 2.3,
> with a simle comment about a security issue.

"Deprecated" doesn't mean anything here. It's just a mention in the
documentation. It doesn't produce actual warnings when used. And for
good reason: there are valid use cases.

> so, from today to 3.9+ there are approximatively 43 months -> 3,5 years.
> I think it's enough in term of time for the big projects to improve
> their code.

Please explain how the "improvement" would look like. What is the
intended replacement for the use case I have explained, and how does it
improve on the statu quo?

And if there is an easy replacement, then how about re-implementing
mktemp() using that replacement, instead of removing it?

Regards

Antoine.

[Guido van Rossum]

On Tue, Mar 19, 2019 at 6:27 AM Antoine Pitrou <soli...@pitrou.net> wrote:

-1.  Please don't remove tempfile.mktemp().  mktemp() is useful to
create a temporary *name*.  All other tempfile functions create an
actual file and impose additional burden, for example by making the
file unaccessible by other processes.  But sometimes all I want is a
temporary name that an *other* program will create / act on, not Python.
It's a very common use case when writing scripts.

The only reasonable workaround I can think of is to first create a
temporary directory using mkdtemp(), then use a well-known name inside
that directory.  But that has the same security implications AFAICT,
since another process can come and create the file / symlink first.

If all you need is a random name, why not just use a random number generator?

E.g. I see code like this:

    binascii.hexlify(os.urandom(8)).decode('ascii')

 

--

--Guido van Rossum (python.org/~guido)

[Antoine Pitrou]

Le 19/03/2019 à 15:52, Guido van Rossum a écrit :
>
> If all you need is a random name, why not just use a random number
> generator?
> E.g. I see code like this:
>
>     binascii.hexlify(os.urandom(8)).decode('ascii')

mktemp() already does this, probably in a better way, including the
notion of prefix, suffix, and parent directory. Why should I have to
reimplement it myself?

Regards

Antoine.

[Antoine Pitrou]

On Tue, 19 Mar 2019 15:12:06 +0100
Sebastian Rittau <sri...@rittau.biz> wrote:
> Am 19.03.19 um 14:53 schrieb Victor Stinner:
> >
> > When I write tests, I don't really care of security, but
> > NamedTemporaryFile caused me many troubles on Windows: you cannot
> > delete a file if it's still open in a another program. It's way more
> > convenient to use tempfile.mktemp().
> >
> > O_EXCL, open(tmpname, "wx"), os.open(tmpname, os.O_CREAT | os.O_EXCL |
> > os.O_WRONLY), etc. can be used to get an error if the file already
> > exists.
> >
> > I agree that for production code where security matters,
> > tempfile.mktemp() must be avoided. But I would prefer to keep it for
> > tests.
>
> If there are valid use cases for mktemp(), I recommend renaming
> it to mkname_unsafe() or something equally obvious.

> [...]

> Adding a new function and following the deprecation process for the
> old one should only be a minor inconvenience for existing users that
> need it, should wake up existing users that should not use it in the
> first place, and still allows it to be used for relevant use cases.

That would be fine with me.

Regards

Antoine.

[Anders Munch]

Antoine Pitrou:

> And if there is an easy replacement, then how about re-implementing
> mktemp() using that replacement, instead of removing it?

Indeed. The principal security issue with mktemp is the difficulty in creating a user-specific thing under a shared /tmp folder in a multi-user setup.

But if it hurts when you use /tmp, why use /tmp? Use a path with no world-accessible ancestor, or at least no world-writable ancestor.

On Windows, that means creating it somewhere under the CSIDL_LOCAL_APPDATA folder. Which is already the default for %TEMP% and %TMP%.
On Unix, it's a $HOME subfolder with access 700 or 600.
How about switching mktemp over to use that?

regards, Anders

[Giampaolo Rodola']

On Tue, Mar 19, 2019 at 2:06 PM Stéphane Wirtel <step...@wirtel.be> wrote:
>
> Hi,
>
> Context: raise a warning or remove tempfile.mktemp()
> BPO: https://bugs.python.org/issue36309
>
> Since 2.3, this function is deprecated in the documentation, just in the
> documentation. In the code, there is a commented RuntimeWarning.
> Commented by Guido in 2002, because the warning was too annoying (and I
> understand ;-)).
>
> So, in this BPO, we start to discuss about the future of this function
> and Serhiy proposed to discuss on the Python-dev mailing list.
>
> Question: Should we drop it or add a (Pending)DeprecationWarning?
>
> Suggestion and timeline:
>
> 3.8, we raise a PendingDeprecationWarning
> * update the code
> * update the documentation
> * update the tests
> (check a PendingDeprecationWarning if sys.version_info == 3.8)
>
> 3.9, we change PendingDeprecationWarning to DeprecationWarning
> (check DeprecationWarning if sys.version_info == 3.9)
>
> 3.9+, we drop tempfile.mktemp()
>
> What do you suggest?

I concur with others who think this should not be removed. It's used
in different stdlib and third party modules' test suites. I see
tempfile.mktemp() very similar to test.support.find_unused_port() and
os.path.exists/isfile/isdir functions: they are all subject to race
conditions but still they are widely used and have reason to exist
(practicality beats purity). I suggest turning the doc deprecation
into a note:: or warning::, so that extra visibility is maintained.
Because the use case is legitimate and many fs-related APIs such as
this one are inevitably racy, I lean more towards a note:: rather than
a warning:: though, and we could probably do the same for os.path.is*
functions.

@Sebastian

> If there are valid use cases for mktemp(), I recommend renaming
> it to mkname_unsafe() or something equally obvious.

I'm -1 about adding an alias (there should be one and preferably only
one way to do it). Also mkstemp() and mkdtemp() are somewhat poorly
named IMO, but I wouldn't add an alias for them either.

--
Giampaolo - http://grodola.blogspot.com

[Giampaolo Rodola']

On Tue, Mar 19, 2019 at 3:57 PM Antoine Pitrou <ant...@python.org> wrote:
>
>
> Le 19/03/2019 à 15:52, Guido van Rossum a écrit :
> >
> > If all you need is a random name, why not just use a random number
> > generator?
> > E.g. I see code like this:
> >
> > binascii.hexlify(os.urandom(8)).decode('ascii')
>
> mktemp() already does this, probably in a better way, including the
> notion of prefix, suffix, and parent directory. Why should I have to
> reimplement it myself?

On top of that mktemp() tries exists() in a loop, diminishing the risk
of names collision.

--
Giampaolo - http://grodola.blogspot.com

[Sebastian Rittau]

Am 19.03.19 um 17:23 schrieb Giampaolo Rodola':

> @Sebastian
>> If there are valid use cases for mktemp(), I recommend renaming
>> it to mkname_unsafe() or something equally obvious.
> I'm -1 about adding an alias (there should be one and preferably only
> one way to do it). Also mkstemp() and mkdtemp() are somewhat poorly
> named IMO, but I wouldn't add an alias for them either.
>

Just to clarify: I was not suggesting creating an alias, I was suggesting
renaming the function, but keeping the old name for a normal
deprecation cycle.

But I had another thought: If I understand correctly, the exploitability
of mktemp() relies on the fact that between determining whether the
file exists and creation an attacker can create the file themselves.
Couldn't this problem be solved by generating a filename of sufficient
length using the secrets module? This way the filename should be
"unguessable" and safe.

 - Sebastian

[Paul Moore]

On Tue, 19 Mar 2019 at 16:47, Sebastian Rittau <sri...@rittau.biz> wrote:
> But I had another thought: If I understand correctly, the exploitability
> of mktemp() relies on the fact that between determining whether the
> file exists and creation an attacker can create the file themselves.
> Couldn't this problem be solved by generating a filename of sufficient
> length using the secrets module? This way the filename should be
> "unguessable" and safe.

IMO, there's not much point trying to "fix" mktemp(). The issues with
it are clear and there are far better alternatives already available
for people who need them. The question here is simply about removing
the function "because people might mistakenly use it and create
security risks".

Personally, I don't think we should break the code of people who are
using mktemp() correctly, in awareness of its limitations, just out of
some idea of protecting people from themselves. Certainly we should
provide safe library functions wherever possible, but we should have
better reasons for removing functions that have been around for many,
many years than just "people might be using it wrongly".

Paul

[Giampaolo Rodola']

On Tue, 19 Mar 2019 at 17:47, Sebastian Rittau <sri...@rittau.biz> wrote:

Am 19.03.19 um 17:23 schrieb Giampaolo Rodola':
> @Sebastian
>> If there are valid use cases for mktemp(), I recommend renaming
>> it to mkname_unsafe() or something equally obvious.
> I'm -1 about adding an alias (there should be one and preferably only
> one way to do it). Also mkstemp() and mkdtemp() are somewhat poorly
> named IMO, but I wouldn't add an alias for them either.
>
Just to clarify: I was not suggesting creating an alias, I was suggesting
renaming the function, but keeping the old name for a normal
deprecation cycle.

But I had another thought: If I understand correctly, the exploitability
of mktemp() relies on the fact that between determining whether the
file exists and creation an attacker can create the file themselves.
Couldn't this problem be solved by generating a filename of sufficient
length using the secrets module? This way the filename should be
"unguessable" and safe.

Technically you cannot make it 100% safe, only less likely to occur. And on a second thought (I retract :)) since this could be used in real apps other than tests (I was too focused on that) I think this should be a doc warning after all, not info. Doc may suggest to use mode=x when creating the file, in order to remove the security implications.

[Guido van Rossum]

On Tue, Mar 19, 2019 at 10:14 AM Giampaolo Rodola' <g.ro...@gmail.com> wrote:

On Tue, 19 Mar 2019 at 17:47, Sebastian Rittau <sri...@rittau.biz> wrote:

Am 19.03.19 um 17:23 schrieb Giampaolo Rodola':
> @Sebastian
>> If there are valid use cases for mktemp(), I recommend renaming
>> it to mkname_unsafe() or something equally obvious.
> I'm -1 about adding an alias (there should be one and preferably only
> one way to do it). Also mkstemp() and mkdtemp() are somewhat poorly
> named IMO, but I wouldn't add an alias for them either.
>
Just to clarify: I was not suggesting creating an alias, I was suggesting
renaming the function, but keeping the old name for a normal
deprecation cycle.

But I had another thought: If I understand correctly, the exploitability
of mktemp() relies on the fact that between determining whether the
file exists and creation an attacker can create the file themselves.
Couldn't this problem be solved by generating a filename of sufficient
length using the secrets module? This way the filename should be
"unguessable" and safe.

Technically you cannot make it 100% safe, only less likely to occur.

That seems unnecessarily pedantic. A cryptographic random generator, like the one in the secrets module, *is* safe enough for all practical purposes (with a large margin).

 

And on a second thought (I retract :)) since this could be used in real apps other than tests (I was too focused on that) I think this should be a doc warning after all, not info. Doc may suggest to use mode=x when creating the file, in order to remove the security implications.

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.

[Sebastian Krause]

Guido van Rossum <gu...@python.org> wrote:
> If all you need is a random name, why not just use a random number
> generator?
> E.g. I see code like this:
>
> binascii.hexlify(os.urandom(8)).decode('ascii')

I tend to use os.path.join(dir, str(uuid.uuid4())) if I need to
create a random filename to pass to another program. However, it
would be nice to have a standard helper function that also allows me
to specify a prefix and suffix. Shouldn't it be possible to just
modify tempfile.mktemp() to generate much longer random filenames so
that it is virtually impossible that another program has already
created a file with the same name? Then the security problem is
gone, there is no need to continue deprecating this function and all
programs using it should continue to work.

[Brett Cannon]

If the only security vulnerability is due to the ability to guess a path that would make sense (but I honestly don't know since I'm not a security expert).

If Guido's suggestion isn't enough then I think that long with a rename of the function to make it obvious that there's potential issues and deprecating the old name makes the most sense.

[Greg Ewing]

Antoine Pitrou wrote:
> Does it always work? According to the docs, """Whether the name can be
> used to open the file a second time, while the named temporary file is
> still open, varies across platforms

So use NamedTemporaryFile(delete = False) and close it before passing
it to the other program.

--
Greg

[Giampaolo Rodola']

On Tue, Mar 19, 2019 at 6:21 PM Guido van Rossum <gu...@python.org> wrote:

>> On Tue, Mar 19, 2019 at 10:14 AM Giampaolo Rodola' <g.ro...@gmail.com> wrote:
>> Technically you cannot make it 100% safe, only less likely to occur.
> That seems unnecessarily pedantic. A cryptographic random generator, like the one in the secrets module, *is* safe enough for all practical purposes (with a large margin).

> [...]

> 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.

Seems to make sense to me. Currently the random string is 8 chars
long, meaning (if I'm not mistaken) max 40320 possible combinations:

>>> len(list(itertools.permutations('wuoa736m'))))
40320

We may use 9 chars and get to 362880 and increase "security".
Regardless, it seems to me the original deprecation warning for
mktemp() was not really justified and should be removed. IMO it
probably makes more sense to just clarify in the doc that
NamedTemporaryFile is the right / safe way to create a file with a
unique name and avoid the theoretical, rare name collision you'd get
by using open(mktemp(), 'w') instead. Also I'm not sure I understand
what the code sample provided in the doc aims to achieve:
https://docs.python.org/3/library/tempfile.html#tempfile.mktemp
The way I see it, the main (or "correct") use case for mktemp() is
when you explicitly want a file name which does not exist, in order to
exercise legitimate scenarios like these ones:

>>> self.assertRaises(FileNotFoundError, os.unlink, tempfile.mktemp())

>>> dst = tempfile.mktemp()
>>> os.rename(src, dst)
>>> assert not os.path.exists(src) and os.path.exists(dst)

--
Giampaolo - http://grodola.blogspot.com

[eryk sun]

On 3/19/19, Victor Stinner <vsti...@redhat.com> wrote:
>
> When I write tests, I don't really care of security, but
> NamedTemporaryFile caused me many troubles on Windows: you cannot
> delete a file if it's still open in a another program. It's way more
> convenient to use tempfile.mktemp().

Opening the file again for normal access is problematic.
NamedTemporaryFile opens it with delete access, but Python's open()
function doesn't support delete-access sharing unless an opener is
used that calls CreateFileW.

NamedTemporaryFile does open files with delete-access sharing, so any
process can delete the file if it's allowed by the file's security and
attributes. You may be thinking of unlinking. In Windows versions
prior to 10, that's always a two-step process. A file with its delete
disposition set doesn't get unlinked until all references for all open
instances are closed.

In Windows 10 (release 1709+), we have the option of using
SetFileInformationByHandle: FileDispositionInfoEx (21) with
FILE_DISPOSITION_FLAG_POSIX_SEMANTICS (2) and
FILE_DISPOSITION_FLAG_DELETE (1). The online documentation hasn't been
updated to include this, but it's supported in the headers for
_WIN32_WINNT_WIN10_RS1 and later. This operation unlinks the file as
soon as we close our handle, even if it has existing references. This
is explained in the remarks for the underlying NT system call [1]. In
particular this resolves the race condition related to handles opened
by anti-malware programs.

It may be worth adding support for deleting files by handle that tries
FileDispositionInfoEx in 1709+. This will work in about half of all
Windows systems. (About 40% still run Windows 7.) It's not a panacea
for Windows file-deleting woes. We still need to be able to open the
file with delete access, which requires existing opens to share delete
access.

[1]: https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/ntddk/ns-ntddk-_file_disposition_information_ex

[Nathaniel Smith]

On Tue, Mar 19, 2019 at 3:43 PM Giampaolo Rodola' <g.ro...@gmail.com> wrote:
>
> On Tue, Mar 19, 2019 at 6:21 PM Guido van Rossum <gu...@python.org> wrote:
>
> >> On Tue, Mar 19, 2019 at 10:14 AM Giampaolo Rodola' <g.ro...@gmail.com> wrote:
> >> Technically you cannot make it 100% safe, only less likely to occur.
> > That seems unnecessarily pedantic. A cryptographic random generator, like the one in the secrets module, *is* safe enough for all practical purposes (with a large margin).
> > [...]
> > 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.
>
> Seems to make sense to me. Currently the random string is 8 chars
> long, meaning (if I'm not mistaken) max 40320 possible combinations:
>
> >>> len(list(itertools.permutations('wuoa736m'))))
> 40320

It's 8 chars taken from a-z0-9_, so there are 37**8 ~= 10**12 possible
names, or ~41 bits.

> We may use 9 chars and get to 362880 and increase "security".
> Regardless, it seems to me the original deprecation warning for
> mktemp() was not really justified and should be removed.

Historically, mktemp variants have caused *tons* of serious security
vulnerabilities. It's not a theoretical issue. See e.g.
https://www.owasp.org/index.php/Insecure_Temporary_File

I believe that if we used sufficient unguessable randomness to
generate the name, then that would resolve the issues and let us
un-deprecate it. Though like Brett I would like to double-check this
with security specialists :-).

This would also simplify the implementation a *lot*, down to just:

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)

The choice of ENTROPY_BYTES is an interesting question. 16 (= 128
bits) would be a nice "obviously safe" number, and gives 22-byte
filenames. We might be able to get away with fewer, if we had a
plausible cost model for the attack. This is another point where a
security specialist might be helpful :-).

-n

--
Nathaniel J. Smith -- https://vorpus.org

[Serhiy Storchaka]

19.03.19 16:21, Paul Ganssle пише:

> I'm not sure the relationship with mkdir and mktemp here. I don't see
> any uses of tempfile.mktemp in pip or setuptools, though they do use
> os.mkdir (which is not deprecated).
>
> Both pip and setuptools use pytest's tmpdir_factory.mktemp() in their
> test suites, but I believe that is not the same thing.

My fault!

Initially I searched mktemp, and have found usages in distutils, tests,
and few other modules in the stdlib. When I wrote the last message I
repeat the search on the wider set of Python sources, but for *mkdir*
instead of *mktemp*! Thank you for catching this mistake Paul.

Actually, seems mktemp is used exclusively in tests in third-party projects.

_______________________________________________
Python-Dev mailing list
Pytho...@python.org
https://mail.python.org/mailman/listinfo/python-dev

Unsubscribe: https://mail.python.org/mailman/options/python-dev/dev-python%2Bgarchive-30976%40googlegroups.com

[Serhiy Storchaka]

19.03.19 15:39, Antoine Pitrou пише:

> The fact that many projects, including well-maintained ones such Sphinx
> or pip, use mktemp(), may be a hint that replacing it is not as easy as
> the people writing the Python documentation seem to think.

Sorry, it was my mistake (searching mkdir instead of mktemp). mktemp is
only used in few tests in third-party projects.

[Anders Munch]

Greg Ewing:

> So use NamedTemporaryFile(delete = False) and close it before passing it to the other program.

That's effectively the same as calling tempfile.mktemp. While it does waste time opening and closing an unused file, that doesn't help with security. If anything, it might worsen security.

If a secure implementation of mktemp is truly impossible, then the same could be said for NamedTemperatoryFile(delete=False). Should that be deprecated as well?

regards, Anders

[Sebastian Rittau]

Am 20.03.19 um 09:47 schrieb Anders Munch:

> Greg Ewing:
>> So use NamedTemporaryFile(delete = False) and close it before passing it to the other program.
> That's effectively the same as calling tempfile.mktemp. While it does waste time opening and closing an unused file, that doesn't help with security. If anything, it might worsen security.

That is not actually true. The important difference is that with
NamedTemporaryFile the file exists with appropriate access right (0600).
This denies access of that file to other users. With mktemp() no file is
created, so another user can "hijack" that name and cause programs to
write potentially privileged data into or read manipulated data from
that file.

 - Sebastian

[Anders Munch]

Anders Munch:

>>> So use NamedTemporaryFile(delete = False) and close it before passing it to the other program.
>> That's effectively the same as calling tempfile.mktemp. While it does waste time opening and closing an unused file, that doesn't help with security

Sebastian Rittau:

> That is not actually true. The important difference is that with NamedTemporaryFile the file exists with appropriate access right (0600).

You are right, I must have mentally reversed the polarity of the delete argument. And I didn't realise that the access right on a file had the power to prevent itself from being removed from the folder that it's in. I thought the access flags were a property of the file itself and not the directory entry. Not sure how that works.

But if NamedTemporaryFile(delete=False) is secure then why not use that to implement mktemp?

def mktemp(suffix="", prefix=template, dir=None):
with NamedTemporaryFile(delete=False, suffix=suffix, prefix=prefix, dir=dir) as f:
return f.name

Yes, it does leave an empty file if the name is not used, but the name is usually created with the intent to use it, so that is rarely going to be a problem. Just document that that's how it is. It does mean that where there's an explicit file-exists check before writing the file, that code will break. But it will break a lot less code than removing mktemp entirely.

regards, Anders

[eryk sun]

On 3/20/19, Anders Munch <a...@flonidan.dk> wrote:
>
> You are right, I must have mentally reversed the polarity of the delete
> argument. And I didn't realise that the access right on a file had the
> power to prevent itself from being removed from the folder that it's in. I
> thought the access flags were a property of the file itself and not the
> directory entry. Not sure how that works.

In POSIX, it's secure so long as we use a directory that doesn't grant
write access to other users, or one that has the sticky bit set such
as "/tmp". A directory that has the sticky bit set allows only root
and the owner of the file to unlink the file.

In Windows, a user's default %TEMP% directory is only accessible by
the user, SYSTEM, and Administrators. The only way others can delete a
file there is if the file security is modified to allow it (possible
for individual files, unlike POSIX). This works even with no access to
the temp directory itself because users have SeChangeNotifyPrivilege,
which bypasses traverse (execute) access checks.

[Anders Munch]

Nathaniel J. Smith:

> Historically, mktemp variants have caused *tons* of serious security
> vulnerabilities. It's not a theoretical issue.

All the more reason to have a standard library function that gets it right.

> The choice of ENTROPY_BYTES is an interesting question. 16 (= 128 bits) would
> be a nice "obviously safe" number, and gives 22-byte filenames. We might be
> able to get away with fewer, if we had a plausible cost model for the
> attack. This is another point where a security specialist might be helpful :-).

I'm not a security specialist but I play one on TV.
Here's my take on it.

Any kind of brute force attack will require at least one syscall per try, to
create a file or check if a file by a given name exists. It's a safe assumption
that names have to be tried individually, because if the attacker has a faster
way of enumerating existing file names, then the entropy of the filename is
worthless anyway.

That means even with only 41 bits of entry, the attacker will have make 2^40
tries on average. For an individual short-lived file, that could be enough;
even with a billion syscalls per second, that's over a thousand seconds, leaving
plenty of time to initiate whatever writes the file.

However, there could be applications where the window of attack is very long,
hours or days even, or that are constantly writing new temporary files, and
where the attacker can keep trying at a rapid pace, and then 41 bits is
definitely not secure.

128 bits seems like overkill: There's no birthday attack because no-one keeps
2^(ENTROPY_BITS/2) files around, and the attack is running on the attackee's
system, so there's no using specialised accelerator hardware. I'd say 64 bits
is enough under those circumstances, but I wouldn't be surprised if a better
security specialist could make a case for more. So maybe go with 80 bits,
that's puts it at 15 or 16 characters.


Med venlig hilsen/Best regards

Anders Munch
Chief Security Architect

T: +45 76266981 * M: +45 51856626
a...@flonidan.dk * www.flonidan.com
FLONIDAN A/S * Islandsvej 29 * DK-8700 Horsens * CVR: 89919916
Winner of the 2018 Frost & Sullivan Customer Leadership Award

[Victor Stinner]

Hi,

I'm not really convinced that mktemp() should be made "more secure".
To be clear: mktemp() is vulnerable by design. It's not a matter of
entropy. You can watch the /tmp directory using inotify and "discover"
immediately the "secret" filename, it doesn't depend on the amount of
entropy used to generate the filename. A function is either unsafe or
secure.

Why mktemp() only uses 8 characters? Well, I guess that humans like to
be able to copy manually (type) a filename :-)

Note: For the ones who didn't notice, "mktemp()" name comes from a
function with the same name in the libc.
http://man7.org/linux/man-pages/man3/mktemp.3.html

Victor

> Unsubscribe: https://mail.python.org/mailman/options/python-dev/vstinner%40redhat.com

--
Night gathers, and now my watch begins. It shall not end until my death.

[Steven D'Aprano]

On Wed, Mar 20, 2019 at 11:25:03AM +0000, Anders Munch wrote:

> 128 bits seems like overkill: There's no birthday attack because no-one keeps
> 2^(ENTROPY_BITS/2) files around,

You haven't seen my Downloads folder... :-)

But seriously:

> and the attack is running on the attackee's
> system, so there's no using specialised accelerator hardware. I'd say 64 bits
> is enough under those circumstances, but I wouldn't be surprised if a better
> security specialist could make a case for more. So maybe go with 80 bits,
> that's puts it at 15 or 16 characters.

Why be so miserly with entropy? This probably isn't a token that goes to
a human, who may have to type it into a web browser, or send it by SMS.
Its likely to be a name used only by the machine. Using 128 bits is just
22 characters using secrets.token_urlsafe().

The default entropy used by secrets is 32 bytes, which gives a 43
character token. I have plenty of files with names longer than that:

"Funny video of cat playing piano while dog does backflips.mp4"

Of course, if you have some specific need for the file name to be
shorter (or longer!) then there ought to be a way to set the entropy
used. But I think the default secrets entropy is fine, and its better to
have longer names than shorter ones, within reason. I don't think 40-50
characters (plus any prefix or suffix) is excessive for a temporary file
intended for use by an application rather than a human.



--
Steven

[Jeroen Demeyer]

On 2019-03-20 12:45, Victor Stinner wrote:
> You can watch the /tmp directory using inotify and "discover"
> immediately the "secret" filename, it doesn't depend on the amount of
> entropy used to generate the filename.

That's not the problem. The security issue here is guessing the filename
*before* it's created and putting a different file or symlink in place.

So I actually do think that mktemp() could be made secure by using a
longer name generated by a secure random generator.

[Steven D'Aprano]

On Wed, Mar 20, 2019 at 12:45:40PM +0100, Victor Stinner wrote:
> Hi,
>
> I'm not really convinced that mktemp() should be made "more secure".
> To be clear: mktemp() is vulnerable by design. It's not a matter of
> entropy. You can watch the /tmp directory using inotify and "discover"
> immediately the "secret" filename, it doesn't depend on the amount of
> entropy used to generate the filename. A function is either unsafe or
> secure.

Security is not a binary state, it is never either-or "unsafe" or
"secure". Secure against what attacks? Unsafe under what circumstances?

I can use the unsafe mktemp on a stand alone single-user computer,
disconnected from the internet, guaranteed to have nothing but trusted
software, and it will be secure in practice.

Or I can use the "safe interfaces" and I'm still vulnerable to an
Advanced Persistent Threat that has compromised the OS specifically to
target my application. If the attacker controls the OS or the hardware,
then effectively they've already won.


--
Steven

[Anders Munch]

Steven D'Aprano:

>> 128 bits seems like overkill: There's no birthday attack because
>> no-one keeps 2^(ENTROPY_BITS/2) files around

> You haven't seen my Downloads folder... :-)

I put it to you that those files are not temporary :-)

> Why be so miserly with entropy?

I don't necessarily disagree.

> Using 128 bits is just 22 characters using secrets.token_urlsafe().

A little more when you take into account case-insensitive file systems.

regards, Anders

[Anders Munch]

Victor Stinner:

> To be clear: mktemp() is vulnerable by design

No: mktemp() is vulnerable by implementation. Specifically, returning a file name in a world-accessible location, /tmp.

[Antoine Pitrou]

On Wed, 20 Mar 2019 11:25:53 +1300
Greg Ewing <greg....@canterbury.ac.nz> wrote:
> Antoine Pitrou wrote:
> > Does it always work? According to the docs, """Whether the name can be
> > used to open the file a second time, while the named temporary file is
> > still open, varies across platforms
>
> So use NamedTemporaryFile(delete = False) and close it before passing
> it to the other program.

How is it more secure than using mktemp()?

Regards

Antoine.

[Ivan Pozdeev via Python-Dev]

Before we can say if something is "secure" or not, we need a threat model -- i.e we need to agree which use cases we are protecting and from
what threats.

So far, I've seen these use cases:

1. File for the current process' private use
2. File/file name generated by the current process; written by another process, read by current one
3. File name generated by the current process; written by the current process, read by another one

And the following threats, three axes:

a. Processes run as other users
b. Processes run as the same user (or a user that otherwise automatically has access to all your files)

1. Accidental collision from a process that uses CREATE_NEW or equivalent
2. Accidental collision from a process that doesn't use CREATE_NEW or equivalent
3. Malicious code creating files at random
4. Malicious code actively monitoring file creation

-1. read
-2. write

E.g. for threat b-4), it's not safe to use named files for IPC at all, only case 1 can be secured (with exclusive open).

On 19.03.2019 16:03, Stéphane Wirtel wrote:

> Hi,
>
> Context: raise a warning or remove tempfile.mktemp()
> BPO: https://bugs.python.org/issue36309
>
> Since 2.3, this function is deprecated in the documentation, just in the
> documentation. In the code, there is a commented RuntimeWarning.
> Commented by Guido in 2002, because the warning was too annoying (and I
> understand ;-)).
>
> So, in this BPO, we start to discuss about the future of this function
> and Serhiy proposed to discuss on the Python-dev mailing list.
>
> Question: Should we drop it or add a (Pending)DeprecationWarning?
>
> Suggestion and timeline:
>
> 3.8, we raise a PendingDeprecationWarning
> * update the code
> * update the documentation
> * update the tests
> (check a PendingDeprecationWarning if sys.version_info == 3.8)
>
> 3.9, we change PendingDeprecationWarning to DeprecationWarning
> (check DeprecationWarning if sys.version_info == 3.9)
>
> 3.9+, we drop tempfile.mktemp()
>
> What do you suggest?
>
> Have a nice day and thank you for your feedback.

> _______________________________________________
> Python-Dev mailing list
> Pytho...@python.org
> https://mail.python.org/mailman/listinfo/python-dev

> Unsubscribe: https://mail.python.org/mailman/options/python-dev/vano%40mail.mipt.ru

--
Regards,
Ivan

[Greg Ewing]

Antoine Pitrou wrote:
> On Wed, 20 Mar 2019 11:25:53 +1300
> Greg Ewing <greg....@canterbury.ac.nz> wrote:
>
>>So use NamedTemporaryFile(delete = False) and close it before passing
>>it to the other program.
>
> How is it more secure than using mktemp()?

It's not, but it solves the problem someone suggested of another
program not being able to access and/or delete the file.

--
Greg

[eryk sun]

On 3/20/19, Greg Ewing <greg....@canterbury.ac.nz> wrote:
> Antoine Pitrou wrote:
>

>> How is it more secure than using mktemp()?
>
> It's not, but it solves the problem someone suggested of another
> program not being able to access and/or delete the file.

NamedTemporaryFile(delete=False) is more secure than naive use of
mktemp(). The file is created exclusively (O_EXCL). Another standard
user can't overwrite it. Nor can another standard user delete it if
it's created in the default temp directory (e.g. POSIX "/tmp" has the
sticky bit set). mkstemp() is similar but lacks the convenience and
reliable resource management of a Python file wrapper.

There's still the problem of accidental name collisions with other
processes that can access the file, i.e. processes running as the same
user or, in POSIX, processes running as the super user. I saw a
suggestion in this thread to increase the length of the random
sequence from 8 characters up to 22 characters in order to make this
problem extremely improbable.

[Cameron Simpson]

On 20Mar2019 12:53, Jeroen Demeyer <J.De...@UGent.be> wrote:
>On 2019-03-20 12:45, Victor Stinner wrote:
>>You can watch the /tmp directory using inotify and "discover"
>>immediately the "secret" filename, it doesn't depend on the amount of
>>entropy used to generate the filename.
>
>That's not the problem. The security issue here is guessing the
>filename *before* it's created and putting a different file or symlink
>in place.
>
>So I actually do think that mktemp() could be made secure by using a
>longer name generated by a secure random generator.

I know it is days later, but to add a little nuance: the security issue
is guessing the filename before it is _used_. Consider:

path = tempfile.mktemp()
with open(path, "w"):
write some secret stuff ...
call_other_function(path)

If an attacker gets in _after_ the open (which creates the file) by
using something like inotify to _observe_ the pathname instead of
guessing and supplants the file then, call_other_function is then
subverted.

Also, the common examples are attackers who are not the user making the
tempfile, in which case the _default_ mktemp is sort of secure with the
above because it gets made in /tmp which on a modern POSIX system
prevents _other_ uses from removing/renaming a file. (And Eryk I think
described the Windows situation which is similarly protected).

However, mktemp somewhere else is not so protected.

And the attacker might be malware running as the orignal user (yes the
game may already be overin that case for other reasons).

However, I wanted to make the point that the security issue isn't around
creation but use - trusting the mktemp pathname to be the same state as
it was earlier.

Cheers,
Cameron Simpson <c...@cskk.id.au>

[eryk sun]

On 3/23/19, Cameron Simpson <c...@cskk.id.au> wrote:
>
> Also, the common examples are attackers who are not the user making the
> tempfile, in which case the _default_ mktemp is sort of secure with the
> above because it gets made in /tmp which on a modern POSIX system
> prevents _other_ uses from removing/renaming a file. (And Eryk I think
> described the Windows situation which is similarly protected).

Using NamedTemporaryFile(delete=False) or mkstemp() ensures that the
file is created and opened securely. in contrast, the filename from
mktemp() might be used naively in POSIX, such as open(path, "w"). This
file might grant read access to everyone depending on the file-mode
creation mask (umask). Also, since it neglects to use exclusive mode
("x"), it might open an existing file that grants read-write
permission to the world, or maybe it's a symlink.

By default, even naive use of the mktemp() name in Windows remains
secure, since every user has a separate temp directory that's only
accessible by privileged users such as SYSTEM, Administrators, and
Backup Operators (with SeBackupPrivilege and SeRestorePrivilege
enabled). The primary issue with a short name is an accidental name
collision with another program that's not as careful as Python's
tempfile. Using a longer name decreases the chance of this to
practically nothing.