[issue28896] Embeddable zip allows Windows registry to override module location

[Donald Stufft]

Changes by Donald Stufft :


--
pull_requests: +1033

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Ned Deily]

Ned Deily added the comment:

[cherrypicked for 3.6.0rc2]

--
priority: release blocker -> 

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Roundup Robot]

Roundup Robot added the comment:

New changeset 6249350e654a by Steve Dower in branch '3.6':
Issue #28896: Deprecate WindowsRegistryFinder
https://hg.python.org/cpython/rev/6249350e654a

--

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Steve Dower]

Steve Dower added the comment:

And that commit removes WindowsRegistryFinder from sys.meta_path on startup (as 
well as fixing regeneration of importlib when building on Windows).

It should *not* be cherry picked for 3.6.0.

--
resolution:  -> fixed
stage: commit review -> resolved
status: open -> closed

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Roundup Robot]

Roundup Robot added the comment:

New changeset 5bd248c2cc75 by Steve Dower in branch '3.6':
Issue #28896: Disable WindowsRegistryFinder by default.
https://hg.python.org/cpython/rev/5bd248c2cc75

New changeset 4bd131b028ce by Steve Dower in branch 'default':
Issue #28896: Disable WindowsRegistryFinder by default.
https://hg.python.org/cpython/rev/4bd131b028ce

--

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Alexey Izbyshev]

Alexey Izbyshev added the comment:

Thanks to Steve and everyone for quick and decisive action!

--

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Steve Dower]

Steve Dower added the comment:

I assumed silence meant everyone was happy with the wording, so I extended it 
to whatsnew and NEWS and pushed.

Ned - the changeset above should be good for you to cherrypick.

I'll leave this issue open to cover actually removing the finder from 
sys.meta_path in 3.7 and potentially 3.6.1.

--
assignee:  -> steve.dower
priority: normal -> release blocker
stage:  -> commit review
versions:  -Python 3.5

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Roundup Robot]

Roundup Robot added the comment:

New changeset 25df9671663b by Steve Dower in branch '3.6':
Issue #28896: Deprecate WindowsRegistryFinder
https://hg.python.org/cpython/rev/25df9671663b

New changeset 5376b3a168c8 by Steve Dower in branch 'default':
Issue #28896: Deprecate WindowsRegistryFinder
https://hg.python.org/cpython/rev/5376b3a168c8

--
nosy: +python-dev

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Steve Dower]

Changes by Steve Dower :


--
keywords: +patch
Added file: http://bugs.python.org/file45794/28896_doc.patch

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Steve Dower]

Steve Dower added the comment:

Here's my proposed doc change for 3.6.0. Any concerns about wording? (The 
change to remove the line from _bootstrap_external.py will be separate, for 
ease of cherry-picking.)


diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst
--- a/Doc/library/importlib.rst
+++ b/Doc/library/importlib.rst
@@ -806,6 +806,10 @@
 
.. versionadded:: 3.3
 
+   .. deprecated:: 3.6
+  Use :mod:`site` configuration instead. Future versions of Python may
+  not enable this finder by default.
+
 
 .. class:: PathFinder
 
diff --git a/Doc/using/windows.rst b/Doc/using/windows.rst
--- a/Doc/using/windows.rst
+++ b/Doc/using/windows.rst
@@ -823,6 +823,14 @@
   * Adds ``pythonXX.zip`` as a potential landmark when directly adjacent
 to the executable.
 
+.. deprecated::
+   3.6
+
+  Modules specified in the registry under ``Modules`` (not ``PythonPath``)
+  may be imported by :class:`importlib.machinery.WindowsRegistryFinder`.
+  This finder is enabled on Windows in 3.6.0 and earlier, but may need to
+  be explicitly added to :attr:`sys.meta_path` in the future.
+
 Additional modules
 ==

--

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Ned Deily]

Ned Deily added the comment:

I'm OK with adding a doc change before 3.6.0 final.  But since this behavior is 
not new with 3.6, I would rather save any code changes for 3.6.1 unless there 
is a consensus that this is an urgent security issue.

--

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Steve Dower]

Steve Dower added the comment:

+Ned

Could we get a doc patch into 3.6 marking this class as deprecated? It appears 
like the importlib docs are the only ones that refer to the class, and none of 
the docs describe the functionality or indicate that it is enabled by default.

I could also pitch this as a security vulnerability and push for removing the 
default .append() right now? Since we wouldn't remove the class itself, 
restoring the previous behavior just requires inserting it into meta_path 
again. And Alexey is right that it actually allows a non-admin user to shadow 
any non-builtin module.

Looking at the latest pywin32 installer, they actually *remove* the keys they 
used to add here because they cause problems. So I think we're fairly safe to 
disable the finder by default and deprecate it into the future.

--
nosy: +ned.deily

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Brett Cannon]

Brett Cannon added the comment:

Deprecate the importer. If I remember correctly it took us a while to even 
notice it was missing due to missing tests prior to importlib coming into 
existence (and getting anyone to care enough to help write those tests also 
took a lot of effort).

--

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Paul Moore]

Paul Moore added the comment:

I thought that most of the users of this functionality had stopped doing so 
(the only one I recall for certain was pywin32, and last time this came up, I 
think someone said they had stopped).

If the functionality isn't used in any of the well-known modules, I'm +1 on 
deprecating it (if it is, I'm all for encouraging them to stop and then *still* 
deprecating it :-))

Use of the registry keys would prevent virtual environments from managing 
proper isolation, as well, so it's not just an issue for the embedded 
distribution.

--

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Steve Dower]

Steve Dower added the comment:

It's not intentional, but we clearly haven't done anything to prevent it.

Arguably this finder should be omitted when you run in isolated mode, and I'm 
on the fence about deprecating it entirely. Adding the importlib experts in 
case they have opinions (relevant ones, ideally).

A one line workaround that can be added to any code base is:

>>> sys.meta_path[:] = [m for m in sys.meta_path if m.__name__ != 
>>> 'WindowsRegistryFinder']

But it would also be good to close off this hole. Thoughts on the best option? 
(-I, move to site.py and -S, something new...)

--
nosy: +brett.cannon, eric.snow, ncoghlan
versions: +Python 3.6, Python 3.7

___
Python tracker 

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

[issue28896] Embeddable zip allows Windows registry to override module location

[Alexey Izbyshev]

New submission from Alexey Izbyshev:

The docs claim: "... the embedded distribution is (almost) fully isolated from 
the user’s system, including environment variables, system registry settings, 
and installed packages."

Via ProcessMonitor tool I've discovered that python.exe still accesses keys 
like "HKLM\Software\Python\PythonCore\3.5\Modules\collections" on every module 
import, allowing registry settings to override the location of any non-builtin 
module.

Digging into the 3.5.2 code revealed that WindowsRegistryFinder is 
unconditionally added to sys.meta_path (Lib/importlib/_bootstrap_external.py, 
line 1422):

if _os.__name__ == 'nt':
sys.meta_path.append(WindowsRegistryFinder)

It can also be confirmed in runtime:

Python 3.5.2 (v3.5.2:4def2a2901a5, Jun 25 2016, 22:18:55) [MSC v.1900 64 bit 
(AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> print(sys.meta_path)
[, , , ]

Is this behavior intended? It seems to be against doc claims and the goal of 
embeddability.

--
components: Windows
messages: 282632
nosy: izbyshev, paul.moore, steve.dower, tim.golden, zach.ware
priority: normal
severity: normal
status: open
title: Embeddable zip allows Windows registry to override module location
type: behavior
versions: Python 3.5

___
Python tracker 

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