PoC: Make it possible to write code that interacts with OpenSSL in Rust #6695

messense · 2021-12-12T15:25:44Z

Implements #6634

TODO:

Bump MSRV to 1.48.0 for the openssl crate.
Fix tests on PyPy
Deal with Py_LIMITED_API

messense · 2021-12-12T15:28:02Z

src/rust/build.rs

+        if line.starts_with("cargo:") {
+            println!("{}", line);
+        } else if line.starts_with("include:") {
+            include = line.replace("include:", "");


I'm thinking the PYO3_PYTHON above and this include from sysconfig should be obtained from pyo3-build-config crate.

This sounds good to me!

src/_cffi_src/build_openssl.py

src/rust/.cargo/config.toml

src/rust/build.rs

src/rust/src/lib.rs

messense · 2021-12-12T16:22:19Z

running: "cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-Werror=implicit-function-declaration" "-I/home/runner/work/cryptography/cryptography/osslcache/openssl-1.1.0l-1c9f97a0fce5ec7feed4e4c4944a9d9cca01bcfc/include" "-I" "" "-I" "/usr/include" "-o" "/home/runner/work/cryptography/cryptography/src/rust/target/release/build/cryptography-rust-c7d21142bca98d61/out/_openssl.o" "-c" "/home/runner/work/cryptography/cryptography/src/rust/target/release/build/cryptography-rust-c7d21142bca98d61/out/_openssl.c"
cargo:warning=cc: error: /home/runner/work/cryptography/cryptography/src/rust/target/release/build/cryptography-rust-c7d21142bca98d61/out/_openssl.c: No such file or directory
cargo:warning=cc: fatal error: no input files

I'm having trouble understand why this fails with tox. Does tox have a cli option to ask it not to delete everything after failure?

alex · 2021-12-12T16:24:44Z

Hmm, I don't think tox should be deleting anything, all the files should still be in .tox/<env name>/. Is it possibly some other file getting deleted?

messense · 2021-12-12T16:26:15Z

Here is the same error on my local machine, looks like tox put files a in temp dir.

running: "cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-arch" "arm64" "-I" "" "-I" "/opt/homebrew/opt/openssl@1.1/include" "-Wall" "-Wextra" "-o" "/private/var/folders/0r/4fqyt_zs2tj4zvglsb7f4zjw0000gn/T/pip-req-build-9o4tjl27/src/rust/target/release/build/cryptography-rust-c81c254f7cb1dd51/out/_openssl.o" "-c" "/private/var/folders/0r/4fqyt_zs2tj4zvglsb7f4zjw0000gn/T/pip-req-build-9o4tjl27/src/rust/target/release/build/cryptography-rust-c81c254f7cb1dd51/out/_openssl.c"
cargo:warning=clang: error: no such file or directory: '/private/var/folders/0r/4fqyt_zs2tj4zvglsb7f4zjw0000gn/T/pip-req-build-9o4tjl27/src/rust/target/release/build/cryptography-rust-c81c254f7cb1dd51/out/_openssl.c'
cargo:warning=clang: error: no input files

alex · 2021-12-12T16:27:37Z

Oh no :-/ this isn't tox itself, it's pip's pep517 support. I think you want --no-clean on pip, but I'm not sure how to pass it down :-/

messense · 2021-12-12T16:29:02Z

I also tried python -m build and it works for me. Not sure why tox doesn't like it.

alex · 2021-12-12T16:40:03Z

I'm very confused by what's happening with tox, if I put 1 / 0 in build_openssl.py it still doesn't seem to fail.

alex · 2021-12-12T16:44:53Z

Ok, this might be because we need println!("cargo:rerun-if-changed=../_cffi_src/build_openssl.py");. Still poking though.

alex · 2021-12-12T17:08:25Z

Ok, this fixes it, it's because of PYTHONPATH:

diff --git a/src/rust/build.rs b/src/rust/build.rs
index 60d28131..0dfc2534 100644
--- a/src/rust/build.rs
+++ b/src/rust/build.rs
@@ -6,12 +6,24 @@ fn main() {
     let out_dir = env::var("OUT_DIR").unwrap();
     // FIXME: maybe pyo3-build-config should provide a way to do this?
     let python = env::var("PYO3_PYTHON").unwrap_or("python3".to_string());
+    println!("cargo:rerun-if-changed=../_cffi_src/");
+    let python_path = match env::var("PYTHONPATH") {
+        Ok(mut val) => {
+            val.push_str(":../");
+            val
+        },
+        Err(_) => {
+            "../".to_string()
+        }
+    };
     let output = Command::new(&python)
-        .env("PYTHONPATH", "../")
+        .env("PYTHONPATH", python_path)
         .env("OUT_DIR", &out_dir)
         .arg("../_cffi_src/build_openssl.py")
         .output()
         .expect("failed to execute build_openssl.py");
+    assert!(output.status.success());
+
     let stdout = String::from_utf8(output.stdout).unwrap();
     let mut include = String::new();
     for line in stdout.lines() {

alex · 2021-12-13T00:32:34Z

Right now we specify our OpenSSL with CFLAGS/LDFLAGS, I guess we'll need to update to use openssl-sys's env vars. See .github/workflows/ci.yml.

messense · 2021-12-13T02:13:40Z

Some of the test matrix passed, hooray!

Debugging unfamiliar CI failure is time consuming, so please feel free to push to my branch to experiment.

alex · 2021-12-13T02:21:27Z

Sent pyca/infra#380 to install pkg-config which should help on the linux distros.

…

On Sun, Dec 12, 2021 at 9:17 PM messense ***@***.***> wrote: @messense commented on this pull request. ________________________________ In src/rust/src/lib.rs: > @@ -91,6 +96,11 @@ fn _rust(py: pyo3::Python<'_>, m: &pyo3::types::PyModule) -> pyo3::PyResult<()> crate::x509::ocsp_resp::add_to_module(ocsp_mod)?; m.add_submodule(ocsp_mod)?; + let openssl_mod = unsafe { + let ptr = PyInit__openssl(); + pyo3::types::PyModule::from_owned_ptr(py, ptr) + }; + m.add_submodule(openssl_mod)?; I can do this after building and testing on CI works. But I'm not sure how to deal with these Python files in https://github.com/pyca/cryptography/tree/main/src/cryptography/hazmat/bindings/openssl — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

-- All that is necessary for evil to succeed is for good people to do nothing.

alex · 2021-12-13T04:03:55Z

Ok, next build should reflect that.

…

On Sun, Dec 12, 2021 at 9:21 PM Alex Gaynor ***@***.***> wrote: Sent pyca/infra#380 to install pkg-config which should help on the linux distros. On Sun, Dec 12, 2021 at 9:17 PM messense ***@***.***> wrote: > > @messense commented on this pull request. > > ________________________________ > > In src/rust/src/lib.rs: > > > @@ -91,6 +96,11 @@ fn _rust(py: pyo3::Python<'_>, m: &pyo3::types::PyModule) -> pyo3::PyResult<()> > crate::x509::ocsp_resp::add_to_module(ocsp_mod)?; > m.add_submodule(ocsp_mod)?; > > + let openssl_mod = unsafe { > + let ptr = PyInit__openssl(); > + pyo3::types::PyModule::from_owned_ptr(py, ptr) > + }; > + m.add_submodule(openssl_mod)?; > > I can do this after building and testing on CI works. But I'm not sure how to deal with these Python files in https://github.com/pyca/cryptography/tree/main/src/cryptography/hazmat/bindings/openssl > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub, or unsubscribe. > Triage notifications on the go with GitHub Mobile for iOS or Android. -- All that is necessary for evil to succeed is for good people to do nothing.

-- All that is necessary for evil to succeed is for good people to do nothing.

messense · 2021-12-13T05:36:08Z

thread 'main' panicked at 'OpenSSL library directory does not exist: /home/runner/work/cryptography/cryptography/osslcache/openssl-3.0.0-1c9f97a0fce5ec7feed4e4c4944a9d9cca01bcfc/lib', /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/openssl-sys-0.9.72/build/main.rs:68:9

Where can I download this openssl-3.0.0 cached build?

alex · 2021-12-13T05:38:06Z

We don't put them anywhere, they're just built ephemerally in CI.

messense · 2021-12-13T05:52:15Z

Looks like openssl-sys is expecting libs in lib/ not lib64 when using the OPENSSL_DIR env var.

/home/runner/work/cryptography/cryptography/osslcache/openssl-3.0.0-1c9f97a0fce5ec7feed4e4c4944a9d9cca01bcfc
├── bin
│   ├── c_rehash
│   └── openssl
├── include
│   └── openssl
├── lib64
│   ├── engines-3
│   ├── libcrypto.a
│   ├── libcrypto.so -> libcrypto.so.3
│   ├── libcrypto.so.3
│   ├── libssl.a
│   ├── libssl.so -> libssl.so.3
│   ├── libssl.so.3
│   ├── ossl-modules
│   └── pkgconfig
└── ssl
    ├── certs
    ├── ct_log_list.cnf
    ├── ct_log_list.cnf.dist
    ├── misc
    ├── openssl.cnf
    ├── openssl.cnf.dist
    └── private

src/rust/build.rs

alex · 2021-12-13T05:56:01Z

How fun, putting it in lib64 is new in OpenSSL 3.0.

reaperhulk · 2021-12-13T06:03:32Z

Yes OpenSSL switched to using lib64 by default in 3.0.0-beta2 (#6192).

messense · 2021-12-13T06:14:43Z

Opened sfackler/rust-openssl#1581

messense · 2021-12-13T06:50:34Z

There are two errors I don't quite understand:

undefined symbol: PyInit__openssl on PyPy
Symbol not found: ___isPlatformVersionAtLeast on macOS Python 3.10

messense · 2021-12-13T07:05:27Z

Symbol not found: ___isPlatformVersionAtLeast

Could this be that the pre-compiled OpenSSL is built on macOS 11 but we're using it on macOS 10.15?

messense · 2021-12-13T12:26:09Z

undefined symbol: PyInit__openssl on PyPy

This is interesting.

root@5b52dbd4d5c2:/io# readelf -s /io/src/cryptography/hazmat/bindings/_rust.pypy38-pp73-aarch64-linux-gnu.so | grep PyInit
    11: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND PyInit__openssl
   148: 0000000000115290    32 FUNC    GLOBAL DEFAULT   11 PyInit__rust
 52405: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND PyInit__openssl
 52520: 0000000000115290    32 FUNC    GLOBAL DEFAULT   11 PyInit__rust

V.S. the _openssl.*.so on main branch for PyPy

root@5b52dbd4d5c2:/io# readelf -s src/cryptography/hazmat/bindings/_openssl.pypy38-pp73-aarch64-linux-gnu.so | grep PyInit
root@5b52dbd4d5c2:/io# readelf -s src/cryptography/hazmat/bindings/_openssl.pypy38-pp73-aarch64-linux-gnu.so | grep _cffi_pypyinit
   709: 0000000000025fa0    32 FUNC    GLOBAL DEFAULT   11 _cffi_pypyinit__openssl
  1407: 0000000000025fa0    32 FUNC    GLOBAL DEFAULT   11 _cffi_pypyinit__openssl

The only useful information I can find is pypa/auditwheel#93

Looking at the generated _openssl.c

#ifdef PYPY_VERSION
PyMODINIT_FUNC
_cffi_pypyinit__openssl(const void *p[])
{
    p[0] = (const void *)0x2601;
    p[1] = &_cffi_type_context;
#if PY_MAJOR_VERSION >= 3
    return NULL;
#endif
}
#  ifdef _MSC_VER
     PyMODINIT_FUNC
#  if PY_MAJOR_VERSION >= 3
     PyInit__openssl(void) { return NULL; }
#  else
     init_openssl(void) { }
#  endif
#  endif
#elif PY_MAJOR_VERSION >= 3
PyMODINIT_FUNC
PyInit__openssl(void)
{
  return _cffi_init("_openssl", 0x2601, &_cffi_type_context);
}
#else
PyMODINIT_FUNC
init_openssl(void)
{
  _cffi_init("_openssl", 0x2601, &_cffi_type_context);
}
#endif

It seems that we need to define PYPY_VERSION for it when building on PyPy. PyInit__openssl(void) { return NULL; } is also interesting.

messense · 2021-12-28T06:06:41Z

src/rust/build.rs

+    // Enable abi3 mode if we're not using PyPy.
+    if python_impl != "PyPy" {
+        // cp36
+        // build.define("Py_LIMITED_API", "0x030600f0");


Unable to get cffi embedding to compile with Py_LIMITED_API.

Oof :-/ Maybe we need to do the embedding way on PyPy and the "normal" way on CPython? PyPy already doesn't support Py_LIMITED_API.

messense · 2021-12-28T06:07:50Z

src/rust/src/lib.rs

+        // let ptr = PyInit__openssl();
+        // pyo3::types::PyModule::from_owned_ptr(py, ptr)
+        make_cryptography_openssl_module();
+        pyo3::types::PyModule::import(py, "_openssl")?


This also makes import _openssl work after importing cryptography, could this be a problem for end users?

Probably not. Maybe we should name it _cryptography_openssl or something to ensure no one accidentally uses it?

messense · 2021-12-28T06:08:45Z

It's not building on Windows, I'm not sure why and I don't have a Windows machine, help appreciated!

Edit: Tried to include Winsock2.h before windows.h, not working.

#ifndef _MSC_VER
   /* --- Assuming a GCC not infinitely old --- */
# define cffi_compare_and_swap(l,o,n)  __sync_bool_compare_and_swap(l,o,n)
# define cffi_write_barrier()          __sync_synchronize()
# if !defined(__amd64__) && !defined(__x86_64__) &&   \
     !defined(__i386__) && !defined(__i386)
#   define cffi_read_barrier()         __sync_synchronize()
# else
#   define cffi_read_barrier()         (void)0
# endif
#else
   /* --- Windows threads version --- */
# include <Windows.h>
# define cffi_compare_and_swap(l,o,n) \
                               (InterlockedCompareExchangePointer(l,n,o) == (o))
# define cffi_write_barrier()       InterlockedCompareExchange(&_cffi_dummy,0,0)
# define cffi_read_barrier()           (void)0
static volatile LONG _cffi_dummy;
#endif

Looks like cffi embedding includes Windows.h before we define WIN32_LEAN_AND_MEAN.

messense · 2021-12-28T06:43:01Z

Unfortunately looks like PyPy tests are hanging with the cffi embedding approach.

Debugged it locally (pypy3.8 setup.py develop then import) and got the following error message

>>>> from cryptography.hazmat.bindings import _rust
Fatal RPython error: a thread is trying to wait for the GIL, but the GIL was not initialized
(For PyPy, see https://bitbucket.org/pypy/pypy/issues/2274)
Aborted

https://foss.heptapod.net/pypy/pypy/-/issues/2274

alex · 2022-01-02T23:00:28Z

Looks like that issue is already closed, I'd suggest either opening a new one or including a comment on the cffi thread we have open. (Sorry I was slow to look at this!)

alex · 2022-01-02T23:01:41Z

src/_cffi_src/build_openssl.py

+    c_file = os.path.join(out_dir, module_name + source_extension)
+    ffi.embedding_api(
+        """
+        extern "Python" void _this_is_not_used(void);


Can we add a comment explaining what this is for?

alex · 2022-01-02T23:02:14Z

src/_cffi_src/utils.py

@@ -73,6 +73,12 @@ def build_ffi(
    verify_source += '\n#define CRYPTOGRAPHY_PACKAGE_VERSION "{}"'.format(
        about["__version__"]
    )
+    verify_source += r"""
+
+int make_cryptography_openssl_module(void) {


Let's name this Cryptography_make_openssl_module(), which is the prefix we generally use.

alex · 2022-01-02T23:02:54Z

src/cryptography/hazmat/bindings/openssl/binding.py

+ffi = _openssl.ffi  # type: ignore
+lib = _openssl.lib  # type: ignore


What actually uses these? Theoretically everything should go through a Binding instance.

alex · 2022-01-02T23:03:55Z

src/rust/build.rs

+    // Enable abi3 mode if we're not using PyPy.
+    if python_impl != "PyPy" {
+        // cp36
+        // build.define("Py_LIMITED_API", "0x030600f0");


Oof :-/ Maybe we need to do the embedding way on PyPy and the "normal" way on CPython? PyPy already doesn't support Py_LIMITED_API.

alex · 2022-01-02T23:04:18Z

src/rust/src/lib.rs

+        // let ptr = PyInit__openssl();
+        // pyo3::types::PyModule::from_owned_ptr(py, ptr)
+        make_cryptography_openssl_module();
+        pyo3::types::PyModule::import(py, "_openssl")?


Probably not. Maybe we should name it _cryptography_openssl or something to ensure no one accidentally uses it?

messense · 2022-01-20T14:11:41Z

Sorry I'm busy recently and unable to make progress on this. I'll come back to this maybe after Chinese lunar new year. In the meantime, feel free to take over.

alex · 2022-01-20T14:37:57Z

Thank you for all your work on this!

…

On Thu, Jan 20, 2022 at 9:11 AM messense ***@***.***> wrote: Sorry I'm busy recently and unable to make progress on this. I'll come back to this maybe after Chinese lunar new year. In the meantime, feel free to take over. — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.Message ID: ***@***.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

alex · 2022-04-30T23:05:56Z

I'm attempting to take over this PR in #7164. Thanks for all your work on this!

Still stuck on the PyPy hangs!

messense commented Dec 12, 2021

View reviewed changes

alex reviewed Dec 12, 2021

View reviewed changes

src/_cffi_src/build_openssl.py Outdated Show resolved Hide resolved

src/rust/.cargo/config.toml Outdated Show resolved Hide resolved

src/rust/build.rs Outdated Show resolved Hide resolved

src/rust/src/lib.rs Show resolved Hide resolved

alex mentioned this pull request Dec 12, 2021

Expose DEP_ var to dependents with path to OpenSSL headers sfackler/rust-openssl#1579

Open

messense force-pushed the rust-openssl branch from ff8663b to 0db7bf6 Compare December 13, 2021 05:20

messense commented Dec 13, 2021

View reviewed changes

src/rust/build.rs Show resolved Hide resolved

messense force-pushed the rust-openssl branch 4 times, most recently from 0447f78 to 11dcf48 Compare December 13, 2021 06:40

messense added 12 commits December 28, 2021 13:15

Print stdout and stderr when build_openssl.py fails

8a6f0f9

Try to fix Windows

c11218a

Set rpath with RUSTFLAGS

b087223

Install cffi in Rust Coverage job

1ef6259

Move extra compile args to Rust side

65340cb

Link against clang runtime on static builds on macOS

a9aa151

Bump MSRV to 1.48.0

e36ba2b

Define Py_LIMITED_API

21208ff

Use OPENSSL_DIR for zuul wheel build

916716c

Install pkg-config in zuul

a114535

Try to fix mypy error

2eea530

Use git dependency for openssl crate

1377cee

messense force-pushed the rust-openssl branch from a04b63f to 1377cee Compare December 28, 2021 05:15

messense added 2 commits December 28, 2021 13:20

Try to fix circleci build failures

9518ea1

Try cffi embedding API

0604d80

messense commented Dec 28, 2021

View reviewed changes

messense force-pushed the rust-openssl branch from 2ad5294 to 0b7abc6 Compare December 28, 2021 06:13

Define WIN32_LEAN_AND_MEAN in Rust side

50a751a

messense force-pushed the rust-openssl branch from 0b7abc6 to 50a751a Compare December 28, 2021 06:31

alex reviewed Jan 2, 2022

View reviewed changes

aganders3 mentioned this pull request Jan 3, 2022

Expose a way to access the PYO3_CONFIG_FILE values PyO3/pyo3#2033

Open

davidhewitt mentioned this pull request Apr 11, 2022

pyo3-build-config: Build "abi3" extensions without an interpreter PyO3/pyo3#2293

Merged

alex closed this Apr 30, 2022

		ffi = _openssl.ffi # type: ignore
		lib = _openssl.lib # type: ignore

PoC: Make it possible to write code that interacts with OpenSSL in Rust #6695

PoC: Make it possible to write code that interacts with OpenSSL in Rust #6695

Conversation

messense commented Dec 12, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

messense commented Dec 12, 2021

alex commented Dec 12, 2021

messense commented Dec 12, 2021

alex commented Dec 12, 2021

messense commented Dec 12, 2021

alex commented Dec 12, 2021 • edited

alex commented Dec 12, 2021

alex commented Dec 12, 2021

alex commented Dec 13, 2021

messense commented Dec 13, 2021

alex commented Dec 13, 2021 via email

alex commented Dec 13, 2021 via email

messense commented Dec 13, 2021

alex commented Dec 13, 2021

messense commented Dec 13, 2021 • edited

alex commented Dec 13, 2021

reaperhulk commented Dec 13, 2021

messense commented Dec 13, 2021

messense commented Dec 13, 2021

messense commented Dec 13, 2021

messense commented Dec 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

messense commented Dec 28, 2021 • edited

messense commented Dec 28, 2021 • edited

alex commented Jan 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

messense commented Jan 20, 2022

alex commented Jan 20, 2022 via email

alex commented Apr 30, 2022

messense commented Dec 12, 2021 •

edited

alex commented Dec 12, 2021 •

edited

messense commented Dec 13, 2021 •

edited

messense commented Dec 28, 2021 •

edited

messense commented Dec 28, 2021 •

edited