fix various bugs related to policy checks (#535)

Signed-off-by: Connor Smith <[email protected]>

host_core/lib/host_core/actors/actor_module.ex

@@ -498,15 +498,17 @@ defmodule HostCore.Actors.ActorModule do
   defp policy_check(%{source_target: true} = token, agent) do
     lattice_prefix = Agent.get(agent, fn contents -> contents.lattice_prefix end)
     host_id = Agent.get(agent, fn contents -> contents.host_id end)
-    config = VirtualHost.config(host_id)
-    origin = token.invocation["origin"]
+    source = token.invocation["origin"]
     target = token.invocation["target"]
 
     decision =
-      with {:ok, _topic} <- PolicyManager.policy_topic(config),
+      with {:ok, {pid, _}} <- VirtualHost.lookup(host_id),
+           config <- VirtualHost.config(pid),
+           labels <- VirtualHost.labels(pid),
+           {:ok, _topic} <- PolicyManager.policy_topic(config),
            {:ok, source_claims} <-
-             ClaimsManager.lookup_claims(lattice_prefix, origin["public_key"]),
-           {:ok, _target_claims} <-
+             ClaimsManager.lookup_claims(lattice_prefix, source["public_key"]),
+           {:ok, target_claims} <-
              ClaimsManager.lookup_claims(lattice_prefix, target["public_key"]) do
         expired =
           case source_claims[:exp] do
@@ -518,18 +520,31 @@ defmodule HostCore.Actors.ActorModule do
         if expired do
           %{permitted: false}
         else
-          config = VirtualHost.config(host_id)
-
           PolicyManager.evaluate_action(
             config,
-            origin,
-            target,
+            labels,
+            %{
+              publicKey: source["public_key"],
+              contractId: source["contract_id"],
+              linkName: source["link_name"],
+              capabilities: source_claims[:caps],
+              issuer: source_claims[:iss],
+              issuedOn: source_claims[:iat],
+              expiresAt: source_claims[:exp],
+              expired: expired
+            },
+            %{
+              publicKey: target["public_key"],
+              contractId: target["contract_id"],
+              linkName: target["link_name"],
+              issuer: target_claims[:iss]
+            },
             @perform_invocation
           )
         end
       else
-        # Failed to check claims for source or target, denying
         :policy_eval_disabled -> %{permitted: true}
+        # Failed to get host info or check claims for source or target, denying
         :error -> %{permitted: false}
       end
 

host_core/lib/host_core/actors/actor_supervisor.ex

@@ -40,104 +40,101 @@ defmodule HostCore.Actors.ActorSupervisor do
       Tracer.set_attribute("byte_size", byte_size(bytes))
       Logger.debug("Start actor request received for #{oci}", oci_ref: oci)
 
-      case Native.extract_claims(bytes) do
+      source = HostCore.Policy.Manager.default_source()
+
+      with {:ok, {pid, _}} <- VirtualHost.lookup(host_id),
+           config <- VirtualHost.config(pid),
+           labels <- VirtualHost.labels(pid),
+           {:ok, claims} <- get_claims(bytes, oci),
+           target <- %{
+             publicKey: claims.public_key,
+             issuer: claims.issuer,
+             contractId: nil,
+             linkName: nil
+           },
+           %{permitted: true} <-
+             HostCore.Policy.Manager.evaluate_action(config, labels, source, target, @start_actor),
+           {:ok} <- check_other_oci_already_running(oci, claims.public_key, host_id),
+           pids <- start_actor_instances(claims, bytes, oci, annotations, host_id, count) do
+        Tracer.add_event("Actor(s) Started", [])
+        Tracer.set_status(:ok, "")
+        {:ok, pids}
+      else
+        %{permitted: false, message: message, requestId: request_id} ->
+          Tracer.set_status(:error, "Policy denied starting actor, request: #{request_id}")
+          {:error, "Starting actor denied: #{message}"}
+
+        :error ->
+          Tracer.set_status(:error, "Host not found")
+          {:error, "Failed to find host #{host_id}"}
+
         {:error, err} ->
           Tracer.set_status(:error, "#{inspect(err)}")
+          {:error, err}
+      end
+    end
+  end
 
-          Logger.error(
-            "Failed to extract claims from WebAssembly module, an actor must be signed with valid capability claims. (#{byte_size(bytes)} bytes)",
-            oci_ref: oci
-          )
+  defp get_claims(bytes, oci) do
+    case Native.extract_claims(bytes) do
+      {:error, err} ->
+        Logger.error(
+          "Failed to extract claims from WebAssembly module, an actor must be signed with valid capability claims. (#{byte_size(bytes)} bytes)",
+          oci_ref: oci
+        )
 
-          {:error, err}
+        {:error, err}
 
-        {:ok, claims} ->
-          source = %{
-            publicKey: "",
-            contractId: "",
-            linkName: "",
-            capabilities: [],
-            issuer: "",
-            issuedOn: "",
-            expiresAt: DateTime.utc_now() |> DateTime.add(60) |> DateTime.to_unix(),
-            expired: false
-          }
-
-          target = %{
-            publicKey: claims.public_key,
-            issuer: claims.issuer,
-            contractId: nil,
-            linkName: nil
-          }
-
-          config = VirtualHost.config(host_id)
-
-          with %{permitted: true} <-
-                 HostCore.Policy.Manager.evaluate_action(
-                   config,
-                   source,
-                   target,
-                   @start_actor
-                 ),
-               false <- other_oci_already_running?(oci, claims.public_key, host_id) do
-            # Start `count` instances of this actor
-            case Enum.reduce_while(1..count, [], fn _count, pids ->
-                   opts = %{
-                     claims: claims,
-                     bytes: bytes,
-                     oci: oci,
-                     annotations: annotations,
-                     host_id: host_id
-                   }
-
-                   case DynamicSupervisor.start_child(
-                          __MODULE__,
-                          {ActorModule, opts}
-                        ) do
-                     {:error, err} ->
-                       {:halt, {:error, "Error: #{inspect(err)}"}}
-
-                     {:ok, pid} ->
-                       {:cont, [pid | pids]}
-
-                     {:ok, pid, _info} ->
-                       {:cont, [pid | pids]}
-
-                     :ignore ->
-                       {:cont, pids}
-                   end
-                 end) do
-              {:error, err} ->
-                Tracer.set_status(:error, "#{inspect(err)}")
-                {:error, err}
-
-              pids ->
-                Tracer.add_event("Actor(s) Started", [])
-                Tracer.set_status(:ok, "")
-                {:ok, pids}
-            end
-          else
-            true ->
-              Tracer.set_status(:error, "Already running")
-
-              {:error,
-               "Cannot start new instance of #{claims.public_key} from ref '#{oci}', it is already running with different reference. To upgrade an actor, use live update."}
-
-            %{permitted: false, message: message, requestId: request_id} ->
-              Tracer.set_status(:error, "Policy denied starting actor, request: #{request_id}")
-              {:error, "Starting actor #{claims.public_key} denied: #{message}"}
-          end
-      end
+      {:ok, claims} ->
+        {:ok, claims}
     end
   end
 
   # Returns whether the given actor's public key has at least one
   # OCI reference running _other_ than the candidate supplied.
-  defp other_oci_already_running?(coci, pk, host_id) do
-    Enum.any?(
-      host_ocirefs(host_id),
-      fn {_pid, tpk, toci} -> tpk == pk && toci != coci end
-    )
+  defp check_other_oci_already_running(oci, pk, host_id) do
+    case Enum.any?(
+           host_ocirefs(host_id),
+           fn {_pid, other_pk, other_oci} -> other_pk == pk && other_oci != oci end
+         ) do
+      true ->
+        {:error,
+         "Cannot start new instance of #{pk} from ref '#{oci}', it is already running with different reference. To upgrade an actor, use live update."}
+
+      false ->
+        {:ok}
+    end
+  end
+
+  defp start_actor_instances(claims, bytes, oci, annotations, host_id, count) do
+    # Start `count` instances of this actor
+    opts = %{
+      claims: claims,
+      bytes: bytes,
+      oci: oci,
+      annotations: annotations,
+      host_id: host_id
+    }
+
+    Enum.reduce_while(1..count, [], fn _count, pids ->
+      case DynamicSupervisor.start_child(
+             __MODULE__,
+             {ActorModule, opts}
+           ) do
+        {:error, err} ->
+          Logger.error("Failed to start actor instance", err)
+          {:halt, {:error, "Error: #{inspect(err)}"}}
+
+        {:ok, pid} ->
+          {:cont, [pid | pids]}
+
+        {:ok, pid, _info} ->
+          {:cont, [pid | pids]}
+
+        :ignore ->
+          {:cont, pids}
+      end
+    end)
   end
 
   def start_actor_from_oci(host_id, ref, count \\ 1, annotations \\ %{}) do

host_core/lib/host_core/policy/manager.ex

@@ -50,11 +50,12 @@ defmodule HostCore.Policy.Manager do
 
   @spec evaluate_action(
           host_config :: HostCore.Vhost.Configuration.t(),
+          labels :: Map.t(),
           source :: Map.t(),
           target :: Map.t(),
           action :: String.t()
         ) :: Map.t()
-  def evaluate_action(host_config, source, target, action) do
+  def evaluate_action(host_config, labels, source, target, action) do
     with {:ok, topic} <- Manager.policy_topic(host_config),
          nil <- cached_decision(source, target, action, host_config.lattice_prefix),
          :ok <- validate_source(source),
@@ -71,7 +72,7 @@ defmodule HostCore.Policy.Manager do
           publicKey: host_config.host_key,
           issuer: host_config.cluster_key,
           latticeId: host_config.lattice_prefix,
-          labels: host_config.labels,
+          labels: labels,
           clusterIssuers: host_config.cluster_issuers
         }
       }
@@ -177,6 +178,20 @@ defmodule HostCore.Policy.Manager do
     end
   end
 
+  @spec default_source() :: Map.t()
+  def default_source() do
+    %{
+      publicKey: "",
+      contractId: "",
+      linkName: "",
+      capabilities: [],
+      issuer: "",
+      issuedOn: "",
+      expiresAt: DateTime.utc_now() |> DateTime.add(60) |> DateTime.to_unix(),
+      expired: false
+    }
+  end
+
   ##
   # Basic validation of source, target, and action ensuring required fields are present
   ##

host_core/lib/host_core/providers/provider_supervisor.ex

@@ -237,18 +237,7 @@ defmodule HostCore.Providers.ProviderSupervisor do
          config_json,
          annotations
        ) do
-    config = VirtualHost.config(host_id)
-
-    source = %{
-      publicKey: "",
-      contractId: "",
-      linkName: "",
-      capabilities: [],
-      issuer: "",
-      issuedOn: "",
-      expiresAt: DateTime.utc_now() |> DateTime.add(60) |> DateTime.to_unix(),
-      expired: false
-    }
+    source = HostCore.Policy.Manager.default_source()
 
     target = %{
       publicKey: claims.public_key,
@@ -257,9 +246,13 @@ defmodule HostCore.Providers.ProviderSupervisor do
       contractId: contract_id
     }
 
-    with %{permitted: true} <-
+    with {:ok, {pid, _}} <- VirtualHost.lookup(host_id),
+         config <- VirtualHost.config(pid),
+         labels <- VirtualHost.labels(pid),
+         %{permitted: true} <-
            HostCore.Policy.Manager.evaluate_action(
              config,
+             labels,
              source,
              target,
              @start_provider
@@ -287,8 +280,12 @@ defmodule HostCore.Providers.ProviderSupervisor do
         Tracer.set_status(:error, "Policy denied starting provider, request: #{request_id}")
         {:error, "Starting provider #{claims.public_key} denied: #{message}"}
 
-      _ ->
+      true ->
         {:error, "Provider is already running on this host"}
+
+      {:error, err} ->
+        Tracer.set_status(:error, "#{inspect(err)}")
+        {:error, err}
     end
   end
 

host_core/lib/host_core/vhost/virtual_host.ex

@@ -191,6 +191,7 @@ defmodule HostCore.Vhost.VirtualHost do
   def lookup(host_id) do
     case Registry.lookup(Registry.HostRegistry, host_id) do
       [] ->
+        Logger.warn("Failed to look up host ID #{host_id}")
         :error
 
       [{pid, value}] ->
@@ -233,6 +234,7 @@ defmodule HostCore.Vhost.VirtualHost do
         config
 
       [] ->
+        Logger.warn("Failed to find config for host ID #{host_id}")
         nil
     end
   end