We'd call loop_write() separately for each char. Let's be nice to
serial console users, and write the full string in one go.
Coverity was complaining that we're not checking the return value
from loop_write(). Rework the code a bit and add voidify.
CID#1402323.
When we read from keyring, a temporary buffer is allocated in order to
determine the size needed for the entire data. However, when zeroing that area,
we use the data size returned by the read instead of the lesser size allocate
for the buffer.
That will cause memory corruption that causes systemd-cryptsetup to crash
either when a single large password is used or when multiple passwords have
already been pushed to the keyring.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
This splits out a bunch of functions from fileio.c that have to do with
temporary files. Simply to make the header files a bit shorter, and to
group things more nicely.
No code changes, just some rearranging of source files.
A race condition happens when calling ask_password_auto() multiple times
to unlock several disks on boot and effectively no password caching is
utilized. This patch fixes it by polling the cache when waiting for
the password.
All over the place we define local variables for the various sockopts
that take a bool-like "int" value. Sometimes they are const, sometimes
static, sometimes both, sometimes neither.
Let's clean this up, introduce a common const variable "const_int_one"
(as well as one matching "const_int_zero") and use it everywhere, all
acorss the codebase.
These lines are generally out-of-date, incomplete and unnecessary. With
SPDX and git repository much more accurate and fine grained information
about licensing and authorship is available, hence let's drop the
per-file copyright notice. Of course, removing copyright lines of others
is problematic, hence this commit only removes my own lines and leaves
all others untouched. It might be nicer if sooner or later those could
go away too, making git the only and accurate source of authorship
information.
This part of the copyright blurb stems from the GPL use recommendations:
https://www.gnu.org/licenses/gpl-howto.en.html
The concept appears to originate in times where version control was per
file, instead of per tree, and was a way to glue the files together.
Ultimately, we nowadays don't live in that world anymore, and this
information is entirely useless anyway, as people are very welcome to
copy these files into any projects they like, and they shouldn't have to
change bits that are part of our copyright header for that.
hence, let's just get rid of this old cruft, and shorten our codebase a
bit.
Files which are installed as-is (any .service and other unit files, .conf
files, .policy files, etc), are left as is. My assumption is that SPDX
identifiers are not yet that well known, so it's better to retain the
extended header to avoid any doubt.
I also kept any copyright lines. We can probably remove them, but it'd nice to
obtain explicit acks from all involved authors before doing that.
This macro will read a pointer of any type, return it, and set the
pointer to NULL. This is useful as an explicit concept of passing
ownership of a memory area between pointers.
This takes inspiration from Rust:
https://doc.rust-lang.org/std/option/enum.Option.html#method.take
and was suggested by Alan Jenkins (@sourcejedi).
It drops ~160 lines of code from our codebase, which makes me like it.
Also, I think it clarifies passing of ownership, and thus helps
readability a bit (at least for the initiated who know the new macro)
A couple of fixes:
1. always bzero_explicit() away what we remove from the passphrase
buffer. The UTF-8 code assumes the string remains NUL-terminated, and
we hence should enforce that. memzero() would do too here, but let's
be paranoid after all this is key material.
2. when clearing '*' characters from string, do so counting UTF-8
codepoints properly. We already have code in place to count UTF-8
codepoints when generating '*' characters, hence we should take the
same care when clearing them again.
3. Treat NUL on input as an alternative terminator to newline or EOF.
4. When removing characters from the password always also reset the
"codepoint" index properly.
We already have the terminal open, hence pass the fd we got to
ask_password_tty(), so that it doesn't have to reopen it a second time.
This is mostly an optimization, but it has the nice benefit of making us
independent from RLIMIT_NOFILE issues and so on, as we don't need to
allocate another fd needlessly.
We should rather sleep to much than too little. This otherwise might
result in a busy loop, because we slept too little and then recheck
again coming to the conclusion we need to go to sleep again, and so on.
This is useful so that callers know whether anything at all and how much
was flushed.
This patches through users of this functions to ensure that the return
values > 0 which may be returned now are not propagated in public APIs.
Also, users that ignore the return value are changed to do so explicitly
now.
This moves pretty much all uses of getpid() over to getpid_raw(). I
didn't specifically check whether the optimization is worth it for each
replacement, but in order to keep things simple and systematic I
switched over everything at once.
explicit_bzero was added in glibc 2.25. Make use of it.
explicit_bzero is hardcoded to zero the memory, so string erase now
truncates the string, instead of overwriting it with 'x'. This causes
a visible difference only in the journalctl case.
We don't have plural in the name of any other -util files and this
inconsistency trips me up every time I try to type this file name
from memory. "formats-util" is even hard to pronounce.
According to its manual page, flags given to mkostemp(3) shouldn't include
O_RDWR, O_CREAT or O_EXCL flags as these are always included. Beyond
those, the only flag that all callers (except a few tests where it
probably doesn't matter) use is O_CLOEXEC, so set that unconditionally.
strv_make_nulstr was creating a nulstr which was not a valid nulstr,
because it was missing the terminating NUL. This didn't cause any issues,
because strv_parse_nulstr correctly parsed the result, using the
separately specified length.
But it's confusing to have something called nulstr which really isn't.
It is likely that somebody will try to use strv_make_nulstr() in
some other place, incorrectly.
This patch changes strv_parse_nulstr() to produce a valid nulstr, and
changes the output length parameter to be the minimum number of bytes
which can be later on parsed by strv_parse_nulstr(). This allows the
only user in ask-password-api to be slightly simplified.
Based-on-patch-by: Jean-Sébastien Bour <jean-sebastien@bour.name>
Fixes#3689.
Yu Watanabe
Zbigniew Jędrzejewski-Szmek
Franck Bui
Thadeu Lima de Souza Cascardo
Lennart Poettering
Xiang Fan
Andreas Rammhold
Topi Miettinen