[issue2528] Change os.access to check ACLs under Windows

[Eryk Sun]

Eryk Sun  added the comment:

With increasing use of os.access() in shutil and tempfile, it would be nice to 
have a real implementation of os.access() for Windows. 

Instead of manually evaluating the security of the file/directory, as 
issue2528.2.patch attempts to do, I'd rather just open the file with the 
desired access (e.g. GENERIC_READ, GENERIC_WRITE, GENERIC_EXECUTE). An 
open-based check supports checking for sharing violations, filesystem policy 
(e.g. FILE_READ_ATTRIBUTES granted by the parent directory), non-filesystem 
devices, and access policy implemented by filter drivers in the device stack. 

The code to open the file/directory can be factored out and generalized from 
the stat() implementation. The common open function can implement the flags 
AT_SYMLINK_NOFOLLOW and AT_EACCESS (without which it should temporarily revert 
to the process access token). Also, when a directory is opened with 
GENERIC_WRITE access, it can re-try the open with FILE_DELETE_CHILD access, 
which POSIX includes in write access for a directory.

An S_OK flag could also be supported to ignore a sharing violation in Windows. 
[MS-FSA] section 2.1.5.1.2 (Open of an Existing File) specifies that access 
sharing is checked after the readonly attribute and file security access check. 
So if an open fails with a sharing violation, the caller knows that access was 
otherwise granted.

--
versions: +Python 3.10, Python 3.8, Python 3.9 -Python 3.5

___
Python tracker 

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

[issue2528] Change os.access to check ACLs under Windows

[Paul Moore]

Changes by Paul Moore p.f.mo...@gmail.com:


--
nosy: +paul.moore

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

[issue2528] Change os.access to check ACLs under Windows

[eryksun]

eryksun added the comment:

In msg243815 you asked me to look over this patch. I hope this helps.

For GetFileSecurity you need to also request LABEL_SECURITY_INFORMATION. To 
test this in Vista+, use a file in the root directory of the system drive. This 
will inherit a high integrity level w/ no write up, which denies write access 
to a medium integrity (non-elevated) process. If you don't include the label 
information, the check will erroneously claim a non-elevated token has write 
access. 

You can set a file's integrity level to high using `icacls filename 
/setintegritylevel High`. This prevents write up. You can prevent read up and 
execute up by using the Windows API directly, or with other administration 
tools such as chml.

As a performance tweak you could get the file security with an initial buffer 
of 512 bytes. That should be big enough for most file security descriptors. I'd 
use PyMem_RawRealloc in a do loop (i.e. starting with pSD == NULL and cbSD == 
512). It should set PyExc_MemoryError on failure.

Regarding ImpersonateSelf, what if the thread is already impersonating? That's 
the token the kernel will actually use when checking access. Given that, I'd 
try OpenThreadToken, and only if it fails call ImpersonateSelf. Then 
RevertToSelf immediately after the 2nd OpenThreadToken call. Reverting doesn't 
have to be delayed until after the check. An alternative to ImpersonateSelf is 
to manually duplicate the process token as an impersonation token (i.e. 
OpenProcessToken; DuplicateToken). The benefit of this approach is that the 
duplicated token can be cached in a static variable.

For the access check itself, use the FILE_GENERIC_* / FILE_ALL_ACCESS 
constants, which are the standard and specific rights for the corresponding 
GENERIC_* rights. These include the standard READ_CONTROL and SYNCHRONIZE 
rights that are required for accessing files.

Also, there's no need to call MapGenericMask here, since you know that 
access_desired doesn't contain any generic rights.

Finally, for backward compatibility this new implementation should default to 
using only the old file-attribute check. Chaining to the ACL-based check should 
require a keyword-only argument such as use_acl.

--
nosy: +eryksun
versions: +Python 3.5 -Python 3.4

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

[issue2528] Change os.access to check ACLs under Windows

[Tim Golden]

Tim Golden added the comment:

Thanks for the very thorough review. This isn't going to make it into 
3.5, but I'll rework it in the light of your comments and see if people 
are happy with it in the optional argument variation.

--

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

[issue2528] Change os.access to check ACLs under Windows

[Mark Lawrence]

Mark Lawrence added the comment:

The solution proposed here could help resolve #22107.

--
nosy: +BreamoreBoy

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

[issue2528] Change os.access to check ACLs under Windows

[Serhiy Storchaka]

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


--
nosy: +serhiy.storchaka

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

[issue2528] Change os.access to check ACLs under Windows

[Mark Lawrence]

Changes by Mark Lawrence breamore...@yahoo.co.uk:


--
nosy:  -BreamoreBoy

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

[issue2528] Change os.access to check ACLs under Windows

[Tim Golden]

Tim Golden added the comment:

Here's an updated patch against trunk with tests  doc changes

--
status: languishing - open
Added file: http://bugs.python.org/file31165/issue2528.2.patch

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

[issue2528] Change os.access to check ACLs under Windows

[Tim Golden]

Tim Golden added the comment:

... and to answer Amaury's question in msg109871 it creates a reasonable 
consistency between the results of os.access and the user's actual ability to 
read / write a file. eg, you might have no permissions whatsoever on the file 
but as long as it wasn't read-only, os.access would return True for reading, 
writing and executing.

--

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

[issue2528] Change os.access to check ACLs under Windows

[Tim Golden]

Changes by Tim Golden m...@timgolden.me.uk:


Removed file: http://bugs.python.org/file9919/os_access-r62091.patch

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

[issue2528] Change os.access to check ACLs under Windows

[Christian Heimes]

Christian Heimes added the comment:

Do you want to provide an updated patch for 3.4?

--
nosy: +christian.heimes
status: open - languishing
versions: +Python 3.4 -Python 3.2

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

[issue2528] Change os.access to check ACLs under Windows

[Tim Golden]

Changes by Tim Golden m...@timgolden.me.uk:


--
assignee:  - tim.golden

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

[issue2528] Change os.access to check ACLs under Windows

[Mark Lawrence]

Mark Lawrence breamore...@yahoo.co.uk added the comment:

Tim, do you want more time to think about this, could we close as won't fix 
or what?

--

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

[issue2528] Change os.access to check ACLs under Windows

[Mark Lawrence]

Mark Lawrence breamore...@yahoo.co.uk added the comment:

A quick look tells me that the patch seems clean.  However it involves changes 
to posixmodule.c and I don't (yet) want to get involved with doing builds, so 
could someone else please give this a try.

--
nosy: +BreamoreBoy
versions:  -Python 2.7

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

[issue2528] Change os.access to check ACLs under Windows

[Amaury Forgeot d'Arc]

Amaury Forgeot d'Arc amaur...@gmail.com added the comment:

But what are the benefits of this change?

--
nosy: +amaury.forgeotdarc

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

[issue2528] Change os.access to check ACLs under Windows

[Tim Golden]

Tim Golden m...@timgolden.me.uk added the comment:

Although I'm the implementer of the patch (the concept
was discussed way back on c.l.py after a naive poster's
original request) I'm probably +0 myself. It's an attempt
to replace os.access' next-to-useless behaviour on Windows
with something which at least uses current security mechanisms
to determine its answer.

However, it's just one function among the many Posixesque
functions which don't quite map to Windows. And one which
you might well be advised to avoid in any case since there's
an race condition inherent in checking for a file's accessibility
and then making use of that fact.  Better to try and fall back.

--

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

[issue2528] Change os.access to check ACLs under Windows

[Daniel Diniz]

Changes by Daniel Diniz aja...@gmail.com:


--
components: +Windows
priority:  - normal
stage:  - patch review
type:  - feature request
versions: +Python 2.7, Python 3.2 -Python 2.6

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

[issue2528] Change os.access to check ACLs under Windows

[Tim Golden]

New submission from Tim Golden [EMAIL PROTECTED]:

At present, os.access under Windows simply calls GetFileAttributes to
determine the readonly attribute (ignoring directories). The patch
attached combines this with the use of the AccessCheck API to compare
the user's permissions with those required for the path.

I'm assuming that ATTRIB and CACLS will be available for use in the unit
tests included.

I haven't altered the structure of the posix_access function at all
although I suspect that it could now be simplified now that we're not
supporting Win9x.

--
components: Library (Lib)
files: os_access-r62091.patch
keywords: patch
messages: 64811
nosy: tim.golden
severity: normal
status: open
title: Change os.access to check ACLs under Windows
versions: Python 2.6
Added file: http://bugs.python.org/file9919/os_access-r62091.patch

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2528
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com