Use the 'SecTrustEvaluate' API for macOS 10.13 and earlier #157

sethmlarson · 2024-10-02T13:55:26Z

Closes #119. This uses the patch provided by @illume as a baseline.

src/truststore/_macos.py

Co-authored-by: =?UTF-8?q?Ren=C3=A9=20Dudfield?= <renesd@gmail.com>

sethmlarson · 2024-10-04T20:37:01Z

Failures are the same as on main (which I need to look at too, looks like aiohttp may have changed?)

sethmlarson · 2024-10-10T17:35:15Z

@illume Are you able to review this implementation and test it on your machine?

ThomasWaldmann · 2024-10-11T22:43:58Z

src/truststore/_macos.py

+    Security.SecTrustEvaluate(sec_trust_ref, ctypes.byref(sec_trust_result_type))
+
+    try:
+        sec_trust_result_type_as_int = int(sec_trust_result_type.value)


Good. If there was a link in #156 to here, that would have saved me some time to find this. See my comments there (and maybe close that PR if you have a superseding one).

I tested your changes on macOS 10.12 by copying the _macos.py from this changeset over the dysfunctional one bundled with pip 24.2, that made pip work again!

ThomasWaldmann

I tested your changes on macOS 10.12 by copying the _macos.py from this changeset over the dysfunctional one bundled with pip 24.2, that made pip work again!

sethmlarson · 2024-10-12T16:15:32Z

@ThomasWaldmann awesome, thank you! Could you also run the test suite on your machine? That would help me immensely.

ThomasWaldmann · 2024-10-12T20:54:27Z

@sethmlarson guess I could do that. but there are failures even on current macOS (see CI). shall I just post the test log output here?

ThomasWaldmann · 2024-10-12T22:03:04Z

Strange, now I needed this to have it not immediately crash:

diff --git a/src/truststore/_macos.py b/src/truststore/_macos.py
index 4d2b44b..762f684 100644
--- a/src/truststore/_macos.py
+++ b/src/truststore/_macos.py
@@ -123,12 +123,6 @@ try:
     ]
     Security.SecTrustEvaluate.restype = OSStatus
 
-    Security.SecTrustEvaluateWithError.argtypes = [
-        SecTrustRef,
-        POINTER(CFErrorRef),
-    ]
-    Security.SecTrustEvaluateWithError.restype = c_bool
-
     Security.SecTrustRef = SecTrustRef  # type: ignore[attr-defined]
     Security.SecTrustResultType = SecTrustResultType  # type: ignore[attr-defined]
     Security.OSStatus = OSStatus  # type: ignore[attr-defined]
@@ -211,8 +205,19 @@ try:
     CoreFoundation.CFStringRef = CFStringRef  # type: ignore[attr-defined]
     CoreFoundation.CFErrorRef = CFErrorRef  # type: ignore[attr-defined]
 
-except AttributeError:
-    raise ImportError("Error initializing ctypes") from None
+except AttributeError as err:
+    raise ImportError("Error initializing ctypes: " + str(err)) from None
+
+
+if _is_macos_version_10_14_or_later:
+    try:
+        Security.SecTrustEvaluateWithError.argtypes = [
+            SecTrustRef,
+            POINTER(CFErrorRef),
+        ]
+        Security.SecTrustEvaluateWithError.restype = c_bool
+    except AttributeError as err:
+        raise ImportError("Error initializing ctypes: " + str(err)) from None

I also improved the ImportError msg to be more helpful.

ThomasWaldmann · 2024-10-12T22:10:12Z

Log output after the change mentioned above (macOS 10.12 Intel):

(truststore-env) This-MacBook-Pro:truststore vagrant$ pytest -v -s -rs --no-flaky-report --max-runs=3 tests/

/vagrant/borg/truststore-env/lib/python3.10/site-packages/pytest_asyncio/plugin.py:208: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset.
The event loop scope for asynchronous fixtures will default to the fixture caching scope. Future versions of pytest-asyncio will default the loop scope for asynchronous fixtures to function scope. Set the default fixture loop scope explicitly in order to avoid unexpected behavior in the future. Valid fixture loop scopes are: "function", "class", "module", "package", "session"

  warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET))
============================= test session starts ==============================
platform darwin -- Python 3.10.2, pytest-8.3.3, pluggy-1.5.0 -- /vagrant/borg/truststore-env/bin/python3
cachedir: .pytest_cache
rootdir: /vagrant/borg/truststore
configfile: pyproject.toml
plugins: anyio-4.6.0, flaky-3.8.1, asyncio-0.24.0, pytest_httpserver-1.1.0
asyncio: mode=strict, default_loop_scope=None
collecting ... collected 64 items

tests/test_api.py::test_success[example.com] PASSED
tests/test_api.py::test_success[1.1.1.1] PASSED
tests/test_api.py::test_failures[wrong.host.badssl.com] PASSED
tests/test_api.py::test_failures[expired.badssl.com] PASSED
tests/test_api.py::test_failures[self-signed.badssl.com] PASSED
tests/test_api.py::test_failures[untrusted-root.badssl.com] PASSED
tests/test_api.py::test_failures[superfish.badssl.com] PASSED
tests/test_api.py::test_failures[revoked.badssl.com] FAILED
tests/test_api.py::test_success_after_loading_additional_anchors[example.com] PASSED
tests/test_api.py::test_success_after_loading_additional_anchors[1.1.1.1] PASSED
tests/test_api.py::test_failure_after_loading_additional_anchors[wrong.host.badssl.com] PASSED
tests/test_api.py::test_failure_after_loading_additional_anchors[expired.badssl.com] PASSED
tests/test_api.py::test_failure_after_loading_additional_anchors[self-signed.badssl.com] PASSED
tests/test_api.py::test_failure_after_loading_additional_anchors[untrusted-root.badssl.com] PASSED
tests/test_api.py::test_failure_after_loading_additional_anchors[superfish.badssl.com] PASSED
tests/test_api.py::test_failure_after_loading_additional_anchors[revoked.badssl.com] FAILED
tests/test_api.py::test_failures_without_revocation_checks[wrong.host.badssl.com] PASSED
tests/test_api.py::test_failures_without_revocation_checks[expired.badssl.com] PASSED
tests/test_api.py::test_failures_without_revocation_checks[self-signed.badssl.com] PASSED
tests/test_api.py::test_failures_without_revocation_checks[untrusted-root.badssl.com] PASSED
tests/test_api.py::test_failures_without_revocation_checks[superfish.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_success[example.com] PASSED
tests/test_api.py::test_sslcontext_api_success[1.1.1.1] SKIPPED (url...)
tests/test_api.py::test_sslcontext_api_success_async[example.com] PASSED
tests/test_api.py::test_sslcontext_api_success_async[1.1.1.1] PASSED
tests/test_api.py::test_sslcontext_api_failures[wrong.host.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures[expired.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures[self-signed.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures[untrusted-root.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures[superfish.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures[revoked.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures_async[wrong.host.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures_async[expired.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures_async[self-signed.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures_async[untrusted-root.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures_async[superfish.badssl.com] PASSED
tests/test_api.py::test_sslcontext_api_failures_async[revoked.badssl.com] PASSED
tests/test_api.py::test_requests_sslcontext_api_success[example.com] PASSED
tests/test_api.py::test_requests_sslcontext_api_success[1.1.1.1] SKIPPED
tests/test_api.py::test_requests_sslcontext_api_failures[wrong.host.badssl.com] PASSED
tests/test_api.py::test_requests_sslcontext_api_failures[expired.badssl.com] PASSED
tests/test_api.py::test_requests_sslcontext_api_failures[self-signed.badssl.com] PASSED
tests/test_api.py::test_requests_sslcontext_api_failures[untrusted-root.badssl.com] PASSED
tests/test_api.py::test_requests_sslcontext_api_failures[superfish.badssl.com] PASSED
tests/test_api.py::test_requests_sslcontext_api_failures[revoked.badssl.com] PASSED
tests/test_api.py::test_trustme_cert PASSED
tests/test_api.py::test_trustme_cert_loaded_via_capath PASSED
tests/test_api.py::test_trustme_cert_still_uses_system_certs PASSED
tests/test_api.py::test_macos_10_7_import_error PASSED
tests/test_custom_ca.py::test_urllib3_custom_ca SKIPPED (Install mkc...)
tests/test_custom_ca.py::test_aiohttp_custom_ca SKIPPED (Install mkc...)
tests/test_custom_ca.py::test_requests_custom_ca SKIPPED (Install mk...)
tests/test_inject.py::test_inject_and_extract PASSED
tests/test_inject.py::test_success_with_inject[example.com] PASSED
tests/test_inject.py::test_success_with_inject[1.1.1.1] PASSED
tests/test_inject.py::test_inject_set_values PASSED
tests/test_inject.py::test_urllib3_works_with_inject SKIPPED (Instal...)
tests/test_inject.py::test_aiohttp_works_with_inject SKIPPED (Instal...)
tests/test_inject.py::test_requests_works_with_inject PASSED
tests/test_inject.py::test_sync_httpx_works_with_inject SKIPPED (Ins...)
tests/test_inject.py::test_async_httpx_works_with_inject SKIPPED (In...)
tests/test_sslcontext.py::test_minimum_maximum_version PASSED
tests/test_sslcontext.py::test_check_hostname_false FAILED
tests/test_sslcontext.py::test_verify_mode_cert_none PASSED

=================================== FAILURES ===================================
______________________ test_failures[revoked.badssl.com] _______________________

failure = FailureHost(host='revoked.badssl.com', error_messages=['“revoked.badssl.com” certificate is revoked', 'Unknown error occurred', 'The certificate is revoked.'])

    @failure_hosts
    def test_failures(failure):
        with pytest.raises(ssl.SSLCertVerificationError) as e:
            connect_to_host(failure.host)
    
        error_repr = repr(e.value)
>       assert any(message in error_repr for message in failure.error_messages), error_repr
E       AssertionError: SSLCertVerificationError('Recoverable trust failure occurred')
E       assert False
E        +  where False = any(<generator object test_failures.<locals>.<genexpr> at 0x10ca47610>)

tests/test_api.py:239: AssertionError
______ test_failure_after_loading_additional_anchors[revoked.badssl.com] _______

failure = FailureHost(host='revoked.badssl.com', error_messages=['“revoked.badssl.com” certificate is revoked', 'Unknown error occurred', 'The certificate is revoked.'])
trustme_ca = <trustme.CA object at 0x10d5fb310>

    @failure_hosts
    def test_failure_after_loading_additional_anchors(failure, trustme_ca):
        with (
            pytest.raises(ssl.SSLCertVerificationError) as e,
            socket.create_connection((failure.host, 443)) as sock,
        ):
            ctx = truststore.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
    
            # See if loading additional anchors still fails.
            trustme_ca.configure_trust(ctx)
            with ctx.wrap_socket(sock, server_hostname=failure.host):
                pass
    
        error_repr = repr(e.value)
>       assert any(message in error_repr for message in failure.error_messages), error_repr
E       AssertionError: SSLCertVerificationError('Recoverable trust failure occurred')
E       assert False
E        +  where False = any(<generator object test_failure_after_loading_additional_anchors.<locals>.<genexpr> at 0x10cc2e880>)

tests/test_api.py:267: AssertionError
__________________________ test_check_hostname_false ___________________________

    @pytest.mark.internet
    def test_check_hostname_false():
        ctx = truststore.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
        assert ctx.check_hostname is True
        assert ctx.verify_mode == ssl.CERT_REQUIRED
    
        with urllib3.PoolManager(ssl_context=ctx, retries=False) as http:
            with pytest.raises(SSLError) as e:
                http.request("GET", "https://wrong.host.badssl.com/")
>           assert "match" in str(e.value)
E           assert 'match' in "('Recoverable trust failure occurred',)"
E            +  where "('Recoverable trust failure occurred',)" = str(SSLError(SSLCertVerificationError('Recoverable trust failure occurred')))
E            +    where SSLError(SSLCertVerificationError('Recoverable trust failure occurred')) = <ExceptionInfo SSLError(SSLCertVerificationError('Recoverable trust failure occurred')) tblen=9>.value

tests/test_sslcontext.py:37: AssertionError
=========================== short test summary info ============================
SKIPPED [1] tests/test_api.py:287: urllib3 doesn't pass server_hostname for IP addresses
SKIPPED [1] tests/test_api.py:343: urllib3 doesn't pass server_hostname for IP addresses
SKIPPED [1] tests/test_custom_ca.py:14: Install mkcert to run custom CA tests
SKIPPED [1] tests/test_custom_ca.py:26: Install mkcert to run custom CA tests
SKIPPED [1] tests/test_custom_ca.py:34: Install mkcert to run custom CA tests
SKIPPED [1] tests/test_inject.py:82: Install mkcert to run custom CA tests
SKIPPED [1] tests/test_inject.py:94: Install mkcert to run custom CA tests
SKIPPED [1] tests/test_inject.py:111: Install mkcert to run custom CA tests
SKIPPED [1] tests/test_inject.py:123: Install mkcert to run custom CA tests
=================== 3 failed, 52 passed, 9 skipped in 26.09s ====================

ThomasWaldmann · 2024-10-12T22:16:41Z

BTW, I had difficulties running this with nox as it always pulled in a broken pip again.

This fix should really go into next pip release to fix these pains (all was fine until pip 24.2).

Also it needs to get bundled into the recent python releases that bundled the broken pip 24.2.

ThomasWaldmann · 2024-10-12T22:40:54Z

src/truststore/_macos.py

+            0: "Invalid trust result type",
+            # 1: "Trust evaluation succeeded",
+            2: "Trust was explicitly denied",
+            3: "Fatal trust failure occurred",
+            # 4: "Trust result is unspecified (but trusted)",
+            5: "Recoverable trust failure occurred",
+            6: "Unknown error occurred",
+            7: "User confirmation required",


Difficult to find something about these numerical codes.

https://opensource.apple.com/source/Security/Security-55471/sec/Security/SecTrust.h.auto.html says

typedef uint32_t SecTrustResultType; enum { kSecTrustResultInvalid = 0, kSecTrustResultProceed = 1, kSecTrustResultConfirm SEC_DEPRECATED_ATTRIBUTE = 2, kSecTrustResultDeny = 3, kSecTrustResultUnspecified = 4, kSecTrustResultRecoverableTrustFailure = 5, kSecTrustResultFatalTrustFailure = 6, kSecTrustResultOtherError = 7 };

https://github.com/xybp888/iOS-SDKs/blob/master/iPhoneOS13.0.sdk/System/Library/Frameworks/Security.framework/Headers/SecTrust.h#L84 seems to confirm the mapping.

Thanks for this, it's quite frustrating that these values aren't documented in Apple's own API documentation.

ThomasWaldmann · 2024-10-12T23:13:02Z

BTW, I tried to install mkcert to make the skipped tests work, but that failed.

brew pulls in go@1.17 as a requirement to compile mkcert, but 1.17 does not work on macOS 10.12.

Then I tried to brew install go@1.16, but they disabled that some months ago. :-(

andlabs · 2024-10-21T05:38:02Z

Can brew be told to use a system-installed Go? If so you should still be able to download the Go 1.16 installer (expanding "Archived versions" at the bottom), or (if you're willing to do some bootstrapping) build a local one from source into your $HOME.

ThomasWaldmann · 2024-10-21T09:52:26Z

@andlabs brew won't discover/use a system-installed Go, it will just keep trying to pull in the one from the dependencies of mkcert brew package.

But your hint was helpful. I installed 1.16.x pkg from the Go site and did a manual install of mkcert to make the skipped tests work.

ThomasWaldmann · 2024-10-21T10:01:46Z

@sethmlarson Updated test output (now including the tests needing mkcert):

test.log

The code still was using the patch mentioned above: #157 (comment)

sethmlarson · 2024-10-21T17:01:24Z

Thanks much @ThomasWaldmann for fixing the SecTrustResultType codes and running on a real macOS 10.13 machine, I pushed a change and hopefully this is good to merge now.

ThomasWaldmann · 2024-10-21T17:32:22Z

@sethmlarson Did you check #157 (comment) ?

Without that, I could not run the tests because the imports already failed.

sethmlarson · 2024-10-21T18:59:27Z

Thanks @ThomasWaldmann, I didn't see that patch. Does everything look good now?

ThomasWaldmann · 2024-10-21T19:36:36Z

@sethmlarson Looks good (tested on macOS 10.12 64bit):

a fresh test.log was made with unmodified code from this PR: 4 test fails.
copying _macos.py from this PR over the broken one in pip 24.2 makes it work again.

test.log

I didn't investigate the test fails, guess you'll know better. :-)

illume · 2024-10-26T04:21:25Z

Hey folks. Awesome work, and thanks a lot for fixing this. 🎉

There's at least one classroom full of donated old macs that will appreciate this.

sethmlarson commented Oct 2, 2024

View reviewed changes

src/truststore/_macos.py Outdated Show resolved Hide resolved

Use the 'SecTrustEvaluate' API for macOS 10.12 and earlier

61f7830

Co-authored-by: =?UTF-8?q?Ren=C3=A9=20Dudfield?= <renesd@gmail.com>

sethmlarson force-pushed the macos-10.12 branch from ed7756c to 61f7830 Compare October 2, 2024 14:05

sethmlarson added 3 commits October 2, 2024 09:17

Add expected error messages to test cases

6a016c9

Allow ignoring certificate errors

08a98a3

Test both configurations

45aaa05

sethmlarson force-pushed the macos-10.12 branch from 147266f to 45aaa05 Compare October 4, 2024 20:28

sethmlarson requested a review from davisagli October 4, 2024 20:36

ThomasWaldmann mentioned this pull request Oct 11, 2024

WIP: Fix for MacOS 10.13 and older to use SecTrustEvaluate #156

Closed

ThomasWaldmann reviewed Oct 11, 2024

View reviewed changes

This was referenced Oct 11, 2024

vagrant: macos1012 box has issues with pip 24.2 borgbackup/borg#8415

Closed

[24.2] installs fail on macOS 10.12 and older due to bundled truststore issue pypa/pip#12901

Closed

ThomasWaldmann approved these changes Oct 11, 2024

View reviewed changes

notatallshaw mentioned this pull request Oct 12, 2024

Release 24.3 pypa/pip#12941

Closed

ThomasWaldmann reviewed Oct 12, 2024

View reviewed changes

Fix mapping of SecTrustResultType codes

690ffaf

sethmlarson changed the title ~~Use the 'SecTrustEvaluate' API for macOS 10.12 and earlier~~ Use the 'SecTrustEvaluate' API for macOS 10.13 and earlier Oct 21, 2024

Delay ctypes for SecTrustEvaluateWithError

d11a003

sethmlarson merged commit 2a9bf4f into main Oct 21, 2024
9 of 19 checks passed

sethmlarson deleted the macos-10.12 branch October 21, 2024 22:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the 'SecTrustEvaluate' API for macOS 10.13 and earlier #157

Use the 'SecTrustEvaluate' API for macOS 10.13 and earlier #157

sethmlarson commented Oct 2, 2024

sethmlarson commented Oct 4, 2024

sethmlarson commented Oct 10, 2024

ThomasWaldmann Oct 11, 2024

ThomasWaldmann Oct 11, 2024

ThomasWaldmann left a comment

sethmlarson commented Oct 12, 2024

ThomasWaldmann commented Oct 12, 2024

ThomasWaldmann commented Oct 12, 2024 •

edited

Loading

ThomasWaldmann commented Oct 12, 2024

ThomasWaldmann commented Oct 12, 2024 •

edited

Loading

ThomasWaldmann Oct 12, 2024

ThomasWaldmann Oct 12, 2024

sethmlarson Oct 21, 2024

ThomasWaldmann commented Oct 12, 2024 •

edited

Loading

andlabs commented Oct 21, 2024

ThomasWaldmann commented Oct 21, 2024

ThomasWaldmann commented Oct 21, 2024 •

edited

Loading

sethmlarson commented Oct 21, 2024

ThomasWaldmann commented Oct 21, 2024

sethmlarson commented Oct 21, 2024

ThomasWaldmann commented Oct 21, 2024 •

edited

Loading

illume commented Oct 26, 2024

Use the 'SecTrustEvaluate' API for macOS 10.13 and earlier #157

Use the 'SecTrustEvaluate' API for macOS 10.13 and earlier #157

Conversation

sethmlarson commented Oct 2, 2024

sethmlarson commented Oct 4, 2024

sethmlarson commented Oct 10, 2024

ThomasWaldmann Oct 11, 2024

Choose a reason for hiding this comment

ThomasWaldmann Oct 11, 2024

Choose a reason for hiding this comment

ThomasWaldmann left a comment

Choose a reason for hiding this comment

sethmlarson commented Oct 12, 2024

ThomasWaldmann commented Oct 12, 2024

ThomasWaldmann commented Oct 12, 2024 • edited Loading

ThomasWaldmann commented Oct 12, 2024

ThomasWaldmann commented Oct 12, 2024 • edited Loading

ThomasWaldmann Oct 12, 2024

Choose a reason for hiding this comment

ThomasWaldmann Oct 12, 2024

Choose a reason for hiding this comment

sethmlarson Oct 21, 2024

Choose a reason for hiding this comment

ThomasWaldmann commented Oct 12, 2024 • edited Loading

andlabs commented Oct 21, 2024

ThomasWaldmann commented Oct 21, 2024

ThomasWaldmann commented Oct 21, 2024 • edited Loading

sethmlarson commented Oct 21, 2024

ThomasWaldmann commented Oct 21, 2024

sethmlarson commented Oct 21, 2024

ThomasWaldmann commented Oct 21, 2024 • edited Loading

illume commented Oct 26, 2024

ThomasWaldmann commented Oct 12, 2024 •

edited

Loading

ThomasWaldmann commented Oct 12, 2024 •

edited

Loading

ThomasWaldmann commented Oct 12, 2024 •

edited

Loading

ThomasWaldmann commented Oct 21, 2024 •

edited

Loading

ThomasWaldmann commented Oct 21, 2024 •

edited

Loading