Thor Lancelot Simon | 5 Oct 2006 14:34

Re: poolifying fileassoc

On Thu, Oct 05, 2006 at 05:30:05AM -0700, Chuck Silvers wrote:
> 
> like I explained in earlier mail, page-checking stuff shouldn't be called
> from getpages but rather from the aiodone code.  not only does that avoid
> any problems like this but it also makes it easier to check pages only when
> they're brought into memory the first time and not on later page-faults.

But veriexec _must_ check them on later page faults, or an adversary can
switch them out from underneath it and it becomes worthless (consider an
executable backed by NFS storage.  The per-page code in veriexec is
explicitly intended to address this failure with other executable
verification systems).

--

-- 
  Thor Lancelot Simon	                                     tls <at> rek.tjls.com

  "We cannot usually in social life pursue a single value or a single moral
   aim, untroubled by the need to compromise with others."      - H.L.A. Hart

Chuck Silvers | 5 Oct 2006 15:02
Favicon

Re: poolifying fileassoc

On Thu, Oct 05, 2006 at 08:34:05AM -0400, Thor Lancelot Simon wrote:
> On Thu, Oct 05, 2006 at 05:30:05AM -0700, Chuck Silvers wrote:
> > 
> > like I explained in earlier mail, page-checking stuff shouldn't be called
> > from getpages but rather from the aiodone code.  not only does that avoid
> > any problems like this but it also makes it easier to check pages only when
> > they're brought into memory the first time and not on later page-faults.
> 
> But veriexec _must_ check them on later page faults, or an adversary can
> switch them out from underneath it and it becomes worthless (consider an
> executable backed by NFS storage.  The per-page code in veriexec is
> explicitly intended to address this failure with other executable
> verification systems).

if a file is changed on an NFS server, the existing cached page on the
NFS client is freed and a new page is allocated to hold the new file data.
the new page will thus be checked again.  even if the same page were somehow
reused, the point where it needs to be checked is when the page is initialized
by reading from the NFS server, not when the page is found in the client's
cache, and the aiodone path is a more appropriate place to do that.

-Chuck

Brett Lymn | 6 Oct 2006 02:34
Picon

Re: poolifying fileassoc

On Thu, Oct 05, 2006 at 06:02:19AM -0700, Chuck Silvers wrote:
> 
> if a file is changed on an NFS server, the existing cached page on the
> NFS client is freed and a new page is allocated to hold the new file data.
> the new page will thus be checked again.  even if the same page were somehow
> reused, the point where it needs to be checked is when the page is initialized
> by reading from the NFS server, not when the page is found in the client's
> cache, and the aiodone path is a more appropriate place to do that.
> 

Unfortunately, the NFS code (and possibly other FS code - I have not
looked too far at the moment) does not call aiodone when it is
finished the i/o - anything that used the genfs routines will be ok
but there is no requirement for the FS code to do so.  As a quick test
hack I added the block verify to the nfs_dio_read() and it worked well
in picking up the backing store modifications on a NFS mount - I need
to work out the "correct" way of doing this as wedging the check into
FS depends code is not a good thing.

I had a look at biodone() but it is too late to be deciding the pages
are bad by then I think.  I don't want to randomly call
uvm_aio_aiodone() as it performs a lot of other work that I am not
sure about.

--

-- 
Brett Lymn

YAMAMOTO Takashi | 6 Oct 2006 02:54
Picon

Re: poolifying fileassoc

> On Thu, Oct 05, 2006 at 06:02:19AM -0700, Chuck Silvers wrote:
> > 
> > if a file is changed on an NFS server, the existing cached page on the
> > NFS client is freed and a new page is allocated to hold the new file data.
> > the new page will thus be checked again.  even if the same page were somehow
> > reused, the point where it needs to be checked is when the page is initialized
> > by reading from the NFS server, not when the page is found in the client's
> > cache, and the aiodone path is a more appropriate place to do that.
> > 
> 
> Unfortunately, the NFS code (and possibly other FS code - I have not
> looked too far at the moment) does not call aiodone when it is
> finished the i/o

nfs uses uvm_aio_aiodone, unless you are using nqnfs.
well, it doesn't for sync i/o, but it's the same as ufs.

> - anything that used the genfs routines will be ok
> but there is no requirement for the FS code to do so.

making it a requiremnt can be a choice, maybe.

> As a quick test
> hack I added the block verify to the nfs_dio_read() and it worked well
> in picking up the backing store modifications on a NFS mount - I need
> to work out the "correct" way of doing this as wedging the check into
> FS depends code is not a good thing.

what's nfs_dio_read?

(Continue reading)

Brett Lymn | 6 Oct 2006 03:43
Picon

Re: poolifying fileassoc

On Fri, Oct 06, 2006 at 09:54:56AM +0900, YAMAMOTO Takashi wrote:
> 
> nfs uses uvm_aio_aiodone, unless you are using nqnfs.
> well, it doesn't for sync i/o, but it's the same as ufs.
> 

Hmmmm - I will update my sources and have another look.  How can I
tell if I am using nqnfs or not?  I have been poking around in
src/sys/nfs.

> > - anything that used the genfs routines will be ok
> > but there is no requirement for the FS code to do so.
> 
> making it a requiremnt can be a choice, maybe.
> 

works for me :)

> 
> what's nfs_dio_read?
> 

me mistyping nfs_doio_read() :)

--

-- 
Brett Lymn

YAMAMOTO Takashi | 6 Oct 2006 04:00
Picon

Re: poolifying fileassoc

> On Fri, Oct 06, 2006 at 09:54:56AM +0900, YAMAMOTO Takashi wrote:
> > 
> > nfs uses uvm_aio_aiodone, unless you are using nqnfs.
> > well, it doesn't for sync i/o, but it's the same as ufs.
> > 
> 
> Hmmmm - I will update my sources and have another look.

it isn't a recent change.

> How can I
> tell if I am using nqnfs or not?

check your mount_nfs option "-q".

YAMAMOTO Takashi

M J Fleming | 9 Oct 2006 13:09
Picon
Favicon

Additional features for veriexecgen(8)

Attached is a patch that implements a number of new features for veriexecgen.

The "-F" command-line option instructs veriexecgen(8) to "guess" which
flags should be written to the fingerprint file for certain files, based on 
characteristics of that file (its path, permissions, etc).

For instance, executing,

"veriexecgen -F"

instructs veriexecgen to search the default system paths for files.
This will cause all files that are on a local filesystem and are executable
to have the flag "PROGRAM" written to the fingerprint file. Any files that
are not executable will have "FILE" written to the fingerprint file. If any
of the files on the default system paths are on non-local filesystems,
the flag "UNTRUSTED" will be appended to the flags for that file.

Of course, a way to make even more intelligent decisions about the flags
that will be written to the fingerprint file is needed. This patch also provides
the user with a way to specify (in conjuection with F) that they want 
veriexecgen to use default values for common library paths, script suffixes
and interpreter paths (/bin/sh, /bin/ksh, etc).

These are wildcards for pathnames which are compared against the files, they
can be turned on with command-line options,

- A default list of interpreter paths (-I)
- A default list of library paths (-L)
- A default list of script suffixes  (-S)

(Continue reading)

Elad Efrat | 9 Oct 2006 19:26
Picon

Using a +t /tmp for chpass(1)

Hi,

Few days ago we discussed a change on #NetBSD-code that made it to our
chpass(1), and someone suggested I bring it up on a public list such
as this "just to make sure" -- so I am.

We used to write the temporary files, pw.XXXXX, to /etc, which created
a problem: a user who started chpass(1) could ^Z it, then SIGKILL it,
leaving a temporary file in /etc, slowly filling it up.

Solutions discussed varied from kernel changes to limit signals to
set-id processes (so that, for example, SIGKILL can't be sent; like
how we limit coredumps of set-id processes), but eventually we chose
a different solution.

What we did is this:

	static char tempname[] = "/tmp/pw.XXXXXX";

	[...]

	dfd = mkstemp(tempname);
	if (dfd < 0 || fcntl(dfd, F_SETFD, 1) < 0)
		(*Pw_error)(tempname, 1, 1);
	if (atexit(cleanup)) {
		cleanup();
		errx(1, "couldn't register cleanup");
	}
	if (stat(dirname(tempname), &sb) == -1)
		err(1, "couldn't stat `%s'", dirname(tempname));
(Continue reading)

Hubert Feyrer | 9 Oct 2006 19:34
Picon
Favicon

Re: Using a +t /tmp for chpass(1)

On Mon, 9 Oct 2006, Elad Efrat wrote:
> Solutions discussed varied from kernel changes to limit signals to
> set-id processes (so that, for example, SIGKILL can't be sent; like
> how we limit coredumps of set-id processes), but eventually we chose
> a different solution.
...
> So, here goes -- is the above okay?

Maybe a solution that does not involve changing userland code (and thus 
leaving a chance to do things wrong) can be found? As a first isdea, 
limiting signal delivery sounds ok to me for that, but I'm no guru in that 
(complex) area...

  - Hubert

Christos Zoulas | 9 Oct 2006 19:49

Re: Using a +t /tmp for chpass(1)

In article <452A864E.8050006 <at> NetBSD.org>, Elad Efrat  <elad <at> NetBSD.org> wrote:
>Hi,
>
>Few days ago we discussed a change on #NetBSD-code that made it to our
>chpass(1), and someone suggested I bring it up on a public list such
>as this "just to make sure" -- so I am.
>
>We used to write the temporary files, pw.XXXXX, to /etc, which created
>a problem: a user who started chpass(1) could ^Z it, then SIGKILL it,
>leaving a temporary file in /etc, slowly filling it up.

I think that chpass should obey the /etc/ptmp lock.

>Solutions discussed varied from kernel changes to limit signals to
>set-id processes (so that, for example, SIGKILL can't be sent; like
>how we limit coredumps of set-id processes), but eventually we chose
>a different solution.
>
>What we did is this:
>
>	static char tempname[] = "/tmp/pw.XXXXXX";
>
>	[...]
>
>	dfd = mkstemp(tempname);
>	if (dfd < 0 || fcntl(dfd, F_SETFD, 1) < 0)
>		(*Pw_error)(tempname, 1, 1);
>	if (atexit(cleanup)) {
>		cleanup();
>		errx(1, "couldn't register cleanup");
(Continue reading)


Gmane