rillian
commented
2 years ago I don't think the non-zero uid is critical, so I'm fine with reverting it.
The primary protection it offers inside the container is denying write access to system files. In the case of star-randsrv, for example, it prevents a compromised server executable from altering the list of trusted tls certs. It's unfortunate the AWS enclave runtime doesn't support the USER directive, but at least our current applications are staticly linked in minimal containers with few support files, so the risk there is low.
Outside the container, it offers some protection to the host system if it launched in a privileged container, and unwise bind mounts or a successful container escape exploit. Hopefully developers use unprivileged containers, or containers on a separate vm like docker does on macos and windows hosts.
Personally, I would:
- revert the in-memory cert cache for nitriding
- document the expected writable cert cache directories in nitriding
- keep the
USERdirective in the star-randsrv Dockerfile, with a comment that it's ineffective inside the enclave - create the expected writable directories with the correct ownership in the star-randsrv Dockerfile
- change the star-randsrv container image to specify uid
1since it's weird fornobodyto be able to write to anything.
That way everything is maximally clear and there's some protection if someone does run the service in a privileged container.
I noticed that the user ID change we implemented in https://github.com/brave-experiments/star-randsrv/pull/33 is ineffective in an enclave. The UID always remains 0. I reached out to our AWS contacts and was told that this is by design: Their minimal container runtime does not support user ID changes.
We could simply document this behavior in the Dockerfiles that contain a
USERinstruction (e.g., here) but I am tempted to partially undo https://github.com/brave/nitriding/pull/36, https://github.com/brave/nitriding/pull/37 (while keeping the refactored code), and https://github.com/brave-experiments/star-randsrv/pull/33 because these changes introduce a non-trivial amount of complexity that only benefits applications that are run outside an enclave, which I would not consider a critical use case. Do you have thoughts on this, @rillian?