gcs: do not trigger container shutdown when signaling init process

When implementing signal container process enforcement policy we
introduced a bug, where instead of signalling just the container
init process we ended up sending signals (SIGTERM or SIGKILL) to
all processes running inside a container (by invoking `runc kill --all`).

This results in an unpleasant behavior, where the init process
could be handling (e.g. ignoring) SIGTERM, where as other processes
inside container don't.

This PR makes a change to the order in which the signal container
policy is enforced:
  - always call `EnforceSignalContainerProcessPolicy` before sending
    any signals. Otherwise, this looks like a bug, since we would
    never call `EnforceSignalContainerProcessPolicy` with
    `signalingInitProcess == true` for `SIGTERM` and `SIGKILL` and
    potentially bypassing policies, which do not allow `SIGTERM` or
    `SIGKILL` to be sent to the init process.
  - no longer call `ShutdownContainer` and instead revert back to
    calling `process.Kill`.
  - call `EnforceShutdownContainerPolicy`, when sending `SIGKILL`
    or `SIGTERM` to container init process was SUCCESSFUL, otherwise
    return error or skip it.

Signed-off-by: Maksim An <maksiman@microsoft.com>

Author: anmaxvl

internal/guest/runtime/hcsv2/uvm.go

@@ -683,25 +683,24 @@ func (h *Host) SignalContainerProcess(ctx context.Context, containerID string, p
 		return err
 	}
 
-	signalingInitProcess := (processID == c.initProcess.pid)
-
-	// Don't allow signalProcessV2 to route around container shutdown policy
-	// Sending SIGTERM or SIGKILL to a containers init process will shut down
-	// the container.
-	if signalingInitProcess {
-		if (signal == unix.SIGTERM) || (signal == unix.SIGKILL) {
-			graceful := (signal == unix.SIGTERM)
-			return h.ShutdownContainer(ctx, containerID, graceful)
-		}
-	}
+	signalingInitProcess := processID == c.initProcess.pid
 
 	startupArgList := p.(*containerProcess).spec.Args
 	err = h.securityPolicyEnforcer.EnforceSignalContainerProcessPolicy(ctx, containerID, signal, signalingInitProcess, startupArgList)
 	if err != nil {
 		return err
 	}
 
-	return p.Kill(ctx, signal)
+	if err := p.Kill(ctx, signal); err != nil {
+		return err
+	}
+
+	// Don't allow signalProcessV2 to route around container shutdown policy.
+	// Sending SIGTERM or SIGKILL to a container's init process should trigger container shutdown policy.
+	if signalingInitProcess && (signal == unix.SIGTERM || signal == unix.SIGKILL) {
+		return h.securityPolicyEnforcer.EnforceShutdownContainerPolicy(ctx, containerID)
+	}
+	return nil
 }
 
 func (h *Host) ExecProcess(ctx context.Context, containerID string, params prot.ProcessParameters, conSettings stdio.ConnectionSettings) (_ int, err error) {

internal/guest/runtime/runc/container.go

@@ -74,7 +74,8 @@ func (c *container) ExecProcess(process *oci.Process, stdioSet *stdio.Connection
 	return p, nil
 }
 
-// Kill sends the specified signal to the container's init process.
+// Kill sends the specified signal to the container's init process. When syscall.SIGTERM or syscall.SIGKILL is sent,
+// send the specified signal to all processes inside the container
 func (c *container) Kill(signal syscall.Signal) error {
 	logrus.WithField(logfields.ContainerID, c.id).Debug("runc::container::Kill")
 	args := []string{"kill"}