From 0ae80ed1cab3d2b3fadfef42c9ae2925f8e5f600 Mon Sep 17 00:00:00 2001 From: Justin W Smith <103147162+justsmth@users.noreply.github.com> Date: Fri, 1 Mar 2024 11:54:34 -0500 Subject: [PATCH] Android 14: Don't set execute-only on FIPS .text segment (#1460) ### Description of changes: * Android 14 enforces execute-only memory (XOM) as is being requested by our call to [mprotect](https://man7.org/linux/man-pages/man2/mprotect.2.html) * ARM64 Android is the only platform for which we were explicitly enabling XOM. * The process of moving all static/const variables from `.text` to `.rodata` segments [has been started](https://github.com/aws/aws-lc/commit/a4bc612579925a21e04b8a4f69f481979c5434ff) but is not yet complete. Follow up in CryptoAlg-2360 ### Testing: * Verified locally with Android 14 emulator: ``` 130|emu64a:/data/local/tmp/aws-lc-build $ uname -a Linux localhost 6.6.9-android15-0-g515a956763d8-ab11275718 #1 SMP PREEMPT Thu Jan 4 21:38:14 UTC 2024 aarch64 Toybox emu64a:/data/local/tmp/aws-lc-build $ ./crypto/crypto_test [==========] Running 2420 tests from 127 test suites. [----------] Global test environment set-up. [----------] 2 tests from ABITest [ RUN ] ABITest.SanityCheck [ OK ] ABITest.SanityCheck (0 ms) [ RUN ] ABITest.AArch64 [ OK ] ABITest.AArch64 (1 ms) ... [----------] 120 tests from TrustTokenAllBadKeyTest/TrustTokenBadKeyTest (1990 ms total) [----------] Global test environment tear-down [==========] 2420 tests from 127 test suites ran. (109149 ms total) [ PASSED ] 2418 tests. ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license. --------- Co-authored-by: Andrew Hopkins --- crypto/fipsmodule/bcm.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crypto/fipsmodule/bcm.c b/crypto/fipsmodule/bcm.c index 84be19d683..517e3fdbfe 100644 --- a/crypto/fipsmodule/bcm.c +++ b/crypto/fipsmodule/bcm.c @@ -19,7 +19,7 @@ #include #include -#if defined(BORINGSSL_FIPS) && defined(OPENSSL_ANDROID) +#if defined(BORINGSSL_FIPS) && !defined(OPENSSL_WINDOWS) #include #include #endif @@ -28,6 +28,7 @@ // to control the order. $b section will place bcm in between the start/end markers // which are in $a and $z. #if defined(BORINGSSL_FIPS) && defined(OPENSSL_WINDOWS) + #pragma code_seg(".fipstx$b") #pragma data_seg(".fipsda$b") #pragma const_seg(".fipsco$b") @@ -207,7 +208,9 @@ static void assert_not_within(const void *start, const void *symbol, BORINGSSL_FIPS_abort(); } -#if defined(OPENSSL_ANDROID) && defined(OPENSSL_AARCH64) +// TODO: Re-enable once all data has been moved out of .text segments CryptoAlg-2360 +#if 0 +//#if defined(OPENSSL_ANDROID) && defined(OPENSSL_AARCH64) static void BORINGSSL_maybe_set_module_text_permissions(int permission) { // Android may be compiled in execute-only-memory mode, in which case the // .text segment cannot be read. That conflicts with the need for a FIPS @@ -224,6 +227,8 @@ static void BORINGSSL_maybe_set_module_text_permissions(int permission) { perror("BoringSSL: mprotect"); } } +#else +static void BORINGSSL_maybe_set_module_text_permissions(int _permission) {} #endif // !ANDROID #endif // !ASAN @@ -329,8 +334,7 @@ int BORINGSSL_integrity_test(void) { fprintf(stderr, "HMAC_Init_ex failed.\n"); return 0; } - -#if defined(OPENSSL_ANDROID) && defined(OPENSSL_AARCH64) +#if !defined(OPENSSL_WINDOWS) BORINGSSL_maybe_set_module_text_permissions(PROT_READ | PROT_EXEC); #endif #if defined(BORINGSSL_SHARED_LIBRARY) @@ -347,9 +351,10 @@ int BORINGSSL_integrity_test(void) { #else HMAC_Update(&hmac_ctx, start, end - start); #endif -#if defined(OPENSSL_ANDROID) && defined(OPENSSL_AARCH64) +#if !defined(OPENSSL_WINDOWS) BORINGSSL_maybe_set_module_text_permissions(PROT_EXEC); #endif + if (!HMAC_Final(&hmac_ctx, result, &result_len) || result_len != sizeof(result)) { fprintf(stderr, "HMAC failed.\n");