From de8417c7076949606bd3767e6112d1e2390d9678 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thi=C3=A9baud=20Weksteen?= Date: Thu, 10 Feb 2022 15:41:46 +1100 Subject: [PATCH] Add AIDL enforce_permissions attribute When set to true, this attribute will pass down the -Wmissing-permission-annotation flag to the aidl compiler. It is possible to declare a set of exceptions (for a graduable adoption). For now, only Java is supported. Test: build having the attribute enabled for frameworks/base Bug: 220214993 Change-Id: I54350199b4d980aef0050519e3daf1fef616d08c --- java/base.go | 25 ++++++++++++++++++++++++- java/droiddoc.go | 2 +- java/gen.go | 15 ++++++++++++--- java/java_test.go | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 5 deletions(-) diff --git a/java/base.go b/java/base.go index 42d7733ca..6ff2d0383 100644 --- a/java/base.go +++ b/java/base.go @@ -227,6 +227,12 @@ type DeviceProperties struct { // whether to generate Binder#GetTransaction name method. Generate_get_transaction_name *bool + // whether all interfaces should be annotated with required permissions. + Enforce_permissions *bool + + // allowlist for interfaces that (temporarily) do not require annotation for permissions. + Enforce_permissions_exceptions []string `android:"path"` + // list of flags that will be passed to the AIDL compiler Flags []string } @@ -418,7 +424,8 @@ type Module struct { outputFile android.Path extraOutputFiles android.Paths - exportAidlIncludeDirs android.Paths + exportAidlIncludeDirs android.Paths + ignoredAidlPermissionList android.Paths logtagsSrcs android.Paths @@ -772,6 +779,17 @@ func (j *Module) hasSrcExt(ext string) bool { return hasSrcExt(j.properties.Srcs, ext) } +func (j *Module) individualAidlFlags(ctx android.ModuleContext, aidlFile android.Path) string { + var flags string + + if Bool(j.deviceProperties.Aidl.Enforce_permissions) { + if !android.InList(aidlFile.String(), j.ignoredAidlPermissionList.Strings()) { + flags = "-Wmissing-permission-annotation -Werror" + } + } + return flags +} + func (j *Module) aidlFlags(ctx android.ModuleContext, aidlPreprocess android.OptionalPath, aidlIncludeDirs android.Paths) (string, android.Paths) { @@ -814,6 +832,11 @@ func (j *Module) aidlFlags(ctx android.ModuleContext, aidlPreprocess android.Opt flags = append(flags, "--transaction_names") } + if Bool(j.deviceProperties.Aidl.Enforce_permissions) { + exceptions := j.deviceProperties.Aidl.Enforce_permissions_exceptions + j.ignoredAidlPermissionList = android.PathsForModuleSrcExcludes(ctx, exceptions, nil) + } + aidlMinSdkVersion := j.MinSdkVersion(ctx).ApiLevel.String() flags = append(flags, "--min_sdk_version="+aidlMinSdkVersion) diff --git a/java/droiddoc.go b/java/droiddoc.go index c84a15c1f..023d61912 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -330,7 +330,7 @@ func (j *Javadoc) genSources(ctx android.ModuleContext, srcFiles android.Paths, // Process all aidl files together to support sharding them into one or more rules that produce srcjars. if len(aidlSrcs) > 0 { - srcJarFiles := genAidl(ctx, aidlSrcs, flags.aidlFlags+aidlIncludeFlags, flags.aidlDeps) + srcJarFiles := genAidl(ctx, aidlSrcs, flags.aidlFlags+aidlIncludeFlags, nil, flags.aidlDeps) outSrcFiles = append(outSrcFiles, srcJarFiles...) } diff --git a/java/gen.go b/java/gen.go index 445a2d8a3..1572bf00a 100644 --- a/java/gen.go +++ b/java/gen.go @@ -44,7 +44,7 @@ var ( }) ) -func genAidl(ctx android.ModuleContext, aidlFiles android.Paths, aidlFlags string, deps android.Paths) android.Paths { +func genAidl(ctx android.ModuleContext, aidlFiles android.Paths, aidlGlobalFlags string, aidlIndividualFlags map[string]string, deps android.Paths) android.Paths { // Shard aidl files into groups of 50 to avoid having to recompile all of them if one changes and to avoid // hitting command line length limits. shards := android.ShardPaths(aidlFiles, 50) @@ -61,15 +61,17 @@ func genAidl(ctx android.ModuleContext, aidlFiles android.Paths, aidlFlags strin rule.Command().Text("rm -rf").Flag(outDir.String()) rule.Command().Text("mkdir -p").Flag(outDir.String()) - rule.Command().Text("FLAGS=' " + aidlFlags + "'") + rule.Command().Text("FLAGS=' " + aidlGlobalFlags + "'") for _, aidlFile := range shard { + localFlag := aidlIndividualFlags[aidlFile.String()] depFile := srcJarFile.InSameDir(ctx, aidlFile.String()+".d") javaFile := outDir.Join(ctx, pathtools.ReplaceExtension(aidlFile.String(), "java")) rule.Command(). Tool(ctx.Config().HostToolPath(ctx, "aidl")). FlagWithDepFile("-d", depFile). Flag("$FLAGS"). + Flag(localFlag). Input(aidlFile). Output(javaFile). Implicits(deps) @@ -159,7 +161,14 @@ func (j *Module) genSources(ctx android.ModuleContext, srcFiles android.Paths, // Process all aidl files together to support sharding them into one or more rules that produce srcjars. if len(aidlSrcs) > 0 { - srcJarFiles := genAidl(ctx, aidlSrcs, flags.aidlFlags+aidlIncludeFlags, flags.aidlDeps) + individualFlags := make(map[string]string) + for _, aidlSrc := range aidlSrcs { + flags := j.individualAidlFlags(ctx, aidlSrc) + if flags != "" { + individualFlags[aidlSrc.String()] = flags + } + } + srcJarFiles := genAidl(ctx, aidlSrcs, flags.aidlFlags+aidlIncludeFlags, individualFlags, flags.aidlDeps) outSrcFiles = append(outSrcFiles, srcJarFiles...) } diff --git a/java/java_test.go b/java/java_test.go index 21c76b6c1..f095c5e48 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -1333,6 +1333,42 @@ func TestAidlFlagsWithMinSdkVersion(t *testing.T) { } } +func TestAidlEnforcePermissions(t *testing.T) { + ctx, _ := testJava(t, ` + java_library { + name: "foo", + srcs: ["aidl/foo/IFoo.aidl"], + aidl: { enforce_permissions: true }, + } + `) + + aidlCommand := ctx.ModuleForTests("foo", "android_common").Rule("aidl").RuleParams.Command + expectedAidlFlag := "-Wmissing-permission-annotation -Werror" + if !strings.Contains(aidlCommand, expectedAidlFlag) { + t.Errorf("aidl command %q does not contain %q", aidlCommand, expectedAidlFlag) + } +} + +func TestAidlEnforcePermissionsException(t *testing.T) { + ctx, _ := testJava(t, ` + java_library { + name: "foo", + srcs: ["aidl/foo/IFoo.aidl", "aidl/foo/IFoo2.aidl"], + aidl: { enforce_permissions: true, enforce_permissions_exceptions: ["aidl/foo/IFoo2.aidl"] }, + } + `) + + aidlCommand := ctx.ModuleForTests("foo", "android_common").Rule("aidl").RuleParams.Command + expectedAidlFlag := "$$FLAGS -Wmissing-permission-annotation -Werror aidl/foo/IFoo.aidl" + if !strings.Contains(aidlCommand, expectedAidlFlag) { + t.Errorf("aidl command %q does not contain %q", aidlCommand, expectedAidlFlag) + } + expectedAidlFlag = "$$FLAGS aidl/foo/IFoo2.aidl" + if !strings.Contains(aidlCommand, expectedAidlFlag) { + t.Errorf("aidl command %q does not contain %q", aidlCommand, expectedAidlFlag) + } +} + func TestDataNativeBinaries(t *testing.T) { ctx, _ := testJava(t, ` java_test_host {