lineageos4microg / docker-lineage-cicd

Docker microservice for LineageOS Continuous Integration and Continous Deployment
https://hub.docker.com/r/lineageos4microg/docker-lineage-cicd
GNU General Public License v3.0
480 stars 189 forks source link

Android 14 signature spoofing patch fails due to API changes #607

Closed nnsee closed 2 months ago

nnsee commented 3 months ago

Hello!

The Android 14 signature spoofing patches fail to compile. The error seems to be:

FAILED: out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava/api-stubs-docs-non-updatable-stubs.srcjar out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava/api-stubs-docs-non-updatable_annotations.zip out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava/api-stubs-docs-non-updatable_api.txt out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava/api-stubs-docs-non-updatable_removed.txt out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava/api_lint.timestamp out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava/api_lint_baseline.txt out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava/api_lint_report.txt out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava/check_last_released_api.timestamp out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava/last_released_baseline.txt
out/host/linux-x86/bin/sbox --sandbox-path out/soong/.temp --output-dir out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava --manifest out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava.sbox.textproto --write-if-changed
The failing command was run inside an sbox sandbox in temporary directory
out/soong/.temp/sbox/dcabbf69fbe8e32ae16e786d21b129ef1759ab95
The failing command line can be found in
out/soong/.temp/sbox/dcabbf69fbe8e32ae16e786d21b129ef1759ab95/sbox_command.0.bash
out/srcjars/android/Manifest.java:2770: error: New API must be flagged with @FlaggedApi: field android.Manifest.permission.FAKE_PACKAGE_SIGNATURE [UnflaggedApi]
out/srcjars/android/Manifest.java:6391: error: New API must be flagged with @FlaggedApi: field android.Manifest.permission_group.FAKE_PACKAGE [UnflaggedApi]
Error: metalava detected the following problems:
out/srcjars/android/Manifest.java:2770: error: New API must be flagged with @FlaggedApi: field android.Manifest.permission.FAKE_PACKAGE_SIGNATURE [UnflaggedApi]
out/srcjars/android/Manifest.java:6391: error: New API must be flagged with @FlaggedApi: field android.Manifest.permission_group.FAKE_PACKAGE [UnflaggedApi]
metalava wrote updated baseline to /srv/src/LINEAGE_21_0/out/soong/.temp/sbox/dcabbf69fbe8e32ae16e786d21b129ef1759ab95/./out/api_lint_baseline.txt
metalava wrote updated baseline to /srv/src/LINEAGE_21_0/out/soong/.temp/sbox/dcabbf69fbe8e32ae16e786d21b129ef1759ab95/./out/last_released_baseline.txt
************************************************************
Your API changes are triggering API Lint warnings or errors.
To make these errors go away, fix the code according to the
error and/or warning messages above.

If it is not possible to do so, there are workarounds:

1. You can suppress the errors with @SuppressLint("<id>")
   where the <id> is given in brackets in the error message above.
2. You can update the baseline by executing the following
   command:
       (cd $ANDROID_BUILD_TOP && cp \
       "out/soong/.intermediates/frameworks/base/api/api-stubs-docs-non-updatable/android_common/metalava/api_lint_baseline.txt" \
       "frameworks/base/core/api/lint-baseline.txt")
   To submit the revised baseline.txt to the main Android
   repository, you will need approval.
************************************************************
exit status 255

Full log can be found here: lineage-21.0-20240408-nns-lemonadep.log.gz

I'm aware that LineageOS now ships with a signature spoofing feature built-in, however, this only applies to debuggable builds (such as userdebug) and not regular user builds. I have to use user builds for technical reasons, so I would appreciate if it were still possible to apply the signature spoofing patch.

I can alternatively write another patch which enables the built-in signature spoofing even in user builds. However, I'm unsure how I would do this - where are patches supposed to go and what are they supposed to look like so that they would be picked up by the build system, is there any documentation for this? Thanks.

petefoth commented 3 months ago

@nnsee thanks for the report.

The patches for Android 14 / lineage-21.1 were added in this commit. They got some testing, but only in a userdebug build (because that's what the project builds, and we don't have the time or resources to test all possible combinations of env variables). Also, they were pretty much done by 'copy & paste' from the Android 13 patches, so it is quite possible there is stuff missing or wrong.

It looks to me as if there are two possible ways of fixing this:

  1. Modify our patches, adding or changing whatever is necessary so that the patches do build correctly in user builds:
  2. Create and maintain a new patch or patches that modify the LineageOS changes to allow them to work in user builds.

A third alternative would be to do both, so that it is possible to make user builds using either LOS changes or the LOS change plus our patches. (This is the current situation for userdebug builds)

I don't have the necessary skills and knowledge to do either 1. or 2, and I don't have time, knowledge or resources to test and debug any changes. If you are able to work out what changes are needed to implement 1 and / or 2, I would be happy to accept them as a pull request. Or if you are not confident about creating PRs, I could take a set of patch files, and work out how to add them to our code.

Thanks again for any help you can give with this

EDIT: PS it would be useful to know the docker run... command you used to make the failing build, so hat we can try to reproduce the error. Thanks

FintasticMan commented 3 months ago

I would say the second option would be the best way to go. To make a patch, you'll want to modify the LineageOS/android_frameworks_base repository, and remove the 4 lines of code starting here. Then you can use git to create a patch.

To get the docker image to pick the patch up, you'll need to replace the contents of the android_frameworks_base-Android14.patch file with your new patch, delete the packages_modules_Permission-Android14.patch file and remove this line. If I haven't missed anything, it should then work with user builds as well as userdebug builds.

I could create a PR doing that if you'd like, but I can't test if it works right now.

petefoth commented 3 months ago

I would say the second option would be the best way to go. To make a patch, you'll want to modify the LineageOS/android_frameworks_base repository, and remove the 4 lines of code starting here. Then you can use git to create a patch.

To get the docker image to pick the patch up, you'll need to replace the contents of the android_frameworks_base-Android14.patch file with your new patch, delete the packages_modules_Permission-Android14.patch file and remove this line. If I haven't missed anything, it should then work with user builds as well as userdebug builds.

From what I can see, that would work for builds where SIGNATURE_SPOOFING is set to restricted. It wouldn't work if SIGNATURE_SPOOFING is set to yes, because the LOS changes only support signature spoofing for GmsCore. For unrestricted signature spoofing to work, we would also need to do the first option, allowing our signature spoofing patches to work in 21.0 user builds.

Alternatively, we can choose to support only restricted signature spoofing in 21.0 and later builds. That would be easier, but less 'user friendly' for anyone who uses our docker engine to make builds with unrestricted signature spoofing. I have no idea whether anyone actually does use it in that way: it may be we can afford not to worry about that :)

And, whatever we choose to do needs to be documented in README.md

FintasticMan commented 3 months ago

I think that not providing support for the unrestricted patch is fine. I used to use the unrestricted patch, but the only reason was that I wasn't installing microG as a system app. This isn't a problem with LOS's signature spoofing, as it works fine with microG as a user app.

We'd probably want a separate option for patching for user builds, as I don't see a reason to apply a patch if we build userdebug.

petefoth commented 3 months ago

OK. Issue marked as 'Fix sometime' and 'Help wanted'. I'd be happy to accept a Pull Request with the required changes

nnsee commented 3 months ago

I've currently been building with the unrestricted patches:

docker run \
    -e "BRANCH_NAME=lineage-21.0" \
    -e "DEVICE_LIST=lemonadep" \
    -e "SIGN_BUILDS=true" \
    -e "ZIP_SUBDIR=false" \
    -e "WITH_GMS=true" \
    -e "SIGNATURE_SPOOFING=yes" \
    -e "OTA_URL=https://[snip]/api" \
    -e "RELEASE_TYPE=nns" \
    -e "USER_NAME=[snip]" \
    -e "USER_MAIL=[snip]" \
    -e "BUILD_TYPE=user" \
    -v "$(pwd)/ccache:/srv/ccache" \
    -v "$(pwd)/keys:/srv/keys" \
    -v "$(pwd)/local_manifests:/srv/local_manifests" \
    -v "$(pwd)/logs:/srv/logs" \
    -v "$(pwd)/mirror:/srv/mirror" \
    -v "$(pwd)/src:/srv/src" \
    -v "$(pwd)/tmp:/srv/tmp" \
    -v "$(pwd)/userscripts:/srv/userscripts" \
    -v "$(pwd)/zips:/srv/zips" \
    lineageos4microg/docker-lineage-cicd

... but as far as I know, I don't actually need it and I'm (personally) fine with the patches being restricted to microG.

I'll have a go at building with the patch that bypasses the isDebuggable check and report back, and formulate it into a PR when I have the time (probably later tonight).

nnsee commented 3 months ago

I've tested with the commit above and the LOS signature spoofing seems to now work fine with an user build. Will draft a PR later.

FintasticMan commented 3 months ago

Now that #608 has been merged, should this be closed?

petefoth commented 3 months ago

Now that #608 has been merged, should this be closed?

No because the changes have been merged into a testing branch. We can close this issue after the changes have been tested and then merged into the master branch

FintasticMan commented 3 months ago

Ah I misread your message then, oops!

videoman614 commented 3 months ago

Not sure if this is related but I use fakestore2playstore to install the patched Play Store to activate the license for Quick Cursor for accessibility reasons since microG even on the latest with some experimental license verification doesn't work with Quick Cursor, then remove it afterwards.

Had no issues on the 20 builds but I tested the newer 21 builds on panther and it just doesn't work anymore no matter what I try, this is one of those situations where if it doesn't work then I'm forced to stick with 20 because it's near unusable for me without Quick Cursor.

Not working on 21

Screenshot

Working on 20

Screenshot
petefoth commented 3 months ago

Not sure if this is related but I use fakestore2playstore to install the patched Play Store to activate the license for Quick Cursor for accessibility reasons since microG even on the latest with some experimental license verification doesn't work with Quick Cursor, then remove it afterwards.

Had no issues on the 20 builds but I tested the newer 21 builds on panther and it just doesn't work anymore no matter what I try, this is one of those situations where if it doesn't work then I'm forced to stick with 20 because it's near unusable for me without Quick Cursor.

Not working on 21 Screenshot

Working on 20 Screenshot

Please raise this as a new issue. Your problems don't seem to be anything to do with signature spoofing patches in user builds which us the subject on this issue.

It looks like the problem may be either

Whatever, this issue is not the place to raise or discuss it :)

petefoth commented 2 months ago

We can close this issue after the changes have been tested a

Is anyone testing the proposed changes? I don't have time to do it. See #612 for an outline of what tests need to be done before merging the changes

petefoth commented 2 months ago

The Android 14 signature spoofing patches fail to compile.

Do they fail to compile for all builds, or only for user builds?

FintasticMan commented 2 months ago

They also fail for userdebug builds.

petefoth commented 2 months ago

Any reason why just adding the @FlaggedAPI annotation to the new APIs in our patches?

The original description of this problem conflates two issues

  1. Our signature spoofing patches fail to build because of the missing @FlaggedAPI annotation for the new APIs
  2. The upstream changes which allow signature spoofing only work for 'debuggablebuilds (e.g.userdebug) and not foruser` builds.

IMO the two issue are separate, and should be fixed separately. The changes in the nns-v21-spoofing-patches branch and PR #612 fixes 2 but doesn't fix 1; it removes the patches rather than fixing them. That is one solution to the problem, but I want to thnk about it a bit more before agreeing to go down that route

petefoth commented 2 months ago

The changes in the nns-v21-spoofing-patches branch and PR #612 fixes 2 but doesn't fix 1; it removes the patches rather than fixing them. That is one solution to the problem, but I want to thnk about it a bit more before agreeing to go down that route

OK I've thought about this, and explored the problem, and the right way to fix 1 (the issue of the patches not building) is to follow the text in the error message

out/srcjars/android/Manifest.java:2770: error: New API must be flagged with @FlaggedApi: field android.Manifest.permission.FAKE_PACKAGE_SIGNATURE [UnflaggedApi]
out/srcjars/android/Manifest.java:6391: error: New API must be flagged with @FlaggedApi: field android.Manifest.permission_group.FAKE_PACKAGE [UnflaggedApi]

and add the @FlaggedApi annotation.

I will do that in a new branch and test it (by making a 21.0 userdebug build for sunfish with SIGNATURE_SPOOFING=yes)

Once that is done we can look at using the code in the nns-v21-spoofing-patches branch to fix https://github.com/lineageos4microg/docker-lineage-cicd/issues/614

johnjacob10 commented 2 months ago

So how did you add the @FlaggedApi annotation?

My userdebug builds for redfin have been failing the same API must be flagged error since March with SIGNATURE_SPOOFING=restricted. The build completes successfully if I set SIGNATURE_SPOOFING=no.

I have been trying to add the FlaggedApi annotation, but I can't figure out how. The only documentation I could find is here but the links in that page don't work for me (they ask for a Google corporate login).

It would be helpful if you could share your pull request for this. Or if you haven't done it yet, we could share information and try to figure it out together?

petefoth commented 2 months ago

So how did you add the @FlaggedApi annotation?

I didn't. I think I fixed the compilation failed error it in this commit. no changes to our patches, we just need to regenerate custom.txt. The changes are in the pf-fix-A14-patches branch

I'm probably ready to create a PR with the changes, but I got sidetracked by #616 (which was an upstream issue and has now gone away). I'll try and get the PR done and merged in the next few days, but in the mean time, you can see the changes made in this comparison https://github.com/lineageos4microg/docker-lineage-cicd/compare/master...pf-fix-A14-patches

for now, building with the docker-lineage-cicd:pf-fix-A14-patches should work. Please let me know if it doesn't docker pull lineageos4microg/docker-lineage-cicd:pf-fix-A14-patches

petefoth commented 2 months ago

Fix has been re-applied in new new-fix-A14-patches) branch, which I am currently testing

And we do need the @FlaggedApi annotation in core/res/AndroidManifest.xml- added in this commit

petefoth commented 2 months ago

Mon 29 Apr 24

Ready to create PR and merge

petefoth commented 2 months ago

The PR #619 is created. I've tested it but it could do with a review before I merge it, if anyone has time today or tomorrow morning. @FintasticMan @johnjacob10 @nnsee ?

nnsee commented 2 months ago

Looks good to me!

johnjacob10 commented 2 months ago

My redfin build with SIGNATURE_SPOOFING=restricted was successful and works fine. So this is a go from me!