Commit Graph

478 Commits

Author SHA1 Message Date
Xi Ruoyao
274e786be9 libmisc: use /dev/urandom as a generic fallback for read_random_bytes()
On systems with Linux kernel < 3.17, getentropy() and getrandom() may
exist but return ENOSYS.  Use /dev/urandom as a fallback to avoid a hard
requirement on Linux kernel version.

Fixes #512.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
2022-06-19 09:16:38 -05:00
Iker Pedrosa
0b51cde162 Remove commented out code and FIXMEs
In order to remove some of the FIXMEs it was necessary to change the
code and call getulong() instead of getlong().

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2022-05-24 07:49:11 -05:00
juyin
a43d0b95c4 libmisc: add check fopen return value in read_random_bytes()
Returns null when fopen fails. Then, using fread with a null pointer will cause a segfault.

Signed-off-by: Yan Zhu <zhuyan34@huawei.com>
2022-04-03 21:07:09 -05:00
Niko
e9bf727253 Handle ERANGE error correctly
The reentrant functions getgrgid_r, getgrnam_r, getpwnam_r, etc. all return an error code instead of setting errno. Adapt the error check accordingly.
2022-03-18 20:24:10 -05:00
Alejandro Colomar
fd5945e533 Use 'void' instead of 'RETSIGTYPE'. Use 'sighandler_t' too.
C89 and POSIX.1-2001 define signal(2) as returning a pointer to a
function returning 'void'.  K&R C signal(2) signature is obsolete.
Use 'void' directly.

Also, instead of writing the function pointer type explicitly, use
POSIX's 'sighandler_t'.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2022-01-15 08:25:53 -06:00
Alejandro Colomar
5450f9a904 Remove old compatibility DIRECT macro.
Use struct dirent directly.  See parent commit.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2022-01-15 08:25:53 -06:00
Serge Hallyn
39eea79d8d
Merge pull request #487 from cgzones/misc_warnings
Resolve several compiler warnings
2022-01-03 09:45:12 -06:00
Serge Hallyn
98f943f2a5
Merge pull request #481 from alejandro-colomar/STDC_HEADERS
Assume C89 is available
2022-01-03 09:37:06 -06:00
Christian Göttsche
119cee142e Declare argument of nss_init const
nss_init() does not modify its path argument, thus declare it const.
Also drop superfluous prototype.

nss.c:54:31: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   54 |                 nsswitch_path = NSSWITCH;
      |                               ^
2022-01-03 15:09:17 +01:00
Christian Göttsche
946eb84182 Do not drop const qualifier for Basename
The private Basename() implementation does not modify its argument, so
a cast to a non-const char pointer is not necessary.

newgrp.c:790:39: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
  790 |         progbase = (char *) Basename ((char *) prog);
      |                                       ^
newgrp.c:790:20: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
  790 |         progbase = (char *) Basename ((char *) prog);
      |                    ^

shell.c:48:70: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
   48 |                 (void) snprintf (arg0, sizeof arg0, "-%s", Basename ((char *) file));
      |                                                                      ^
2022-01-03 15:09:17 +01:00
Christian Göttsche
45bba0e190 Use strict prototypes
Function declarations with no argument declare functions taking an
arbitrary number of arguments. Use the special type void to declare
functions taking no argument.
2022-01-03 15:09:17 +01:00
Serge Hallyn
e8a2cfa7dc
Merge pull request #451 from hallyn/2021-12-05/license 2022-01-02 18:38:42 -06:00
ed neville
b4472167c2 Adding nofollow to opens 2022-01-01 21:13:41 +00:00
Alejandro Colomar
8f134c0bea Use isdigit(3) instead of a reimplementation of it
C89 defined isdigit as a function that tests for any decimal-digit
character, defining the decimal digits as 0 1 2 3 4 5 6 7 8 9.

I don't own a copy of C89 to check, but check in C17:

7.4.1.5
5.2.1

More specifically:

> In both the source and execution basic character sets, the value
> of each character after 0 in the above list of decimal digits
> shall be one greater than the value of the previous.

And since in ascii(7), the character after '9' is ':', it's highly
unlikely that any implementation will ever accept any
_decimal digit_ other than 0..9.

POSIX simply defers to the ISO C standard.

This is exactly what we wanted from ISDIGIT(c), so just use it.
Non-standard implementations might have been slower or considered
other characters as digits in the past, but let's assume
implementations available today conform to ISO C89.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-29 02:41:09 +01:00
Alejandro Colomar
44126d85ee Remove definition of ISDIGIT_LOCALE(c)
It wasn't being used at all.  Let's remove it.

Use isdigit(3) directly in comments that referenced it.

Also, in those comments, remove an outdated reference to the fact
that ISDIGIT_LOCALE(c) might evaluate its argument more than once,
which could be true a few commits ago, until
IN_CTYPE_DEFINITION(c) was removed.  Previously, the definition
for ISDIGIT_LOCALE(c) was:

 #if defined (STDC_HEADERS) || (!defined (isascii) && !defined (HAVE_ISASCII))
 # define IN_CTYPE_DOMAIN(c) 1
 #else
 # define IN_CTYPE_DOMAIN(c) isascii(c)
 #endif

 #define ISDIGIT_LOCALE(c) (IN_CTYPE_DOMAIN (c) && isdigit (c))

Which could evaluate 'c' twice on pre-C89 systems (which I hope
don't exist nowadays).

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-29 02:41:09 +01:00
Alejandro Colomar
2a41a72b8c Use standard isspace(3), isalpha(3), and isupper(3)
Due to the recent removal of IN_CTYPE_DOMAIN(), the uppercase
macros that wrapped these standard calls are now defined to be
equivalent.  Therefore, there's no need for the wrappers, and it
is much more readable to use the standard calls directly.

However, hold on with ISDIGIT*(), since it's not so obvious what
to do with it.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-29 02:41:09 +01:00
Alejandro Colomar
45d2e6dff0 Remove IN_CTYPE_DOMAIN, which was always true
The recent removal of STDC_HEADERS made IN_CTYPE_DOMAIN be defined
to 1 unconditionally.  Remove the now unnecessary definition, and
propagate its truthness to expressions where it was used.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-29 02:41:09 +01:00
Alejandro Colomar
4e1afcd662 Assume STDC_HEADERS will always be defined
We're in 2021.  C89 is everywhere; in fact, there are many other
assumptions in the code that wouldn't probably hold on
pre-standard C environments.  Let's simplify and assume that C89
is available.

The specific assumptions are that:
- <string.h>, and <stdlib.h> are available
- strchr(3), strrchr(3), and strtok(3) are available
- isalpha(3), isspace(3), isdigit(3), and isupper(3) are available

I think we can safely assume we have all of those.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-29 02:39:04 +01:00
Serge Hallyn
9e5a852ee0
Merge pull request #480 from alejandro-colomar/memcpy
Remove HAVE_MEMCPY and HAVE_MEMSET ifdefs
2021-12-27 19:10:48 -06:00
Alejandro Colomar
047bfc47c6 Remove HAVE_MEMCPY ifdefs
memcpy(3) has been in standard C since C89.  It is also in
POSIX.1-2001, in SVr4, and in 4.3BSD (see memcpy(3) and memcpy(3p)).
We can assume that this function is always available.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-27 21:38:47 +01:00
Alejandro Colomar
3e602b58a2 Remove HAVE_STRFTIME ifdefs
strftime(3) has been in standard C since C89.  It is also in
POSIX.1-2001, and in SVr4 (see strftime(3) and strftime(3p)).
We can assume that this function is always available.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-27 20:50:13 +01:00
Serge Hallyn
e1b1d187f4
Merge pull request #467 from alejandro-colomar/date_to_str
Have a single definition of date_to_str()
2021-12-27 09:53:00 -06:00
Alejandro Colomar
355ad6a9e0 Have a single definition of date_to_str()
PARAMETERS:

According to the C2x charter, I reordered the parameters 'size'
and 'buf' from previously existing date_to_str() definitions.

C2x charter:
> 15. Application Programming Interfaces (APIs) should be
> self-documenting when possible.  In particular, the order of
> parameters in function declarations should be arranged such that
> the size of an array appears before the array.  The purpose is to
> allow Variable-Length Array (VLA) notation to be used.  This not
> only makes the code's purpose clearer to human readers, but also
> makes static analysis easier.  Any new APIs added to the Standard
> should take this into consideration.

I used 'long' for the date parameter, as some uses of the function
need to pass a negative value meaning "never".

FUNCTION BODY:

I didn't check '#ifdef HAVE_STRFTIME', which old definitions did,
since strftime(3) is guaranteed by the C89 standard, and all of
the conversion specifiers that we use are also specified by that
standard, so we don't need any extensions at all.

Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
2021-12-26 18:55:39 +01:00
Serge Hallyn
f93cf255d4 Update licensing info
Closes #238

Update all files to list SPDX license shortname.  Most files are
BSD 3 clause license.

The exceptions are:

serge@sl ~/src/shadow$ git grep SPDX-License | grep -v BSD-3-Clause
contrib/atudel:# SPDX-License-Identifier: BSD-4-Clause
lib/tcbfuncs.c: * SPDX-License-Identifier: 0BSD
libmisc/salt.c: * SPDX-License-Identifier: Unlicense
src/login_nopam.c: * SPDX-License-Identifier: Unlicense
src/nologin.c: * SPDX-License-Identifier: BSD-2-Clause
src/vipw.c: * SPDX-License-Identifier: GPL-2.0-or-later

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-12-23 19:36:50 -06:00
Serge Hallyn
79157cbad8 Make shadow_logfd and Prog not extern
Closes #444
Closes #465

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-12-23 15:18:07 -06:00
François Rigault
a757b458ff groupdel: fix SIGSEGV when passwd does not exist
When using groupdel with a prefix, groupdel will attempt to read a
passwd file to look for any user in the group. When the file does not
exist it cores with segmentation fault.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1986111
2021-11-01 13:54:25 +01:00
Tobias Stoeckmann
63a96706b1 Handle malformed lines in hushlogins file.
If a line in hushlogins file, e.g. /etc/hushlogins, starts with
'\0', then current code performs an out of boundary write.
If the line lacks a newline at the end, then another character is
overridden.

With strcspn both cases are solved.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
2021-10-29 19:50:38 +02:00
a1346054
d7c9550b7f fix spelling 2021-09-13 15:11:40 +00:00
a1346054
7687ae4dbd fix spelling and unify whitespace 2021-08-18 18:06:02 +00:00
Mike Gilbert
234e8fa7b1 libmisc: fix default value in SHA_get_salt_rounds()
If SHA_CRYPT_MIN_ROUNDS and SHA_CRYPT_MAX_ROUNDS are both unspecified,
use SHA_ROUNDS_DEFAULT.

Previously, the code fell through, calling shadow_random(-1, -1). This
ultimately set rounds = (unsigned long) -1, which ends up being a very
large number! This then got capped to SHA_ROUNDS_MAX later in the
function.

The new behavior matches BCRYPT_get_salt_rounds().

Bug: https://bugs.gentoo.org/808195
Fixes: https://github.com/shadow-maint/shadow/issues/393
2021-08-14 13:43:26 -04:00
Serge Hallyn
537b8cd90b Fix out of tree builds with respect to libsubid includes
There's a better way to do this, and I hope to clean that up,
but this fixes out of tree builds for me right now.

Closes #386

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-07-23 17:51:13 -05:00
Serge Hallyn
0c821fcf0c
Merge pull request #383 from ikerexxe/wrong_free
libmisc: don't free members variable
2021-07-14 07:25:55 -05:00
Iker Pedrosa
c3cf23b00c libmisc: don't free members variable
In 9eb191edc4 I included a free() that
frees the members variable, which in turn causes the comma_to_list()
function to return an array of empty elements. The array variable holds
a list of pointers that point to offsets of the members variable. When
the function succeeds freeing members variable causes the elements of
the array variable to point to an empty string.

This is causing several regressions in our internal testing environment.
So, I'm reverting the change.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2021-07-14 12:13:25 +02:00
steven Y Gui
ffd35d8902 fread returns element count, not element size 2021-07-14 16:17:48 +08:00
Björn Esser
ea04eb301d
libmisc/salt.c: Use crypt_gensalt(), if available in libcrypt.
Most Linux distributions, including Fedora and RHEL 8, are shipping
with libxcrypt >= 4.0.

Since that version of libxcrypt the provided family of crypt_gensalt()
functions are able to use automatic entropy drawn from secure system
ressources, like arc4random(), getentropy() or getrandom().

Anyways, the settings generated by crypt_gensalt() are always
guaranteed to works with the crypt() function.

Using crypt_gensalt() is also needed to make proper use of newer
hashing methods, like yescrypt, provided by libxcrypt.

Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-07-04 13:01:22 +02:00
Björn Esser
c82ed0c15e
libmisc/salt.c: Use secure system ressources to obtain random bytes.
In a previous commit we introduced /dev/urandom as a source to obtain
random bytes from.  This may not be available on all systems, or when
operating inside of a chroot.

Almost all systems provide functions to obtain random bytes from
secure system ressources.  Thus we should prefer to use these, and
fall back to /dev/urandom, if there is no such function present, as
a last resort.

Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-07-04 13:00:32 +02:00
Serge Hallyn
9eb191edc4
Merge pull request #359 from ikerexxe/rest_resource_leak
Fix covscan RESOURCE_LEAK
2021-06-24 13:14:15 -05:00
Iker Pedrosa
e65cc6aebc Fix covscan RESOURCE_LEAK
Error: RESOURCE_LEAK (CWE-772): [#def1]
shadow-4.8.1/lib/commonio.c:320: alloc_fn: Storage is returned from allocation function "fopen_set_perms".
shadow-4.8.1/lib/commonio.c:320: var_assign: Assigning: "bkfp" = storage returned from "fopen_set_perms(backup, "w", &sb)".
shadow-4.8.1/lib/commonio.c:329: noescape: Resource "bkfp" is not freed or pointed-to in "putc".
shadow-4.8.1/lib/commonio.c:334: noescape: Resource "bkfp" is not freed or pointed-to in "fflush".
shadow-4.8.1/lib/commonio.c:339: noescape: Resource "bkfp" is not freed or pointed-to in "fileno".
shadow-4.8.1/lib/commonio.c:342: leaked_storage: Variable "bkfp" going out of scope leaks the storage it points to.
  340|   	    || (fclose (bkfp) != 0)) {
  341|   		/* FIXME: unlink the backup file? */
  342|-> 		return -1;
  343|   	}
  344|

Error: RESOURCE_LEAK (CWE-772): [#def2]
shadow-4.8.1/libmisc/addgrps.c:69: alloc_fn: Storage is returned from allocation function "malloc".
shadow-4.8.1/libmisc/addgrps.c:69: var_assign: Assigning: "grouplist" = storage returned from "malloc(i * 4UL)".
shadow-4.8.1/libmisc/addgrps.c:73: noescape: Resource "grouplist" is not freed or pointed-to in "getgroups". [Note: The source code implementation of the function has been overridden by a builtin model.]
shadow-4.8.1/libmisc/addgrps.c:126: leaked_storage: Variable "grouplist" going out of scope leaks the storage it points to.
  124|   	}
  125|
  126|-> 	return 0;
  127|   }
  128|   #else				/* HAVE_SETGROUPS && !USE_PAM */

Error: RESOURCE_LEAK (CWE-772): [#def3]
shadow-4.8.1/libmisc/chowntty.c:62: alloc_fn: Storage is returned from allocation function "getgr_nam_gid".
shadow-4.8.1/libmisc/chowntty.c:62: var_assign: Assigning: "grent" = storage returned from "getgr_nam_gid(getdef_str("TTYGROUP"))".
shadow-4.8.1/libmisc/chowntty.c:98: leaked_storage: Variable "grent" going out of scope leaks the storage it points to.
   96|   	 */
   97|   #endif
   98|-> }
   99|

Error: RESOURCE_LEAK (CWE-772): [#def4]
shadow-4.8.1/libmisc/copydir.c:742: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
shadow-4.8.1/libmisc/copydir.c:742: var_assign: Assigning: "ifd" = handle returned from "open(src, 0)".
shadow-4.8.1/libmisc/copydir.c:748: leaked_handle: Handle variable "ifd" going out of scope leaks the handle.
  746|   #ifdef WITH_SELINUX
  747|   	if (set_selinux_file_context (dst, NULL) != 0) {
  748|-> 		return -1;
  749|   	}
  750|   #endif				/* WITH_SELINUX */

Error: RESOURCE_LEAK (CWE-772): [#def5]
shadow-4.8.1/libmisc/copydir.c:751: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
shadow-4.8.1/libmisc/copydir.c:751: var_assign: Assigning: "ofd" = handle returned from "open(dst, 577, statp->st_mode & 0xfffU)".
shadow-4.8.1/libmisc/copydir.c:752: noescape: Resource "ofd" is not freed or pointed-to in "fchown_if_needed".
shadow-4.8.1/libmisc/copydir.c:775: leaked_handle: Handle variable "ofd" going out of scope leaks the handle.
  773|   	   ) {
  774|   		(void) close (ifd);
  775|-> 		return -1;
  776|   	}
  777|

Error: RESOURCE_LEAK (CWE-772): [#def7]
shadow-4.8.1/libmisc/idmapping.c:188: alloc_fn: Storage is returned from allocation function "xmalloc".
shadow-4.8.1/libmisc/idmapping.c:188: var_assign: Assigning: "buf" = storage returned from "xmalloc(bufsize)".
shadow-4.8.1/libmisc/idmapping.c:188: var_assign: Assigning: "pos" = "buf".
shadow-4.8.1/libmisc/idmapping.c:213: noescape: Resource "buf" is not freed or pointed-to in "write".
shadow-4.8.1/libmisc/idmapping.c:219: leaked_storage: Variable "pos" going out of scope leaks the storage it points to.
shadow-4.8.1/libmisc/idmapping.c:219: leaked_storage: Variable "buf" going out of scope leaks the storage it points to.
  217|   	}
  218|   	close(fd);
  219|-> }

Error: RESOURCE_LEAK (CWE-772): [#def8]
shadow-4.8.1/libmisc/list.c:211: alloc_fn: Storage is returned from allocation function "xstrdup".
shadow-4.8.1/libmisc/list.c:211: var_assign: Assigning: "members" = storage returned from "xstrdup(comma)".
shadow-4.8.1/libmisc/list.c:217: var_assign: Assigning: "cp" = "members".
shadow-4.8.1/libmisc/list.c:218: noescape: Resource "cp" is not freed or pointed-to in "strchr".
shadow-4.8.1/libmisc/list.c:244: leaked_storage: Variable "cp" going out of scope leaks the storage it points to.
shadow-4.8.1/libmisc/list.c:244: leaked_storage: Variable "members" going out of scope leaks the storage it points to.
  242|   	if ('\0' == *members) {
  243|   		*array = (char *) 0;
  244|-> 		return array;
  245|   	}
  246|

Error: RESOURCE_LEAK (CWE-772): [#def11]
shadow-4.8.1/libmisc/myname.c:61: alloc_fn: Storage is returned from allocation function "xgetpwnam".
shadow-4.8.1/libmisc/myname.c:61: var_assign: Assigning: "pw" = storage returned from "xgetpwnam(cp)".
shadow-4.8.1/libmisc/myname.c:67: leaked_storage: Variable "pw" going out of scope leaks the storage it points to.
   65|   	}
   66|
   67|-> 	return xgetpwuid (ruid);
   68|   }
   69|

Error: RESOURCE_LEAK (CWE-772): [#def12]
shadow-4.8.1/libmisc/user_busy.c:260: alloc_fn: Storage is returned from allocation function "opendir".
shadow-4.8.1/libmisc/user_busy.c:260: var_assign: Assigning: "task_dir" = storage returned from "opendir(task_path)".
shadow-4.8.1/libmisc/user_busy.c:262: noescape: Resource "task_dir" is not freed or pointed-to in "readdir".
shadow-4.8.1/libmisc/user_busy.c:278: leaked_storage: Variable "task_dir" going out of scope leaks the storage it points to.
  276|   					         _("%s: user %s is currently used by process %d\n"),
  277|   					         Prog, name, pid);
  278|-> 					return 1;
  279|   				}
  280|   			}

Error: RESOURCE_LEAK (CWE-772): [#def20]
shadow-4.8.1/src/newgrp.c:162: alloc_fn: Storage is returned from allocation function "xgetspnam".
shadow-4.8.1/src/newgrp.c:162: var_assign: Assigning: "spwd" = storage returned from "xgetspnam(pwd->pw_name)".
shadow-4.8.1/src/newgrp.c:234: leaked_storage: Variable "spwd" going out of scope leaks the storage it points to.
  232|   	}
  233|
  234|-> 	return;
  235|
  236|   failure:

Error: RESOURCE_LEAK (CWE-772): [#def21]
shadow-4.8.1/src/passwd.c:530: alloc_fn: Storage is returned from allocation function "xstrdup".
shadow-4.8.1/src/passwd.c:530: var_assign: Assigning: "cp" = storage returned from "xstrdup(crypt_passwd)".
shadow-4.8.1/src/passwd.c:551: noescape: Resource "cp" is not freed or pointed-to in "strlen".
shadow-4.8.1/src/passwd.c:554: noescape: Resource "cp" is not freed or pointed-to in "strcat". [Note: The source code implementation of the function has been overridden by a builtin model.]
shadow-4.8.1/src/passwd.c:555: overwrite_var: Overwriting "cp" in "cp = newpw" leaks the storage that "cp" points to.
  553|   		strcpy (newpw, "!");
  554|   		strcat (newpw, cp);
  555|-> 		cp = newpw;
  556|   	}
  557|   	return cp;
2021-06-24 09:18:35 +02:00
Björn Esser
bc8257cf73
libmisc/salt.c: Obtain random bytes from /dev/urandom.
Using the random() function to obtain pseudo-random bytes
for generating salt strings is considered to be dangerous.
See CWE-327.

We really should use a more reliable source for obtaining
pseudo-random bytes like /dev/urandom.

Fixes #376.

Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-06-23 16:30:21 +02:00
Björn Esser
dbf230e4cf
libmisc/salt.c: Add comments how the minmum buffer length is computed.
In the previous commit we refactored the functions converting the
rounds number into a string for use with the crypt() function, to
not require any static buffer anymore.

Add some clarifying comments about how the minimum required buffer
length is computed inside of these functions.

Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-06-23 16:29:24 +02:00
Serge Hallyn
2f0d14eb21
Merge pull request #361 from besser82/topic/besser82/simplyfy_libmisc_salt
Sanitize code in libmisc/salt.c
2021-06-23 09:18:05 -05:00
Serge Hallyn
d4535015a0
Merge pull request #362 from ikerexxe/buffer_size
Fix covscan BUFFER_SIZE
2021-06-22 20:55:31 -05:00
Björn Esser
14b108728a
libmisc/salt.c: Sanitize code.
* Move all pre-processor defines to the top of the file.
* Unify the gensalt() function to be useable for all supported
  hash methods.
* Drop the gensalt_{b,yes}crypt() functions in favor of the
  previous change.
* Refactor the functions converting the rounds number into
  a string for use with the crypt() function, to not require
  any static buffer anymore.
* Clarify the comment about how crypt_make_salt() chooses the used
  hash method from the settings in the login.defs file.
* Use memset() to fill static buffers with zero before using them.
* Use a fixed amount of 16 random base64-chars for the
  sha{256,512}crypt hash methods, which is effectively still less
  than the recommendation from NIST (>= 128 bits), but the maximum
  those methods can effectively use (approx. 90 bits).
* Rename ROUNDS_{MIN,MAX} to SHA_ROUNDS_{MIN,MAX}.
* Bugfixes in the logic of setting rounds in BCRYPT_salt_rounds().
* Likewise for YESCRYPT_salt_cost().
* Fix formatting and white-space errors.

Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-06-22 22:03:21 +02:00
Björn Esser
738d92a4bd
libmisc/salt.c: bcrypt should use $2b$ as prefix for setting.
This prefix is the recommended one for new bcrypt hashes
for a long time.

Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-06-22 18:52:39 +02:00
Björn Esser
7a3bb4d0ea
libmisc/salt.c: Use int pointer for YESCRYPT_salt_cost().
The corresponding functions for the other hash methods all take
a pointer to an integer value as the only paramater, so this
particular function should do so as well.

Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-06-22 18:51:59 +02:00
Iker Pedrosa
5d0d784197 Fix covscan BUFFER_SIZE
Error: BUFFER_SIZE (CWE-170): [#def6]
shadow-4.8.1/libmisc/failure.c:101: buffer_size_warning: Calling "strncpy" with a maximum size argument of 12 bytes on destination array "fl->fail_line" of size 12 bytes might leave the destination string unterminated.
   99|   	}
  100|
  101|-> 	strncpy (fl->fail_line, tty, sizeof fl->fail_line);
  102|   	(void) time (&fl->fail_time);
  103|

Error: BUFFER_SIZE (CWE-170): [#def9]
shadow-4.8.1/libmisc/log.c:103: buffer_size_warning: Calling "strncpy" with a maximum size argument of 32 bytes on destination array "newlog.ll_line" of size 32 bytes might leave the destination string unterminated.
  101|   	(void) time (&ll_time);
  102|   	newlog.ll_time = ll_time;
  103|-> 	strncpy (newlog.ll_line, line, sizeof newlog.ll_line);
  104|   #if HAVE_LL_HOST
  105|   	strncpy (newlog.ll_host, host, sizeof newlog.ll_host);

Error: BUFFER_SIZE (CWE-170): [#def10]
shadow-4.8.1/libmisc/log.c:105: buffer_size_warning: Calling "strncpy" with a maximum size argument of 256 bytes on destination array "newlog.ll_host" of size 256 bytes might leave the destination string unterminated.
  103|   	strncpy (newlog.ll_line, line, sizeof newlog.ll_line);
  104|   #if HAVE_LL_HOST
  105|-> 	strncpy (newlog.ll_host, host, sizeof newlog.ll_host);
  106|   #endif
  107|   	if (   (lseek (fd, offset, SEEK_SET) != offset)

Error: BUFFER_SIZE (CWE-170): [#def13]
shadow-4.8.1/libmisc/utmp.c:260: buffer_size_warning: Calling "strncpy" with a maximum size argument of 32 bytes on destination array "utent->ut_line" of size 32 bytes might leave the destination string unterminated.
  258|   #endif				/* HAVE_STRUCT_UTMP_UT_TYPE */
  259|   	utent->ut_pid = getpid ();
  260|-> 	strncpy (utent->ut_line, line,      sizeof (utent->ut_line));
  261|   #ifdef HAVE_STRUCT_UTMP_UT_ID
  262|   	if (NULL != ut) {

Error: BUFFER_SIZE (CWE-170): [#def14]
shadow-4.8.1/libmisc/utmp.c:266: buffer_size_warning: Calling "strncpy" with a maximum size argument of 4 bytes on destination array "utent->ut_id" of size 4 bytes might leave the destination string unterminated.
  264|   	} else {
  265|   		/* XXX - assumes /dev/tty?? */
  266|-> 		strncpy (utent->ut_id, line + 3, sizeof (utent->ut_id));
  267|   	}
  268|   #endif				/* HAVE_STRUCT_UTMP_UT_ID */

Error: BUFFER_SIZE (CWE-170): [#def15]
shadow-4.8.1/libmisc/utmp.c:273: buffer_size_warning: Calling "strncpy" with a maximum size argument of 32 bytes on destination array "utent->ut_user" of size 32 bytes might leave the destination string unterminated.
  271|   #endif				/* HAVE_STRUCT_UTMP_UT_NAME */
  272|   #ifdef HAVE_STRUCT_UTMP_UT_USER
  273|-> 	strncpy (utent->ut_user, name,      sizeof (utent->ut_user));
  274|   #endif				/* HAVE_STRUCT_UTMP_UT_USER */
  275|   	if (NULL != hostname) {

Error: BUFFER_SIZE (CWE-170): [#def16]
shadow-4.8.1/libmisc/utmp.c:278: buffer_size_warning: Calling "strncpy" with a maximum size argument of 256 bytes on destination array "utent->ut_host" of size 256 bytes might leave the destination string unterminated.
  276|   		struct addrinfo *info = NULL;
  277|   #ifdef HAVE_STRUCT_UTMP_UT_HOST
  278|-> 		strncpy (utent->ut_host, hostname, sizeof (utent->ut_host));
  279|   #endif				/* HAVE_STRUCT_UTMP_UT_HOST */
  280|   #ifdef HAVE_STRUCT_UTMP_UT_SYSLEN

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2021-06-16 09:50:53 +02:00
Serge Hallyn
9d37173b24 usermod, newusers, prefix: enforce absolute paths for homedir
useradd already was enforcing this, but these were not.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-06-01 22:12:24 -05:00
Serge Hallyn
2b22a6909d libsubid: don't print error messages on stderr by default
Closes #325

Add a new subid_init() function which can be used to specify the
stream on which error messages should be printed.  (If you want to
get fancy you can redirect that to memory :)  If subid_init() is
not called, use stderr.  If NULL is passed, then /dev/null will
be used.

This patch also fixes up the 'Prog', which previously had to be
defined by any program linking against libsubid.  Now, by default
in libsubid it will show (subid).  Once subid_init() is called,
it will use the first variable passed to subid_init().

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-05-15 12:38:55 -05:00
Serge Hallyn
3ac8d97825
Merge pull request #323 from cgzones/selinux
SELinux modernizations
2021-05-07 08:32:01 -05:00
Christian Brauner
91d4ab622b
libmisc: retain setfcap when mapping uid 0
When uid 0 maps host uid 0 into the child userns newer kernels require
CAP_SETFCAP be retained as this allows the caller to create fscaps that
are valid in the ancestor userns. This was a security issue (in very
rare circumstances). So whenever host uid 0 is mapped, retain
CAP_SETFCAP if the caller had it.
Userspace won't need to set CAP_SETFCAP on newuidmap as this is really
only a scenario that real root should be doing which always has
CAP_SETFCAP. And if they don't then they are in a locked-down userns.
(LXC sometimes maps host uid 0 during chown operations in a helper
 userns but will not rely on newuidmap for that. But we don't want to
 risk regressing callers that want to rely on this behavior.)

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
2021-05-06 19:04:42 +02:00