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

Install fails if hasSystemFeature method not found #1

Closed
CaptainThrowback opened this issue May 30, 2024 · 12 comments
Closed

Install fails if hasSystemFeature method not found #1

CaptainThrowback opened this issue May 30, 2024 · 12 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@CaptainThrowback
Copy link

CaptainThrowback commented May 30, 2024

First of all, great work on this! I just tried to install it, and everything is fine up to the above item. I believe that portion of the process is optional, so perhaps if that method can't be found, the install should continue rather than exit?

Install log below:

  _  __                    _ ____  _   _ 
 | |/ /___ _ __ _ __   ___| / ___|| | | |
 | ' // _ \ '__| '_ \ / _ \ \___ \| | | |
 | . \  __/ |  | | | |  __/ |___) | |_| |
 |_|\_\___|_|  |_| |_|\___|_|____/ \___/ 

- Preparing image
- Module size: 8.22 MB
- Mounting image
- Current boot slot: _a
- Device is system-as-root
Archive:  /data/user/0/me.weishu.kernelsu/cache/module.zip
  inflating: module.prop
**********************
 Framework Patcher GO 
 by changhuapeng 
**********************
*********************
 Powered by KernelSU 
*********************
Archive:  /data/user/0/me.weishu.kernelsu/cache/module.zip
  inflating: customize.sh
- Extracting module files
Archive:  /data/user/0/me.weishu.kernelsu/cache/module.zip
  inflating: customize.sh
  inflating: module.prop
Getting: busybox
Getting: bash
Getting: bin
Getting: core
------------Device INFO------------
API=34
ABI=arm
ABILONG=arm64-v8a
arch=arm64
arch32=arm
max_arch=arm64
is64bit=true
status=Enforcing
encrypted=true
slot=_a
dynamic_partitions=true
virtual_partitions=true
free_root=0
free_system=1071231320064
free_vendor=0
------------Setup INFO------------
CUSTOM_SETUP=2
TMP=/dev/tmp19970
MODPATH=/data/adb/modules_update/FrameworkPatcherGo
SKIPUNZIP=0
KSU=true
KSU_VER=0.9.4-33-gf381e324
KSU_VER_CODE=11871
KSU_KERNEL_VER_CODE=11871
ARCH=arm64
di_version=4.8-b
main_version=4.8
----------------------------------
 
Currently using: "/dev/tmp19970/ugu/unzip" binary
No possible changes found for: "unzip" binary
 
---------Installer Configs----------
devices=off
apex_mount=off
magisk_support=on
ensure_root=on
import_addons=off
extraction_speed=default
permissions=0:0:0755:0644
-----------------------------------
ZIP: Cant find folder [META-INF/addons]
----------------Running SCRIPTs------------
- Setting common permissions/contexts
 
- Installing from KernelSU app
Installation may seem stuck at times, please wait patiently ...
 
******************************
> Decompiling framework.jar ...
******************************
I: Using Apktool 2.7.0 on framework.jar
I: Baksmaling classes.dex...
I: Baksmaling classes2.dex...
I: Baksmaling classes3.dex...
I: Baksmaling classes4.dex...
I: Baksmaling classes5.dex...
I: Copying assets and libs...
I: Copying unknown files...
I: Copying original files...
 
******************************
> Patching AndroidKeyStoreSpi.smali file...
******************************
--------------------
Patching engineGetCertificateChain method:

    invoke-static {v3}, Lcom/android/internal/util/framework/Android;->engineGetCertificateChain([Ljava/security/cert/Certificate;)[Ljava/security/cert/Certificate;

    move-result-object v3

added.
Edited: "/dev/tmp19970/framework/smali_classes3/android/security/keystore2/AndroidKeyStoreSpi.smali"
--------------------
 
******************************
> Patching Instrumentation.smali file ...
******************************
 
--------------------
Patching newApplication static method:

    invoke-static {p1}, Lcom/android/internal/util/framework/Android;->newApplication(Landroid/content/Context;)V

added.
Edited: "/dev/tmp19970/framework/smali/android/app/Instrumentation.smali"
--------------------
 
--------------------
Patching newApplication method:

    invoke-static {p3}, Lcom/android/internal/util/framework/Android;->newApplication(Landroid/content/Context;)V

added.
Edited: "/dev/tmp19970/framework/smali/android/app/Instrumentation.smali"
--------------------
 
******************************
> Patching ApplicationPackageManager.smali file ...
******************************
sed: unmatched '/'
Name register not found in hasSystemFeature method
 Mounting:auto:ro:1: /dev/block/dm-3 /system_ext
 Mounting:auto:ro:1: /dev/block/dm-4 /vendor
 Mounting:auto:ro:1: /dev/block/dm-1 /product
 Mounting:auto:ro:1: /dev/block/dm-0 /odm
- Error: Failed to install module script
Error: Failed to install module script

EDIT: It could also just be an error with the sed command, upon reviewing the above output...

EDIT 2: Seems like the sed command reporting the error is from line 158 in META-INF/com/google/android/magisk/customize.sh, which uses the "name_regex" variable from line 157. That's the one that seems to be causing the error.

EDIT 3: Okay, I did some actual Debugging by adding a few echo commands to the code to see where that sed error is coming from. This is what I added:

ui_print " "
ui_print "******************************"
ui_print "> Patching ApplicationPackageManager.smali file ..."
ui_print "******************************"
apm_line=$(grep -w "$app_package_manager_file" -e "$apm_method")
echo -e "DEBUG: apm_line: $apm_line"
apm_method_code=$(string -f "$app_package_manager_file" extract "$apm_line" ".end method")
echo -e "DEBUG: apm_method_code: $apm_method_code"

And this is the corresponding log output:

******************************
> Patching ApplicationPackageManager.smali file ...
******************************
DEBUG: apm_line:     invoke-virtual {p0, v0}, Landroid/app/ApplicationPackageManager;->hasSystemFeature(Ljava/lang/String;)Z
.method public whitelist hasSystemFeature(Ljava/lang/String;)Z
sed: unmatched '/'
DEBUG: apm_method_code: 
Name register not found in hasSystemFeature method

So I guess the issue is the apm_method_code variable? Something in that command is causing a sed error, and making that variable end up blank.

@VisionR1
Copy link

Yeah as I see, after fix it, if can add a option like, if you want the patch for ApplicationPackageManager.smali press the Volume+ if not press Volume-

@changhuapeng
Copy link
Owner

changhuapeng commented May 31, 2024

I believe that portion of the process is optional, so perhaps if that method can't be found, the install should continue rather than exit?

I agree. Will make the patching portion to be optional in the upcoming release.

And this is the corresponding log output:

******************************
> Patching ApplicationPackageManager.smali file ...
******************************
DEBUG: apm_line:     invoke-virtual {p0, v0}, Landroid/app/ApplicationPackageManager;->hasSystemFeature(Ljava/lang/String;)Z
.method public whitelist hasSystemFeature(Ljava/lang/String;)Z
sed: unmatched '/'
DEBUG: apm_method_code: 
Name register not found in hasSystemFeature method

Thanks for the debug log. The log does shows some unexpected output from $apm_line, which I'm thinking is what causing the error. Please try this test version to see if it solves the issue that @VisionR1 and you are encountering when patching the hasSystemFeature method.

FrameworkPatcherGo-test.zip

@VisionR1
Copy link

VisionR1 commented May 31, 2024

@changhuapeng
Unfortunately still say the same.
IMG_20240530_202438

And mine, original (A11) ApplicationPackageManager.smali
IMG_20240531_142417

@CaptainThrowback
Copy link
Author

@changhuapeng
Unfortunately still say the same.
IMG_20240530_202438

And mine, original (A11) ApplicationPackageManager.smali
IMG_20240531_142417

Same result with the test build here for me.

@changhuapeng
Copy link
Owner

changhuapeng commented May 31, 2024

Thanks for testing. Would it be possible for the both of you to send me your unedited framework.jar? You can get it by running adb pull /system/framework/framework.jar

@changhuapeng changhuapeng self-assigned this May 31, 2024
@changhuapeng changhuapeng added bug Something isn't working enhancement New feature or request labels May 31, 2024
@CaptainThrowback
Copy link
Author

Thanks for testing. Would it be possible for the both of you to send me your unedited framework.jar? You can get it by running adb pull /system/framework/framework.jar

framework.zip

@VisionR1
Copy link

Thanks for testing. Would it be possible for the both of you to send me your unedited framework.jar? You can get it by running adb pull /system/framework/framework.jar

framework.zip

@changhuapeng
Copy link
Owner

Hopefully this will do the job: FrameworkPatcherGo-test2.zip

@VisionR1
Copy link

VisionR1 commented May 31, 2024

Hopefully this will do the job: FrameworkPatcherGo-test2.zip

Yeah, this working, even in emulator i tried it and it works.
Thanks for all your hard work on this!
And to see if all is added proper, I decompile the classes.dex and 2 all is the same.
So now this is fix it, if you can add a option like, if you want the patch for ApplicationPackageManager.smali press the Volume+ if not press Volume-

@CaptainThrowback
Copy link
Author

CaptainThrowback commented May 31, 2024

Hopefully this will do the job: FrameworkPatcherGo-test2.zip

Yes! This one works.

Interestingly, on the version where I commented out the "optional" portion so that the patch would complete, I ended up with a "bad signature" error in Key Attestation Demo, resulting in the certificate appearing to be revoked. However, now that it patched completely, the app no longer shows that error.

Unfortunately, I'm still only passing Basic Integrity, so I still need to find a different keybox that actually works. I tried adding a different keybox with 3 certs, but it ends up in a bootloop. I'm going to try changing the props to see if maybe I need to use props from a device that supports HARDWARE-BACKED Attestation.

Screenshot_20240531-110730_Key_Attestation.png

Screenshot_20240531-110750_Google_Play_Store.png

Thanks for all your hard work on this!

P.S. While the installation and patching went fine, I think the logging may be slightly incorrect, as seen below:

******************************
> Patching ApplicationPackageManager.smali file ...
******************************
 
--------------------
Patching hasSystemFeature method:
 
    move-result v0
 
replaced by:

    move-result v0

Edited: "/dev/tmp2065/framework/smali/android/app/ApplicationPackageManager.smali"
--------------------

    invoke-static {v0, p1}, Lcom/android/internal/util/framework/Android;->hasSystemFeature(ZLjava/lang/String;)Z

    move-result v0

added.
Edited: "/dev/tmp2065/framework/smali/android/app/ApplicationPackageManager.smali"
--------------------
 
    return v0
 
replaced by:

    return v0

Edited: "/dev/tmp2065/framework/smali/android/app/ApplicationPackageManager.smali"
--------------------

The "replaced by" section appears to be the same as what precedes it, so not sure what it's supposed to be showing there. But again, that's just a visual error and the actual patching appears to be working correctly.

@VisionR1
Copy link

VisionR1 commented May 31, 2024

Edited: "/dev/tmp2065/framework/smali/android/app/ApplicationPackageManager.smali"


The "replaced by" section appears to be the same as what precedes it, so not sure what it's supposed to be showing there. But again, that's just a visual error and the actual patching appears to be working correctly.

Yeah and mine the same, but like you say is just a visual error, the main purpose works.

@changhuapeng
Copy link
Owner

Thanks to you both for testing and verifying that this new fix is working correctly! Upcoming release will contain this fix as well as providing user a choice to patch ApplicationPackageManager or not.

The "replaced by" section appears to be the same as what precedes it, so not sure what it's supposed to be showing there. But again, that's just a visual error and the actual patching appears to be working correctly.

That's not a visual bug since the script actually did replaced the original string with a new similar string. The reason is that from the few framework.jar that I referenced from during development, none of the original string was the correct string right from the start. Hence, the script wasn't expecting such and simply overwriting what was originally correct with what it should be corrected with. I supposed I can write a simple check to fix this confusion. Thanks for highlighting this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants