[environment] Enforce uniqueness of environment label
This enforces in appmgr that the environment label provided through
fuchsia.sys.Environment.CreateNestedEnvironment must be unique within
the scope of the containing enviroment. This will let us rely on the
label as a unique identity for the environment for the purposes of
isolating persistent mutable storage.
CF-28 #comment
Test: fx run-test appmgr_integration_tests -t
appmgr_realm_integration_tests
Change-Id: I2b4831402ec569cc72d7a5451317e76e6b2765a6
diff --git a/bin/appmgr/integration_tests/realm_integration_test.cc b/bin/appmgr/integration_tests/realm_integration_test.cc
index 6e0053e..b2defb6 100644
--- a/bin/appmgr/integration_tests/realm_integration_test.cc
+++ b/bin/appmgr/integration_tests/realm_integration_test.cc
@@ -265,6 +265,34 @@
[&] { return env_controller_status == ZX_ERR_INVALID_ARGS; }, kTimeout));
}
+TEST_F(RealmTest, EnvironmentLabelMustBeUnique) {
+ // Create first environment with label kRealm using EnclosingEnvironment since
+ // that's easy.
+ auto enclosing_environment =
+ CreateNewEnclosingEnvironment(kRealm, CreateServices());
+
+ // Can't use EnclosingEnvironment here since there's no way to discern between
+ // 'not yet created' and 'failed to create'. This also lets us check the
+ // specific status returned.
+ fuchsia::sys::EnvironmentPtr env;
+ fuchsia::sys::EnvironmentControllerPtr env_controller;
+
+ zx_status_t env_status, env_controller_status;
+ env.set_error_handler([&](zx_status_t status) { env_status = status; });
+ env_controller.set_error_handler(
+ [&](zx_status_t status) { env_controller_status = status; });
+
+ // Same environment label as EnclosingEnvironment created above.
+ real_env()->CreateNestedEnvironment(
+ env.NewRequest(), env_controller.NewRequest(), kRealm, nullptr,
+ fuchsia::sys::EnvironmentOptions{});
+
+ EXPECT_TRUE(RunLoopWithTimeoutOrUntil(
+ [&] { return env_status == ZX_ERR_BAD_STATE; }, kTimeout));
+ EXPECT_TRUE(RunLoopWithTimeoutOrUntil(
+ [&] { return env_controller_status == ZX_ERR_BAD_STATE; }, kTimeout));
+}
+
class RealmFakeLoaderTest : public RealmTest, public fuchsia::sys::Loader {
protected:
RealmFakeLoaderTest() {
diff --git a/bin/appmgr/realm.cc b/bin/appmgr/realm.cc
index 8d78a4a..5de8a78 100644
--- a/bin/appmgr/realm.cc
+++ b/bin/appmgr/realm.cc
@@ -315,9 +315,12 @@
}
for (const auto& child : children_) {
if (label == child.first->label_) {
- FXL_LOG(WARNING) << "Attempt to create nested environment '"
- << label.c_str() << "' under '" << label_
- << "' but label matches existing environment";
+ FXL_LOG(ERROR) << "Attempt to create nested environment '"
+ << label.c_str() << "' under '" << label_
+ << "' but label matches existing environment";
+ environment.Close(ZX_ERR_BAD_STATE);
+ controller_request.Close(ZX_ERR_BAD_STATE);
+ return;
}
}
diff --git a/public/fidl/fuchsia.sys/environment.fidl b/public/fidl/fuchsia.sys/environment.fidl
index 90dbd69..f353a8c 100644
--- a/public/fidl/fuchsia.sys/environment.fidl
+++ b/public/fidl/fuchsia.sys/environment.fidl
@@ -38,9 +38,10 @@
// automatically when the |EnvironmentController|'s interface is closed. You
// can use |EnvironmentController.Detach| to disable this behavior.
//
- // |label| defines the new environment's label/name. It can also be used for
- // diagnostic purposes. The label will be truncated if it is longer than
- // |kLabelMaxLength|.
+ // |label| defines the new environment's label/name. It must be unique within
+ // the parent environment (though not globally) and is used for isolating
+ // separate environments. It can also be used for diagnostic purposes. The
+ // label will be truncated if it is longer than |kLabelMaxLength|.
//
// |additional_services|, which may be empty, contains a list of services
// that the environment provides, which are hosted by
