| Age | Commit message (Collapse) | Author |
|---|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
adapted from the changes made in lbutils
i'm just patching this crappy code. lbutils doesn't
work properly on openbsd yet, and i just want nvmutil
to work properly there, so i'm using the old code for
now, on openbsd.
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
guaranteed not to break on openbsd (tm)
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
i'm trying to make nvmutil work on openbsd. the new code
in lbutils is a bit buggy, likely somewhere in mkhtemp.
i'm still debugging it.
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
This will also be used in lbmk itself at some point,
which currently just uses regular mktemp, for tmpdir
handling during the build process.
Renamed util/nvmutil to util/libreboot-utils, which
now contains two tools. The new tool, mkhtemp, is a
hardened implementation of mktemp, which nvmutil
also uses now. Still experimental, but good enough
for nvmutil.
Mkhtemp attempts to provide TOCTOU resistance on
Linux, by using modern features in Linux such as
Openat2 (syscall) with O_EXCL and O_TMPFILE,
and many various security checks e.g.
inode/dev during creation. Checks are done constantly,
to try to detect race conditions. The code is very
strict about things like sticky bits in world writeable
directories, also ownership (it can be made to bar even
root access on files and directories it doesn't own).
It's a security-first implementation of mktemp, likely
even more secure than the OpenBSD mkstemp, but more
auditing and testing is needed - more features are
also planned, including a compatibility mode to make
it also work like traditional mktemp/mkstemp. The
intention, once this becomes stable, is that it will
become a modern drop-in replacement for mkstemp on
Linux and BSD systems.
Some legacy code has been removed, and in general
cleaned up. I wrote mkhtemp for nvmutil, as part of
its atomic write behaviour, but mktemp was the last
remaining liability, so I rewrote that too!
Docs/manpage/website will be made for mkhtemp once
the code is mature.
Other changes have also been made. This is from another
experimental branch of Libreboot, that I'm pushing
early. For example, nvmutil's state machine has been
tidied up, moving more logic back into main.
Mktemp is historically prone to race conditions,
e.g. symlink attacks, directory replacement, remounting
during operation, all sorts of things. Mkhtemp has
been written to solve, or otherwise mitigate, that
problem. Mkhtemp is currently experimental and will
require a major cleanup at some point, but it
already works well enough, and you can in fact use
it; at this time, the -d, -p and -q flags are
supported, and you can add a custom template at
the end, e.g.
mkhtemp -p test -d
Eventually, I will make this have complete parity
with the GNU and BSD implementations, so that it is
fully useable on existing setups, while optionally
providing the hardening as well.
A lot of code has also been tidied up. I didn't
track the changes I made with this one, because
it was a major re-write of nvmutil; it is now
libreboot-utils, and I will continue to write
more programs in here over time. It's basically
now a bunch of hardened wrappers around various
libc functions, e.g. there is also a secure I/O
wrapper for read/write.
There is a custom randomisation function, rlong,
which simply uses arc4random or getrandom, on
BSD and Linux respectively. Efforts are made to
make it as reliable as possible, to the extent
that it never returns with failure; in the unlikely
event that it fails, it aborts. It also sleeps
between failure, to mitigate certain DoS attacks.
You can just go in util/libreboot-utils and
type make, then you will have the nvmutil and
mkhtemp binaries, which you can just use. It
all works. Everything was massively rewritten.
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
handle init in xstatus()
it's now a singleton design
also tidied up some other code
also removed todo.c. bloat.
will do all those anyway.
too much change. i just kept
touching the code until it
looked good
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
this is a big program now. act like it.
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
make a singleton function instead
now there are technically no global variables,
so i can more easily start splitting this up
into multiple linked programs
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
arc4random added in openbsd 2,1
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
sizeof includes the null
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
must be world writeable and not have sticky bits
a bit theoretical, but we're also reading TMPDIR,
which could be anything
due to how this is called, it defaults back to /tmp
if null is returned, so itt's safe
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
set it really high though, so it's still
basically reliably
an EINTR/EAGAIN storm could cause problems
in prw()
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
that function i added was a load of crap. it
worked, but it was a bit dumb, and crap.
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
not secure. i'll just re-add arc4random
and use urandom as the fallback
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
yeah. obvious bug
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
otherwise, a stale descriptor could be manipulated
easily by an attacker over time
very theoretical to be honest
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
whoops
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
the actual cat function just writes to stdout
we need only check that the input is null, which
i've now done.
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
where feasible, don't assign them at declaration
this is especially important for the next change
i'm working on
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
check yourself before you execute yourself
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
masking O_ACCMODE tells you which flag it is
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
call it sooner. set new_state afterward.
i had to uncouple nv from some functions
for this, and i also added some extra
checks especially at exit, about whether
to touch nv (whether it is initialised)
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
check it right after initialisation
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
older compilers might not have -std for example.
the code is portable, but old compilers can't
compile with just "make", you have to add lots
of flags
i will now use "make strict" and "make hell"
in testing, but otherwise make without flags
are fine.
move the current strictness to command:
make strict
added an extra command:
make hell
hell uses -Weverything, and is useful with
clang's strict testing, on which i only got
a very small number of errors (it's way less
than a lot of programs would get with this
flag, because -Weverything is REALLY STRICT):
ja, mich nvmutil$ make hell CC=clang
clang -I. -Wall -Wextra -pedantic -std=c90 -Os -Werror -Weverything nvmutil.c -o nvmutil
In file included from nvmutil.c:35:
./nvmutil.h:225:16: error: padding struct 'struct commands' with 1 byte to align 'rw_size' [-Werror,-Wpadded]
225 | unsigned long rw_size; /* within the 4KB GbE part */
| ^
./nvmutil.h:217:8: error: padding size of 'struct commands' with 4 bytes to alignment boundary [-Werror,-Wpadded]
217 | struct commands {
| ^
./nvmutil.h:235:8: error: padding size of 'struct xfile' with 4 bytes to alignment boundary [-Werror,-Wpadded]
235 | struct xfile {
| ^
./nvmutil.h:288:16: error: padding struct 'struct xstate' with 4 bytes to align 'xsize' [-Werror,-Wpadded]
288 | unsigned long xsize;
| ^
nvmutil.c:617:43: error: implicit conversion changes signedness: 'off_t' (aka 'long') to 'unsigned long' [-Werror,-Wsign-conversion]
617 | _r = rw_file_exact(f->gbe_fd, f->buf, f->gbe_file_size,
| ~~~~~~~~~~~~~ ~~~^~~~~~~~~~~~~
nvmutil.c:626:43: error: implicit conversion changes signedness: 'off_t' (aka 'long') to 'unsigned long' [-Werror,-Wsign-conversion]
626 | _r = rw_file_exact(f->tmp_fd, f->buf, f->gbe_file_size,
| ~~~~~~~~~~~~~ ~~~^~~~~~~~~~~~~
nvmutil.c:654:46: error: implicit conversion changes signedness: 'off_t' (aka 'long') to 'unsigned long' [-Werror,-Wsign-conversion]
654 | _r = rw_file_exact(f->tmp_fd, f->bufcmp, f->gbe_file_size,
| ~~~~~~~~~~~~~ ~~~^~~~~~~~~~~~~
nvmutil.c:661:39: error: implicit conversion changes signedness: 'off_t' (aka 'long') to 'unsigned long' [-Werror,-Wsign-conversion]
661 | if (x_i_memcmp(f->buf, f->bufcmp, f->gbe_file_size) != 0)
| ~~~~~~~~~~ ~~~^~~~~~~~~~~~~
nvmutil.c:702:23: error: implicit conversion loses integer precision: 'int' to 'unsigned char' [-Werror,-Wimplicit-int-conversion]
702 | f->part_valid[_p] = good_checksum(_p);
| ~ ^~~~~~~~~~~~~~~~~
nvmutil.c:1045:21: error: implicit conversion loses integer precision: 'int' to 'unsigned char' [-Werror,-Wimplicit-int-conversion]
1045 | f->part_valid[0] = good_checksum(0);
| ~ ^~~~~~~~~~~~~~~~
nvmutil.c:1046:21: error: implicit conversion loses integer precision: 'int' to 'unsigned char' [-Werror,-Wimplicit-int-conversion]
1046 | f->part_valid[1] = good_checksum(1);
| ~ ^~~~~~~~~~~~~~~~
nvmutil.c:1170:45: error: implicit conversion changes signedness: 'off_t' (aka 'long') to 'unsigned long' [-Werror,-Wsign-conversion]
1170 | (unsigned long)(p * (f->gbe_file_size >> 1)));
| ~ ~~~~~~~~~~~~~~~~~^~~~
nvmutil.c:1269:37: error: implicit conversion loses integer precision: 'int' to 'unsigned short' [-Werror,-Wimplicit-int-conversion]
1269 | return (unsigned short)f->buf[pos] |
| ~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
1270 | ((unsigned short)f->buf[pos + 1] << 8);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
nvmutil.c:1610:9: error: implicit conversion changes signedness: 'off_t' (aka 'long') to 'unsigned long' [-Werror,-Wsign-conversion]
1609 | r = rw_file_exact(f->tmp_fd, f->bufcmp,
| ~~~~~~~~~~~~~
1610 | f->gbe_file_size, 0, IO_PREAD,
| ~~~^~~~~~~~~~~~~
nvmutil.c:1618:9: error: implicit conversion changes signedness: 'off_t' (aka 'long') to 'unsigned long' [-Werror,-Wsign-conversion]
1617 | r = rw_file_exact(dest_fd, f->bufcmp,
| ~~~~~~~~~~~~~
1618 | f->gbe_file_size, 0, IO_PWRITE,
| ~~~^~~~~~~~~~~~~
nvmutil.c:1609:6: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
1609 | r = rw_file_exact(f->tmp_fd, f->bufcmp,
| ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1610 | f->gbe_file_size, 0, IO_PREAD,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1611 | NO_LOOP_EAGAIN, LOOP_EINTR,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~
1612 | MAX_ZERO_RW_RETRY, OFF_ERR);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~
nvmutil.c:1617:6: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
1617 | r = rw_file_exact(dest_fd, f->bufcmp,
| ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1618 | f->gbe_file_size, 0, IO_PWRITE,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1619 | NO_LOOP_EAGAIN, LOOP_EINTR,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~
1620 | MAX_ZERO_RW_RETRY, OFF_ERR);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~
nvmutil.c:1936:45: error: implicit conversion changes signedness: 'long' to 'unsigned long' [-Werror,-Wsign-conversion]
1936 | if (rv >= 0 && (unsigned long)rv > (nrw - rc))
| ~ ^~
nvmutil.c:2193:27: error: signed shift result (0x8000000000000000) sets the sign bit of the shift expression's type ('long') and becomes negative [-Werror,-Wshift-sign-overflow]
2193 | if (nrw > (unsigned long)X_LONG_MAX)
| ^~~~~~~~~~
./nvmutil.h:147:38: note: expanded from macro 'X_LONG_MAX'
147 | #define X_LONG_MAX ((long)(~((long)1 << (sizeof(long)*CHAR_BIT-1))))
| ~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
make: *** [Makefile:42: hell] Fehler 1
in a future commit, i intend to fix all of these issues,
so that the code reliably compiles in hell-mode.
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
this program needs bits to be 8
some obscure systems set it to something else
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
and add another
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
i need to re-initialise r each time.
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
|
yes, this begins the next phase of nvmutil:
remove global status in functions that should be
generic, and make functions that are not generic,
generic. make everything as re-useable in a library
as possible.
most of the program is error control, as it should
be, but much of it is mixed in with functions
that really should just be split up for libraries.
so that is what i'm now beginning.
Signed-off-by: Leah Rowe <leah@libreboot.org>
|
