Skip to content

python: use PyConfig_InitIsolatedConfig to initialize the embedded Python#5515

Closed
philiptaron wants to merge 1 commit intofontforge:masterfrom
philiptaron:PyConfig_InitIsolatedConfig
Closed

python: use PyConfig_InitIsolatedConfig to initialize the embedded Python#5515
philiptaron wants to merge 1 commit intofontforge:masterfrom
philiptaron:PyConfig_InitIsolatedConfig

Conversation

@philiptaron
Copy link
Copy Markdown

Motivation for change

Reduce surprising and security-sensitive loading of Python modules based on the current working directory where the fontforge program is run. After this change, the Python scripts which are explicitly loaded based on fontforge's command line flags, plus hard-coded ones like pkg_resources, are the only ones used.

This is a breaking change!

From the docs:

PyConfig_InitIsolatedConfig() creates a configuration to isolate Python
from the system. This is ideal to embed Python into an application.

This configuration ignores global configuration variables, environment
variables, command line arguments (PyConfig.argv is not parsed) and
user site directory. The C standard streams (ex: stdout) and the
LC_CTYPE locale are left unchanged. Signal handlers are not installed.

Configuration files are still used with this configuration to determine
paths that are unspecified. Ensure PyConfig.home is specified to avoid
computing the default path configuration.

This will change the default behavior of Python in FontForge from loading whatever is in the current working directory to loading what's installed as part of the system Python or the venv Python, which ever gets loaded as the shared library.

Breaking changes

…thon

From the [docs](https://docs.python.org/3/c-api/init_config.html#init-isolated-conf):

> `PyConfig_InitIsolatedConfig()` creates a configuration to isolate Python
> from the system. This is ideal to embed Python into an application.
>
> This configuration ignores global configuration variables, environment
> variables, command line arguments (`PyConfig.argv` is not parsed) and
> user site directory. The C standard streams (ex: stdout) and the
> `LC_CTYPE` locale are left unchanged. Signal handlers are not installed.
>
> Configuration files are still used with this configuration to determine
> paths that are unspecified. Ensure `PyConfig.home` is specified to avoid
> computing the default path configuration.

This will change the default behavior of Python in FontForge from loading
whatever is in the current working directory to loading what's installed
as part of the system Python or the venv Python, which ever gets loaded
as the shared library.
@philiptaron
Copy link
Copy Markdown
Author

Some of the tests failed with Python module errors: 👀

The following tests FAILED:
	 58 - test1001_py (Failed)
	 64 - test1001c_py (Failed)
	 68 - test1003_py (Failed)
	104 - test926_py (Failed)
	116 - test932_py (Failed)
	122 - test_fea_hyphens_py (Failed)
	124 - test_sfd_suppl_plane_py (Failed)
	126 - test_fea_context_sub_py (Failed)

@skef
Copy link
Copy Markdown
Contributor

skef commented Dec 31, 2024

It seems like this raises a lot of potential issues and would require some communication at a minimum. The main benefit would be a slightly decreased attack surface but FontForge is already incredibly insecure.

Our guideline about untrusted input isn't directly related but I'm tempted to refer to it anyway: https://github.com/fontforge/fontforge/wiki/Community-guidelines#D1

@philiptaron
Copy link
Copy Markdown
Author

Our guideline about untrusted input isn't directly related but I'm tempted to refer to it anyway: https://github.com/fontforge/fontforge/wiki/Community-guidelines#D1

Actually, that was an incredibly helpful reference. Plus the set of failing tests, it made me realize the impact here for making this the default is... not worth the squeeze.

For nixpkgs's packaging of FontForge, I might pursue this approach, since it makes much more sense in that context. In NixOS/nixpkgs, there is an assumption about hermeticity that's not present in the broader Python ecosystem. There are also simple ways of constructing a Python "system" environment with a crafted set of packages just for FontForge. None of those hold true in the general case.

Given all that, I'm closing this PR. Thank you for your time and attention.

@philiptaron philiptaron deleted the PyConfig_InitIsolatedConfig branch January 2, 2025 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants