From 51d7bfd9a1c71300e0254aca599e3cb2e5df4cce Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Thu, 7 Sep 2023 05:31:32 +0000 Subject: [PATCH] Revert^2 "Only allow setting presigned without preprocessed on targetSdk < 30" This reverts commit d293e28f521f37d4e07c307090e5db829545d774. Reason for revert: The underlying issue was fixed in ag/24685010 Change-Id: I06810d37dba37aa12f9a1e14b0749f1e1eb41136 --- java/app_import.go | 30 ++++++++++++++++++--- java/app_import_test.go | 34 +++++++++--------------- java/base.go | 2 +- java/builder.go | 11 +++++++- java/dex.go | 2 +- java/hiddenapi.go | 2 +- scripts/check_target_sdk_less_than_30.py | 30 +++++++++++++++++++++ 7 files changed, 81 insertions(+), 30 deletions(-) create mode 100755 scripts/check_target_sdk_less_than_30.py diff --git a/java/app_import.go b/java/app_import.go index ad1765e9d..1718d936c 100644 --- a/java/app_import.go +++ b/java/app_import.go @@ -277,6 +277,14 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext a.hideApexVariantFromMake = true } + if Bool(a.properties.Preprocessed) { + if a.properties.Presigned != nil && !*a.properties.Presigned { + ctx.ModuleErrorf("Setting preprocessed: true implies presigned: true, so you cannot set presigned to false") + } + t := true + a.properties.Presigned = &t + } + numCertPropsSet := 0 if String(a.properties.Certificate) != "" { numCertPropsSet++ @@ -288,11 +296,9 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext numCertPropsSet++ } if numCertPropsSet != 1 { - ctx.ModuleErrorf("One and only one of certficate, presigned, and default_dev_cert properties must be set") + ctx.ModuleErrorf("One and only one of certficate, presigned (implied by preprocessed), and default_dev_cert properties must be set") } - _, _, certificates := collectAppDeps(ctx, a, false, false) - // TODO: LOCAL_EXTRACT_APK/LOCAL_EXTRACT_DPI_APK // TODO: LOCAL_PACKAGE_SPLITS @@ -365,6 +371,7 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext } else if !Bool(a.properties.Presigned) { // If the certificate property is empty at this point, default_dev_cert must be set to true. // Which makes processMainCert's behavior for the empty cert string WAI. + _, _, certificates := collectAppDeps(ctx, a, false, false) a.certificate, certificates = processMainCert(a.ModuleBase, String(a.properties.Certificate), certificates, ctx) signed := android.PathForModuleOut(ctx, "signed", apkFilename) var lineageFile android.Path @@ -377,8 +384,13 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext SignAppPackage(ctx, signed, jnisUncompressed, certificates, nil, lineageFile, rotationMinSdkVersion) a.outputFile = signed } else { + // Presigned without Preprocessed shouldn't really be a thing, currently we disallow + // it for apps with targetSdk >= 30, because on those targetSdks you must be using signature + // v2 or later, and signature v2 would be wrecked by uncompressing libs / zipaligning. + // But ideally we would disallow it for all prebuilt apks, and remove the presigned property. + targetSdkCheck := a.validateTargetSdkLessThan30(ctx, srcApk) alignedApk := android.PathForModuleOut(ctx, "zip-aligned", apkFilename) - TransformZipAlign(ctx, alignedApk, jnisUncompressed) + TransformZipAlign(ctx, alignedApk, jnisUncompressed, []android.Path{targetSdkCheck}) a.outputFile = alignedApk a.certificate = PresignedCertificate } @@ -432,6 +444,16 @@ func (a *AndroidAppImport) validatePreprocessedApk(ctx android.ModuleContext, sr }) } +func (a *AndroidAppImport) validateTargetSdkLessThan30(ctx android.ModuleContext, srcApk android.Path) android.Path { + alignmentStamp := android.PathForModuleOut(ctx, "validated-prebuilt", "old_target_sdk.stamp") + ctx.Build(pctx, android.BuildParams{ + Rule: checkBelowTargetSdk30ForNonPreprocessedApks, + Input: srcApk, + Output: alignmentStamp, + }) + return alignmentStamp +} + func (a *AndroidAppImport) Prebuilt() *android.Prebuilt { return &a.prebuilt } diff --git a/java/app_import_test.go b/java/app_import_test.go index bb8fab93b..506c7344b 100644 --- a/java/app_import_test.go +++ b/java/app_import_test.go @@ -629,31 +629,21 @@ func TestAndroidTestImport_Preprocessed(t *testing.T) { presigned: true, preprocessed: true, } - - android_test_import { - name: "foo_cert", - apk: "prebuilts/apk/app.apk", - certificate: "cert/new_cert", - preprocessed: true, - } `) - testModules := []string{"foo", "foo_cert"} - for _, m := range testModules { - apkName := m + ".apk" - variant := ctx.ModuleForTests(m, "android_common") - jniRule := variant.Output("jnis-uncompressed/" + apkName).BuildParams.Rule.String() - if jniRule != android.Cp.String() { - t.Errorf("Unexpected JNI uncompress rule: " + jniRule) - } + apkName := "foo.apk" + variant := ctx.ModuleForTests("foo", "android_common") + jniRule := variant.Output("jnis-uncompressed/" + apkName).BuildParams.Rule.String() + if jniRule != android.Cp.String() { + t.Errorf("Unexpected JNI uncompress rule: " + jniRule) + } - // Make sure signing and aligning were skipped. - if variant.MaybeOutput("signed/"+apkName).Rule != nil { - t.Errorf("signing rule shouldn't be included for preprocessed.") - } - if variant.MaybeOutput("zip-aligned/"+apkName).Rule != nil { - t.Errorf("aligning rule shouldn't be for preprocessed") - } + // Make sure signing and aligning were skipped. + if variant.MaybeOutput("signed/"+apkName).Rule != nil { + t.Errorf("signing rule shouldn't be included for preprocessed.") + } + if variant.MaybeOutput("zip-aligned/"+apkName).Rule != nil { + t.Errorf("aligning rule shouldn't be for preprocessed") } } diff --git a/java/base.go b/java/base.go index 2c0b3ea7c..8f483989e 100644 --- a/java/base.go +++ b/java/base.go @@ -1608,7 +1608,7 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath false, nil, nil) if *j.dexProperties.Uncompress_dex { combinedAlignedJar := android.PathForModuleOut(ctx, "dex-withres-aligned", jarName).OutputPath - TransformZipAlign(ctx, combinedAlignedJar, combinedJar) + TransformZipAlign(ctx, combinedAlignedJar, combinedJar, nil) dexOutputFile = combinedAlignedJar } else { dexOutputFile = combinedJar diff --git a/java/builder.go b/java/builder.go index debf49a00..bf919175d 100644 --- a/java/builder.go +++ b/java/builder.go @@ -277,6 +277,14 @@ var ( Command: `${config.Zip2ZipCmd} -i ${in} -o ${out} -x 'META-INF/services/**/*'`, CommandDeps: []string{"${config.Zip2ZipCmd}"}, }) + + checkBelowTargetSdk30ForNonPreprocessedApks = pctx.AndroidStaticRule("checkBelowTargetSdk30ForNonPreprocessedApks", + blueprint.RuleParams{ + Command: "build/soong/scripts/check_target_sdk_less_than_30.py ${config.Aapt2Cmd} $in $out", + CommandDeps: []string{"build/soong/scripts/check_target_sdk_less_than_30.py", "${config.Aapt2Cmd}"}, + Description: "Check prebuilt target sdk version", + }, + ) ) func init() { @@ -689,12 +697,13 @@ func GenerateMainClassManifest(ctx android.ModuleContext, outputFile android.Wri android.WriteFileRule(ctx, outputFile, "Main-Class: "+mainClass+"\n") } -func TransformZipAlign(ctx android.ModuleContext, outputFile android.WritablePath, inputFile android.Path) { +func TransformZipAlign(ctx android.ModuleContext, outputFile android.WritablePath, inputFile android.Path, validations android.Paths) { ctx.Build(pctx, android.BuildParams{ Rule: zipalign, Description: "align", Input: inputFile, Output: outputFile, + Validations: validations, }) } diff --git a/java/dex.go b/java/dex.go index df501bffa..511ae5eb3 100644 --- a/java/dex.go +++ b/java/dex.go @@ -438,7 +438,7 @@ func (d *dexer) compileDex(ctx android.ModuleContext, dexParams *compileDexParam } if proptools.Bool(d.dexProperties.Uncompress_dex) { alignedJavalibJar := android.PathForModuleOut(ctx, "aligned", dexParams.jarName).OutputPath - TransformZipAlign(ctx, alignedJavalibJar, javalibJar) + TransformZipAlign(ctx, alignedJavalibJar, javalibJar, nil) javalibJar = alignedJavalibJar } diff --git a/java/hiddenapi.go b/java/hiddenapi.go index 4d08b8307..fe3fe7b61 100644 --- a/java/hiddenapi.go +++ b/java/hiddenapi.go @@ -305,7 +305,7 @@ func hiddenAPIEncodeDex(ctx android.ModuleContext, dexInput, flagsCSV android.Pa }) if uncompressDex { - TransformZipAlign(ctx, output, encodeRuleOutput) + TransformZipAlign(ctx, output, encodeRuleOutput, nil) } return output diff --git a/scripts/check_target_sdk_less_than_30.py b/scripts/check_target_sdk_less_than_30.py new file mode 100755 index 000000000..69b0bf011 --- /dev/null +++ b/scripts/check_target_sdk_less_than_30.py @@ -0,0 +1,30 @@ +#!/usr/bin/env python3 + +import subprocess +import argparse +import re +import sys + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument('aapt2', help = "the path to the aapt2 executable") + parser.add_argument('apk', help = "the apk to check") + parser.add_argument('stampfile', help = "a file to touch if successful") + args = parser.parse_args() + + regex = re.compile(r"targetSdkVersion: *'([0-9]+)'") + output = subprocess.check_output([args.aapt2, "dump", "badging", args.apk], text=True) + targetSdkVersion = None + for line in output.splitlines(): + match = regex.fullmatch(line.strip()) + if match: + targetSdkVersion = int(match.group(1)) + break + + if targetSdkVersion is None or targetSdkVersion >= 30: + sys.exit(args.apk + ": Prebuilt, presigned apks with targetSdkVersion >= 30 (or a codename targetSdkVersion) must set preprocessed: true in the Android.bp definition (because they must be signed with signature v2, and the build system would wreck that signature otherwise)") + + subprocess.check_call(["touch", args.stampfile]) + +if __name__ == "__main__": + main()