[Extensions] Only allow SHA-1 hashes in whitelist/blacklist properties

Feature properties whitelist and blacklist should only contain SHA-1
hashes of actual extension IDs. We can't verify whether the hash was
generated from an actual extension ID, but we can ensure that only valid
SHA-1 hashes are contained. Invalid values will cause an compile error.

Bug: none
Change-Id: I4a62d3bee9737a7c87963b7179e89ea974e12e60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1554498Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648696}

tools/json_schema_compiler/feature_compiler.py

@@ -64,11 +64,10 @@ CC_FILE_END = """
 }  // namespace extensions
 """
 
-# Returns true if the list 'l' does not contain any strings that look like
-# extension ids.
-def ListDoesNotContainPlainExtensionIds(l):
-  # For now, let's just say anything 32 characters in length is an id.
-  return len(filter(lambda s: len(s) == 32, l)) == 0
+# Returns true if the list 'l' only contains strings that are a hex-encoded SHA1
+# hashes.
+def ListContainsOnlySha1Hashes(l):
+  return len(filter(lambda s: not re.match("^[A-F0-9]{40}$", s), l)) == 0
 
 # A "grammar" for what is and isn't allowed in the features.json files. This
 # grammar has to list all possible keys and the requirements for each. The
@@ -120,7 +119,7 @@ FEATURE_GRAMMAR = (
       list: {
         'subtype': unicode,
         'validators': [
-          (ListDoesNotContainPlainExtensionIds,
+          (ListContainsOnlySha1Hashes,
            'list should only have hex-encoded SHA1 hashes of extension ids')
         ]
       }
@@ -227,7 +226,7 @@ FEATURE_GRAMMAR = (
       list: {
         'subtype': unicode,
         'validators': [
-          (ListDoesNotContainPlainExtensionIds,
+          (ListContainsOnlySha1Hashes,
            'list should only have hex-encoded SHA1 hashes of extension ids')
         ]
       }

tools/json_schema_compiler/feature_compiler_test.py

@@ -38,7 +38,10 @@ class FeatureCompilerTest(unittest.TestCase):
   def testFeature(self):
     # Test some basic feature parsing for a sanity check.
     f = self._parseFeature({
-      'blacklist': ['aaa', 'bbb'],
+      'blacklist': [
+        'ABCDEF0123456789ABCDEF0123456789ABCDEF01',
+        '10FEDCBA9876543210FEDCBA9876543210FEDCBA'
+      ],
       'channel': 'stable',
       'command_line_switch': 'switch',
       'component_extensions_auto_granted': False,
@@ -57,7 +60,10 @@ class FeatureCompilerTest(unittest.TestCase):
       'noparent': True,
       'platforms': ['mac', 'win'],
       'session_types': ['kiosk', 'regular'],
-      'whitelist': ['zzz', 'yyy']
+      'whitelist': [
+        '0123456789ABCDEF0123456789ABCDEF01234567',
+        '76543210FEDCBA9876543210FEDCBA9876543210'
+      ]
     })
     self.assertFalse(f.GetErrors())
 

tools/json_schema_compiler/test/features_generation_unittest.cc

@@ -125,8 +125,10 @@ TEST(FeaturesGenerationTest, FeaturesTest) {
     comparator.extension_types = {Manifest::TYPE_EXTENSION,
                                   Manifest::TYPE_PLATFORM_APP};
     comparator.location = SimpleFeature::COMPONENT_LOCATION;
-    comparator.allowlist = {"aaa", "bbb"};
-    comparator.blocklist = {"zzz", "yyy"};
+    comparator.allowlist = {"ABCDEF0123456789ABCDEF0123456789ABCDEF01",
+                            "10FEDCBA9876543210FEDCBA9876543210FEDCBA"};
+    comparator.blocklist = {"0123456789ABCDEF0123456789ABCDEF01234567",
+                            "76543210FEDCBA9876543210FEDCBA9876543210"};
     comparator.component_extensions_auto_granted = false;
     comparator.CompareFeature(feature);
   }
@@ -145,7 +147,7 @@ TEST(FeaturesGenerationTest, FeaturesTest) {
     // case that it specifies its own value. Thus, we reuse |comparator|.
     feature = GetAsSimpleFeature("gamma.child");
     comparator.name = "gamma.child";
-    comparator.allowlist = {"ccc"};
+    comparator.allowlist = {"0123456789ABCDEF0123456789ABCDEF01234567"};
     comparator.platforms = {Feature::LINUX_PLATFORM};
     comparator.dependencies.clear();
     comparator.CompareFeature(feature);
@@ -155,7 +157,7 @@ TEST(FeaturesGenerationTest, FeaturesTest) {
     // other feature.
     const SimpleFeature* feature = GetAsSimpleFeature("gamma.unparented");
     FeatureComparator comparator("gamma.unparented");
-    comparator.blocklist = {"ddd"};
+    comparator.blocklist = {"0123456789ABCDEF0123456789ABCDEF01234567"};
     comparator.contexts = {Feature::UNBLESSED_EXTENSION_CONTEXT};
     comparator.channel = version_info::Channel::DEV;
     comparator.CompareFeature(feature);
@@ -259,7 +261,7 @@ TEST(FeaturesGenerationTest, FeaturesTest) {
       comparator.channel = version_info::Channel::BETA;
       comparator.contexts = {Feature::BLESSED_EXTENSION_CONTEXT};
       comparator.extension_types = {Manifest::TYPE_EXTENSION};
-      comparator.allowlist = {"aaa"};
+      comparator.allowlist = {"0123456789ABCDEF0123456789ABCDEF01234567"};
       comparator.CompareFeature(other_parent);
     }
   }

tools/json_schema_compiler/test/features_test.json

@@ -13,8 +13,14 @@
     "contexts": ["blessed_extension"],
     "extension_types": ["extension", "platform_app"],
     "location": "component",
-    "whitelist": ["aaa", "bbb"],
-    "blacklist": ["zzz", "yyy"],
+    "whitelist": [
+      "ABCDEF0123456789ABCDEF0123456789ABCDEF01",
+      "10FEDCBA9876543210FEDCBA9876543210FEDCBA"
+    ],
+    "blacklist": [
+      "0123456789ABCDEF0123456789ABCDEF01234567",
+      "76543210FEDCBA9876543210FEDCBA9876543210"
+    ],
     "component_extensions_auto_granted": false
   },
   "gamma": {
@@ -26,13 +32,13 @@
     "internal": true
   },
   "gamma.child": {
-    "whitelist": ["ccc"],
+    "whitelist": ["0123456789ABCDEF0123456789ABCDEF01234567"],
     "dependencies": [],
     "platforms": ["linux"]
   },
   "gamma.unparented": {
     "channel": "dev",
-    "blacklist": ["ddd"],
+    "blacklist": ["0123456789ABCDEF0123456789ABCDEF01234567"],
     "contexts": ["unblessed_extension"],
     "noparent": true
   },
@@ -55,7 +61,7 @@
     "channel": "beta",
     "contexts": ["blessed_extension"],
     "extension_types": ["extension"],
-    "whitelist": ["aaa"]
+    "whitelist": ["0123456789ABCDEF0123456789ABCDEF01234567"]
   }, {
     "channel": "stable",
     "contexts": ["blessed_extension"],