kornelski
Implement old version fallback using `dlsym`
rustls-platform-verifier (nowadays used in major Rust projects like Rustup) uses security-framework, and enables the "OSX_10_14" Cargo feature.
As such, trying to run rustup update on a machine with macOS 10.12 installed (which the Rust project otherwise still supports) results in the following dynamic linker error:
info: syncing channel updates for 'stable-x86_64-apple-darwin'
dyld: lazy symbol binding failed: Symbol not found: _SecTrustEvaluateWithError
Referenced from: /Users/madsmtm/.cargo/bin/rustup
Expected in: /System/Library/Frameworks/Security.framework/Versions/A/Security
dyld: Symbol not found: _SecTrustEvaluateWithError
Referenced from: /Users/madsmtm/.cargo/bin/rustup
Expected in: /System/Library/Frameworks/Security.framework/Versions/A/Security
Abort trap: 6
And trying to compile it manually results in a static linker error:
error: linking with `cc` failed: exit status: 1
|
= note: "cc" "$WORKSPACE/*.o" "$WORKSPACE/*.rlib" "-framework" "Security" "-liconv" "-framework" "CoreFoundation" "-lSystem" "-lc" "-lm" "-arch" "x86_64" "-mmacosx-version-min=10.12.0" "-o" "$WORKSPACE/target/debug/deps/rustls_platform_verifier-f3c84624becba9b5" "-Wl,-dead_strip" "-nodefaultlibs"
= note: Undefined symbols for architecture x86_64:
"_SecTrustEvaluateWithError", referenced from:
security_framework::trust::SecTrust::evaluate_with_error::hb6ce8f9ed18e6bcb in libsecurity_framework-063e6ee960d31664.rlib(security_framework-063e6ee960d31664.security_framework.d276614c73a19b26-cgu.10.rcgu.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
error: could not compile `rustls-platform-verifier` (lib test) due to 1 previous error
This could of course be "fixed" in rustls-platform-verifier by simply not enabling the feature at all, but there's (probably) a reason why they've chosen to do so. Ideally, we should be able to use SecTrustEvaluateWithError when available, and only need to fall back to SecTrustEvaluate when not available.
Weak linking offers such a mechanism, though it has been unstable for years with difficult blockers, so another solution would be to look the symbol up dynamically using dlsym. Something like the following would work:
let fnptr = dlsym(RTLD_DEFAULT, c"SecTrustEvaluateWithError".as_ptr());
let fnptr = mem::transmute<*const c_void, Option<unsafe extern "C" fn(SecTrustRef, *mut CFErrorRef) -> bool>>();
if let Some(fnptr) = fnptr {
// SecTrustEvaluateWithError available
fnptr(...)
} else {
// Not available, fall back to SecTrustEvaluate
}
Does the library work on 10.12 without the OSX_10_14 feature?
It seems to me that if 10.12 support is required, then they shouldn't be configuring the library to require 10.14 as the minimum OS version. I get that Cargo features are a pain, but that's the interface we have to work with.
I could add some compatibility workarounds for older OSes when the OSX_10_14 feature is disabled. Having hacks for older OSes when the "I don't need the hacks for older OSes" feature is enabled goes against its purpose.
Does the library work on 10.12 without the OSX_10_14 feature?
The tests do not pass because they expect specific error messages on cert validation failure (which SecTrustEvaluate won't give them). Too unfamiliar with rustls-platform-verifier to tell for certain, but I'm fairly sure it isn't required.
I could add some compatibility workarounds for older OSes when the OSX_10_14 feature is disabled. Having hacks for older OSes when the "I don't need the hacks for older OSes" feature is enabled goes against its purpose.
That's what I was arguing for, yeah. The problem with disabling the "OSX_10_14" feature right now is that it worsens the experience for users on macOS 10.14 or above.
To be clear, the ideal solution to me would be something like the following:
#[cfg(feature = "OSX_10_14")]
{
SecTrustEvaluateWithError(...); // Use directly
}
#[cfg(not(feature = "OSX_10_14"))]
{
let fnptr = dlsym(RTLD_DEFAULT, c"SecTrustEvaluateWithError".as_ptr());
// ...
}