Skip to content

Conversation

@TerminalFi
Copy link
Contributor

MacOS Big Sur had dynamic linker changes. Shared libraries are now stored only in /System/Library/dyld/dyld_shared_cache_x86_64 and no longer present on the actual file system. This changes makes the find_library not return the paths of the libraries such as find_library("ssl")

MacOS Big Sur had dynamic linker changes. Shared libraries are now
stored only in `/System/Library/dyld/dyld_shared_cache_x86_64`
and no longer present on the actual file system. This changes makes the
`find_library` not return the paths of the libraries such as
`find_library("ssl")`
Copy link
Owner

@wbond wbond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we just need a few little tweaks making sure that we only set the OpenSSL paths if we didn't find anything.

libcrypto_path == '/usr/lib/libcrypto.42.dylib'
# if we are on macOS 10.15+, we want to strongly version libcrypto since unversioned libcrypto has a non-stable ABI
if sys.platform == 'darwin' and list(map(int, platform.mac_ver()[0].split('.')))[1] >= 15 and \
if sys.platform == 'darwin' and tuple(map(int, platform.mac_ver()[0].split('.'))) >= (10, 15) and \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably become an elif. While it running for 10.16+ doesn't change because the last condition won't apply, this seems more fragile than needs be.

libcrypto_path == '/usr/lib/libcrypto.42.dylib'
# if we are on macOS 10.15+, we want to strongly version libcrypto since unversioned libcrypto has a non-stable ABI
if sys.platform == 'darwin' and list(map(int, platform.mac_ver()[0].split('.')))[1] >= 15 and \
if sys.platform == 'darwin' and tuple(map(int, platform.mac_ver()[0].split('.'))) >= (10, 15) and \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

tuple(map(int, platform.mac_ver()[0].split('.'))) >= (10, 16):
libssl_path = '/usr/lib/libssl.44.dylib'
# if we are on macOS 10.15+, we want to strongly version libssl since unversioned libcrypto has a non-stable ABI
if sys.platform == 'darwin' and list(map(int, platform.mac_ver()[0].split('.')))[1] >= 15 and \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

tuple(map(int, platform.mac_ver()[0].split('.'))) >= (10, 16):
libssl_path = '/usr/lib/libssl.44.dylib'
# if we are on macOS 10.15+, we want to strongly version libssl since unversioned libcrypto has a non-stable ABI
if sys.platform == 'darwin' and list(map(int, platform.mac_ver()[0].split('.')))[1] >= 15 and \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

@FichteFoll
Copy link

Actually, seeing how frequently this pattern is used, it seems reasonable to factor it out into a function of some sort.

@TerminalFi
Copy link
Contributor Author

Actually, seeing how frequently this pattern is used, it seems reasonable to factor it out into a function of some sort.

Have something in mind? This? Welcomed suggestions, I am on the road currently.

def get_library(library, fallback, unverisoned):
    ...

@TerminalFi
Copy link
Contributor Author

TerminalFi commented Jun 28, 2020

Not sure I like this

def get_library(name, unversioned, fallback):
    library = find_library(name)
    if not library and sys.platform == 'darwin' and tuple(map(int,  platform.mac_ver()[0].split('.'))) >= (10, 16):
        library == fallback
    elif sys.platform == 'darwin' and tuple(map(int,  platform.mac_ver()[0].split('.'))) >= (10, 15) and \
            library == unversioned:
        library = fallback
    return library

in use

libssl_path = _backend_config().get('libssl_path')
if libssl_path is None:
    libssl_path = get_library('ssl', '/usr/lib/libssl.dylib', '/usr/lib/libssl.44.dylib')
if not libssl_path:
    raise LibraryNotFoundError('The library libssl could not be found')

@TerminalFi
Copy link
Contributor Author

@FichteFoll mind helping out with this test issue? I I am guessing I need to fix something in the tests themselves, unless my logic is wrong.

Running tests (via coverage.py)
Traceback (most recent call last):
  File "run.py", line 8, in <module>
    run_task()
  File "/Users/terminal/.projects/oscrypto/dev/_task.py", line 162, in run_task
    result = run(*args, **kwargs)
  File "/Users/terminal/.projects/oscrypto/dev/ci.py", line 49, in run
    tests_result = run_coverage(ci=True)
  File "/Users/terminal/.projects/oscrypto/dev/coverage.py", line 57, in run
    result = run_tests(ci=ci)
  File "/Users/terminal/.projects/oscrypto/dev/tests.py", line 61, in run
    for test_class in test_classes():
  File "/Users/terminal/.projects/oscrypto/tests/__init__.py", line 136, in test_classes
    from .test_kdf import KDFTests
  File "/Users/terminal/.projects/oscrypto/tests/test_kdf.py", line 7, in <module>
    from oscrypto import kdf, _pkcs5
  File "/Users/terminal/.projects/oscrypto/oscrypto/kdf.py", line 9, in <module>
    from .util import rand_bytes
  File "/Users/terminal/.projects/oscrypto/oscrypto/util.py", line 13, in <module>
    from ._mac.util import rand_bytes
  File "/Users/terminal/.projects/oscrypto/oscrypto/_mac/util.py", line 208, in <module>
    from .._openssl._libcrypto import libcrypto
  File "/Users/terminal/.projects/oscrypto/oscrypto/_openssl/_libcrypto.py", line 9, in <module>
    from ._libcrypto_cffi import (
  File "/Users/terminal/.projects/oscrypto/oscrypto/_openssl/_libcrypto_cffi.py", line 7, in <module>
    from ..util import get_library
ImportError: cannot import name 'get_library' from 'oscrypto.util' (/Users/terminal/.projects/oscrypto/oscrypto/util.py)

@wbond
Copy link
Owner

wbond commented Jun 28, 2020

I believe you have an include loop since the root util.py is importing the platform-specific one to get rand_bytes().

This helper function really shouldn’t be part of the public API, so it should go in a file with an _, probably _ffi.py.

Copy link

@FichteFoll FichteFoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function made this much better to handle, but there were still some rough edges.

TerminalFi and others added 5 commits June 30, 2020 08:14
Co-authored-by: FichteFoll <fichtefoll2@googlemail.com>
Co-authored-by: FichteFoll <fichtefoll2@googlemail.com>
Co-authored-by: FichteFoll <fichtefoll2@googlemail.com>
Co-authored-by: FichteFoll <fichtefoll2@googlemail.com>
Co-authored-by: FichteFoll <fichtefoll2@googlemail.com>
@michaelblyons
Copy link

/backlink-ing to wbond/package_control#1477

@wbond
Copy link
Owner

wbond commented Jul 7, 2020

The error seen with Python 3.7 on Mac can be resolved by rebasing on master, although you may not see it every time the tests are run.

@wbond wbond merged commit 6c5abfd into wbond:master Jul 8, 2020
@wbond
Copy link
Owner

wbond commented Jul 8, 2020

Apparently installing PyPy via homebrew is broken, so I'm just going to ignore that error for now.

Thanks for the work on this!

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.

4 participants