Bug 1393259 - Enable sandbox read access extensions for font access. r=Alex_Gaynor draft
authorHaik Aftandilian <haftandilian@mozilla.com>
Thu, 11 Jan 2018 10:53:11 -0800
changeset 724334 b757480398e3f0d9720ab845e9f10fb70a794d77
parent 724333 2a03a5dc34b08614e0018ef3861c3095afcc00cf
child 724463 6d2a25e94ecc30ee76d2570aa187d44e2bf4f1af
child 724466 93559c3896e8c6e2db1997ea9a0b53184d2f5876
push id96735
push userhaftandilian@mozilla.com
push dateWed, 24 Jan 2018 22:01:01 +0000
reviewersAlex_Gaynor
bugs1393259
milestone60.0a1
Bug 1393259 - Enable sandbox read access extensions for font access. r=Alex_Gaynor Enable sandbox read access extensions to allow content processes to access fonts stored in non-standard locations without whitelisting hardcoded directories. This is needed for configurations with third party font managers that store fonts in their own directories or user-specified directories. Now that font access is not dependent on the filename extension such as .otf and .ttf, remove the relevent tests. MozReview-Commit-ID: 8hSMrocGwIm
security/sandbox/mac/SandboxPolicies.h
security/sandbox/test/browser_content_sandbox_fs.js
--- a/security/sandbox/mac/SandboxPolicies.h
+++ b/security/sandbox/mac/SandboxPolicies.h
@@ -225,34 +225,27 @@ static const char contentSandboxRules[] 
 ; depending on systems, the 1st, 2nd or both rules are necessary
   (allow user-preference-read (preference-domain "com.apple.HIToolbox"))
   (allow file-read-data (literal "/Library/Preferences/com.apple.HIToolbox.plist"))
 
   (allow user-preference-read (preference-domain "com.apple.ATS"))
   (allow file-read-data (literal "/Library/Preferences/.GlobalPreferences.plist"))
 
   (allow file-read*
-      (subpath "/Library/Fonts")
       (subpath "/Library/Audio/Plug-Ins")
       (subpath "/Library/Spelling")
       (literal "/")
       (literal "/private/tmp")
       (literal "/private/var/tmp")
-
       (home-literal "/.CFUserTextEncoding")
       (home-literal "/Library/Preferences/com.apple.DownloadAssessment.plist")
       (home-subpath "/Library/Colors")
-      (home-subpath "/Library/Fonts")
-      (home-subpath "/Library/FontCollections")
       (home-subpath "/Library/Keyboard Layouts")
       (home-subpath "/Library/Input Methods")
       (home-subpath "/Library/Spelling")
-      (home-subpath "/Library/Application Support/Adobe/CoreSync/plugins/livetype")
-      (home-subpath "/Library/Application Support/FontAgent")
-
       (literal appPath)
       (literal appBinaryPath))
 
   (if (defined? 'file-map-executable)
     (begin
       (when testingReadPath1
         (allow file-read* file-map-executable (subpath testingReadPath1)))
       (when testingReadPath2
@@ -357,36 +350,27 @@ static const char contentSandboxRules[] 
   ; bug 1237847
   (allow file-read* file-write-data
     (subpath appTempDir))
   (allow file-write-create
     (require-all
       (subpath appTempDir)
       (vnode-type REGULAR-FILE)))
 
-  ; bug 1382260
-  ; We may need to load fonts from outside of the standard
-  ; font directories whitelisted above. This is typically caused
-  ; by a font manager. For now, whitelist any file with a
-  ; font extension. Limit this to the common font types:
-  ; files ending in .otf, .ttf, .ttc, .otc, and .dfont.
+  ; Fonts
   (allow file-read*
-    (regex #"\.[oO][tT][fF]$"           ; otf
-           #"\.[tT][tT][fF]$"           ; ttf
-           #"\.[tT][tT][cC]$"           ; ttc
-           #"\.[oO][tT][cC]$"           ; otc
-           #"\.[dD][fF][oO][nN][tT]$")) ; dfont
-
-  ; bug 1404919
-  ; Read access (recursively) within directories ending in .fontvault
-  (allow file-read* (regex #"\.fontvault/"))
-
-  ; bug 1429133
-  ; Read access to the default FontExplorer font directory
-  (allow file-read* (home-subpath "/FontExplorer X/Font Library"))
+    (subpath "/Library/Fonts")
+    (subpath "/Library/Application Support/Apple/Fonts")
+    (home-subpath "/Library/Fonts")
+    ; Allow read access to paths allowed via sandbox extensions.
+    ; This is needed for fonts in non-standard locations normally
+    ; due to third party font managers. The extensions are
+    ; automatically issued by the font server in response to font
+    ; API calls.
+    (extension "com.apple.app-sandbox.read"))
 )";
 
 static const char fileContentProcessAddend[] = R"(
   ; This process has blanket file read privileges
   (allow file-read*)
 
   ; File content processes need access to iconservices to draw file icons in
   ; directory listings
--- a/security/sandbox/test/browser_content_sandbox_fs.js
+++ b/security/sandbox/test/browser_content_sandbox_fs.js
@@ -1,18 +1,16 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
  /* import-globals-from browser_content_sandbox_utils.js */
  "use strict";
 
 Services.scriptloader.loadSubScript("chrome://mochitests/content/browser/" +
     "security/sandbox/test/browser_content_sandbox_utils.js", this);
 
-const FONT_EXTENSIONS = [ "otf", "ttf", "ttc", "otc", "dfont" ];
-
 /*
  * This test exercises file I/O from web and file content processes using
  * OS.File methods to validate that calls that are meant to be blocked by
  * content sandboxing are blocked.
  */
 
 // Creates file at |path| and returns a promise that resolves with true
 // if the file was successfully created, otherwise false. Include imports
@@ -252,56 +250,16 @@ async function createTempFile() {
     let symlinkCreated = await ContentTask.spawn(browser, path, createSymlink);
     ok(symlinkCreated == false,
        "created a symlink in content temp is not permitted");
   } else {
     ok(fileDeleted == true, "deleting a file in content temp is permitted");
   }
 }
 
-// Build a list of nonexistent font file paths (lower and upper case) with
-// all the font extensions we want the sandbox to allow read access to.
-// Generate paths within base directory |baseDir|.
-function getFontTestPaths(baseDir) {
-  baseDir = baseDir + "/";
-
-  let basename = uuid();
-  let testPaths = [];
-
-  for (let ext of FONT_EXTENSIONS) {
-    // lower case filename
-    let lcFilename = baseDir + (basename + "lc." + ext).toLowerCase();
-    testPaths.push(lcFilename);
-    // upper case filename
-    let ucFilename = baseDir + (basename + "UC." + ext).toUpperCase();
-    testPaths.push(ucFilename);
-  }
-  return testPaths;
-}
-
-// Build a list of nonexistent invalid font file paths. Specifically,
-// paths that include the valid font extensions but should fail to load.
-// For example, if a font extension happens to be a substring of the filename
-// but isn't the extension. Generate paths within base directory |baseDir|.
-function getBadFontTestPaths(baseDir) {
-  baseDir = baseDir + "/";
-
-  let basename = uuid();
-  let testPaths = [];
-
-  for (let ext of FONT_EXTENSIONS) {
-    let filename = baseDir + basename + "." + ext + ".txt";
-    testPaths.push(filename);
-
-    filename = baseDir + basename + "." + ext + ext + ".txt";
-    testPaths.push(filename);
-  }
-  return testPaths;
-}
-
 // Test reading files and dirs from web and file content processes.
 async function testFileAccess() {
   // for tests that run in a web content process
   let webBrowser = gBrowser.selectedBrowser;
 
   // Ensure that the file content process is enabled.
   let fileContentProcessEnabled =
     Services.prefs.getBoolPref("browser.tabs.remote.separateFileUriProcess");
@@ -322,65 +280,16 @@ async function testFileAccess() {
 
   // Directories/files to test accessing from content processes.
   // For directories, we test whether a directory listing is allowed
   // or blocked. For files, we test if we can read from the file.
   // Each entry in the array represents a test file or directory
   // that will be read from either a web or file process.
   let tests = [];
 
-  // Test that Mac content processes can read files with font extensions
-  // and fail to read files that include the font extension as a
-  // non-extension substring.
-  if (isMac()) {
-    // Use the same directory for valid/invalid font path tests to ensure
-    // the font isn't allowed because the directory is already allowed.
-    let fontTestDir = "/private/tmp";
-    let fontTestPaths = getFontTestPaths(fontTestDir);
-    let badFontTestPaths = getBadFontTestPaths(fontTestDir);
-
-    // before we start creating dummy font files,
-    // register a cleanup func to remove them
-    registerCleanupFunction(async function() {
-      for (let fontPath of fontTestPaths.concat(badFontTestPaths)) {
-        await OS.File.remove(fontPath, {ignoreAbsent: true});
-      }
-    });
-
-    // create each dummy font file and add a test for it
-    for (let fontPath of fontTestPaths) {
-      let result = await createFile(fontPath);
-      Assert.ok(result, `${fontPath} created`);
-
-      let fontFile = GetFile(fontPath);
-      tests.push({
-        desc:     "font file",                  // description
-        ok:       true,                         // expected to succeed?
-        browser:  webBrowser,                   // browser to run test in
-        file:     fontFile,                     // nsIFile object
-        minLevel: minHomeReadSandboxLevel(),    // min level to enable test
-        func:     readFile,                     // the test function to use
-      });
-    }
-    for (let fontPath of badFontTestPaths) {
-      let result = await createFile(fontPath);
-      Assert.ok(result, `${fontPath} created`);
-
-      let fontFile = GetFile(fontPath);
-      tests.push({
-        desc:     "invalid font file",          // description
-        ok:       false,                        // expected to succeed?
-        browser:  webBrowser,                   // browser to run test in
-        file:     fontFile,                     // nsIFile object
-        minLevel: minHomeReadSandboxLevel(),    // min level to enable test
-        func:     readFile,                     // the test function to use
-      });
-    }
-  }
-
   // The Linux test runners create the temporary profile in the same
   // system temp dir we give write access to, so this gives a false
   // positive.
   let profileDir = GetProfileDir();
   if (!isLinux()) {
     tests.push({
       desc:     "profile dir",                // description
       ok:       false,                        // expected to succeed?