Commit Graph

2917 Commits

Author SHA1 Message Date
Alejandro Colomar
82480995b4 Remove unused function: gr_append_member()
Reported-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-02-03 22:03:38 -06:00
Serge Hallyn
1058872a0b Improve TTYGROUP description in login.defs manpage
Closes #457

The existing prose was confusing, or simply wrong.  Make it clear
that only the group ownership of the tty is affected, and how.
Also move the paragraph about defaults after the discussion of
acceptable TTYGROUPs, as this seems more natural.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2023-02-02 22:03:45 -06:00
Alejandro Colomar
1f6f1669cf Remove superfluous casts to 'void*'
Every non-const pointer converts automatically to it.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-02-02 22:03:26 -06:00
Alejandro Colomar
62172f6fb5 Call NULL by its name
In variadic functions we still do the cast.  In POSIX, it's not
necessary, since NULL is required to be of type 'void *', and 'void *'
is guaranteed to have the same alignment and representation as 'char *'.
However, since ISO C still doesn't mandate that, and moreover they're
doing dubious stuff by adding nullptr, let's be on the cautious side.
Also, C++ requires that NULL is _not_ 'void *', but either plain 0 or
some magic stuff.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-02-02 13:08:30 -06:00
Alejandro Colomar
1482224c54 Use freezero(3) where suitable
It originated in OpenBSD, and is available in libbsd.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-02-02 12:04:28 +01:00
Samanta Navarro
8e0ad48c21 Prevent out of boundary access
If lines start with '\0' then it is possible to trigger out of
boundary accesses.

Check if indices are valid before accessing them.

Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
2023-02-01 15:47:35 -06:00
Samanta Navarro
ffc480c2e9 Explicitly override only newlines
Override only newlines with '\0' to avoid undesired truncation of
actual line content.

Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
2023-02-01 15:47:35 -06:00
Samanta Navarro
37ae232080 Correctly handle illegal system file in tz
If the file referenced by ENV_TZ has a zero length string, then an out
of boundary write occurs. Also the result can be wrong because it is
assumed that the file will always end with a newline.

Only override a newline character with '\0' to avoid these cases.

This cannot be considered to be security relevant because login.defs
and its contained references to system files should be trusted to begin
with.

Proof of Concept:

1. Compile shadow's su with address sanitizer and --without-libpam

2. Setup your /etc/login.defs to contain ENV_TZ=/etc/tzname

3. Prepare /etc/tzname to contain a '\0' byte at the beginning

`python -c "print('\x00')" > /etc/tzname`

4. Use su

`su -l`

You can see the following output:

`tz.c:45:8: runtime error: index 18446744073709551615 out of bounds for type 'char [8192]'`

Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
2023-02-01 15:47:35 -06:00
Alejandro Colomar
03bbe6c418 leading_zerosul(): Fix bug
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-02-01 09:10:34 +01:00
Alejandro Colomar
2a61122b5e Unoptimize the higher part of the domain of csrand_uniform()
__int128, which is needed for optimizing that part of the range, is not
always available.  We need the unoptimized version for portability
reasons.

Closes: <https://github.com/shadow-maint/shadow/issues/634>
Fixes: 1a0e13f94e ("Optimize csrand_uniform()")
Reported-by: Adam Sampson <ats@offog.org>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-30 18:24:15 +01:00
Alejandro Colomar
0712b236c3 Add bit manipulation functions
We do need the unoptimized version of csrand_uniform() for high values
of `n`, since the optimized version depends on having __int128, and it's
not available on several platforms, including ARMv7, IA32, and MK68k.

This reverts commit 848f53c1d3c1362c86d3baab6906e1e4419d2634; however,
I applied some tweaks to the reverted commit.

Reported-by: Adam Sampson <ats@offog.org>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-30 18:24:15 +01:00
Alejandro Colomar
848f53c1d3 Revert "Add bit manipulation functions"
Now that we optimized csrand_uniform(), we don't need these functions.

This reverts commit 7c8fe291b1260e127c10562bfd7616961013730f.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Alejandro Colomar
1a0e13f94e Optimize csrand_uniform()
Use a different algorithm to minimize rejection.  This is essentially
the same algorithm implemented in the Linux kernel for
__get_random_u32_below(), but written in a more readable way, and
avoiding microopimizations that make it less readable.

Which (the Linux kernel implementation) is itself based on Daniel
Lemire's algorithm from "Fast Random Integer Generation in an Interval",
linked below.  However, I couldn't really understand that paper very
much, so I had to reconstruct the proofs from scratch, just from what I
could understand from the Linux kernel implementation source code.

I constructed some graphical explanation of how it works, and why it
is optimal, because I needed to visualize it to understand it.  It is
published in the GitHub pull request linked below.

Here goes a wordy explanation of why this algorithm based on
multiplication is better optimized than my original implementation based
on masking.

masking:

	It discards the extra bits of entropy that are not necessary for
	this operation.  This works as if dividing the entire space of
	possible csrand() values into smaller spaces of a size that is
	a smaller power of 2.  Each of those smaller spaces has a
	rejection band, so we get as many rejection bands as spaces
	there are.  For smaller values of 'n', the size of each
	rejection band is smaller, but having more rejection bands
	compensates for this, and results in the same inefficiency as
	for large values of 'n'.

multiplication:

	It divides the entire space of possible random numbers in
	chunks of size exactly 'n', so that there is only one rejection
	band that is the remainder of `2^64 % n`.  The worst case is
	still similar to the masking algorithm, a rejection band that is
	almost half the entire space (n = 2^63 + 1), but for lower
	values of 'n', by only having one small rejection band, it is
	much faster than the masking algorithm.

	This algorithm, however, has one caveat: the implementation
	is harder to read, since it relies on several bitwise tricky
	operations to perform operations like `2^64 % n`, `mult % 2^64`,
	and `mult / 2^64`.  And those operations are different depending
	on the number of bits of the maximum possible random number
	generated by the function.  This means that while this algorithm
	could also be applied to get uniform random numbers in the range
	[0, n-1] quickly from a function like rand(3), which only
	produces 31 bits of (non-CS) random numbers, it would need to be
	implemented differently.  However, that's not a concern for us,
	it's just a note so that nobody picks this code and expects it
	to just work with rand(3) (which BTW I tried for testing it, and
	got a bit confused until I realized this).

Finally, here's some light testing of this implementation, just to know
that I didn't goof it.  I pasted this function into a standalone
program, and run it many times to find if it has any bias (I tested also
to see how many iterations it performs, and it's also almost always 1,
but that test is big enough to not paste it here).

int main(int argc, char *argv[])
{
	printf("%lu\n", csrand_uniform(atoi(argv[1])));
}

$ seq 1 1000 | while read _; do ./a.out 3; done | grep 1 | wc -l
341
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 1 | wc -l
339
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 1 | wc -l
338
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 2 | wc -l
336
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 2 | wc -l
328
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 2 | wc -l
335
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 0 | wc -l
332
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 0 | wc -l
331
$ seq 1 1000 | while read _; do ./a.out 3; done | grep 0 | wc -l
327

This isn't a complete test for a cryptographically-secure random number
generator, of course, but I leave that for interested parties.

Link: <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e9a688bcb19348862afe30d7c85bc37c4c293471>
Link: <https://github.com/shadow-maint/shadow/pull/624#discussion_r1059574358>
Link: <https://arxiv.org/abs/1805.10941>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Björn Esser <besser82@fedoraproject.org>
Cc: Yann Droneaud <ydroneaud@opteya.com>
Cc: Joseph Myers <joseph@codesourcery.com>
Cc: Sam James <sam@gentoo.org>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
[Daniel Lemire: Added link to research paper in source code]
Cc: Daniel Lemire <daniel@lemire.me>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Alejandro Colomar
217b054cf5 Use WIDTHOF() instead of its expansion
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Alejandro Colomar
3f90eff494 Add WIDTHOF() to get the width in bits
It is common to use the expression 'sizeof(x) * CHAR_BIT' to mean the
width in bits of a type or object.  Now that there are _WIDTH macros for
some types, indicating the number of bits that they use, it makes sense
to wrap this calculation in a macro of a similar name.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Alejandro Colomar
1db190cb66 Rewrite csrand_interval() as a wrapper around csrand_uniform()
The old code didn't produce very good random numbers.  It had a bias.
And that was from performing some unnecessary floating-point
calculations that overcomplicate the problem.

Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Björn Esser <besser82@fedoraproject.org>
Cc: Yann Droneaud <ydroneaud@opteya.com>
Cc: Joseph Myers <joseph@codesourcery.com>
Cc: Sam James <sam@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Alejandro Colomar
31375d48ca Add csrand_uniform()
This API is similar to arc4random_uniform(3).  However, for an input of
0, this function is equivalent to csrand(), while arc4random_uniform(0)
returns 0.

This function will be used to reimplement csrand_interval() as a wrapper
around this one.

The current implementation of csrand_interval() doesn't produce very good
random numbers.  It has a bias.  And that comes from performing some
unnecessary floating-point calculations that overcomplicate the problem.

Looping until the random number hits within bounds is unbiased, and
truncating unwanted bits makes the overhead of the loop very small.

We could reduce loop overhead even more, by keeping unused bits of the
random number, if the width of the mask is not greater than
ULONG_WIDTH/2, however, that complicates the code considerably, and I
prefer to be a bit slower but have simple code.

BTW, Björn really deserves the copyright for csrand() (previously known
as read_random_bytes()), since he rewrote it almost from scratch last
year, and I kept most of its contents.  Since he didn't put himself in
the copyright back then, and BSD-3-Clause doesn't allow me to attribute
derived works, I won't add his name, but if he asks, he should be put in
the copyright too.

Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Björn Esser <besser82@fedoraproject.org>
Cc: Yann Droneaud <ydroneaud@opteya.com>
Cc: Joseph Myers <joseph@codesourcery.com>
Cc: Sam James <sam@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Alejandro Colomar
4a56f2baab Add bit manipulation functions
These functions implement bit manipulation APIs, which will be added to
C23, so that in the far future, we will be able to replace our functions
by the standard ones, just by adding the stdc_ prefix, and including
<stdbit.h>.

However, we need to avoid UB for an input of 0, so slightly deviate from
C23, and use a different name (with _wrap) for distunguishing our API
from the standard one.

Cc: Joseph Myers <joseph@codesourcery.com>
Cc: Yann Droneaud <ydroneaud@opteya.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Alejandro Colomar
be1f4f7972 Move csrand() to a new file csrand.c
A set of APIs similar to arc4random(3) is complex enough to deserve its
own file.

Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Björn Esser <besser82@fedoraproject.org>
Cc: Yann Droneaud <ydroneaud@opteya.com>
Cc: Joseph Myers <joseph@codesourcery.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Alejandro Colomar
986ef4e69c Use naming consistent with other common functions
arc4random(3) returns a number.
arc4random_buf(3) fills a buffer.
arc4random_uniform(3) returns a number less than a bound.

and I'd add a hypothetical one which we use:

*_interval() should return a number within the interval [min, max].

In reality, the function being called csrand() in this patch is not
really cryptographically secure, since it had a bias, but a subsequent
patch will fix that.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Alejandro Colomar
6d2337d9e8 Fix types of the csrand_interval() API
We were always casting the result to u_long.  Better just use that type
in the function.  Since we're returning u_long, it makes sense to also
specify the input as u_long.  In fact, that'll help for doing bitwise
operations inside this function.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Alejandro Colomar
8f441c9f7a Use a more precise name for a CSPRNG API with an interval
I have plans to split this function in smaller functions that implement
bits of this functionallity, to simplify the implementation.  So, let's
use names that distinguish them.

This one produces a number within an interval, so make that clear.  Also
make clear that the function produces cryptographically-secure numbers.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-27 21:48:37 -06:00
Stefan Schubert
a27d5c51f1 Supporting vendor given -shells- configuration file 2023-01-26 22:45:32 -06:00
Samanta Navarro
b2d202cb5d libmisc: fix grammar
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
2023-01-26 22:44:39 -06:00
Samanta Navarro
b312bc0b4d Fix typos
Typos found with codespell.

Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
2023-01-26 22:44:39 -06:00
Christian Göttsche
194014678e Declare constant data structure const
./lib/pam_defs.h:18:24: warning: ‘conv’ defined but not used [-Wunused-variable]
       18 | static struct pam_conv conv = {
          |                        ^~~~
2023-01-25 12:31:17 +01:00
Christian Göttsche
89be7c0465 Provide strlcpy declaration
strlcpy(3) might not be visible since it is declared in <bsd/string.h>.
This can lead to warnings, like:

    fields.c: In function 'change_field':
    fields.c:103:17: warning: implicit declaration of function 'strlcpy'; did you mean 'strncpy'? [-Wimplicit-function-declaration]
      103 |                 strlcpy (buf, cp, maxsize);
          |                 ^~~~~~~
          |                 strncpy

    ../lib/fields.c:103:17: warning: type of 'strlcpy' does not match original declaration [-Wlto-type-mismatch]
      103 |                 strlcpy (buf, cp, maxsize);
          |                 ^
    /usr/include/bsd/string.h:44:8: note: return value type mismatch
       44 | size_t strlcpy(char *dst, const char *src, size_t siz);
          |        ^
    /usr/include/bsd/string.h:44:8: note: type 'size_t' should match type 'int'
    /usr/include/bsd/string.h:44:8: note: 'strlcpy' was previously declared here
    /usr/include/bsd/string.h:44:8: note: code may be misoptimized unless '-fno-strict-aliasing' is used
2023-01-25 12:31:17 +01:00
Christian Göttsche
c99d8d0a08 Avoid comparisons of different signs
Comparisons if different signedness can result in unexpected results.
Add casts to ensure operants are of the same type.

    gettime.c: In function 'gettime':
    gettime.c:58:26: warning: comparison of integer expressions of different signedness: 'long long unsigned int' and 'time_t' {aka 'long int'} [-Wsign-compare]
       58 |         } else if (epoch > fallback) {
          |                          ^

Cast to time_t, since epoch is less than ULONG_MAX at this point.

    idmapping.c: In function 'write_mapping':
    idmapping.c:202:48: warning: comparison of integer expressions of different signedness: 'int' and 'long unsigned int' [-Wsign-compare]
      202 |                 if ((written <= 0) || (written >= (bufsize - (pos - buf)))) {
          |                                                ^~

    newgidmap.c: In function ‘main’:
    newgidmap.c:178:40: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
      178 |         if ((written <= 0) || (written >= sizeof(proc_dir_name))) {
          |                                        ^~
    newuidmap.c: In function ‘main’:
    newuidmap.c:107:40: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
      107 |         if ((written <= 0) || (written >= sizeof(proc_dir_name))) {
          |                                        ^~
2023-01-25 12:31:17 +01:00
Christian Göttsche
43508ac476 Drop redundant declaration
environ is exported in <unistd.h>.

    env.c:29:15: warning: redundant redeclaration of 'environ' [-Wredundant-decls]
       29 | extern char **environ;
          |               ^~~~~~~
    login.c:92:15: warning: redundant redeclaration of ‘environ’ [-Wredundant-decls]
       92 | extern char **environ;
          |               ^~~~~~~
    sulogin.c:40:15: warning: redundant redeclaration of ‘environ’ [-Wredundant-decls]
       40 | extern char **environ;
          |               ^~~~~~~
    newgrp.c:32:15: warning: redundant redeclaration of ‘environ’ [-Wredundant-decls]
       32 | extern char **environ;
          |               ^~~~~~~
2023-01-25 12:31:17 +01:00
Christian Göttsche
46d3058341 copydir: fix impl usage
copydir.c: In function 'copy_dir':
    copydir.c:517:32: warning: passing argument 1 of 'copy_tree' from incompatible pointer type [-Wincompatible-pointer-types]
      517 |             return (copy_tree (src, dst, false, reset_selinux,
          |                                ^~~
          |                                |
          |                                const struct path_info *
    In file included from copydir.c:20:
    ../lib/prototypes.h:108:35: note: expected 'const char *' but argument is of type 'const struct path_info *'
      108 | extern int copy_tree (const char *src_root, const char *dst_root,
          |                       ~~~~~~~~~~~~^~~~~~~~
    copydir.c:517:37: warning: passing argument 2 of 'copy_tree' from incompatible pointer type [-Wincompatible-pointer-types]
      517 |             return (copy_tree (src, dst, false, reset_selinux,
          |                                     ^~~
          |                                     |
          |                                     const struct path_info *
    ../lib/prototypes.h:108:57: note: expected 'const char *' but argument is of type 'const struct path_info *'
      108 | extern int copy_tree (const char *src_root, const char *dst_root,
          |                                             ~~~~~~~~~~~~^~~~~~~~

Fixes: 74c17c71 ("Add support for skeleton files from /usr/etc/skel")
2023-01-25 12:31:17 +01:00
Christian Göttsche
e0d79ee032 Modernize manual memzero implementation
Instead of using volatile pointers to prevent the compiler from
optimizing the call away, use a memory barrier.
This requires support for embedded assembly, which should be fine after
the recent requirement bumps.
2023-01-25 11:07:25 +01:00
Christian Göttsche
90ead3cfb8 Replace flawed memset_s usage
memset_s() has a different signature than memset(3) or explicit_bzero(),
thus the current code would not compile.  Also memset_s()
implementations are quite rare.
Use the C23 standardized version memset_explicit(3).

Fixes: 7a799ebb ("Ensure memory cleaning")
2023-01-25 11:07:25 +01:00
Alejandro Colomar
b2bed465e8 Use getnameinfo(3) instead of our own equivalent
I didn't know getnameinfo(3) existed, so I implemented it, or something
similar to it called inet_sockaddr2str().  Let's use the standard API.

Link: <https://inbox.sourceware.org/libc-alpha/0f25d60f-f183-b518-b6c1-6d46aa63ee57@gmail.com/T/>
Link: <https://stackoverflow.com/a/42190913/6872717>
Link: <https://github.com/shadow-maint/shadow/pull/617>
Link: <https://software.codidact.com/posts/287748>
Cc: Zack Weinberg <zack@owlfolio.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-20 10:23:03 -06:00
Alejandro Colomar
ac8b81c2b7 Prefer getrandom(3)/getentropy(3) over arc4random(3bsd)
arc4random(3) without kernel support is unsafe, as it can't know when to
drop the buffer.  Since we depend on libbsd since recently, we have
arc4random(3) functions always available, and thus, this code would have
always called arc4random_buf(3bsd), which is unsafe.  Put it after some
better alternatives, at least until in a decade or so all systems have a
recent enough glibc.

glibc implements arc4random(3) safely, since it's just a wrapper around
getrandom(2).

Link: <https://inbox.sourceware.org/libc-alpha/20220722122137.3270666-1-adhemerval.zanella@linaro.org/>
Link: <https://inbox.sourceware.org/libc-alpha/5c29df04-6283-9eee-6648-215b52cfa26b@cs.ucla.edu/T/>
Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Guillem Jover <guillem@hadrons.org>
Cc: Björn Esser <besser82@fedoraproject.org>
Reviewed-by: "Jason A. Donenfeld" <Jason@zx2c4.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-16 10:12:31 +01:00
Serge Hallyn
39ecca84d4 workflow: update checkout acton v2 to v3
Signed-off-by: Serge Hallyn <serge@hallyn.com>
2023-01-13 09:51:05 +01:00
SoumyaWind
670cae8348 shadow: Fix can not print full login timeout message
Login timed out message prints only first few bytes when write is immediately followed by exit.
Calling exit from new handler provides enough time to display full message.
2023-01-12 18:30:32 -06:00
lilinjie
abeb5f3794 fix typo
Signed-off-by: lilinjie <lilinjie@uniontech.com>
2023-01-12 12:10:57 +01:00
Christian Göttsche
1d936c968a Warn if failed to read existing /etc/nsswitch.conf
Commit 90424e7c ("Don't warn when failed to open /etc/nsswitch.conf")
removed the logging for failing to read /etc/nsswitch.conf to reduce the
noise in the case the file does not exists (e.g. musl based systems).

Reintroduce a warning if /etc/nsswitch.conf exists but we failed to read
it (e.g. permission denied).

Improves: 90424e7c ("Don't warn when failed to open /etc/nsswitch.conf")
2023-01-04 14:21:43 -06:00
Alejandro Colomar
609c641323 Call inet_sockaddr2str() instead of inet_ntop(3)
To simplify.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-02 08:20:43 +01:00
Alejandro Colomar
bb3a89577c Add inet_sockaddr2str() to wrap inet_ntop(3)
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-02 08:20:43 +01:00
Alejandro Colomar
eec5f9fccc Replace gethostbyname(3) by getaddrinfo(3)
gethostbyname(3) was removed in POSIX.1-2008.  It has been obsoleted,
and replaced by getaddrinfo(3), which is superior in several ways:

-  gethostbyname(3) is not reentrant.  There's a GNU extension,
   gethostbyname_r(3) which is reentrant, but it's not likely to be
   standardized for the following reason.  And we don't care too much
   about this point either.

-  gethostbyname(3) only supports IPv4, but getaddrinfo(3) supports both
   IPv4 and IPv6 (and may support other address families in the future).

We don't care about reentrancy, so for keeping the code simple (i.e.,
not touch call site to add code to free(3) an allocated buffer), I added
a static buffer for inet_ntop(3).  We could address that in the future,
but I don't think it's worth it.

BTW, we also replace inet_ntoa(3) by inet_ntop(3), as a consequence of
using getaddrinfo(3).  inet_ntoa(3) is also marked as deprecated, but
that deprecation seems to have been documented only in the manual page,
and POSIX doesn't mark it as deprecated.  The deprecation notice goes
back to when the inet_ntop(3) manual page was added by Sam Varshavchik
to the Linux man-pages in version 1.30 (year 2000).

So, this, apart from updating the code to POSIX.1-2008, is also adding
support for IPv6 :)  Although, probably many other parts of the code are
written for IPv4 only, so I wouldn't yet claim support for it.

A few notes:

-  I didn't check the return value of inet_ntop(3), since it can't fail
   for the given input:

   -  EAFNOSUPPORT:  We only call it with AF_INET and AF_INET6.
   -  ENOSPC:  We calculate the size of the buffer to be wide enough:
               MAX(INET_ADDRSTRLEN, INET6_ADDRSTRLEN) so it always fits.

Cc: Dave Hagewood <admin@arrowweb.com>
Cc: Sam Varshavchik
Cc: Jakub Jelinek <jakub@redhat.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2023-01-02 08:20:43 +01:00
ed neville
65470e5c7d changing lock mechanism
Systems can suffer power interruptions whilst .lock files are in /etc,
preventing scripts and other automation tools from updating shadow's
files which persist across boots.

This commit replaces that mechanism with file locking to avoid problems
of power interruption/crashing.

Minor tweak to groupmems man page, requested by 'xx' on IRC.

Signed-off-by: ed neville <ed@s5h.net>
2022-12-29 13:58:49 -06:00
Serge Hallyn
bc18c184e5 chfn: new_fields: fix wrong fields printed
When the caller may not change the room number, work phone, or
home number, then rather than prompting for the new one it will
print the existing one.  But due to a typo it printed the full name
in place of each of those.

Fix the fields being printed.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2022-12-23 09:04:02 +01:00
Alejandro Colomar
eb164165f6 Add NITEMS(arr) to get the number of elements of an array
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2022-12-22 18:20:02 -06:00
Alejandro Colomar
220b352b70 Use strlcpy(3) instead of its pattern
-  Since strncpy(3) is not designed to write strings, but rather
   (null-padded) character sequences (a.k.a. unterminated strings), we
   had to manually append a '\0'.  strlcpy(3) creates strings, so they
   are always terminated.  This removes dependencies between lines, and
   also removes chances of accidents.

-  Repurposing strncpy(3) to create strings requires calculating the
   location of the terminating null byte, which involves a '-1'
   calculation.  This is a source of off-by-one bugs.  The new code has
   no '-1' calculations, so there's almost-zero chance of these bugs.

-  strlcpy(3) doesn't padd with null bytes.  Padding is relevant when
   writing fixed-width buffers to binary files, when interfacing certain
   APIs (I believe utmpx requires null padding at lease in some
   systems), or when sending them to other processes or through the
   network.  This is not the case, so padding is effectively ignored.

-  strlcpy(3) requires that the input string is really a string;
   otherwise it crashes (SIGSEGV).  Let's check if the input strings are
   really strings:

   -  lib/fields.c:
      -  'cp' was assigned from 'newft', and 'newft' comes from fgets(3).

   -  lib/gshadow.c:
      -  strlen(string) is calculated a few lines above.

   -  libmisc/console.c:
      -  'cons' comes from getdef_str, which is a bit cryptic, but seems
         to generate strings, I guess.1

   -  libmisc/date_to_str.c:
      -  It receives a string literal.  :)

   -  libmisc/utmp.c:
      -  'tname' comes from ttyname(3), which returns a string.

   -  src/su.c:
      -  'tmp_name' has been passed to strcmp(3) a few lines above.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
2022-12-22 18:03:39 -06:00
Iker Pedrosa
a48d77bdef strtoday.c: remove unused defines.h inclusion
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2022-12-22 10:39:45 -06:00
Iker Pedrosa
bb0c89d944 strtoday.c: remove USE_GETDATE as it was always used
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2022-12-22 10:39:45 -06:00
Iker Pedrosa
e4441489bc strtoday.c: remove POSIX 1995 conditional dependency
Since the project is supposed to be POSIX.1-2001 compliant it doesn't
make sense to have that added conditionally.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2022-12-22 10:39:45 -06:00
Alejandro Colomar
647d46507d Assume struct tm is defined in <time.h>
It has been a requirement since at least C90, according to tm(3type).

Link: <https://github.com/shadow-maint/shadow/pull/600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2022-12-22 09:49:02 -06:00
Alejandro Colomar
d96bb2868d Assume struct stat has st_atim and st_mtim fields
That's required by POSIX.1-2008.

Link: <https://github.com/shadow-maint/shadow/pull/600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
2022-12-22 09:49:02 -06:00