Test suite crashes on OS X w/ SIGBUS #391
Comments
|
I can replicate this with the current cryptography master on my Mac. No idea why we're not seeing this on our CI though. This is the vector that causes the failure (and this script fails with a bus error on my Mac) from cryptography.hazmat.backends.openssl.backend import backend
from cryptography.hazmat.primitives.asymmetric import ec
vector = {'y': 17502091976454499611505625879374774917623246385936044091564633236054L, 'x': 1075547452530705099852971289727135188477809665711500674593974877078L, 'curve': 'secp224r1', 'd': 24409883047820789832454338490434427874387169505509737608417835723568L}
numbers = ec.EllipticCurvePrivateNumbers(
vector['d'],
ec.EllipticCurvePublicNumbers(
vector['x'],
vector['y'],
ec.SECP224R1()
)
)
key = numbers.private_key(backend) |
|
FWIW, it doesn’t crash on my local Ubuntu with OpenSSL 1.0.1f too. Do you think that’s a bug in 1.0.2 on OS X? |
|
It only happens with universal builds of OpenSSL, so it's definitely OS X only. I don't have a good explanation for why this occurs right now though.
|
|
Figured it out. Here we go... Cryptography binds functions that may or may not be available. Unfortunately, cffi provides us no method by which we can say "include this symbol if present, otherwise leave it out". Instead we need to call In this particular case we declared a flag #if defined(OPENSSL_NO_EC) || OPENSSL_VERSION_NUMBER < 0x1000100f || \
defined(OPENSSL_NO_EC_NISTP_64_GCC_128)
static const long Cryptography_HAS_EC_NISTP_64_GCC_128 = 0;
const EC_METHOD *(*EC_GFp_nistp224_method)(void) = NULL;
const EC_METHOD *(*EC_GFp_nistp256_method)(void) = NULL;
const EC_METHOD *(*EC_GFp_nistp521_method)(void) = NULL;
#else
static const long Cryptography_HAS_EC_NISTP_64_GCC_128 = 1;
#endifSo far so good. The problem arises when building a universal library on OS X and then linking against the static version of it. To understand that we need a quick digression into what a universal library is on OS X... Universal libraries are simply multiple CPU architectures crammed together in one file (typically using a tool called When compiling cryptography (or any project) against a universal lib you specify the headers and then the library location (if they're not already in your include and lib search paths) and then the compiler does the rest. Unfortunately, universal libraries apparently assume that the headers will be the same between architectures. This is not true for any project that leaks build flags into the public headers. In OpenSSL's case this comes through with Of course, this should still be caught right? After all, the # ifndef OPENSSL_NO_EC_NISTP_64_GCC_128
/** Returns 64-bit optimized methods for nistp224
* \return EC_METHOD object
*/
const EC_METHOD *EC_GFp_nistp224_method(void);
/** Returns 64-bit optimized methods for nistp256
* \return EC_METHOD object
*/
const EC_METHOD *EC_GFp_nistp256_method(void);
/** Returns 64-bit optimized methods for nistp521
* \return EC_METHOD object
*/
const EC_METHOD *EC_GFp_nistp521_method(void);
# endifBut wait, even if this all gets compiled the symbol is still present in the Finally, if we're removing the function conditionally why is it being called and failing? Well, internally Summary and What We're Gonna Do™So, when running in 64-bit mode linked against a universal OpenSSL built using homebrew the current cryptography release (and master) will The functions in question have no need to be bound at all (they're really internal implementation details and are of no benefit to us), so dropping them from the bindings will resolve this issue. Going forward, I'm going to propose altering our NULL function pointer approach to instead be something more like const EC_METHOD *EC_GFp_nistp224_method(void) {
printf("EC_GFp_nistp224_method was called. Aborting!\n");
abort();
}rather than const EC_METHOD *(*EC_GFp_nistp224_method)(void) = NULL; |
|
This bug is stupid and @reaperhulk is awesome for catching it. Well done! |
|
This has been fixed by the amazing @reaperhulk. |
This is a new one:
The backtrace:
Help us @reaperhulk you’re our only hope.
The text was updated successfully, but these errors were encountered: