Skip to content

Commit 8ffbc6f

Browse files
committed
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`. Additionally update framework.rego to have SIGTERM and SIGKILL as default kill signals for init process for framework API versions "0.10.0" and below. New policies must explicitly have kill signals present. Signed-off-by: Maksim An <[email protected]>
1 parent 6efa5fd commit 8ffbc6f

File tree

3 files changed

+10
-12
lines changed

3 files changed

+10
-12
lines changed

internal/guest/runtime/hcsv2/uvm.go

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -683,17 +683,7 @@ func (h *Host) SignalContainerProcess(ctx context.Context, containerID string, p
683683
return err
684684
}
685685

686-
signalingInitProcess := (processID == c.initProcess.pid)
687-
688-
// Don't allow signalProcessV2 to route around container shutdown policy
689-
// Sending SIGTERM or SIGKILL to a containers init process will shut down
690-
// the container.
691-
if signalingInitProcess {
692-
if (signal == unix.SIGTERM) || (signal == unix.SIGKILL) {
693-
graceful := (signal == unix.SIGTERM)
694-
return h.ShutdownContainer(ctx, containerID, graceful)
695-
}
696-
}
686+
signalingInitProcess := processID == c.initProcess.pid
697687

698688
startupArgList := p.(*containerProcess).spec.Args
699689
err = h.securityPolicyEnforcer.EnforceSignalContainerProcessPolicy(ctx, containerID, signal, signalingInitProcess, startupArgList)

internal/guest/runtime/runc/container.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ func (c *container) ExecProcess(process *oci.Process, stdioSet *stdio.Connection
7474
return p, nil
7575
}
7676

77-
// Kill sends the specified signal to the container's init process.
77+
// Kill sends the specified signal to the container's init process. When syscall.SIGTERM or syscall.SIGKILL is sent,
78+
// send the specified signal to all processes inside the container
7879
func (c *container) Kill(signal syscall.Signal) error {
7980
logrus.WithField(logfields.ContainerID, c.id).Debug("runc::container::Kill")
8081
args := []string{"kill"}

pkg/securitypolicy/framework.rego

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -909,6 +909,13 @@ signal_ok(signals) {
909909
input.signal == signal
910910
}
911911

912+
signal_ok(signals) {
913+
count(signals) == 0
914+
input.isInitProcess
915+
semver.compare(data.api.version, "0.11.0") < 0
916+
input.signal in [9, 15]
917+
}
918+
912919
plan9_mounted(target) {
913920
data.metadata.p9mounts[target]
914921
}

0 commit comments

Comments
 (0)