Skip to content

Commit

Permalink
Android 14: Don't set execute-only on FIPS .text segment (#1460)
Browse files Browse the repository at this point in the history
### 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](a4bc612)
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 <andhop@amazon.com>
  • Loading branch information
justsmth and andrewhop authored Mar 1, 2024
1 parent 7600809 commit 0ae80ed
Showing 1 changed file with 10 additions and 5 deletions.
15 changes: 10 additions & 5 deletions crypto/fipsmodule/bcm.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include <openssl/crypto.h>

#include <stdlib.h>
#if defined(BORINGSSL_FIPS) && defined(OPENSSL_ANDROID)
#if defined(BORINGSSL_FIPS) && !defined(OPENSSL_WINDOWS)
#include <sys/mman.h>
#include <unistd.h>
#endif
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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");
Expand Down

0 comments on commit 0ae80ed

Please sign in to comment.