Subject: bug#45198: 28.0.50; Sandbox mode

From: Philipp <[email-masked]>
Date: Sat, 17 Apr 2021 17:44:06 +0200

> Am 17.04.2021 um 17:26 schrieb Mattias Engdegård <[email-masked]>:
> 
> Slightly updated patch for macOS. Obviously not nearly as fancy as the seccomp one but for running something in batch mode that reads from files and writes to stdout/stderr it should do.
> 
> It works and can be pushed right away but it would be nice to have a place to use it, for validation and for tuning the interface. Any plans for that?
> 

I think it would be better to first implement the mechanism and not the high-level `sandbox-enter' function (I think that one needs a bit more discussion), and implement the mechanism as a command-line flag.  This would not only be consistent with the Seccomp implementation, but also be somewhat more conservative in that it wouldn't require the sandboxing functionality to work in arbitrary running Emacs processes.  As we gain more experience with these sandboxing mechanisms, we can look at relaxing these restrictions, but I think initially we should be conservative.

> diff --git a/lisp/subr.el b/lisp/subr.el
> index c2be26a15f..4994771c33 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -6262,4 +6262,20 @@ internal--format-docstring-line
>  This is intended for internal use only."
>    (internal--fill-string-single-line (apply #'format string objects)))
>  
> +(when (eq system-type 'darwin)
> +  (defun sandbox-enter (dirs)
> +    "Enter a sandbox only permitting reading files under DIRS.
> +DIRS is a list of directory names.  Most other operations such as
> +writing files and network access are disallowed.
> +Existing open descriptors can still be used freely."
> +    (darwin-sandbox-init
> +     (concat "(version 1)\n"
> +             "(deny default)\n"
> +             ;; Emacs seems to need /dev/null; allowing it does no harm.
> +             "(allow file-read* (path \"/dev/null\"))\n"
> +             (mapconcat (lambda (dir)
> +                          (format "(allow file-read* (subpath %S))\n" dir))
> +                        dirs ""))))
> +  )
> +
>  ;;; subr.el ends here

I think it would be better to not commit to a high-level interface like `sandbox-enter' yet.  I intentionally held off adding such an interface in my patch because I think it deserves more discussion about the right design and interface.

> diff --git a/src/sysdep.c b/src/sysdep.c
> index d940acc4e0..b6c402ba33 100644
> --- a/src/sysdep.c
> +++ b/src/sysdep.c
> @@ -4286,8 +4286,33 @@ str_collate (Lisp_Object s1, Lisp_Object s2,
>  }
>  #endif	/* WINDOWSNT */
>  
> +#ifdef DARWIN_OS
> +
> +/* This function prototype is not in the platform header files. */

Is there any documentation you could refer to, even only an unofficial one?

> +int sandbox_init_with_parameters(const char *profile,
> +                                 uint64_t flags,
> +                                 const char *const parameters[],
> +                                 char **errorbuf);
> +
> +DEFUN ("darwin-sandbox-init", Fdarwin_sandbox_init, Sdarwin_sandbox_init,
> +       1, 1, 0,
> +       doc: /* Enter a sandbox whose permitted access is curtailed by PROFILE.

I think it would be better to define this as command-line flag, at least initially.  That way, the sandbox can protect code that happens early on, e.g. the startup code.

This needs to somehow document what PROFILE is.

> +Already open descriptors can be used freely. */)

What does this mean?  Emacs doesn't really expose file descriptors to users.

> +  (Lisp_Object profile)
> +{
> +  char *err = NULL;
> +  if (sandbox_init_with_parameters (SSDATA (profile), 0, NULL, &err) != 0)

Missing CHECK_STRING (profile).