On Thu, Jan 29, 2015 at 7:19 AM, Andrei Borzenkov <[email protected]>
wrote:

> В Thu, 29 Jan 2015 06:17:42 -0800
> Jonathan McCune <[email protected]> пишет:
>
> > On Jan 29, 2015 1:19 AM, "Andrei Borzenkov" <[email protected]> wrote:
> > >
> > > What sematic of file tests should be? I think they should just test
> > > file existence; this already happens for compressed files that checks
> > > that on-disk file size, not uncompressed. I think same should apply to
> > > signature checks.
> > >
> >
> > Where the alternative is that an existence check will only succeed if a
> > file has a corresponding (and verifiable) .sig?
> >
> > I think existence-only is the right semantics because verify_detached can
> > be used to achieve the signature-check in a standalone fashion.
> >
> > (I.e., the existing behavior of test and verify_detached seems correct to
> > me.)
> >
>
> Existing behavior is to simply open file so any filter in effect will
> be applied.
>
>
I think it's more subtle than that. I think it depends on which arguments
are provided to test. As I understand it filters are applied in
grub_file_open() (
http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/file.c), and
I think I can use -f to test the existence of an unsigned file while
check_signatures=enforce, without causing signature verification to fail.

Let's consider the case of a grub.cfg using 'test -f', checking for file
existence. From
http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/commands/test.c:

if (grub_strcmp (args[*argn], "-f") == 0)
    {
      get_fileinfo (args[*argn + 1], &ctx);
      /* FIXME: check for other types. */
      update_val (ctx.file_exists && ! ctx.file_info.dir, &ctx);
      (*argn) += 2;
      return ctx.or || ctx.and;
    }

In get_fileinfo() (also test.c) I don't think grub_file_open() is called.
The final else branch calls through a filesystem-specific function pointer
that invokes a callback with entries in some directory ("/* Call HOOK with
each file under DIR.  */" from
http://git.savannah.gnu.org/cgit/grub.git/tree/include/grub/fs.h):

(fs->dir) (dev, path, find_file, ctx);

, which leads to a call to the find_file() function (also in test.c) for
each entry in the directory. The files do *not* seem to be opened.

It looks like the handler for the "-s" ("file exists and has a size greater
than zero") *does* cause the file to get opened (i.e., calls
grub_file_open(), causing filters to get applied), but I think the basic
existence check ("-f") *not* leading to a call to grub_file_open() (and
hence applying all the filters / enforcing mandatory signature checks if
check_signatures=enforce) is the right behavior.

I think this behavior is reasonable, because the attack surface for testing
file existence is the filesystem parsing code, as opposed to the
[potentially unsigned, and populated with some kind of evil] file whose
existence is being checked.

Please do let me know if I've misunderstood the code somehow.

-Jon





> > > May be file checks should simply disable all filters unconditionally
> > > to become more lightweight.
> > >
> > > _______________________________________________
> > > Grub-devel mailing list
> > > [email protected]
> > > https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to