Re: [PATCH RFC v8 01/10] namei: obey trailing magic-link DAC permissions

[Andy Lutomirski]

> On May 23, 2019, at 8:11 PM, Aleksa Sarai <cyphar@xxxxxxxxxx> wrote:
> 
>> On 2019-05-23, Aleksa Sarai <cyphar@xxxxxxxxxx> wrote:
>>> On 2019-05-22, Andy Lutomirski <luto@xxxxxxxxxx> wrote:
>>> What are actual examples of uses for this exception?  Breaking
>>> selftests is not, in and of itself, a huge problem.
>> 
>> Not as far as I know. All of the re-opening users I know of do re-opens
>> of O_PATH or are re-opening with the same (or fewer) privileges. I also
>> ran this for a few days on my laptop without this exception, and didn't
>> have any visible issues.
> 
> I have modified the patch to WARN_ON(may_open_magiclink() == -EACCES).
> 
> So far (in the past day on my openSUSE machines) I have only seen two
> programs which have hit this case: kbd[1]'s "loadkeys" and "kbd_mode"
> binaries. In addition to there not being any user-visible errors -- they
> actually handle permission errors gracefully!
> 
>  static int
>  open_a_console(const char *fnam)
>  {
>      int fd;
> 
>      /*
>       * For ioctl purposes we only need some fd and permissions
>       * do not matter. But setfont:activatemap() does a write.
>       */
>      fd = open(fnam, O_RDWR);
>      if (fd < 0)
>          fd = open(fnam, O_WRONLY);
>      if (fd < 0)
>          fd = open(fnam, O_RDONLY);
>      if (fd < 0)
>          return -1;
>      return fd;
>  }
> 
> The above gets called with "/proc/self/fd/0" as an argument (as well as
> other console candidates like "/dev/console"). And setfont:activatemap()
> actually does handle read-only fds:
> 
>  static void
>  send_escseq(int fd, const char *seq, int n)
>  {
>      if (write(fd, seq, n) != n) /* maybe fd is read-only */
>          printf("%s", seq);
>  }
> 
>  void activatemap(int fd)
>  {
>      send_escseq(fd, "\033(K", 3);
>  }
> 
> So, thus far, not only have I not seen anything go wrong -- the only
> program which actually hits this case handles the error gracefully.
> Obviously we got lucky here, but the lack of any users of this
> mis-feature leads me to have some hope that we can block it without
> anyone noticing.
> 
> But I emphatically do not want to break userspace here (except for
> attackers, obviously).

Hmm. This will break any script that does echo foo >/dev/stdin too.

Just to throw an idea out there, what if the open were allowed if the file mode is sufficient or if the magic link target is openable with the correct mode without magic?  In other words, first check as in your code but without the exception and, if that check fails, then walk the same path that d_path would return and see if it would work as a normal open?  Of course, that second attempt would need to disable magic links to avoid recursing.  Iâm not sure I love this idea...

Otherwise, I imagine we can live with the exception, especially if the new open API turns it off by default.