Make encryption action an argument to mkdir

FscryptSetDirectoryPolicy no longer tries to infer the action from the
filename. Well mostly; it still assumes top-level directories in /data
should be encrypted unless the mkdir arguments say otherwise, but
it warns.

Bug: 26641735
Test: boot, check log messages
Change-Id: Id6d2cea7fb856f17323897d85cf6190c981b443c

Author: ciphergoth

init/README.md

@@ -505,12 +505,23 @@ Commands
 > Used to mark the point right after /data is mounted. Used to implement the
   `class_reset_post_data` and `class_start_post_data` commands.
 
-`mkdir <path> [mode] [owner] [group]`
+`mkdir <path> [<mode>] [<owner>] [<group>] [encryption=<action>] [key=<key>]`
 > Create a directory at _path_, optionally with the given mode, owner, and
   group. If not provided, the directory is created with permissions 755 and
   owned by the root user and root group. If provided, the mode, owner and group
   will be updated if the directory exists already.
 
+ > _action_ can be one of:
+  * `None`: take no encryption action; directory will be encrypted if parent is.
+  * `Require`: encrypt directory, abort boot process if encryption fails
+  * `Attempt`: try to set an encryption policy, but continue if it fails
+  * `DeleteIfNecessary`: recursively delete directory if necessary to set
+  encryption policy.
+
+  > _key_ can be one of:
+  * `ref`: use the systemwide DE key
+  * `per_boot_ref`: use the key freshly generated on each boot.
+
 `mount_all <fstab> [ <path> ]\* [--<option>]`
 > Calls fs\_mgr\_mount\_all on the given fs\_mgr-format fstab with optional
   options "early" and "late".

init/builtins.cpp

@@ -364,67 +364,52 @@ static Result<void> do_interface_stop(const BuiltinArguments& args) {
     return {};
 }
 
-// mkdir <path> [mode] [owner] [group]
+// mkdir <path> [mode] [owner] [group] [<option> ...]
 static Result<void> do_mkdir(const BuiltinArguments& args) {
-    mode_t mode = 0755;
-    Result<uid_t> uid = -1;
-    Result<gid_t> gid = -1;
-
-    switch (args.size()) {
-        case 5:
-            gid = DecodeUid(args[4]);
-            if (!gid) {
-                return Error() << "Unable to decode GID for '" << args[4] << "': " << gid.error();
-            }
-            FALLTHROUGH_INTENDED;
-        case 4:
-            uid = DecodeUid(args[3]);
-            if (!uid) {
-                return Error() << "Unable to decode UID for '" << args[3] << "': " << uid.error();
-            }
-            FALLTHROUGH_INTENDED;
-        case 3:
-            mode = std::strtoul(args[2].c_str(), 0, 8);
-            FALLTHROUGH_INTENDED;
-        case 2:
-            break;
-        default:
-            return Error() << "Unexpected argument count: " << args.size();
+    auto options = ParseMkdir(args.args);
+    if (!options) return options.error();
+    std::string ref_basename;
+    if (options->ref_option == "ref") {
+        ref_basename = fscrypt_key_ref;
+    } else if (options->ref_option == "per_boot_ref") {
+        ref_basename = fscrypt_key_per_boot_ref;
+    } else {
+        return Error() << "Unknown key option: '" << options->ref_option << "'";
     }
-    std::string target = args[1];
+
     struct stat mstat;
-    if (lstat(target.c_str(), &mstat) != 0) {
+    if (lstat(options->target.c_str(), &mstat) != 0) {
         if (errno != ENOENT) {
-            return ErrnoError() << "lstat() failed on " << target;
+            return ErrnoError() << "lstat() failed on " << options->target;
         }
-        if (!make_dir(target, mode)) {
-            return ErrnoErrorIgnoreEnoent() << "mkdir() failed on " << target;
+        if (!make_dir(options->target, options->mode)) {
+            return ErrnoErrorIgnoreEnoent() << "mkdir() failed on " << options->target;
         }
-        if (lstat(target.c_str(), &mstat) != 0) {
-            return ErrnoError() << "lstat() failed on new " << target;
+        if (lstat(options->target.c_str(), &mstat) != 0) {
+            return ErrnoError() << "lstat() failed on new " << options->target;
         }
     }
     if (!S_ISDIR(mstat.st_mode)) {
-        return Error() << "Not a directory on " << target;
+        return Error() << "Not a directory on " << options->target;
     }
-    bool needs_chmod = (mstat.st_mode & ~S_IFMT) != mode;
-    if ((*uid != static_cast<uid_t>(-1) && *uid != mstat.st_uid) ||
-        (*gid != static_cast<gid_t>(-1) && *gid != mstat.st_gid)) {
-        if (lchown(target.c_str(), *uid, *gid) == -1) {
-            return ErrnoError() << "lchown failed on " << target;
+    bool needs_chmod = (mstat.st_mode & ~S_IFMT) != options->mode;
+    if ((options->uid != static_cast<uid_t>(-1) && options->uid != mstat.st_uid) ||
+        (options->gid != static_cast<gid_t>(-1) && options->gid != mstat.st_gid)) {
+        if (lchown(options->target.c_str(), options->uid, options->gid) == -1) {
+            return ErrnoError() << "lchown failed on " << options->target;
         }
         // chown may have cleared S_ISUID and S_ISGID, chmod again
         needs_chmod = true;
     }
     if (needs_chmod) {
-        if (fchmodat(AT_FDCWD, target.c_str(), mode, AT_SYMLINK_NOFOLLOW) == -1) {
-            return ErrnoError() << "fchmodat() failed on " << target;
+        if (fchmodat(AT_FDCWD, options->target.c_str(), options->mode, AT_SYMLINK_NOFOLLOW) == -1) {
+            return ErrnoError() << "fchmodat() failed on " << options->target;
         }
     }
     if (fscrypt_is_native()) {
-        if (fscrypt_set_directory_policy(target)) {
+        if (!FscryptSetDirectoryPolicy(ref_basename, options->fscrypt_action, options->target)) {
             return reboot_into_recovery(
-                    {"--prompt_and_wipe_data", "--reason=set_policy_failed:"s + target});
+                    {"--prompt_and_wipe_data", "--reason=set_policy_failed:"s + options->target});
         }
     }
     return {};
@@ -589,8 +574,8 @@ static Result<void> queue_fs_event(int code) {
         return reboot_into_recovery(options);
         /* If reboot worked, there is no return. */
     } else if (code == FS_MGR_MNTALL_DEV_FILE_ENCRYPTED) {
-        if (fscrypt_install_keyring()) {
-            return Error() << "fscrypt_install_keyring() failed";
+        if (!FscryptInstallKeyring()) {
+            return Error() << "FscryptInstallKeyring() failed";
         }
         property_set("ro.crypto.state", "encrypted");
         property_set("ro.crypto.type", "file");
@@ -600,8 +585,8 @@ static Result<void> queue_fs_event(int code) {
         ActionManager::GetInstance().QueueEventTrigger("nonencrypted");
         return {};
     } else if (code == FS_MGR_MNTALL_DEV_IS_METADATA_ENCRYPTED) {
-        if (fscrypt_install_keyring()) {
-            return Error() << "fscrypt_install_keyring() failed";
+        if (!FscryptInstallKeyring()) {
+            return Error() << "FscryptInstallKeyring() failed";
         }
         property_set("ro.crypto.state", "encrypted");
         property_set("ro.crypto.type", "file");
@@ -611,8 +596,8 @@ static Result<void> queue_fs_event(int code) {
         ActionManager::GetInstance().QueueEventTrigger("nonencrypted");
         return {};
     } else if (code == FS_MGR_MNTALL_DEV_NEEDS_METADATA_ENCRYPTION) {
-        if (fscrypt_install_keyring()) {
-            return Error() << "fscrypt_install_keyring() failed";
+        if (!FscryptInstallKeyring()) {
+            return Error() << "FscryptInstallKeyring() failed";
         }
         property_set("ro.crypto.state", "encrypted");
         property_set("ro.crypto.type", "file");
@@ -1257,7 +1242,7 @@ const BuiltinFunctionMap& GetBuiltinFunctionMap() {
         {"load_system_props",       {0,     0,    {false,  do_load_system_props}}},
         {"loglevel",                {1,     1,    {false,  do_loglevel}}},
         {"mark_post_data",          {0,     0,    {false,  do_mark_post_data}}},
-        {"mkdir",                   {1,     4,    {true,   do_mkdir}}},
+        {"mkdir",                   {1,     6,    {true,   do_mkdir}}},
         // TODO: Do mount operations in vendor_init.
         // mount_all is currently too complex to run in vendor_init as it queues action triggers,
         // imports rc scripts, etc.  It should be simplified and run in vendor_init context.

init/check_builtins.cpp

@@ -121,22 +121,10 @@ Result<void> check_loglevel(const BuiltinArguments& args) {
 }
 
 Result<void> check_mkdir(const BuiltinArguments& args) {
-    if (args.size() >= 4) {
-        if (!args[3].empty()) {
-            auto uid = DecodeUid(args[3]);
-            if (!uid) {
-                return Error() << "Unable to decode UID for '" << args[3] << "': " << uid.error();
-            }
-        }
-
-        if (args.size() == 5 && !args[4].empty()) {
-            auto gid = DecodeUid(args[4]);
-            if (!gid) {
-                return Error() << "Unable to decode GID for '" << args[4] << "': " << gid.error();
-            }
-        }
+    auto options = ParseMkdir(args.args);
+    if (!options) {
+        return options.error();
     }
-
     return {};
 }
 

init/fscrypt_init_extensions.cpp

@@ -41,19 +41,15 @@
 
 using namespace android::fscrypt;
 
-static int set_policy_on(const std::string& ref_basename, const std::string& dir);
-
-int fscrypt_install_keyring() {
+bool FscryptInstallKeyring() {
     key_serial_t device_keyring = add_key("keyring", "fscrypt", 0, 0, KEY_SPEC_SESSION_KEYRING);
 
     if (device_keyring == -1) {
         PLOG(ERROR) << "Failed to create keyring";
-        return -1;
+        return false;
     }
-
     LOG(INFO) << "Keyring created with id " << device_keyring << " in process " << getpid();
-
-    return 0;
+    return true;
 }
 
 // TODO(b/139378601): use a single central implementation of this.
@@ -97,102 +93,57 @@ static void delete_dir_contents(const std::string& dir) {
     }
 }
 
-int fscrypt_set_directory_policy(const std::string& dir) {
-    const std::string prefix = "/data/";
-
-    if (!android::base::StartsWith(dir, prefix)) {
-        return 0;
-    }
-
-    // Special-case /data/media/obb per b/64566063
-    if (dir == "/data/media/obb") {
-        // Try to set policy on this directory, but if it is non-empty this may fail.
-        set_policy_on(fscrypt_key_ref, dir);
-        return 0;
-    }
-
-    // Only set policy on first level /data directories
-    // To make this less restrictive, consider using a policy file.
-    // However this is overkill for as long as the policy is simply
-    // to apply a global policy to all /data folders created via makedir
-    if (dir.find_first_of('/', prefix.size()) != std::string::npos) {
-        return 0;
-    }
-
-    // Special case various directories that must not be encrypted,
-    // often because their subdirectories must be encrypted.
-    // This isn't a nice way to do this, see b/26641735
-    std::vector<std::string> directories_to_exclude = {
-        "lost+found",
-        "system_ce", "system_de",
-        "misc_ce", "misc_de",
-        "vendor_ce", "vendor_de",
-        "media",
-        "data", "user", "user_de",
-        "apex", "preloads", "app-staging",
-        "gsi",
-    };
-    for (const auto& d : directories_to_exclude) {
-        if ((prefix + d) == dir) {
-            LOG(INFO) << "Not setting policy on " << dir;
-            return 0;
-        }
-    }
-    std::vector<std::string> per_boot_directories = {
-            "per_boot",
-    };
-    for (const auto& d : per_boot_directories) {
-        if ((prefix + d) == dir) {
-            LOG(INFO) << "Setting per_boot key on " << dir;
-            return set_policy_on(fscrypt_key_per_boot_ref, dir);
-        }
-    }
-    int err = set_policy_on(fscrypt_key_ref, dir);
-    if (err == 0) {
-        return 0;
-    }
-    // Empty these directories if policy setting fails.
-    std::vector<std::string> wipe_on_failure = {
-            "rollback", "rollback-observer",  // b/139193659
-    };
-    for (const auto& d : wipe_on_failure) {
-        if ((prefix + d) == dir) {
-            LOG(ERROR) << "Setting policy failed, deleting: " << dir;
-            delete_dir_contents(dir);
-            err = set_policy_on(fscrypt_key_ref, dir);
-            break;
-        }
-    }
-    return err;
-}
-
-// Set an encryption policy on the given directory.  The policy (key reference
+// Look up an encryption policy  The policy (key reference
 // and encryption options) to use is read from files that were written by vold.
-static int set_policy_on(const std::string& ref_basename, const std::string& dir) {
-    EncryptionPolicy policy;
+static bool LookupPolicy(const std::string& ref_basename, EncryptionPolicy* policy) {
     std::string ref_filename = std::string("/data") + ref_basename;
-    if (!android::base::ReadFileToString(ref_filename, &policy.key_raw_ref)) {
-        LOG(ERROR) << "Unable to read system policy to set on " << dir;
-        return -1;
+    if (!android::base::ReadFileToString(ref_filename, &policy->key_raw_ref)) {
+        LOG(ERROR) << "Unable to read system policy with name " << ref_filename;
+        return false;
     }
 
     auto options_filename = std::string("/data") + fscrypt_key_mode;
     std::string options_string;
     if (!android::base::ReadFileToString(options_filename, &options_string)) {
         LOG(ERROR) << "Cannot read encryption options string";
-        return -1;
+        return false;
     }
-    if (!ParseOptions(options_string, &policy.options)) {
+    if (!ParseOptions(options_string, &policy->options)) {
         LOG(ERROR) << "Invalid encryption options string: " << options_string;
-        return -1;
+        return false;
     }
+    return true;
+}
 
+static bool EnsurePolicyOrLog(const EncryptionPolicy& policy, const std::string& dir) {
     if (!EnsurePolicy(policy, dir)) {
         std::string ref_hex;
         BytesToHex(policy.key_raw_ref, &ref_hex);
         LOG(ERROR) << "Setting " << ref_hex << " policy on " << dir << " failed!";
-        return -1;
+        return false;
     }
+    return true;
+}
+
+static bool SetPolicyOn(const std::string& ref_basename, const std::string& dir) {
+    EncryptionPolicy policy;
+    if (!LookupPolicy(ref_basename, &policy)) return false;
+    if (!EnsurePolicyOrLog(policy, dir)) return false;
+    return true;
+}
 
-    return 0;
+bool FscryptSetDirectoryPolicy(const std::string& ref_basename, FscryptAction action,
+                               const std::string& dir) {
+    if (action == FscryptAction::kNone) {
+        return true;
+    }
+    if (SetPolicyOn(ref_basename, dir) || action == FscryptAction::kAttempt) {
+        return true;
+    }
+    if (action == FscryptAction::kDeleteIfNecessary) {
+        LOG(ERROR) << "Setting policy failed, deleting: " << dir;
+        delete_dir_contents(dir);
+        return SetPolicyOn(ref_basename, dir);
+    }
+    return false;
 }

init/fscrypt_init_extensions.h

@@ -18,5 +18,13 @@
 
 #include <string>
 
-int fscrypt_install_keyring();
-int fscrypt_set_directory_policy(const std::string& dir);
+enum class FscryptAction {
+    kNone,
+    kAttempt,
+    kRequire,
+    kDeleteIfNecessary,
+};
+
+bool FscryptInstallKeyring();
+bool FscryptSetDirectoryPolicy(const std::string& ref_basename, FscryptAction action,
+                               const std::string& dir);