[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: test command and signature checks

From: Jonathan McCune
Subject: Re: test command and signature checks
Date: Thu, 29 Jan 2015 09:12:47 -0800

On Thu, Jan 29, 2015 at 7:19 AM, Andrei Borzenkov <address@hidden> wrote:
В Thu, 29 Jan 2015 06:17:42 -0800
Jonathan McCune <address@hidden> пишет:

> On Jan 29, 2015 1:19 AM, "Andrei Borzenkov" <address@hidden> 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() (, 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

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

(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.


> > May be file checks should simply disable all filters unconditionally
> > to become more lightweight.
> >
> > _______________________________________________
> > Grub-devel mailing list
> > address@hidden
> >

reply via email to

[Prev in Thread] Current Thread [Next in Thread]