tequilaOS/platform_build_soong
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge changes Icb91de93,I5a2edaf4

Browse source 
* changes:
  Change permitted_packages check to be per-jar rather than per-apex
  Revert "Relax apex package restriction for T+ jars"
This commit is contained in:
Spandan Das 2022-04-05 04:06:55 +00:00 committed by Gerrit Code Review
commit 2430a04b2e
4 changed files with 71 additions and 208 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

88
android/neverallow.go
View file

@ -249,7 +249,7 @@ func neverallowMutator(ctx BottomUpMutatorContext) {
continue continue
} }
if !n.appliesToProperties(ctx, properties) { if !n.appliesToProperties(properties) {
continue continue
} }
@ -261,20 +261,12 @@ func neverallowMutator(ctx BottomUpMutatorContext) {
continue continue
} }
if !n.appliesToBootclasspathJar(ctx) {
continue
}
ctx.ModuleErrorf("violates " + n.String()) ctx.ModuleErrorf("violates " + n.String())
} }
} }
type ValueMatcherContext interface {
Config() Config
}
type ValueMatcher interface { type ValueMatcher interface {
Test(ValueMatcherContext, string) bool Test(string) bool
String() string String() string
} }
@ -282,7 +274,7 @@ type equalMatcher struct {
expected string expected string
} }
func (m *equalMatcher) Test(ctx ValueMatcherContext, value string) bool { func (m *equalMatcher) Test(value string) bool {
return m.expected == value return m.expected == value
} }
@ -293,7 +285,7 @@ func (m *equalMatcher) String() string {
type anyMatcher struct { type anyMatcher struct {
} }
func (m *anyMatcher) Test(ctx ValueMatcherContext, value string) bool { func (m *anyMatcher) Test(value string) bool {
return true return true
} }
@ -307,7 +299,7 @@ type startsWithMatcher struct {
prefix string prefix string
} }
func (m *startsWithMatcher) Test(ctx ValueMatcherContext, value string) bool { func (m *startsWithMatcher) Test(value string) bool {
return strings.HasPrefix(value, m.prefix) return strings.HasPrefix(value, m.prefix)
} }
@ -319,7 +311,7 @@ type regexMatcher struct {
re *regexp.Regexp re *regexp.Regexp
} }
func (m *regexMatcher) Test(ctx ValueMatcherContext, value string) bool { func (m *regexMatcher) Test(value string) bool {
return m.re.MatchString(value) return m.re.MatchString(value)
} }
@ -331,7 +323,7 @@ type notInListMatcher struct {
allowed []string allowed []string
} }
func (m *notInListMatcher) Test(ctx ValueMatcherContext, value string) bool { func (m *notInListMatcher) Test(value string) bool {
return !InList(value, m.allowed) return !InList(value, m.allowed)
} }
@ -341,7 +333,7 @@ func (m *notInListMatcher) String() string {
type isSetMatcher struct{} type isSetMatcher struct{}
func (m *isSetMatcher) Test(ctx ValueMatcherContext, value string) bool { func (m *isSetMatcher) Test(value string) bool {
return value != "" return value != ""
} }
@ -351,19 +343,6 @@ func (m *isSetMatcher) String() string {
var isSetMatcherInstance = &isSetMatcher{} var isSetMatcherInstance = &isSetMatcher{}
type sdkVersionMatcher struct {
condition func(ctx ValueMatcherContext, spec SdkSpec) bool
description string
}
func (m *sdkVersionMatcher) Test(ctx ValueMatcherContext, value string) bool {
return m.condition(ctx, SdkSpecFromWithConfig(ctx.Config(), value))
}
func (m *sdkVersionMatcher) String() string {
return ".sdk-version(" + m.description + ")"
}
type ruleProperty struct { type ruleProperty struct {
fields []string // e.x.: Vndk.Enabled fields []string // e.x.: Vndk.Enabled
matcher ValueMatcher matcher ValueMatcher
@ -397,8 +376,6 @@ type Rule interface {
NotModuleType(types ...string) Rule NotModuleType(types ...string) Rule
BootclasspathJar() Rule
With(properties, value string) Rule With(properties, value string) Rule
WithMatcher(properties string, matcher ValueMatcher) Rule WithMatcher(properties string, matcher ValueMatcher) Rule
@ -514,12 +491,6 @@ func (r *rule) Because(reason string) Rule {
return r return r
} }
// BootclasspathJar whether this rule only applies to Jars in the Bootclasspath
func (r *rule) BootclasspathJar() Rule {
r.onlyBootclasspathJar = true
return r
}
func (r *rule) String() string { func (r *rule) String() string {
s := []string{"neverallow requirements. Not allowed:"} s := []string{"neverallow requirements. Not allowed:"}
if len(r.paths) > 0 { if len(r.paths) > 0 {
@ -537,9 +508,6 @@ func (r *rule) String() string {
if len(r.osClasses) > 0 { if len(r.osClasses) > 0 {
s = append(s, fmt.Sprintf("os class(es): %q", r.osClasses)) s = append(s, fmt.Sprintf("os class(es): %q", r.osClasses))
} }
if r.onlyBootclasspathJar {
s = append(s, "in bootclasspath jar")
}
if len(r.unlessPaths) > 0 { if len(r.unlessPaths) > 0 {
s = append(s, fmt.Sprintf("EXCEPT in dirs: %q", r.unlessPaths)) s = append(s, fmt.Sprintf("EXCEPT in dirs: %q", r.unlessPaths))
} }
@ -580,14 +548,6 @@ func (r *rule) appliesToDirectDeps(ctx BottomUpMutatorContext) bool {
return matches return matches
} }
func (r *rule) appliesToBootclasspathJar(ctx BottomUpMutatorContext) bool {
if !r.onlyBootclasspathJar {
return true
}
return InList(ctx.ModuleName(), ctx.Config().BootJars())
}
func (r *rule) appliesToOsClass(osClass OsClass) bool { func (r *rule) appliesToOsClass(osClass OsClass) bool {
if len(r.osClasses) == 0 { if len(r.osClasses) == 0 {
return true return true
@ -606,10 +566,9 @@ func (r *rule) appliesToModuleType(moduleType string) bool {
return (len(r.moduleTypes) == 0 || InList(moduleType, r.moduleTypes)) && !InList(moduleType, r.unlessModuleTypes) return (len(r.moduleTypes) == 0 || InList(moduleType, r.moduleTypes)) && !InList(moduleType, r.unlessModuleTypes)
} }
func (r *rule) appliesToProperties(ctx ValueMatcherContext, func (r *rule) appliesToProperties(properties []interface{}) bool {
properties []interface{}) bool { includeProps := hasAllProperties(properties, r.props)
includeProps := hasAllProperties(ctx, properties, r.props) excludeProps := hasAnyProperty(properties, r.unlessProps)
excludeProps := hasAnyProperty(ctx, properties, r.unlessProps)
return includeProps && !excludeProps return includeProps && !excludeProps
} }
@ -629,16 +588,6 @@ func NotInList(allowed []string) ValueMatcher {
return &notInListMatcher{allowed} return &notInListMatcher{allowed}
} }
func LessThanSdkVersion(sdk string) ValueMatcher {
return &sdkVersionMatcher{
condition: func(ctx ValueMatcherContext, spec SdkSpec) bool {
return spec.ApiLevel.LessThan(
SdkSpecFromWithConfig(ctx.Config(), sdk).ApiLevel)
},
description: "lessThan=" + sdk,
}
}
// assorted utils // assorted utils
func cleanPaths(paths []string) []string { func cleanPaths(paths []string) []string {
@ -657,28 +606,25 @@ func fieldNamesForProperties(propertyNames string) []string {
return names return names
} }
func hasAnyProperty(ctx ValueMatcherContext, properties []interface{}, func hasAnyProperty(properties []interface{}, props []ruleProperty) bool {
props []ruleProperty) bool {
for _, v := range props { for _, v := range props {
if hasProperty(ctx, properties, v) { if hasProperty(properties, v) {
return true return true
} }
} }
return false return false
} }
func hasAllProperties(ctx ValueMatcherContext, properties []interface{}, func hasAllProperties(properties []interface{}, props []ruleProperty) bool {
props []ruleProperty) bool {
for _, v := range props { for _, v := range props {
if !hasProperty(ctx, properties, v) { if !hasProperty(properties, v) {
return false return false
} }
} }
return true return true
} }
func hasProperty(ctx ValueMatcherContext, properties []interface{}, func hasProperty(properties []interface{}, prop ruleProperty) bool {
prop ruleProperty) bool {
for _, propertyStruct := range properties { for _, propertyStruct := range properties {
propertiesValue := reflect.ValueOf(propertyStruct).Elem() propertiesValue := reflect.ValueOf(propertyStruct).Elem()
for _, v := range prop.fields { for _, v := range prop.fields {
@ -692,7 +638,7 @@ func hasProperty(ctx ValueMatcherContext, properties []interface{},
} }
check := func(value string) bool { check := func(value string) bool {
return prop.matcher.Test(ctx, value) return prop.matcher.Test(value)
} }
if matchValue(propertiesValue, check) { if matchValue(propertiesValue, check) {

49
android/neverallow_test.go
View file

@ -327,48 +327,6 @@ var neverallowTests = []struct {
"Only boot images may be imported as a makefile goal.", "Only boot images may be imported as a makefile goal.",
}, },
}, },
{
name: "min_sdk too low",
fs: map[string][]byte{
"Android.bp": []byte(`
java_library {
name: "min_sdk_too_low",
min_sdk_version: "30",
}`),
},
rules: []Rule{
NeverAllow().WithMatcher("min_sdk_version", LessThanSdkVersion("31")),
},
expectedErrors: []string{
"module \"min_sdk_too_low\": violates neverallow",
},
},
{
name: "min_sdk high enough",
fs: map[string][]byte{
"Android.bp": []byte(`
java_library {
name: "min_sdk_high_enough",
min_sdk_version: "31",
}`),
},
rules: []Rule{
NeverAllow().WithMatcher("min_sdk_version", LessThanSdkVersion("31")),
},
},
{
name: "current min_sdk high enough",
fs: map[string][]byte{
"Android.bp": []byte(`
java_library {
name: "current_min_sdk_high_enough",
min_sdk_version: "current",
}`),
},
rules: []Rule{
NeverAllow().WithMatcher("min_sdk_version", LessThanSdkVersion("31")),
},
},
} }
var prepareForNeverAllowTest = GroupFixturePreparers( var prepareForNeverAllowTest = GroupFixturePreparers(
@ -452,10 +410,9 @@ func (p *mockCcLibraryModule) GenerateAndroidBuildActions(ModuleContext) {
} }
type mockJavaLibraryProperties struct { type mockJavaLibraryProperties struct {
Libs []string Libs []string
Min_sdk_version *string Sdk_version *string
Sdk_version *string Uncompress_dex *bool
Uncompress_dex *bool
} }
type mockJavaLibraryModule struct { type mockJavaLibraryModule struct {

47
apex/apex.go
View file

@ -3278,21 +3278,19 @@ func makeApexAvailableBaseline() map[string][]string {
} }
func init() { func init() {
android.AddNeverAllowRules(createApexPermittedPackagesRules(qModulesPackages())...) android.AddNeverAllowRules(createBcpPermittedPackagesRules(qBcpPackages())...)
android.AddNeverAllowRules(createApexPermittedPackagesRules(rModulesPackages())...) android.AddNeverAllowRules(createBcpPermittedPackagesRules(rBcpPackages())...)
} }
func createApexPermittedPackagesRules(modules_packages map[string][]string) []android.Rule { func createBcpPermittedPackagesRules(bcpPermittedPackages map[string][]string) []android.Rule {
rules := make([]android.Rule, 0, len(modules_packages)) rules := make([]android.Rule, 0, len(bcpPermittedPackages))
for module_name, module_packages := range modules_packages { for jar, permittedPackages := range bcpPermittedPackages {
permittedPackagesRule := android.NeverAllow(). permittedPackagesRule := android.NeverAllow().
BootclasspathJar(). With("name", jar).
With("apex_available", module_name). WithMatcher("permitted_packages", android.NotInList(permittedPackages)).
WithMatcher("permitted_packages", android.NotInList(module_packages)). Because(jar +
WithMatcher("min_sdk_version", android.LessThanSdkVersion("Tiramisu")). " bootjar may only use these package prefixes: " + strings.Join(permittedPackages, ",") +
Because("jars that are part of the " + module_name + ". Please consider the following alternatives:\n" +
" module may only use these package prefixes: " + strings.Join(module_packages, ",") +
" with min_sdk < T. Please consider the following alternatives:\n" +
" 1. If the offending code is from a statically linked library, consider " + " 1. If the offending code is from a statically linked library, consider " +
"removing that dependency and using an alternative already in the " + "removing that dependency and using an alternative already in the " +
"bootclasspath, or perhaps a shared library." + "bootclasspath, or perhaps a shared library." +
@ -3300,55 +3298,56 @@ func createApexPermittedPackagesRules(modules_packages map[string][]string) []an
" 3. Jarjar the offending code. Please be mindful of the potential system " + " 3. Jarjar the offending code. Please be mindful of the potential system " +
"health implications of bundling that code, particularly if the offending jar " + "health implications of bundling that code, particularly if the offending jar " +
"is part of the bootclasspath.") "is part of the bootclasspath.")
rules = append(rules, permittedPackagesRule) rules = append(rules, permittedPackagesRule)
} }
return rules return rules
} }
// DO NOT EDIT! These are the package prefixes that are exempted from being AOT'ed by ART on Q/R/S. // DO NOT EDIT! These are the package prefixes that are exempted from being AOT'ed by ART.
// Adding code to the bootclasspath in new packages will cause issues on module update. // Adding code to the bootclasspath in new packages will cause issues on module update.
func qModulesPackages() map[string][]string { func qBcpPackages() map[string][]string {
return map[string][]string{ return map[string][]string{
"com.android.conscrypt": []string{ "conscrypt": []string{
"android.net.ssl", "android.net.ssl",
"com.android.org.conscrypt", "com.android.org.conscrypt",
}, },
"com.android.media": []string{ "updatable-media": []string{
"android.media", "android.media",
}, },
} }
} }
// DO NOT EDIT! These are the package prefixes that are exempted from being AOT'ed by ART on R/S. // DO NOT EDIT! These are the package prefixes that are exempted from being AOT'ed by ART.
// Adding code to the bootclasspath in new packages will cause issues on module update. // Adding code to the bootclasspath in new packages will cause issues on module update.
func rModulesPackages() map[string][]string { func rBcpPackages() map[string][]string {
return map[string][]string{ return map[string][]string{
"com.android.mediaprovider": []string{ "framework-mediaprovider": []string{
"android.provider", "android.provider",
}, },
"com.android.permission": []string{ "framework-permission": []string{
"android.permission", "android.permission",
"android.app.role", "android.app.role",
"com.android.permission", "com.android.permission",
"com.android.role", "com.android.role",
}, },
"com.android.sdkext": []string{ "framework-sdkextensions": []string{
"android.os.ext", "android.os.ext",
}, },
"com.android.os.statsd": []string{ "framework-statsd": []string{
"android.app", "android.app",
"android.os", "android.os",
"android.util", "android.util",
"com.android.internal.statsd", "com.android.internal.statsd",
"com.android.server.stats", "com.android.server.stats",
}, },
"com.android.wifi": []string{ "framework-wifi": []string{
"com.android.server.wifi", "com.android.server.wifi",
"com.android.wifi.x", "com.android.wifi.x",
"android.hardware.wifi", "android.hardware.wifi",
"android.net.wifi", "android.net.wifi",
}, },
"com.android.tethering": []string{ "framework-tethering": []string{
"android.net", "android.net",
}, },
} }

95
apex/apex_test.go
View file

@ -7580,7 +7580,7 @@ func TestDexpreoptAccessDexFilesFromPrebuiltApex(t *testing.T) {
}) })
} }
func testApexPermittedPackagesRules(t *testing.T, errmsg, bp string, bootJars []string, rules []android.Rule) { func testBootJarPermittedPackagesRules(t *testing.T, errmsg, bp string, bootJars []string, rules []android.Rule) {
t.Helper() t.Helper()
bp += ` bp += `
apex_key { apex_key {
@ -7619,11 +7619,11 @@ func testApexPermittedPackagesRules(t *testing.T, errmsg, bp string, bootJars []
func TestApexPermittedPackagesRules(t *testing.T) { func TestApexPermittedPackagesRules(t *testing.T) {
testcases := []struct { testcases := []struct {
name string name string
expectedError string expectedError string
bp string bp string
bootJars []string bootJars []string
modulesPackages map[string][]string bcpPermittedPackages map[string][]string
}{ }{
{ {
@ -7637,7 +7637,6 @@ func TestApexPermittedPackagesRules(t *testing.T) {
apex_available: ["myapex"], apex_available: ["myapex"],
sdk_version: "none", sdk_version: "none",
system_modules: "none", system_modules: "none",
min_sdk_version: "30",
} }
java_library { java_library {
name: "nonbcp_lib2", name: "nonbcp_lib2",
@ -7646,25 +7645,23 @@ func TestApexPermittedPackagesRules(t *testing.T) {
permitted_packages: ["a.b"], permitted_packages: ["a.b"],
sdk_version: "none", sdk_version: "none",
system_modules: "none", system_modules: "none",
min_sdk_version: "30",
} }
apex { apex {
name: "myapex", name: "myapex",
min_sdk_version: "30",
key: "myapex.key", key: "myapex.key",
java_libs: ["bcp_lib1", "nonbcp_lib2"], java_libs: ["bcp_lib1", "nonbcp_lib2"],
updatable: false, updatable: false,
}`, }`,
bootJars: []string{"bcp_lib1"}, bootJars: []string{"bcp_lib1"},
modulesPackages: map[string][]string{ bcpPermittedPackages: map[string][]string{
"myapex": []string{ "bcp_lib1": []string{
"foo.bar", "foo.bar",
}, },
}, },
}, },
{ {
name: "Bootclasspath apex jar not satisfying allowed module packages on Q.", name: "Bootclasspath apex jar not satisfying allowed module packages.",
expectedError: `(?s)module "bcp_lib2" .* which is restricted because jars that are part of the myapex module may only use these package prefixes: foo.bar with min_sdk < T. Please consider the following alternatives:\n 1. If the offending code is from a statically linked library, consider removing that dependency and using an alternative already in the bootclasspath, or perhaps a shared library. 2. Move the offending code into an allowed package.\n 3. Jarjar the offending code. Please be mindful of the potential system health implications of bundling that code, particularly if the offending jar is part of the bootclasspath.`, expectedError: `(?s)module "bcp_lib2" .* which is restricted because bcp_lib2 bootjar may only use these package prefixes: foo.bar. Please consider the following alternatives:\n 1. If the offending code is from a statically linked library, consider removing that dependency and using an alternative already in the bootclasspath, or perhaps a shared library. 2. Move the offending code into an allowed package.\n 3. Jarjar the offending code. Please be mindful of the potential system health implications of bundling that code, particularly if the offending jar is part of the bootclasspath.`,
bp: ` bp: `
java_library { java_library {
name: "bcp_lib1", name: "bcp_lib1",
@ -7673,7 +7670,6 @@ func TestApexPermittedPackagesRules(t *testing.T) {
permitted_packages: ["foo.bar"], permitted_packages: ["foo.bar"],
sdk_version: "none", sdk_version: "none",
system_modules: "none", system_modules: "none",
min_sdk_version: "29",
} }
java_library { java_library {
name: "bcp_lib2", name: "bcp_lib2",
@ -7682,102 +7678,67 @@ func TestApexPermittedPackagesRules(t *testing.T) {
permitted_packages: ["foo.bar", "bar.baz"], permitted_packages: ["foo.bar", "bar.baz"],
sdk_version: "none", sdk_version: "none",
system_modules: "none", system_modules: "none",
min_sdk_version: "29",
} }
apex { apex {
name: "myapex", name: "myapex",
min_sdk_version: "29",
key: "myapex.key", key: "myapex.key",
java_libs: ["bcp_lib1", "bcp_lib2"], java_libs: ["bcp_lib1", "bcp_lib2"],
updatable: false, updatable: false,
} }
`, `,
bootJars: []string{"bcp_lib1", "bcp_lib2"}, bootJars: []string{"bcp_lib1", "bcp_lib2"},
modulesPackages: map[string][]string{ bcpPermittedPackages: map[string][]string{
"myapex": []string{ "bcp_lib1": []string{
"foo.bar",
},
"bcp_lib2": []string{
"foo.bar", "foo.bar",
}, },
}, },
}, },
{ {
name: "Bootclasspath apex jar not satisfying allowed module packages on R.", name: "Updateable Bootclasspath apex jar not satisfying allowed module packages.",
expectedError: `(?s)module "bcp_lib2" .* which is restricted because jars that are part of the myapex module may only use these package prefixes: foo.bar with min_sdk < T. Please consider the following alternatives:\n 1. If the offending code is from a statically linked library, consider removing that dependency and using an alternative already in the bootclasspath, or perhaps a shared library. 2. Move the offending code into an allowed package.\n 3. Jarjar the offending code. Please be mindful of the potential system health implications of bundling that code, particularly if the offending jar is part of the bootclasspath.`,
bp: `
java_library {
name: "bcp_lib1",
srcs: ["lib1/src/*.java"],
apex_available: ["myapex"],
permitted_packages: ["foo.bar"],
sdk_version: "none",
system_modules: "none",
min_sdk_version: "30",
}
java_library {
name: "bcp_lib2",
srcs: ["lib2/src/*.java"],
apex_available: ["myapex"],
permitted_packages: ["foo.bar", "bar.baz"],
sdk_version: "none",
system_modules: "none",
min_sdk_version: "30",
}
apex {
name: "myapex",
min_sdk_version: "30",
key: "myapex.key",
java_libs: ["bcp_lib1", "bcp_lib2"],
updatable: false,
}
`,
bootJars: []string{"bcp_lib1", "bcp_lib2"},
modulesPackages: map[string][]string{
"myapex": []string{
"foo.bar",
},
},
},
{
name: "Bootclasspath apex jar >= T not satisfying Q/R/S allowed module packages.",
expectedError: "", expectedError: "",
bp: ` bp: `
java_library { java_library {
name: "bcp_lib1", name: "bcp_lib_restricted",
srcs: ["lib1/src/*.java"], srcs: ["lib1/src/*.java"],
apex_available: ["myapex"], apex_available: ["myapex"],
permitted_packages: ["foo.bar"], permitted_packages: ["foo.bar"],
sdk_version: "none", sdk_version: "none",
min_sdk_version: "29",
system_modules: "none", system_modules: "none",
min_sdk_version: "current",
} }
java_library { java_library {
name: "bcp_lib2", name: "bcp_lib_unrestricted",
srcs: ["lib2/src/*.java"], srcs: ["lib2/src/*.java"],
apex_available: ["myapex"], apex_available: ["myapex"],
permitted_packages: ["foo.bar", "bar.baz"], permitted_packages: ["foo.bar", "bar.baz"],
sdk_version: "none", sdk_version: "none",
min_sdk_version: "29",
system_modules: "none", system_modules: "none",
min_sdk_version: "current",
} }
apex { apex {
name: "myapex", name: "myapex",
min_sdk_version: "current",
key: "myapex.key", key: "myapex.key",
java_libs: ["bcp_lib1", "bcp_lib2"], java_libs: ["bcp_lib_restricted", "bcp_lib_unrestricted"],
updatable: false, updatable: true,
min_sdk_version: "29",
} }
`, `,
bootJars: []string{"bcp_lib1", "bcp_lib2"}, bootJars: []string{"bcp_lib1", "bcp_lib2"},
modulesPackages: map[string][]string{ bcpPermittedPackages: map[string][]string{
"myapex": []string{ "bcp_lib1_non_updateable": []string{
"foo.bar", "foo.bar",
}, },
// bcp_lib2_updateable has no entry here since updateable bcp can contain new packages - tracking via an allowlist is not necessary
}, },
}, },
} }
for _, tc := range testcases { for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
rules := createApexPermittedPackagesRules(tc.modulesPackages) rules := createBcpPermittedPackagesRules(tc.bcpPermittedPackages)
testApexPermittedPackagesRules(t, tc.expectedError, tc.bp, tc.bootJars, rules) testBootJarPermittedPackagesRules(t, tc.expectedError, tc.bp, tc.bootJars, rules)
}) })
} }
} }