improve security of sentinel mechanism

[rptaylor]

The default sentinel behaviour is to check for the existence of a file on the host. This is done using a test -f command executed on the host with nsenter:
https://github.com/weaveworks/kured/blob/main/cmd/kured/main.go#L661

I would propose refactoring this to achieve the same thing in a different way, using a (read-only) hostPath to check for existence of a file on the host, while keeping the non-default option to run an arbitrary sentinel command in the host namespace unchanged. In conjunction with #416 this would allow kured to be non-privileged. This could mostly be handled by the Helm chart, mounting a configurable directory on the host (the sentinel file itself could not be mounted as a file-type hostPath because it would normally not exist).

volumes: 
  - name: sentinel
    hostPath:
      path: /somewhere_configurable
      type: Directory

volumeMounts:
  - name: sentinel
    mountPath: /somewhere_arbitrary
    readOnly: true

A rebootSentinel option already exists in the helm chart and kured CLI.
Looks like these helm path helpers would be perfect to split up the given rebootSentinel path; Dir could be used to get the hostPath directory to mount and Base could be passed to the kured executable, to find the file name inside the arbitrary directory in the pod.

[ckotzbauer]

I like this idea, it could improve the security for the "default way". However, for custom sentinel-commands this would not be possible. nsenter is still needed there. Maybe this is a job for the helm-chart to limit privileges when the default-sentinel-mechanism is used and use the current privilege-level when using custom commands.

[rptaylor]

Yes, I was thinking along the same lines as #416, there could be a new option rebootSentinelMethod with default value "file"; can also be "command". The helm chart could have a bit of logic that sets the pod to privileged if either rebootMethod or rebootSentinelMethod is "command".

[github-actions]

This issue was automatically considered stale due to lack of activity. Please update it and/or join our slack channels to promote it, before it automatically closes (in 7 days).

[rptaylor]

Still relevant.

[ckotzbauer]

PR #813 is open, there's a detailed description of the implementation and why the mount-point will not be readonly.

[ckotzbauer]

#814 is merged