Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid executing ELF files directly #24

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

fornwall
Copy link
Member

@fornwall fornwall commented Oct 2, 2023

This is change to work around the Android 10 R^W violation (execution of binary files are not allowed).

See the README for more information.

Helping with testing

Help to test this would be great! For context and more information, see Problem 1 and Solution 1 in the updated README.

NOTE: May totally break your system and need a reinstallation (or require a manual repair in a failsafe session - see below).

Steps (on aarch64 - let me know if you want to test on some other arch):

  • Be sure you have updated packages:
    • pkg up
  • Take a backup (if this is not a installation that you can reinstall termux quickly):
    • cp $PREFIX/lib/libtermux-exec.so $PREFIX/lib/libtermux-exec.so.bak
  • Download and install a test build of termux-exec containing this code:
    • curl -o ~/te-997.deb https://fornwall.me/te-997.deb && apt install --reinstall ~/te-997.deb

After this, new terminal sessions will be using the updated termux-exec version. A way to check this is by running ls -l /proc/self/exe - it should show a symlink pointing to a path ending in bin/linker64, since the linker is what is being executed.

Try things out (run scripts, compile programs, etc) and report if something does not work as a comment in this issue!

This should only affect behaviour on Android 10+ devices, but if you have an old Android version you are welcome to test that as well - that nothing has changed there.

Helping with testing: Debugging tools

  • export TERMUX_EXEC_DEBUG=1 to get verbose logging to stderr
    • This log output may interfere with programs
  • export TERMUX_EXEC_OPTOUT=1 to opt out from termux-exec - it will not do any modification to execve calls

Helping with testing: Known issues

  • Statically linked executables such as zig does not work (linker errors with has unexpected e_type: 2)

Helping with testing: To restore

If things are not completely broken, apt install --reinstall termux-exec=1:1.0 will install the stable version of termux-exec.

If the installation is completely broken, so you can't even start a shell:

  • Start a failsafe session by dragging out the drawer from the left, long press on the NEW SESSION button, and click FAILSAFE in the dialog that shows up.
  • cd $PREFIX/lib && cp libtermux-exec.so.bak libtermux-exec.so
  • You can now start a normal shell session again
    • Restore the termux-exec package properly with: apt install --reinstall termux-exec=1:1.0

Updates

  • 2023-10-17 (file te-997.deb, apt show termux-exec should show version 9:9.7):
    • Various fixes to edge cases and logging
    • Introduce TERMUX_SELF_EXE, the absolute path to the executed file, which can be used as a /proc/self/exec replacement.
  • 2023-10-05
    • Various fixes to edge cases and logging

fornwall added a commit to termux-play-store/termux-packages that referenced this pull request Oct 2, 2023
LLVM looks at /proc/self/exe to determine which binary has been called.
This does not work with the proposed changes for termux-exec:
termux/termux-exec#24

Luckily the fallback to look at argv0 works instead.
fornwall added a commit to termux-play-store/termux-packages that referenced this pull request Oct 2, 2023
LLVM looks at /proc/self/exe to determine which binary has been called.
This does not work with the proposed changes for termux-exec:
termux/termux-exec#24

Luckily the fallback to look at argv0 works instead.
fornwall added a commit to termux-play-store/termux-packages that referenced this pull request Oct 2, 2023
Without -f/--full, pgrep&pkill matches searches by default only on
the binary name, which will not work with the changes proposed in
termux/termux-exec#24, as there the binary
will always be /system/bin/linker64.
fornwall added a commit to termux-play-store/termux-packages that referenced this pull request Oct 2, 2023
Without -f/--full, pgrep&pkill matches searches by default only on
the binary name, which will not work with the changes proposed in
termux/termux-exec#24, as there the binary
will always be /system/bin/linker64.
@fornwall fornwall force-pushed the master branch 4 times, most recently from 2a852ec to 6baed3d Compare October 2, 2023 14:22
@agnostic-apollo
Copy link
Member

Very interesting solution. But yeah, it likely won't be acceptable by play store policies, but could be tried. Although not sure if android could add further selinux restrictions to prevent this.

Btw are you sure in testing, you uninstalled all termux apps, rebooted, and reinstalled, since otherwise untrusted_app_27 selinux process would still be assigned to the app, you can confirm with Termux in-app settings -> About, just making sure.

The executable being /system/bin/linker64 issues may be significant though.

termux-exec.c Outdated

#ifndef TERMUX_BASE_DIR
# define TERMUX_BASE_DIR "/data/data/com.termux/files"
#define TERMUX_BASE_DIR "/data/data/com.termux/files"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use hardcoded com.termux values here and elsewhere, it affects termux forks. To get prefix, first check TERMUX__PREFIX env variable, then PREFIX, otherwise default to package build time path @TERMUX_BASE_DIR@. In future, multiple rootfs support may also be added and $PREFIX has been deprecated in favour of TERMUX__PREFIX (note double dash as scope), as it conflicts with other packages like npm, etc. Haven't pushed change to termux-app yet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I'll update!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to look at TERMUX__PREFIX now, with a TERMUX_BASE_DIR value given at compilation time as fallback.

I think PREFIX is too dangerous to use, as you say it can totally break when reused for other purposes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. But do not use literal com.termux value. The whole path @TERMUX_BASE_DIR@ should be replaced during build time in the source by value set in properties.sh. Possibly tests should use this too.

https://github.com/termux/termux-packages/blob/26b05538619ffd93cfd52f252f34d1c025d51be7/scripts/properties.sh#L34

For inspiration, check termux-tools design by grimler.

https://github.com/termux/termux-tools/blob/master/configure.ac

https://github.com/termux/termux-tools/blob/master/Makefile.am

Also do not limit prefix length to 48, package names can be much longer. For example /data/user/10/net.dinglisch.android.taskerm/files will fail with length 49. Even /data/data paths can be longer. Package name alone on playstore can be of length 50. Probably just check if it starts with / since technically packages can be compiled for any prefix, even /data/local/tmp for adb or /mnt paths for external sd.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good idea not to use $PREFIX, will have to push TERMUX__PREFIX export to termux-app as well.

Copy link
Member Author

@fornwall fornwall Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But do not use literal com.termux value

With the current version (I made changes and squashed commits): We start by using the TERMUX__PREFIX env variable now. If that does not exist, we fall back to TERMUX_BASE_DIR.

TERMUX_BASE_DIR is given at compilation time, as in make TERMUX_BASE_DIR=/a/path/to/package/prefix (the
It's not really hardcoded any longer, but supplied at compile time (the existing package already supplies that at build time: https://github.com/termux/termux-packages/blob/master/packages/termux-exec/build.sh#L10).

Remaining usage of com.termux are ok I think:

  • In Makefile, as a default value if make is executed without any argument
  • In termux-exec.c, but only inside #ifdef UNIT_TEST, so only used for testing. We also test with other prefixes as well in the tests.
  • In test-program.c, which is just a utility test program that can be run manually, not part of the package

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your thorough review and feedback @agnostic-apollo !

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will check code in detail later, outside, but if app is installed on adoptable external sd, then its app data parent directory prefix will be /mnt/expand/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx/user/0, which alone has length 55, so 100 is not going to be enough, especially if multiple sub dirs under package name.

You are very welcome. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so 100 is not going to be enough

Thanks again! So what about 200, that should be enough for everyone perhaps :)? That allows a path such as /mnt/expand/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx/user/0/yyyyyyyyyy-yyyyyyyyyy/yyyyyyyyyy-yyyyyyyyyyyyyyyyyyyy/yyyyyyyyyyyyyyyyyyyy/yyyyyyyyyyyyyyyyyyyy-yyyyyy yyyy/xxxxx-xxxxx/yyyyy-yyyyy/zzzzzz-zzzzz/.

The reason I don't want an arbitrary big length is that it would be good to have a reasonable tight limit - this code is going to be injected in a lot of places, so not taking up more stack or heap space than necessary would be great, to decrease risks of interfering with the surrounding problem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lolz technically the only valid length is PATH_MAX(4096 bytes), but that would be for the full path, not just the prefix. That is also what libraries including mine use as the standard for passing to sys calls. You could reduce it if necessary, but couple of hundred bytes won't matter, but at least log a warning if value is ignored.

termux-exec.c Outdated Show resolved Hide resolved
@sylirre
Copy link
Member

sylirre commented Oct 2, 2023

But yeah, it likely won't be acceptable by play store policies, but could be tried.

As Play Store accepts apps that run binaries by proot and have target SDK 29+, I doubt that would be an issue. Maybe they consider the whole purpose of app (e.g. terminal emulators run binaries and need exec but games do not), who knows...

@ETERNALBLUEbullrun

This comment was marked as off-topic.

@agnostic-apollo
Copy link
Member

I think it may be more likely that they don't know proot is being used, otherwise if someone took interest, proot and this clearly does violate the policy.

An app distributed via Google Play may not modify, replace, or update itself using any method other than Google Play's update mechanism. Likewise, an app may not download executable code (such as dex, JAR, .so files) from a source other than Google Play. This restriction does not apply to code that runs in a virtual machine or an interpreter where either provides indirect access to Android APIs (such as JavaScript in a webview or browser).
Apps or third-party code, like SDKs, with interpreted languages (JavaScript, Python, Lua, etc.) loaded at run time (for example, not packaged with the app) must not allow potential violations of Google Play policies.

https://support.google.com/googleplay/android-developer/answer/9888379?hl=en#zippy=%2Cexamples-of-common-violations

@fornwall
Copy link
Member Author

fornwall commented Oct 2, 2023

Btw are you sure in testing, you uninstalled all termux apps, rebooted, and reinstalled, since otherwise untrusted_app_27 selinux process would still be assigned to the app, you can confirm with Termux in-app settings -> About, just making sure.

The executable being /system/bin/linker64 issues may be significant though.

Thanks, I noticed the behaviour you described as well, was really confusing first! But now I can confirm that I am indeed testing with effective targetSdk of 34! I'll share a branch of termux-app for testing shortly.

@fornwall
Copy link
Member Author

fornwall commented Oct 2, 2023

I think we should focus on the technical issue here, and leave Google Play policy out of the discussion, since I think that's a separate one. This change is hopefully good on its own, for bumping targetSdk also for distribution outside of Google Play.

@agnostic-apollo
Copy link
Member

Thanks, I noticed the behaviour you described as well, was really confusing first! But now I can confirm that I am indeed testing with effective targetSdk of 34! I'll share a branch of termux-app for testing shortly.

Welcome. Android also sometimes assigns latest target sdk SE_INFO, even if using older target sdk, and requires a reboot to fix. Have added checks for that too locally. I am far ahead of termux-app master locally, most of the non-terminal stuff has been rewritten/refactored, will be pushing in coming weeks when I manage to complete the changes.

leave Google Play policy out of the discussion

Fair enough. targetSdkVersion is far important, I also don't care too much about playstore either.

@agnostic-apollo

This comment was marked as off-topic.

@termux termux deleted a comment from ETERNALBLUEbullrun Oct 2, 2023
@ETERNALBLUEbullrun

This comment was marked as off-topic.

@twaik
Copy link
Member

twaik commented Oct 2, 2023

That solution may be problematic. Linker on old Android version does not allow to do this. But I am not sure when it starts.

@chenxiaolong
Copy link

That solution may be problematic. Linker on old Android version does not allow to do this. But I am not sure when it starts.

Looks like it was introduced in https://android.googlesource.com/platform/bionic/+/8f639a40966c630c64166d2657da3ee641303194, which was first included in Android 8.

@Grimler91
Copy link
Member

That solution may be problematic. Linker on old Android version does not allow to do this. But I am not sure when it starts.

Looks like it was introduced in https://android.googlesource.com/platform/bionic/+/8f639a40966c630c64166d2657da3ee641303194, which was first included in Android 8.

Only supporting android >= 8 (or even a version or two higher) for an app re-introduced to google play seems reasonable to me

@sylirre
Copy link
Member

sylirre commented Oct 2, 2023

Android 10+ solution is not needed to be used on older versions. It can be enabled on-demand, possibly with letting user to opt-out from this solution. Don't forget that:

  • Not everyone uses Android 10 or higher there. Some people actually use Termux to make obsolete devices useful, e.g. to run home server.
  • Some people have SELinux in permissive mode, whether intentionally or because custom ROM doesn't support enforcing. In this case exec restriction doesn't work.

Though I found one problem with this solution: users will no longer be able to run static binaries. When I attempt to run static binary, I get error unexpected e_type: 2. Can't remember any Termux package using static binaries, but sideloaded third-party prebuilt software definitely will no longer run without proot.

agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 20, 2024
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 20, 2024
…__S_TERMUX` in patch files, currently for `$TERMUX_EXEC__PROC_SELF_EXE`

Related pull termux/termux-exec#24
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 20, 2024
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 20, 2024
…__S_TERMUX` in patch files, currently for `$TERMUX_EXEC__PROC_SELF_EXE`

Related pull termux/termux-exec#24
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 21, 2024
@agnostic-apollo
Copy link
Member

agnostic-apollo commented Dec 21, 2024

I have pushed the new changes to https://github.com/termux/termux-exec/tree/2.0.0-prerelease

@fornwall you can push them yourself to this pull branch by the running following commands from this repo. If we merge my changes from this pull, then it will require changing commit hash in a commit message and docs and will need to merged locally with fast forward.

git checkout master
git reset --hard f3ae554e
git remote add upstream [email protected]:termux/termux-exec.git
git fetch upstream 2.0.0-prerelease
git checkout -b 2.0.0-prerelease upstream/2.0.0-prerelease
git checkout master
git merge --ff-only 2.0.0-prerelease
git push --force origin master

New Changes Since Last Push

  • For system_linker_exec, executable or interpreter paths will now be checked under TERMUX_APP__DATA_DIR or TERMUX_APP__LEGACY_DATA_DIR instead of TERMUX__ROOTFS as files could be executed from TERMUX__APPS_DIR and TERMUX__CACHE_DIR, which are not under the Termux rootfs. Additionally, Termux rootfs may not exist under app data directory at all and could be under another directory under Android rootfs /, like if compiling packages for shell user for the com.android.shell package with the Termux rootfs under /data/local/tmp instead of /data/data/com.android.shell (and using force mode) or compiling packages for /system directory.
  • The $TERMUX__PREFIX (and $TERMUX_APP__DATA_DIR) environment variables are now lazy read only if they are actually needed instead of being always read.
  • Read ANDROID__BUILD_VERSION_SDK environment variable exported by Termux app to get Android build version sdk, and if its not set, then from the android_get_device_api_level() call provided by <android/api-level.h>, which gets it from the system properties, which should be slower (untested).
  • Use find to build or move all matched files instead of specifying them individually.
  • Create or move all build files under the build/ directory. It's better to build a directory structure for the files to be added to prefix under the build/usr directory than have a mix of using files under both src/ and build/ during installation. This way src/ directory also doesn't get modified and will not contain the files with replaced constants that were created from *.in files. This also makes it easier to clean and build the deb since all built files exist under the same directory, which termux-create-package can also use with source_recurse.
  • The recursive replacement of termux constants is done with find -exec sed with TERMUX_CONSTANTS_SED_ARGS passed to it. Using foreach with the call function for replace-termux-constants does not work for some reason if multiple files exists, because somehow function definition itself like @sed gets passed to sed command as argument. So replace-termux-constants is only called for individual files. The sed replacements surround arguments with double quotes instead of single quotes, so it could potentially break on more cases during shell expansion, although regex variables weren't being escaped before either.
  • The termux-exec-package.json will now have correct version and be consistent with build.sh, and also include tests files, which wasn't being done before.
  • The termux-exec-package.json previously had hardcoded aarch64 as architecture, now we find and replace it for the compiler based on which predefined architecture macro is defined.
  • There was a typo in default TERMUX_API_APP_ scope used for version name environment variable that was preventing termux-api tests to run when termux-exec was built with make manually. @Grimler91 reported this earlier.
  • Use TERMUX_EXEC_PKG__VERSION instead of TERMUX_EXEC__VERSION.
  • Added termux-core, tudo and sudo tests.
  • Added tests for Termux app data and prefix directory, and checking if path is under Termux app data directory.
  • Changed termux-exec-tests scope from TERMUX_EXEC_TESTS__ to TERMUX_EXEC__TESTS__.
  • Using new design for reading TERMUX*__ scoped variables in termux-exec-tests. Previously, the way build time placeholders @TERMUX_VAR@ were being used inside shell code everywhere would break shell syntax checkers and highlighters and made code harder to read. Now the required variables from the scope set in TERMUX_ENV__S_* (like FOO_ instead of default TERMUX_) are read into the TERMUX*__ scoped variables with the same variable sub name at the start of the script, and then the TERMUX*__ scoped variables are used afterwards where ever they need to be referenced, like FOO__PREFIX may be copied to TERMUX__PREFIX. The termux-scoped-env-variable from the termux-core package does not need to be used to get the variable values and names as termux-exec is built with our build infrastructure in which @TERMUX_ENV__S_*@ can be replaced during build time. The TERMUX_API_APP__APP_VERSION_NAME is read from the termux-apps-info.env file using termux-apps-app-version-name.bash, the file isn't created yet by Termux app.

 

Check termux-scoped-env-variable, termux-apps-info-env-variable and termux-apps-info-app-version-name utils docs added in termux-core package for more info on new scoped design. I need to implement the new scoped environment in Termux app, haven't done that yet, things had to be finalized first, I seem satisfied now.

 

 

 

Run tests with TERMUX_EXEC__LOG_LEVEL=1 TERMUX_API_APP__APP_VERSION_NAME=0.50.1 TERMUX_ROOTFS__PACKAGE_MANAGER=apt "${TERMUX__PREFIX:-$PREFIX}/libexec/installed-tests/termux-exec/termux-exec-tests" -vv all.

Install the termux-exec, and optionally the termux-core, tudo and sudo packages from termux/termux-packages#18872 action run or build them locally, build.sh files have comments for local file:// builds.

Exporting TERMUX_API_APP__APP_VERSION_NAME and TERMUX_ROOTFS__PACKAGE_MANAGER variables is necessary for running tests on older Termux app versions without scoped environment variable changes for package manager and Termux:API app tests to run.

To only run termux-exec package specific tests and not other Termux packages/commands, pass --only-termux-exec-tests.
To skip termux-core package tests, pass --skip-termux-core-tests, as under normal circumstances with Termux app in foreground, tests take ~5min to run depending on device as there are 8708 test variants that are run. Older devices will take longer.
If running sudo package has been installed and its tests are run, then sometimes stdout is returned in an unsynchronized manner which fails the validation, rerun the tests and they should pass.

 

Respective pulls/releases for termux-exec, termux-core, tudo and sudo packages are at following links. tudo and sudo will be provided by Termxu repositories now and sudo will replace tsu. All these packages and termux-exec support Android 5/6 as well now.

 

The primary issue currently remaining is that Pixel 7 Pro on Android 14 has some unknown performance problem. It takes ~26min to run the same termux-core 8708 tests that run in 3-5mins on AVD or older devices. The issue is not termux-exec as with all the changes added in this pull, there has been no performance loss as per tests compared to 1.0. Additionally, I tried the tests with 1.0 as well with targetSdkVersion=28 and it took the same time. Also tried with selinux disabled.

Although, interestingly, tests take longer on each Android version increment tested.

Not sure what the issue is. termux-exec, system_linker_exec, targetSdkVersion and selinux don't seem to be it. External storage is never used so that can't affect time with all its emulation. It would be good if others test it on devices with newer/latest Android versions, maybe its just performance regression that has already been fixed, my Pixel 7 Pro has security patch from Jan 2024, I could update to new patches or Android 15, but maybe would be good to find the cause before updating.

A few tests were added after some of the below tests were already run, so there can be few second variance in test time, although that already happens on reruns.

Android 14 aarch64 (Pixel 7 Pro):
termux-exec 1.0 (direct, targetSdkVersion=28): 24min, 58s
termux-exec 2.0.0 (direct, targetSdkVersion=28): 26min, 49s
termux-exec 2.0.0 (system_linker_exec, targetSdkVersion=35): 27min, 23s
termux-exec 2.0.0 (direct, selinux_disabled, targetSdkVersion=35): 27min, 23s

Android 15 x86_64 (AVD):
termux-exec 1.0 (direct, targetSdkVersion=28): 4min, 7s
termux-exec 2.0.0 (direct, targetSdkVersion=28): 3min, 58s
termux-exec 2.0.0 (direct, root, targetSdkVersion=28): 4min, 14s
termux-exec 2.0.0 (system_linker_exec, targetSdkVersion=28): 3min, 44s
termux-exec 2.0.0 (system_linker_exec, targetSdkVersion=35): 3min, 44s //fixme

Android 11 aarch64 (Poco F2 Pro):
termux-exec 1.0 (direct, targetSdkVersion=28): 5min, 24s
termux-exec 2.0.0 (direct, targetSdkVersion=28): 5min, 34s
termux-exec 2.0.0 (system_linker_exec, targetSdkVersion=28): 5min, 37s

Android 11 x86 (AVD):
termux-exec 2.0.0 (direct, targetSdkVersion=28): 3min, 5s

Android 7 aarch64 (LG G5):
termux-exec 2.0.0 (direct, targetSdkVersion=28): 14min, 17s

Android 7 arm (AVD, emulated on x86_64 host):
termux-exec 2.0.0 (direct, targetSdkVersion=28): 46min, 29s

Android 6 x86_64 (AVD):
termux-exec 2.0.0 (direct, targetSdkVersion=28): 1min, 50s

Android 5 x86_64 (AVD):
termux-exec 2.0.0 (direct, targetSdkVersion=28): 1min, 28s

agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 21, 2024
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 21, 2024
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 21, 2024
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 21, 2024
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 22, 2024
@agnostic-apollo
Copy link
Member

The termux-core, termux-exec, tudo and sudo packages can now be tested from the https://github.com/termux/termux-packages/actions/runs/12453170903?pr=18872 run of the termux/termux-packages#18872 pull for Android >= 7.

agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 22, 2024
@fornwall
Copy link
Member Author

@agnostic-apollo I've invited you to get write permissions to the https://github.com/termux-play-store/termux-exec repository, so you should be able to push to the master branch (from which this PR is created from) - does it seem to work?

@agnostic-apollo
Copy link
Member

Looks like that worked, thanks a lot. That should make things easier.

…ENSE` to `licenses/termux__termux-exec__Apache-2.0.md` file and use `debian/copyright` format in `LICENSE` file

The `LICENSE` file will list other licenses in future.

Also fix copyright statement.
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 27, 2024
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Dec 27, 2024
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Jan 5, 2025
agnostic-apollo added a commit to termux/termux-packages that referenced this pull request Jan 5, 2025
@agnostic-apollo agnostic-apollo force-pushed the master branch 2 times, most recently from c816aab to f460791 Compare January 6, 2025 00:49
…anges in f261b26 for `system_linker_exec`

- The `src/exec-variants.c` has been moved to `src/exec/exec-variants.c` which handles hooks into the entire `exec()` family of functions except `execve()`. The handling of `execve` done by `src/exe/termux-exec.c` has been moved to `src/exec/exec.c`. The `src/exe/termux-exec.c` now exclusively only defines the functions intercepted by `libtermux-exec.so` and directs them to their hooks in respective source files.
- The `__attribute__((visibility("default")))` needs to be set for all the intercepted functions in `src/exe/termux-exec.c` as `libtermux-exec.so` is compiled with `-fvisibility=hidden`, so that no other function in the library other than ones explicitly labelled are exported. This can be confirmed by running `nm --demangle --dynamic --defined-only --extern-only /home/builder/.termux-build/termux-exec/src/build/usr/lib/libtermux-exec.so` after building the package.
- Revert order of functions in `src/exec/exec_variants.c` back to the one in AOSP and uncomment the `NULL` path checks for `execvpe()` and add its original license to file header. Log entries are also added to each variant to know which variant was originally hooked.

- Removed the singular `TERMUX_EXEC_OPTOUT` environment variable. Opt outs should be confined to specific hooks and logics.
- Added the `bool` environment variable `TERMUX_EXEC__INTERCEPT_EXECVE` for whether `execve` would be hooked for shebang fix or `system_linker_exec`. The default value is `true`. The other wrapper functions in the `exec()` family of functions declared in `unistd.h` are always intercepted to solve some other issues on older Android versions, check [`libc/bionic/exec.cpp`](https://cs.android.com/android/platform/superproject/+/android-14.0.0_r1:bionic/libc/bionic/exec.cpp) git history.
- Added the `string` `TERMUX_EXEC__SYSTEM_LINKER_EXEC` environment variable for whether to use `system_linker_exec` if `TERMUX_EXEC__INTERCEPT_EXECVE` is enabled. If set to `disable`, `system_linker_exec` will be disabled. If set to `enable`, then `system_linker_exec` will be enabled but only if required. If set to `force`, then `system_linker_exec` will be force enabled even if not required and is supported. The default value is `enable`. Check `should_system_linker_exec()` in `exec.h` and `exec.h` for more info and how its handled. Docs will be added in a later commit. The `system_linker_exec` will now engage for executable or interpreter paths that are under `TERMUX_APP__DATA_DIR` or `TERMUX_APP__LEGACY_DATA_DIR` instead of `TERMUX__ROOTFS` (`TERMUX_BASE_DIR`).
- Read `ANDROID__BUILD_VERSION_SDK` environment variable exported by Termux app to get Android build version sdk, and if its not set, then from the `android_get_device_api_level()` call provided by `<android/api-level.h>`, which gets it from the system properties, which should be slower.

- Port normalize and absolutize path functions from `portal-io` library and AOSP that has been tested on hundreds of test cases to handle all the required cases.
- Fix the order for normalize, absolutize and replacing termux bin prefix for both executable path and interpreter, check comment in `execve_intercept()`.
- Fix relative paths for interpreter path by absolutizing it. Previously, only prefix was being replaced.

- Fix `argv[0]` for executing shell scripts where it should be set to the original interpreter set in the file as is instead of the `argv[0]` to `execve()` being hooked for the executable.

- Increase buffer size for executable file shebang header from `256` to `340` defined by `FILE_HEADER__BUFFER_LEN` as per termux path limits, check comment in `exec.h` file and [Termux File Path Limits](https://github.com/termux/termux-packages/wiki/Termux-file-system-layout#file-path-limits) docs.
- Decrease max length of valid `TERMUX__ROOTFS` from `200` to `86` defined by `TERMUX__ROOTFS_DIR_MAX_LEN`, check `termux_files.h` file and [Termux File Path Limits](https://github.com/termux/termux-packages/wiki/Termux-file-system-layout#file-path-limits) docs.

- Add buffer overflow checks in `termux_prefix_path()` and return errors for it.
- Fix replacing prefix if termux rootfs is `/` or `/system` and executable equals the bin directory itself instead of a subfile.

- Fix `fexecve()` where executable path would be `/proc/self/fd/<n>` and checking if its under Termux app data directory directory would give wrong results. The `is_path_under_termux_app_data_dir()` function handles this by getting real path of file and ensuring that the real path is for the same file for which fd was open by comparing `stat.st_dev` and `stat.st_ino`.
- Fix checking if executable is under Termux directories, like Termux app data directory (previously rootfs/base). Previously, `strstr(executable_path, termux_base_dir)` was used, which would check first occurrence of the substring `termux_base_dir` in `executable_path`, and not whether `executable_path` is under `termux_base_dir`. This is now properly handled by `is_path_in_dir_path()` via `is_path_under_termux_app_data_dir()`.

- Do not use `TERMUX__PREFIX` to get Termux rootfs directory by getting its parent directory and use `TERMUX__ROOTFS` environment variable directly. There may also be cases where they `TERMUX__PREFIX` equals `TERMUX__ROOTFS`, and getting parent directory would result in wrong results. If environment variable is not set or is invalid as per `TERMUX__ROOTFS_DIR_MAX_LEN`, then we use the default Termux prefix for which package was compiled for as long as its executable and readable to ensure termux-exec was not compiled for a different package.

- Fix the environment `envp` being copied twice during `execve` for system bin paths, once for unsetting `LD_` variables and then to set `TERMUX_EXEC__PROC_SELF_EXE`. The `modify_exec_env()` function now handles all changes to environment with a single copy.
- Fix `TERMUX_EXEC__PROC_SELF_EXE` being set even if `system_linker_exec` is not being used on `targetSdkVersion <= 28`, etc, and also not being unset when going from `system_linker_exec` to direct execution like system binaries. Packages will be patched to detect if `system_linker_exec` is being used to modify their behaviour, which shouldn't be modified for direct execution.
- The `modify_exec_args()` function now handles all changes to the arguments.
- Fix issues where `errno` may already be set when `execve` is entered, check comment in `init()` function of `termux-exec.c` where it is set to `0`.

- Fix hardcoded `com.termux` values being used, all constants are replaced during building including the root scope of environment variables that are read and as per `TERMUX_ENV__S_ROOT` defined in `properties.sh` of `termux-pacakges` as `TERMUX_`.

- Fix `string_starts_with()` giving wrong results for `NULL` and empty strings.

- Added logger framework with multiple log levels with log entries for all the important variable states to track logic. The singular `TERMUX_EXEC_DEBUG` environment variable has been removed. The `int` `TERMUX_EXEC__LOG_LEVEL` environment variable controls the log level based on `(OFF=0, NORMAL=1, DEBUG=2, VERBOSE=3, VVERBOSE=4 and VVVERBOSE=5)`. The default value is `1`. Normally, `termux-exec` does not log anything at default log level `1` (`NORMAL`) for hooks even and will require setting log level to `>= 2` (`DEBUG`) to see log messages. To enable `VVERBOSE` logging for a command, you can run something like `TERMUX_EXEC__LOG_LEVEL=4 id -u`.

- Added `TERMUX_EXEC_PKG__VERSION` `Makefile` parameter that gets logged on hook for `termux-exec` package version currently installed.

- Create or move all build files under the `build/` directory. It's better to build a directory structure for the files to be added to prefix under the `build/usr` directory than have a mix of using files under both `src/` and `build/` during installation. This way `src/` directory also doesn't get modified and will not contain the files with replaced constants that were created from `*.in` files. This also makes it easier to clean and build the deb since all built files exist under the same directory, which `termux-create-package` can also use with `source_recurse`.

- Move unit tests from `termux-exec.c` to `tests/unit-tests.c` since tests shouldn't be in source files.
- Added testing framework via `tests/termux-exec-tests.in` that calls `tests/unit-tests.c` to run unit tests, and `runtime-binary-tests.c` and `tests/runtime-script-tests.in` for runtime tests. Old tests files in random places have been removed. The testing is done for `termux-exec` and other termux internal packages including `termux-core`, `termux-tools`, `termux-am`, `termux-api`, `tudo` and `sudo` to ensure a fully working termux environment. The entire `exec()` family of functions is also tested by `runtime-binary-tests.c`.  Currently, `termux-api` will not work in secondary users due to abstact namespace sockets being used. Docs will be added in a later commit. In future, tests can be moved to each package's own repo and a separate "tests caller" package can be added that runs tests for all packages that are installed. Tests can be run with `TERMUX_EXEC__LOG_LEVEL=1 "${TERMUX__PREFIX:-$PREFIX}/libexec/installed-tests/termux-exec/termux-exec-tests -vv all"`.

- The recursive replacement of termux constants is done with `find -exec sed` with `TERMUX_CONSTANTS_SED_ARGS` passed to it. Using `foreach` with the `call` function for `replace-termux-constants` does not work for some reason if multiple files exists, because somehow function definition itself like `@sed` gets passed to `sed` command as argument. So `replace-termux-constants` is only called for individual files. The sed replacements surround arguments with double quotes instead of single quotes, so it could potentially break on more cases during shell expansion, although regex variables weren't being escaped before either.

- The `termux-exec-package.json` will now have correct version and be consistent with `build.sh`, and also include tests files, which wasn't being done before.
- The `termux-exec-package.json` previously had hardcoded `aarch64` as architecture, now we find and replace it for the compiler based on which predefined architecture macro is defined.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.