[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[WGH]

WGH  added the comment:

> My patch uses O_EXCL. It makes possible to use linkat() to create a path for 
> the temporary file (I didn't try it, but I read that it's possible). I don't 
> know if using O_EXCL should be the default.

I think it is the other way around. From the manual: "If O_EXCL is not 
specified, then linkat(2) can ..."

--
nosy: +WGH

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

> Suppose conditions:
> - Old linux kernel ignoring flag
> - malicious hacker force use of PLAIN FILE instead of directory

Is it a theorical bug, or are you able to reproduce it?

Old Linux kernel ignores the 0o2000 bit but O_TMPFILE is 0o2000 | 
os.O_DIRECTORY. So the kernel still ensures that the path is a directory. 
tempfile.TemporaryFile() tries to open the path with:

   os.open(path, os.O_RDWR |os.O_EXCL | os.O_TMPFILE)

if the 0o2000 bit is ignored by old kernel, it becomes:

   os.open(path, os.O_RDWR |os.O_EXCL | os.O_DIRECTORY)

You cannot open a regular file with these flags:

>>> open('x', 'w').close()
>>> os.open('x', os.O_RDWR |os.O_EXCL | os.O_DIRECTORY)
Traceback (most recent call last):
  File "", line 1, in 
NotADirectoryError: [Errno 20] Not a directory: 'x'

You cannot open a directory with these flags:

>>> os.open('.', os.O_RDWR |os.O_EXCL | os.O_DIRECTORY)
Traceback (most recent call last):
  File "", line 1, in 
IsADirectoryError: [Errno 21] Is a directory: '.'

Same behaviour for symbolic link to a regular file or to a directory.

Please open a new issue if you consider that you found a bug, but please write 
a short script reproducing the bug.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Марк Коренберг]

Марк Коренберг added the comment:

Okay, seemes it is not documented that

os.open('.', os.O_RDWR |os.O_EXCL | os.O_DIRECTORY)

Should return EISDIR  

I did not found that in Linux manpages. Using undocumented features is bad. 
Maybe I should report this to Michael Kerrisk to update manpage ?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

2015-10-20 20:02 GMT+02:00 Марк Коренберг :
> Okay, seemes it is not documented that
>
> os.open('.', os.O_RDWR |os.O_EXCL | os.O_DIRECTORY)
>
> Should return EISDIR

You cannot open a directory to write, only to read.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Марк Коренберг]

Марк Коренберг added the comment:

Well, it's not said explicit, that O_DIRECTORY cannot be combined with O_RDWR.

So, everything is valid now, very hacky, but works without bugs.

It will be nice, if someone comment that hacks in source code

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

> It will be nice, if someone comment that hacks in source code

I don't understand why you keep calling this a hack. It's part of open() 
contract, and I'm quite sure that it was a deliberate choice to declare 
O_TMPFILE as O_DIRECTY|new_bit. See for example this comment:
https://lwn.net/Articles/560834/

I wrote a patch to explain that it's fine to call open() with O_TMPFILE on old 
kernels to check if the flag is supported: see attached patch.

--
resolution: fixed -> 
status: closed -> open
Added file: http://bugs.python.org/file40824/tempfile_comment.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Марк Коренберг]

Марк Коренберг added the comment:

Huge thanks for that patch. Now things are much cleaner.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Roundup Robot]

Roundup Robot added the comment:

New changeset dc2deecb2346 by Victor Stinner in branch '3.5':
Issue #21515: Elaborate tempfile.TemporaryFile() comment
https://hg.python.org/cpython/rev/dc2deecb2346

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

> Huge thanks for that patch. Now things are much cleaner.

I understand that the patch looks good to you, so I pushed it to Python 3.5 & 
3.6. I close again the issue. Thanks for your analasys of 
tempfile.TemporaryFile() :-)

--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Марк Коренберг]

Марк Коренберг added the comment:

Suppose conditions:
- Old linux kernel ignoring flag
- malicious hacker force use of PLAIN FILE instead of directory

On new kernel it will fail
On old kernel it will just open that file!

So, we can make a HACK! Just add last slash to directory name. This will not 
hurt on new kernels, but protect on old kernels.

tests should also test a case when directory is symlink really.

--
nosy: +mmarkk

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

Changes by STINNER Victor victor.stin...@gmail.com:


--
resolution:  - fixed
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Arfrever Frehtes Taifersar Arahesis]

Arfrever Frehtes Taifersar Arahesis added the comment:

Minor inconsistency in Lib/tempfile.py:
# Set flag to None to not try again.
_O_TMPFILE_WORKS = False

s/None/False/

--
nosy: +Arfrever

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Roundup Robot]

Roundup Robot added the comment:

New changeset 8b93cdccd872 by Victor Stinner in branch 'default':
Issue #21515: Fix typo in a comment, thanks Arfrever for the report
http://hg.python.org/cpython/rev/8b93cdccd872

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

Can someone please review  tempfile_o_tmpfile3.patch ?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Antoine Pitrou]

Antoine Pitrou added the comment:

It would be nice if the patch added a pointer to the O_TMPFILE documentation 
(if that exists) and mentioned that it is Linux-specific.
Otherwise, it looks good to me.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Roundup Robot]

Roundup Robot added the comment:

New changeset 4b51a992cb70 by Victor Stinner in branch 'default':
Issue #21515: tempfile.TemporaryFile now uses os.O_TMPFILE flag is available
http://hg.python.org/cpython/rev/4b51a992cb70

--
nosy: +python-dev

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

 It would be nice if the patch added a pointer to the O_TMPFILE documentation 
 (if that exists) and mentioned that it is Linux-specific.

I modified TemporaryFile documentation to mention that the O_TMPFILE
flag is used if available and if the flag works.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Serhiy Storchaka]

Changes by Serhiy Storchaka storch...@gmail.com:


--
nosy: +serhiy.storchaka

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

tempfile_o_tmpfile2.patch: Updated patch to handle OS errors.

I'm not sure that __O_TMPFILE has the same value on all architectures.

The O_TMPFILE flag was added to fcntl.h in the glibc 2.19 (released the 8 Feb 
2014):
https://sourceware.org/git/?p=glibc.git;a=commit;h=ffdd31816a67f48697ea4d6b852e58d2886d42ca

My Linux Fedora 20 uses the glibc 2.18.

I removed the hardcoded constant from my patch. Add this to Lib/tempfile.py to 
test manually if you have a glibc older than 2.19:

# after if hasattr(_os, 'O_TMPFILE'):
elif _sys.platform == 'linux':
__O_TMPFILE = 0o2000
_O_TMPFILE = (__O_TMPFILE | _os.O_DIRECTORY)

--
Added file: http://bugs.python.org/file35276/tempfile_o_tmpfile2.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

Changes by STINNER Victor victor.stin...@gmail.com:


Removed file: http://bugs.python.org/file35276/tempfile_o_tmpfile2.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

(Oops, I made a mistake, the hardcoded constant was still present in my patch 
2.)

Patch 3 uses tempfile._O_TMPFILE_WORKS variable to check if the O_TMPFILE flag 
is avaialble and works.

Use os.O_TMPFILE = 0o2000 | os.O_DIRECTORY to try my patch with glibc 
older than 2.19.

--
Added file: http://bugs.python.org/file35277/tempfile_o_tmpfile3.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

New submission from STINNER Victor:

Linux 3.11 introduced a new file flag O_TMPFILE. The flag is exposed in 
Python, see the issue #18673.

O_TMPFILE is a new open(2)/openat(2) flag that makes easier the creation of 
secure temporary files. Files opened with the O_TMPFILE flag are created but 
they are not visible in the filesystem. And as soon as they are closed, they 
get deleted - just as a file you would have opened and unlinked.
http://kernelnewbies.org/Linux_3.11#head-8be09d59438b31c2a724547838f234cb33c40357

Does it make sense to use this flag in tempfile.TemporaryFile?

Attached patch is a work-in-progress patch for tempfile.

 if hasattr(_os, 'O_TMPFILE'):
 _O_TMPFILE = _os.O_TMPFILE
 elif _sys.platform == 'linux':
 __O_TMPFILE = 0o2000
 _O_TMPFILE = (__O_TMPFILE | _os.O_DIRECTORY)

The second if should be removed. I used it because my Linux kernel (3.14) 
supports the flag, but the constant is not defined yet in C headers of my C 
library (glibc 2.18).

 flags = (flags | _O_TMPFILE)  ~_os.O_CREAT

O_CREAT is incompatible with O_TMPFILE.

Bonus point of the flag: no need to compute a random name! Just pass the 
temporary directory.

To do: test the patch on Linux  3.11 to see how the flag is interpreted. If 
the flag is ignored, we open the directory in write mode! That's insafe. If the 
flag raises an error, we should fallback to the current implementation and 
remember that the flag is not supported.

I implemented something similar for O_CLOEXEC and SOCK_CLOEXEC flags (PEP 433).

--
files: tempfile_o_tmpfile.patch
keywords: patch
messages: 218648
nosy: haypo, neologix
priority: normal
severity: normal
status: open
title: Use Linux O_TMPFILE flag in tempfile.TemporaryFile?
versions: Python 3.5
Added file: http://bugs.python.org/file35261/tempfile_o_tmpfile.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Antoine Pitrou]

Antoine Pitrou added the comment:

I don't think we can use this by default, or it will break the expected 
semantics of temporary files under Unix (visible by other processes).

--
nosy: +georg.brandl, ncoghlan, pitrou

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

I don't think we can use this by default, or it will break the expected 
semantics of temporary files under Unix (visible by other processes).

I proposed to change TemporaryFile, not NamedTemporaryFile. Do you mean that 
other processes are supposed to have access to the temporary file descriptor? 
Access through /proc/pid/fd/tmp_fd?

O_TMPFILE should increase the security because there is no more race condition 
between os.open() and os.unlink() (window where an attack can access the file).

My patch uses O_EXCL. It makes possible to use linkat() to create a path for 
the temporary file (I didn't try it, but I read that it's possible). I don't 
know if using O_EXCL should be the default.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Antoine Pitrou]

Antoine Pitrou added the comment:

 I proposed to change TemporaryFile, not NamedTemporaryFile.

Ah, sorry. Then it sounds ok.
(I couldn't find any documentation for O_TMPFILE, though)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

It looks like O_TMPFILE is supported by tmpfs (3.11), ext3 (3.11), ext4 (3.11), 
XFS (3.15). It looks like BTRFS will also support the O_TMPFILE:
https://btrfs.wiki.kernel.org/index.php/Project_ideas#Implement_O_TMPFILE_support

--

It looks like os.open() fails with OSError(95, 'Operation not supported') if 
the filesystem of the directory doesn't support TMPFILE. In this case, a 
fallback to the current implementation should be enough. I don't think that we 
need to remember that the directory doesn't support TMPFILE. The directory may 
be on a different filesystem at the next call.

haypo@smithers$ ~/prog/python/default/python 
Python 3.5.0a0 (default:5e98a50e0f55, May 16 2014, 10:44:10) 
 import tempfile
 tempfile._O_TMPFILE
4259840
 f=tempfile.TemporaryFile(dir='.')
Traceback (most recent call last):
  File stdin, line 1, in module
  File /home/haypo/prog/python/default/Lib/tempfile.py, line 507, in 
TemporaryFile
fd = _os.open(dir, flags, 0o600)
OSError: [Errno 95] Operation not supported: '.'

haypo@smithers$ df .
Sys. de fichiers   Taille Utilisé Dispo Uti% Monté sur
192.168.0.42:/test96G9,1G   83G  10% /home/haypo/nfs

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

It looks like open() ignores O_TMPFILE (0o2000) on old Linux kernels. Test 
on Linux 3.2:

 fd=os.open(/tmp, os.O_RDWR | O_TMPFILE, 0o600)
Traceback (most recent call last):
  File stdin, line 1, in module
OSError: [Errno 21] Is a directory: '/tmp'

 fd=os.open(/tmp, os.O_RDWR | os.O_DIRECTORY, 0o600)
Traceback (most recent call last):
  File stdin, line 1, in module
OSError: [Errno 21] Is a directory: '/tmp'

So we should catch OSError(21, Is a directory: '/tmp') and fallback to the 
current implementation (random name, unlink), and remember that the kernel 
version is too old.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Antoine Pitrou]

Antoine Pitrou added the comment:

 So we should catch OSError(21, Is a directory: '/tmp') and fallback
 to the current implementation (random name, unlink), and remember that
 the kernel version is too old.

Just catch any OSError?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[STINNER Victor]

STINNER Victor added the comment:

 Just catch any OSError?

If possible, I would prefer to not retry O_TMPFILE each time if the kernel 
version does not support the flag.

Pseudo-code:
--
if o_tmpfile_supported:
   try:
  fd = os.open(dir, os.O_TMPFILE | ...)
   except IsADirectoryError:
  # Linux kernel older than 3.11 ignores O_TMPFILE flag
  o_tmpfile_supported = False
   except OSError:
  # the filesystem doesn't support O_TMPFILE
  pass
   else:
  return io.open(fd, ...)
   # fallback to unsafe but portable implementation

# current code generating a name and using unlink
---

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Antoine Pitrou]

Antoine Pitrou added the comment:

  Just catch any OSError?
 
 If possible, I would prefer to not retry O_TMPFILE each time if the kernel 
 version does not support the flag.

How likely it is to have a glibc flag that's not supported by the kernel
(on a normal setup, not a self-compiled distro)?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Nick Coghlan]

Nick Coghlan added the comment:

Reasonably common, I believe. For example, Red Hat ships a Developer
Toolset, so you may be building with an up to date gcc on RHEL 6 or 7, but
still support deploying against the older kernel in RHEL 5.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21515] Use Linux O_TMPFILE flag in tempfile.TemporaryFile?

[Josh Rosenberg]

Changes by Josh Rosenberg shadowranger+pyt...@gmail.com:


--
nosy: +josh.rosenberg

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21515
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com