Skip to content

Cryptography isn't installable if we don't have OpenSSL INC/LIB dirs specified in global INCLUDE/LIB environment variables. #1093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
OCherniaiev opened this issue Jun 3, 2014 · 11 comments

Comments

@OCherniaiev
Copy link

Cryptography isn't installable if we don't have OpenSSL INC/LIB dirs specified in global INCLUDE/LIB environment variables.

On windows platform if we want to have both 32/64 bit versions of OpenSSL it becomes quite problematic to specify these environment variables.

I've done a few of modifications. Please see the diff:

diff --git a/cryptography/hazmat/bindings/openssl/binding.py b/cryptography/hazmat/bindings/openssl/binding.py
index 464081b..1ab4463 100644
--- a/cryptography/hazmat/bindings/openssl/binding.py
+++ b/cryptography/hazmat/bindings/openssl/binding.py
@@ -15,6 +15,7 @@ from __future__ import absolute_import, division, print_function

 import sys
 import threading
+import os

 from cryptography.hazmat.bindings.utils import build_ffi

@@ -98,13 +99,19 @@ class Binding(object):
         else:  # pragma: no cover
             libraries = ["libeay32", "ssleay32", "advapi32"]

-        cls.ffi, cls.lib = build_ffi(
+        build_cffi_kwargs = dict(
             module_prefix=cls._module_prefix,
             modules=cls._modules,
             pre_include=_OSX_PRE_INCLUDE,
             post_include=_OSX_POST_INCLUDE,
             libraries=libraries,
         )
+
+        if 'OPENSSL_INC_DIR' in os.environ and 'OPENSSL_LIB_DIR' in os.environ:
+            build_cffi_kwargs['include_dirs'] = [os.path.expandvars('$OPENSSL_INC_DIR')]
+            build_cffi_kwargs['library_dirs'] = [os.path.expandvars('$OPENSSL_LIB_DIR')]
+
+        cls.ffi, cls.lib = build_ffi(**build_cffi_kwargs)
         res = cls.lib.Cryptography_add_osrandom_engine()
         assert res != 0

@Ayrx
Copy link
Contributor

Ayrx commented Jun 3, 2014

Do you mind resubmitting the diff as a pull request if your goal is to get this merged so we can tell if it breaks anything with our test runners?

@OCherniaiev
Copy link
Author

I don't think this is a full diff required to check if this works ok. For now build_ffi also requires changes. Next I'm not sure that CFFI.FFI.verify accepts such sort of additional data. Yet I don't have a permission to pull. It's just an idea of how to solve such kind of problems. Wheel/egg bundled packages are ok, but sdist one(tar.gz) always tries to compile from scatch.

@OCherniaiev
Copy link
Author

BTW have a look over pypa/pip#1840 . I think that CFFI can produce such sort of problems.

@reaperhulk
Copy link
Member

Another potential fix to this problem is static linking (which simplifies the Windows install process immensely at the cost of requiring us to issue new wheels for every OpenSSL point release).

@OCherniaiev
Copy link
Author

But binary packing already fixes the problem - wheel/egg. The main problem arises when setuptools tries to satisfy requirements - due to easy_install's unsupportability of wheels it can only use eggs/sdists. In this case it finds only sdist. Sdist requires to build extensions from sources. Anyways this step requires paths to OpenSSL INC/LIB dirs whenewer we want to link statically or dynamically.

@reaperhulk
Copy link
Member

I'm not sure I fully understand here.

We currently build wheels for 32-bit and 64-bit. For these to operate properly the required OpenSSL shared library must be present in the Windows library lookup path. This requires setting the LIB variable or installing OpenSSL's libraries in the system directories (which is the default for the Windows installer). So to run 32-bit and 64-bit side by side you'll likely need to install OpenSSL in a non-default location and then specify which library you want to load, correct?

When compiling the same is true, but you can just @set LIB="C:\OpenSSL-Win32\lib";%LIB% or @set LIB="C:\OpenSSL-Win64\lib";%LIB% before you execute python. These variables are only set in the scope of the current shell, so I'm unsure why this isn't essentially the same as your proposed solution.

@OCherniaiev
Copy link
Author

For instance pyinstaller has a dependency on cryptography through its requirements.txt file. Setup tools as to my knowledge doesn't support wheel packed packages. Therefore it fetches sdist package that must be built from sources. As to lib/inc path - as an end user I use both 32 and 64 bit python configurations simultaneously. That's why we cannot set up these variables globally - they cannot contain paths to both 32 and 64 bit OpenSSL libs. At the same time if we want to include dependency to your package in our requirements.txt - it takes extra work to override default setup develop command for our package.

@reaperhulk
Copy link
Member

Could you elaborate a bit on what you mean when you say you run 32 and 64 bit python configurations simultaneously? Do you mean you execute each one via its own batch script or command prompt? So to run 32 and 64 you execute two scripts or start them on two prompts?

@OCherniaiev
Copy link
Author

We use virtualenvwrapper/virtualenvwrapper-win and we have two installations of python - 32/64 accordingly. Next we use two different virtual environments with appropriate python path passed as --python arg to mkvirtualenv. Our package depends on pyinstaller, which in its turn depends on cryptography(REQUIREMENTS.txt or install_requires), When setuptools performs to install pyinstaller it finds that it needs to satisfy cryptography dependency(it's not pip I guess). Next if finds source distribution package and tries to setup it. This in turn triggers extension build process. At least this situation can be reproduced with pip install --egg pyinstaller==2.1.1dev-89e99dd

@OCherniaiev
Copy link
Author

And yes - we perform the same steps on both virtual environments: setup develop - it triggers dependencies installation.

@Ayrx
Copy link
Contributor

Ayrx commented Jun 4, 2014

Going by what you have said, I'm not convinced that this is a common enough issue or big enough a problem that we should modify our install process to workaround it.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants