mgr:python: avoid pyo3 errors by running certain cryptographic functions in a child process #62951

phlogistonjohn · 2025-04-24T14:17:34Z

Based on #61737
Fixes (workaround): https://tracker.ceph.com/issues/64213

This PR takes some of the work done by @pecastro along with suggestions by @epuertat and combines them into a series of patches that create two implementations with a common interface (via ABCs) for calling basic cryptographic and TSL/SSL related functionality. This interface is named CryptoCaller.

The new 'remote' implementation uses subprocesses to execute the cryptography functions outside of the mgr. This avoids conflicts between the pyo3 using libs (specifically cryptography) across sub-interpreters. The 'internal' implementation uses the library calls directly but does so through a class based interface. A helper module allows mgr code to choose or get the currently enabled (per sub-interpreter) crypto caller implementation.

The mgr utils are updated to use this interface. The cephadm module uses the internal implemenation. The dashboard defaults to the remote implementation but can be configured.

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- Updates tests

Show available Jenkins commands

jenkins test classic perf Jenkins Job | Jenkins Job Definition
jenkins test crimson perf Jenkins Job | Jenkins Job Definition
jenkins test signed Jenkins Job | Jenkins Job Definition
jenkins test make check Jenkins Job | Jenkins Job Definition
jenkins test make check arm64 Jenkins Job | Jenkins Job Definition
jenkins test submodules Jenkins Job | Jenkins Job Definition
jenkins test dashboard Jenkins Job | Jenkins Job Definition
jenkins test dashboard cephadm Jenkins Job | Jenkins Job Definition
jenkins test api Jenkins Job | Jenkins Job Definition
jenkins test docs ReadTheDocs | Github Workflow Definition
jenkins test ceph-volume all Jenkins Jobs | Jenkins Jobs Definition
jenkins test windows Jenkins Job | Jenkins Job Definition
jenkins test rook e2e Jenkins Job | Jenkins Job Definition

phlogistonjohn · 2025-04-30T16:46:53Z

https://pulpito.ceph.com/adking-2025-04-29_23:34:48-orch:cephadm-jjm-wip-pycry-remoted-distro-default-smithi/
This test run with this PR applied + a "tainted" container image ran many tests successfully.

phlogistonjohn · 2025-04-30T16:56:27Z

I'm ready for initial reviews: PTAL @epuertat @pecastro @adk3798

Feel free to ignore the experimental module shield commit for now, I will remove it when I take this out of draft, ideally early next week.

pecastro · 2025-04-30T22:39:48Z

LGTM.

I just have a question around the code duplication, I can see it defaulting to the remote option, which is the forked subprocess. Great.
How are you planning on exercising the internal branch to make sure that other piece of code doesn't bitrot ?

phlogistonjohn · 2025-05-01T14:02:37Z

LGTM.

I just have a question around the code duplication, I can see it defaulting to the remote option, which is the forked subprocess. Great. How are you planning on exercising the internal branch to make sure that other piece of code doesn't bitrot ?

Good Q: The remote default impacts the dashboard. The cephadm module defaults to using internal both modules make use of a number of the crypto caller functions. Functional testing (teuthology) that uses both modules should get us fair coverage of both options.

The unit testing story is a bit more complicated with the extremely heavy use of mocks that mgr module units tests use. In that case I think it's good enough to let all code use the default as that's the more complex path. That said, we could probably add a set of dedicated "unit" tests to the new dir for focused testing on the new interface and backend implementations. I will look into that - but perhaps in a follow up PR.

phlogistonjohn · 2025-05-01T14:05:11Z

One thing I meant to note in my previous comment: when I mention that the remote path is the complex one be aware that the remote crypto caller eventually calls the internal crypto caller - so every time the remote caller is used the internal caller gets used too (in the child process). It's just indirect.

phlogistonjohn · 2025-05-02T17:26:31Z

jenkins test make check arm64

phlogistonjohn · 2025-05-05T15:40:13Z

@epuertat - I am ready to take this out of draft but I'd like a general "thumbs up/down" from you before I do so. I don't need a full review at the moment just a quick check on the overall approach.

phlogistonjohn · 2025-05-06T13:16:03Z

Teuthology run results - default base OS packages:
https://pulpito.ceph.com/adking-2025-05-06_04:21:07-orch:cephadm-wip-adk2-testing-2025-05-05-1337-distro-default-smithi/

epuertat · 2025-05-06T19:50:28Z

@epuertat - I am ready to take this out of draft but I'd like a general "thumbs up/down" from you before I do so. I don't need a full review at the moment just a quick check on the overall approach.

Looks good to me, @phlogistonjohn !

When I saw the "remote" term over there, I remembered that someone (@batrick) suggested to have these crypto methods in a new helper-only mgr module, and use the mgr.remote() method to invoke them. It'd be a middle ground between the sub-interpreter and the stand-alone interpreter approaches.

batrick

When I saw the "remote" term over there, I remembered that someone (@batrick) suggested to have these crypto methods in a new helper-only mgr module, and use the mgr.remote() method to invoke them. It'd be a middle ground between the sub-interpreter and the stand-alone interpreter approaches.

Thanks for the ping @epuertat . Note I don't think we typically fork in Ceph processes so this may run afoul of restrictive container settings like NPROC=1. Something to be on the lookout for. A pending release note would be appropriate.

batrick · 2025-05-06T21:25:50Z

src/python-common/ceph/cryptotools/remote.py

+# sharing the same protocol.
+# For instance we could have a persistent process listening on a unix
+# socket accepting the crypto functions as RPCs. For now, we keep it
+# simple though :-)


batrick · 2025-05-06T21:30:35Z

src/python-common/ceph/cryptotools/select.py

+        name = os.environ.get(_CC_ENV, '')
+        _check_name(name)
+    if not name:
+        name = _CC_REMOTE


Is it not possible to attempt an internal import and fallback to remote?

In any case, debug messages for these branches would be a good idea.

Debug sounds good. Unfortunately, an attempt to import the module in the "wrong" subintepreter that succeeds will pollute the system. IOW, if we import the dashboard and it probes the cryptography module and it succeeds it will then break a subsequent import of cephadm. So it's not really "safe" to try to import it proactively.

batrick · 2025-05-06T21:32:36Z

src/python-common/ceph/cryptotools/cryptotools.py

+
+
+def main() -> None:
+    # create the top-level parser


I wonder if there is a generic mechanism for this. It has to be somewhat common.

I'm not sure what you are referring to here, setting up an argument parser?

I mean a python module that is providing RPCs over a socket / pipe.

phlogistonjohn · 2025-05-06T23:54:32Z

The idea of moving the use of cryptography into a central mgr module was discussed in a ceph orch weekly. One of the issues we discussed was that cephadm "needs cryptography the most" so that it can use asyncssh. Thus we'd probably need to do it in a less clean way with cephadm handling the crypto calls - rather than a nice central crypto module that does one thing assisting the other modules.

Of course, all of this is a hopefully temporary workaround once the pyo3 team does something on their side. I kept thinking that with python 3.13 they were going to expose subintepreters to the api but I guess that never went all the way (the module exists in 3.13 but is _interpreters so not totally official). So my hope that would motivate the pyo3 devs is probably irrelevant now.

Signed-off-by: John Mulligan <jmulligan@redhat.com>

Add a module to select a desired crypto caller. Update the callers to use the crypto caller interface. Signed-off-by: John Mulligan <jmulligan@redhat.com>

Previously, the internal crypto caller would catch (and convert) some errors when reading the cert but not all cases. Move the logic to catch the errors to a common location and do it once consistently. Signed-off-by: John Mulligan <jmulligan@redhat.com>

The cephadm modules needs to use python cryptography module for ssh (via asyncssh) and thus there's no need to use the remote crypto caller in cephadm. Configure cephadm to always use the internal cryptocaller. Signed-off-by: John Mulligan <jmulligan@redhat.com>

Add a mgr config option `crypto_caller` that lets a ceph user override the default behavior of using the remote crypto caller. Supported values are `internal` and `remote`. Signed-off-by: John Mulligan <jmulligan@redhat.com>

adk3798 · 2025-07-17T12:31:10Z

https://pulpito.ceph.com/adking-2025-07-16_15:43:08-orch:cephadm-wip-adk2-testing-2025-07-10-1207-distro-default-smithi/

no special failures I can see related to pyo3 stuff

These patches were taken from PR #62951 [0] and backported onto upstream tag "19.2.2" (0eceb0defba). This provides a workaround for the PyO3 ImportError regarding sub-interpreters for the Ceph Dashboard. [0]: ceph/ceph#62951 Signed-off-by: Max R. Carrara <m.carrara@proxmox.com> Link: https://lore.proxmox.com/20250715093237.650039-2-m.carrara@proxmox.com

These patches were taken from PR #62951 [0] and backported onto upstream tag "19.2.2" (0eceb0defba). This provides a workaround for the PyO3 ImportError regarding sub-interpreters for the Ceph Dashboard. [0]: ceph/ceph#62951 Signed-off-by: Max R. Carrara <m.carrara@proxmox.com> Link: https://lore.proxmox.com/20250715093237.650039-2-m.carrara@proxmox.com Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

By moving the self-signed cert generation into a separate module and consequently running it via `subprocess.run`--so, as a new, independent Python process--the PyO3 ImportError is successfully avoided. Inspired by an upstream PR [0]. [0]: ceph/ceph#62951 Signed-off-by: Max R. Carrara <m.carrara@proxmox.com> Link: https://lore.proxmox.com/20250716173956.980112-2-m.carrara@proxmox.com

nizamial09 · 2025-08-25T05:12:38Z

@phlogistonjohn shouldn't we backport this to tentacle as well?

phlogistonjohn · 2025-08-25T13:55:03Z

This has been in main for over a month and I haven't heard of any regressions related to these changes, so sure we can probably backport it to tentacle.

Move the self-signed cert generation into a separate module inside python-common/ceph and run the module in a separate Python process. This provides a workaround for the ImportError thrown by PyO3 when the `restful` module is loaded in the context of multiple Python sub-interpreters being present. In particular, the ImportError is thrown by the `crypto` module of the `OpenSSL` package. Inspired by an upstream PR [0]. [0]: ceph#62951 Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>

rsommer · 2025-09-30T11:09:20Z

Will this be backported to reef or at least squid, as these versions are affected as well? It is currently blocking updates to reef/squid on Debian-based systems.

bazaah · 2025-10-02T23:28:44Z

I'm playing around with a cherry-picked squash of this branch for v19/Archlinux, the merge conflicts weren't so tricky, but I want to test it out a bit. I can then submit a proper backport for v19 if there's interest from the Ceph side.

@phlogistonjohn

This is a backport of ceph/ceph#62951, rebased onto v19.2.3. All credit for this work goes to @phlogistonjohn. Upstream-ref: ceph/ceph@7094a5a References: ceph/ceph#62951

qiuxinyidian · 2025-11-24T09:32:15Z

@phlogistonjohn hi, should pybind/mgr/cephadm/ssl_cert_utils.py use this interface ?

github-actions bot added cephadm dashboard mgr nfs orchestrator pybind rook labels Apr 24, 2025

github-project-automation bot added this to Ceph-Dashboard Apr 24, 2025

github-project-automation bot moved this to New in Ceph-Dashboard Apr 24, 2025

phlogistonjohn mentioned this pull request Apr 24, 2025

pybind/mgr: Hack around the 'ImportError: PyO3 modules may only be in… #61737

Closed

14 tasks

phlogistonjohn force-pushed the jjm-pe-crypto-plus branch 3 times, most recently from a2721b3 to 008d669 Compare April 29, 2025 18:05

phlogistonjohn changed the title ~~[WIP] mgr:python: avoid pyo3 errors by running certain crypgraphic functions in a child process~~ mgr:python: avoid pyo3 errors by running certain crypgraphic functions in a child process Apr 30, 2025

adk3798 added the wip-adk2-testing label May 1, 2025

phlogistonjohn force-pushed the jjm-pe-crypto-plus branch from 008d669 to 13e31c9 Compare May 1, 2025 18:28

batrick reviewed May 6, 2025

View reviewed changes

phlogistonjohn marked this pull request as ready for review May 8, 2025 17:34

phlogistonjohn requested review from a team as code owners May 8, 2025 17:34

phlogistonjohn added 2 commits July 7, 2025 09:32

python-common/cryptotools: add caller module for base class

c3dc34a

Signed-off-by: John Mulligan <jmulligan@redhat.com>

python-common/cryptotools: move internal crypto caller to new file

0c774d5

Signed-off-by: John Mulligan <jmulligan@redhat.com>

phlogistonjohn force-pushed the jjm-pe-crypto-plus branch from 0c4b10c to b1903da Compare July 7, 2025 13:32

phlogistonjohn added 4 commits July 7, 2025 09:34

python-common/cryptotools: create module for selecting crypto caller

0eb2f4b

Add a module to select a desired crypto caller. Update the callers to use the crypto caller interface. Signed-off-by: John Mulligan <jmulligan@redhat.com>

phlogistonjohn force-pushed the jjm-pe-crypto-plus branch from b1903da to 27c2050 Compare July 7, 2025 13:34

phlogistonjohn changed the title ~~mgr:python: avoid pyo3 errors by running certain crypgraphic functions in a child process~~ mgr:python: avoid pyo3 errors by running certain cryptographic functions in a child process Jul 14, 2025

adk3798 approved these changes Jul 17, 2025

View reviewed changes

adk3798 merged commit 7094a5a into ceph:main Jul 17, 2025
18 of 19 checks passed

github-project-automation bot moved this from Reviewer approved to Done in Ceph-Dashboard Jul 17, 2025

bazaah mentioned this pull request Jul 17, 2025

ceph-mgr/dashboard: python-cryptography PyO3 modules may only be initialized once per interpreter process bazaah/aur-ceph#20

Open

phlogistonjohn deleted the jjm-pe-crypto-plus branch July 23, 2025 21:04

sabaini mentioned this pull request Jul 28, 2025

Ceph Dashboard Certs Dependency Problem canonical/microceph#587

Closed

bazaah mentioned this pull request Sep 16, 2025

prepare for Ceph v20 bazaah/aur-ceph#33

Open

nh2 mentioned this pull request Sep 30, 2025

Build ceph with Python 3.12; move to pkgs/by-name NixOS/nixpkgs#443671

Open

Aequitosh mentioned this pull request Oct 13, 2025

Tracking Issue: Sub-Interpreter Support PyO3/pyo3#3451

Open

5 tasks

numinit mentioned this pull request Nov 29, 2025

ceph: Build with Python 3.12 NixOS/nixpkgs#455889

Open

13 tasks

mgr:python: avoid pyo3 errors by running certain cryptographic functions in a child process #62951

mgr:python: avoid pyo3 errors by running certain cryptographic functions in a child process #62951

Uh oh!

Conversation

phlogistonjohn commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

phlogistonjohn commented Apr 30, 2025

Uh oh!

phlogistonjohn commented Apr 30, 2025

Uh oh!

pecastro commented Apr 30, 2025

Uh oh!

phlogistonjohn commented May 1, 2025

Uh oh!

phlogistonjohn commented May 1, 2025

Uh oh!

phlogistonjohn commented May 2, 2025

Uh oh!

phlogistonjohn commented May 5, 2025

Uh oh!

phlogistonjohn commented May 6, 2025

Uh oh!

epuertat commented May 6, 2025

Uh oh!

batrick left a comment

Choose a reason for hiding this comment

Uh oh!

batrick May 6, 2025

Choose a reason for hiding this comment

Uh oh!

batrick May 6, 2025

Choose a reason for hiding this comment

Uh oh!

batrick May 6, 2025

Choose a reason for hiding this comment

Uh oh!

phlogistonjohn May 6, 2025

Choose a reason for hiding this comment

Uh oh!

batrick May 6, 2025

Choose a reason for hiding this comment

Uh oh!

phlogistonjohn May 6, 2025

Choose a reason for hiding this comment

Uh oh!

batrick May 8, 2025

Choose a reason for hiding this comment

Uh oh!

phlogistonjohn commented May 6, 2025

Uh oh!

adk3798 commented Jul 17, 2025

Uh oh!

Uh oh!

nizamial09 commented Aug 25, 2025

Uh oh!

phlogistonjohn commented Aug 25, 2025

Uh oh!

rsommer commented Sep 30, 2025

Uh oh!

bazaah commented Oct 2, 2025

Uh oh!

qiuxinyidian commented Nov 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

phlogistonjohn commented Apr 24, 2025 •

edited

Loading