Add activation script to check for system-libs that override openssl install

[h-vetinari]

Checklist

• Used a fork of the feedstock to propose changes
• Bumped the build number (if the version is unchanged)
• Reset the build number to 0 (if the version changed)
• Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
• Ensured the license file is being packaged.

This comes out of discussions with @isuruf and @mingwandroid in conda-forge/cryptography-feedstock#37 and conda-forge/python-feedstock#345.

The short story is that the cryptography test suite luckily picked up that the presence of outdated system libs in C:\Windows\System32 caused a wrong openssl-version to be linked under the hood.

While setting CONDA_DLL_SEARCH_MODIFICATION_ENABLE allows to work around this, it was deemed "too big of a hammer" to be generally enabled. However, in order not to leave affected users (with such outdated libs on the system path) unknowingly vulnerable, the suggestion was to check this in an activation script for openssl.

The script works as follows:

• check which libssl-1_1-x64.dll is the first on PATH
• check which libcrypto-1_1-x64.dll is the first on PATH
• if any of the respective paths contains "System32":
  • if CONDA_DLL_SEARCH_MODIFICATION_ENABLE is not set: set it and try again
  • if it had been set already: display a warning to the user

[conda-forge-linter]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

[h-vetinari]

@conda-forge-admin, please rerender

[isuruf]

check which libssl-1_1-x64.dll is the first on PATH

It doesn't matter which DLL is the first on PATH. Windows loader looks at C:\Windows\System32 first and then the paths on PATH. So, we should just look at whether the file C:\Windows\System32\libcrypto-1_1-x64.dll exists or not as I mentioned at conda-forge/cryptography-feedstock#37 (comment)

[h-vetinari]

It doesn't matter which DLL is the first on PATH. Windows loader looks at C:\Windows\System32 first and then the paths on PATH. So, we should just look at whether the file C:\Windows\System32\libcrypto-1_1-x64.dll exists or not as I mentioned at conda-forge/cryptography-feedstock#37 (comment)

I wasn't aware of this intricacy of the windows loader. It unfortunately messes up my idea to set CONDA_DLL_SEARCH_MODIFICATION_ENABLE only if necessary (and test the result), because I haven't found a way to determine which DLL would be loaded first in that case (completely aside from that envvar, not even ctypes.util.find_library yields the correct result when C:\Windows\System32 is involved).

I mean, running python and checking the out put of

from cryptography.hazmat.backends.openssl import backend
backend.openssl_version_text()

would work in principle (but not for this feedstock). Does someone have an idea on how I could check which library the windows loader would pick up (say, from within python)?

[h-vetinari]

Travis is just a timeout, but I don't have the rights to restart it.

Nevermind, I found & fixed a mistake and repushed.

[h-vetinari]

@conda-forge-admin, please rerender

[h-vetinari]

I had made an oversight while refactoring - %LIBSSL_PATH% was not defined. I now fixed and re-tested it (both cases), and also added the outstanding bash-activation scripts (tested with git-bash).

[h-vetinari]

Thinking ahead to implementing a similar activation script for cryptography as described, I'm wondering if we have any control over the order of the activation scripts? Or should the bld.bat of cryptography overwrite/replace the activation script from openssl (or maybe use a shared name)?

[mingwandroid]

Why don't we make a common package for this check that both openssl and anything else can depend on? I need to check that activation order is correct though yeah..

[h-vetinari]

Why don't we make a common package for this check that both openssl and anything else can depend on? I need to check that activation order is correct though yeah..

That's certainly a smart approach, but there are non-python environments that just use openssl (and should see a warning like in this PR).

But I'm guessing this could be solved with some additional checks in the activation script of that common dependency?

[isuruf]

This warning should always be there because CONDA_DLL_SEARCH_MODIFICATION_ENABLE applies only to the python processes and any openssl binaries will not use it.

[h-vetinari]

This warning should always be there because CONDA_DLL_SEARCH_MODIFICATION_ENABLE applies only to the python processes and any openssl binaries will not use it.

This is pretty much what I've done in conda-forge/staged-recipes#11452 :)
Still a draft, but should be enough as a basis for discussion.

[isuruf]

Why not merge this as is?

[h-vetinari]

Why not merge this as is?

We can, but once we also add an activation script for cryptography, we'll be duplicating warnings, or having to muck around with order/dependencies of those scripts. It wasn't my idea, but I tend to agree with @mingwandroid that the cleanest solution is common dependency that covers all cases.

[isuruf]

Why do we need another warning for cryptography?

[h-vetinari]

Why do we need another warning for cryptography?

Because only there can we try to activate CONDA_DLL_SEARCH_MODIFICATION_ENABLE and test whether that was successful. It had certainly seemed like you had agreed with that approach a few hours ago.

[isuruf]

Yes, but the warning for openssl should be there regardless. So, the two scripts does two different things. cryptography script can use an env variable from openssl script to warn.

[h-vetinari]

Yes, but the warning for openssl should be there regardless. So, the two scripts does two different things. cryptography script can use an env variable from openssl script to warn.

That's an option, true. It's just not very user friendly because they'll be getting the same or similar warnings twice, because we (or at least I) don't know in which order the activation scripts will be triggered.

That might mean (assuming cryptography comes first and succeeds with CONDA_DLL_SEARCH_MODIFICATION_ENABLE):

C:\Users\Someone>conda activate my_env
# cryptography.activate
Warning: Found (potentially) outdated libssl/libcrypto libraries on system;
Warning: Attempting to re-activate with CONDA_DLL_SEARCH_MODIFICATION_ENABLE

Warning: Your system contains (potentially) outdated libraries under:
Warning: C:\Windows\System32\libssl-1_1-x64.dll
Warning: C:\Windows\System32\libcrypto-1_1-x64.dll
Warning: Within this environment, the python-runtime will correctly load
Warning: the openssl version of the environment, but be aware that anything
Warning: *outside* of python that is trying to load libssl/libcrypto will
Warning: load the DLLs above, which might make that application vulnerable!

# openssl.activate
WARNING: Your system contains (potentially) outdated libraries under:
WARNING: C:\Windows\System32\libssl-1_1-x64.dll
WARNING: C:\Windows\System32\libcrypto-1_1-x64.dll
WARNING: These libraries will be used before those in the conda
WARNING: environment and might make your installation vulnerable!
(my_env) > C:\Users\Someone>

The third warning is IMO unnecessarily loud, and can be avoided with the approach in conda-forge/staged-recipes#11452. The openssl block would only get shown if no cryptography is installed.

[isuruf]

because we (or at least I) don't know in which order the activation scripts will be triggered.

I think the order is dependency order and if there's a cycle, alphabetically by the name of the script.

[h-vetinari]

I think the order is dependency order and if there's a cycle, alphabetically by the name of the script.

Assuming we get the order deterministically, we still need to either block or overwrite one or the other (depending on which libraries are present in the environment), if we want to avoid duplicated warnings. This sounds like much more pain than it's worth (especially if there's an easy alternative).

[h-vetinari]

@isuruf
In one aspect you have a point - to avoid cycles, conda-forge/staged-recipes#11452 does not depend on openssl / cryptography, but still checks for their presence.

Arguably then, this does not need a new common dependency, but the activation script here could just as well probe for the presence of cryptography, and react accordingly.

PS. But it's probably harder to get outdated DLLs into the test enviroment (in conda-forge/staged-recipes#11452, I have the luxury of having source==recipe)

[hmaarrfk]

Do we still need this?

[h-vetinari]

Do we still need this?

Would still be nice to have IMO, but my efforts in staged-recipes stalled for lack of review and then keeping things up to date. I don't know the current status of path-resolution on windows around CONDA_DLL_SEARCH_MODIFICATION_ENABLE (e.g. if C:\Windows\System32 wins against conda envs), but if there's any chance that outdated system libs are still being picked up, I'd still want this safeguard.