Commit Graph

2764 Commits

Author SHA1 Message Date
Serge Hallyn
049b08481a useradd.c: Fix undeclared subuid_count when not using subids
subuid_count won't get used by usr_update(), but since we're passing it
as an argument we have to make sure it's always defined.  So just define
it as pre-set to 0.

Closes #402

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-08-14 19:25:51 -05:00
Serge Hallyn
02b200c9aa man/po/Makefile.in: switch from xml2po to itstool
xml2po is deprecated.  We've previously replaced xml2po with
itstool in man/generate_translations.mak, but there was still
an instance of it that only is exercised for 'make dist'.
Update that one.  Now 'make dist' succeeds on a ubuntu focal
or newer host where xml2po is not available.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-08-14 14:51:08 -05: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
c4539fc4f9
Merge pull request #399 from ikerexxe/useradd_empty_subid_range
useradd: avoid generating an empty subid range
2021-08-14 09:23:26 -05:00
Iker Pedrosa
9dd720a285 useradd: avoid generating an empty subid range
useradd generates an empty subid range when adding a new user. This is
caused because there are two variables, one local and the other one
global, that have a very similar name and they are used indistinctly in
the code. The local variable loads the SUB_*ID_COUNT configuration from
the login.defs file, while the global variable, which holds a value of
0, is used to generate the subid range. Causing the empty subid range
problem.

I've merged the two variables in the local one and removed the global
variable. I prefer to do it this way to reduce the scope of it but I'm
open to doing it the other way round.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1990653

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2021-08-10 10:04:39 +02:00
Serge Hallyn
1c1e0b0c68
Merge pull request #397 from ikerexxe/revert_local_groups_check
usermod: allow all group types with -G option
2021-08-04 10:41:56 -05:00
Serge Hallyn
adb83f7796
Merge pull request #398 from Dzejrou/master
passwd: handle NULL pw_passwd when printing password status
2021-08-04 10:38:30 -05:00
Serge Hallyn
28a02dc83e
Merge pull request #396 from floppym/libcrack
libsubid: fix build with libcrack
2021-08-04 10:37:08 -05:00
Serge Hallyn
e318bb9b06
Merge pull request #395 from ikerexxe/downstream_4_9
Makefile: include libeconf dependency in new*idmap
2021-08-04 10:36:44 -05:00
Jaroslav Jindrak
05388f748d passwd: handle NULL pw_passwd when printing password status
When the -S and -a options are used for passwd to list the status
of all passwords, there is a chance the pw_passwd field of struct
passwd will be NULL. This can be due to 'files compat' being set
for passwd in /etc/nsswitch.conf and the usage of some features
not available in the 'files' mode (e.g. a plus sign at the start
of a line).

Example:

germ161:~ # grep passwd /etc/nsswitch.conf
passwd: files compat
germ161:~ # rpm -qa shadow
shadow-4.2.1-34.20.x86_64
germ161:~ # grep passwd /etc/nsswitch.conf
passwd: files compat
germ161:~ # grep + /etc/passwd
+@nisgroup
germ161:~ # passwd -S -a > /dev/null
Segmentation fault (core dumped)

With this commit:

germ161:~ # passwd -S -a > /dev/null
passwd: malformed password data obtained for user +@nisgroup
2021-08-03 20:03:46 +02:00
Iker Pedrosa
e481437ab9 usermod: allow all group types with -G option
The only way of removing a group from the supplementary list is to use
-G option, and list all groups that the user is a member of except for
the one that wants to be removed. The problem lies when there's a user
that contains both local and remote groups, and the group to be removed
is a local one. As we need to include the remote group with -G option
the command will fail.

This reverts commit 140510de9d. This way,
it would be possible to remove the remote groups from the supplementary
list.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1967641
Resolves: https://github.com/shadow-maint/shadow/issues/338

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2021-08-03 11:14:09 +02:00
Mike Gilbert
6becc82e26 libsubid: fix build with libcrack
Fixes a link failure:

  ../libsubid/.libs/libsubid.so: undefined reference to `FascistCheck'

Bug: https://bugs.gentoo.org/806124
Signed-off-by: Mike Gilbert <floppym@gentoo.org>
2021-08-02 12:00:48 -04:00
Iker Pedrosa
c6847011e8 Makefile: include libeconf dependency in new*idmap
new*idmap has a dependency with libeconf since commit
c464ec5570. I'm just adding it to the
Makefile to be able to compile in distributions that include libeconf.

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
2021-08-02 15:54:20 +02:00
Christian Brauner
065aae682c
Merge pull request #392 from hallyn/2021-07-25/useradd-defaults
useradd: create /etc/default saving defaults.
2021-07-26 10:20:56 +02:00
Serge Hallyn
33c33a32a2 useradd: create /etc/default saving defaults.
Since bbf4b79, we stopped shipping /etc/default/useradd, and therefore
install of shadow does not auto-create /etc/default.  So when useradd
tries to save a new default, it needs to create the directory.

Closes #390.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-07-25 16:19:56 -05:00
Serge Hallyn
cd551ef728
Merge pull request #391 from hallyn/2021-07-25/static
Respect --enable-static=no in libsubid
2021-07-25 14:01:11 -05:00
Serge Hallyn
fa986b1d73 Respect --enable-static=no in libsubid
libsubid's Makefile.am was always setting enable-shared in its LDFLAGS.
Do that only if not building static.

Closes #387

Signed-off-by: Serge Hallyn <shallyn@cisco.com>
2021-07-25 12:21:55 -05:00
Serge Hallyn
4616fe71e7
Merge pull request #388 from hallyn/2021-07-23/outoftreebuild
Fix out of tree builds with respect to libsubid includes
2021-07-23 18:11:51 -05: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
d4b6d1549b
Merge pull request #385 from xry111/ftbfs-fix
libsubid: link to PAM libraries
2021-07-23 04:53:34 -05:00
Xi Ruoyao
f4a84efb46
libsubid: link to PAM libraries
libsubid.so links to libmisc.a, which contains several routines referring to
PAM functions.
2021-07-23 14:38:08 +08:00
Serge Hallyn
6f9124b7f7 configure.ac: get ready to release 4.9
Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-07-22 16:50:51 -05:00
Serge Hallyn
496fc2919d update Changelog
Signed-off-by: Serge Hallyn <serge@hallyn.com>
2021-07-22 16:49:26 -05:00
Serge Hallyn
905eb76cec
Merge pull request #378 from besser82/topic/besser82/update_po
Update translation files.
2021-07-14 09:35:31 -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
Serge Hallyn
93086e9aa7
Merge pull request #382 from StevenYGui/master
fread returns element count, not element size
2021-07-14 07:20:24 -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
c7b4f317f9
po/de.po: Update German translations.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-07-05 11:49:45 +02:00
Björn Esser
9b58a623f4
po: Update translation files from updated template.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-07-05 11:49:45 +02:00
Björn Esser
1a20189b83
po: Update pot file.
Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-07-05 11:49:36 +02:00
Serge Hallyn
bd102eaa35
Merge pull request #357 from besser82/topic/besser82/xcrypt_gensalt
Use crypt_gensalt(), if available in libcrypt.
2021-07-04 22:23:56 -05: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
c93897a8d7
lib/defines.h: Include <crypt.h> if present on the system.
The functions crypt(3), crypt_gensalt(3), and their
feature test macros may be defined in there.

Signed-off-by: Björn Esser <besser82@fedoraproject.org>
2021-07-04 13:01:21 +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
Serge Hallyn
b3b1f5d4a4
Merge pull request #377 from besser82/topic/besser82/salt_c_random
libmisc/salt.c: Obtain random bytes from /dev/urandom.
2021-06-23 14:54:25 -05: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
Serge Hallyn
dbfad7d1c9
Merge pull request #360 from besser82/topic/besser82/bcrypt_prefix
bcrypt should use $2b$ as prefix for setting.
2021-06-22 14:46:41 -05: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
Serge Hallyn
a026ea6bbf
Merge pull request #354 from ikerexxe/useradd_resource_leak
useradd.c: fix covscan RESOURCE_LEAK
2021-06-22 09:22:09 -05:00
Serge Hallyn
5939e066db
Merge pull request #353 from ikerexxe/man_subid
man: improve subid documentation
2021-06-22 09:20:11 -05:00
Serge Hallyn
af9984bff1
Merge pull request #356 from tzccinct/pass-min-days-doc
man: use the consistent value 0 to disable PASS_MIN_DAYS restriction
2021-06-22 08:51:08 -05:00
Iker Pedrosa
0faec51bf0 man: definition and configuration of subid
Define the subid functionality and explain the way to configure its
delegation.
2021-06-16 12:25:06 +02:00