Re: svn commit: r322678 - in head/usr.sbin/pw: . tests

[Ed Maste]

On 18 August 2017 at 21:08, Ngie Cooper (yaneurabeya)
 wrote:
>
>> On Aug 18, 2017, at 17:32, Ed Maste  wrote:
>>
>> Author: emaste
>> Date: Sat Aug 19 00:32:26 2017
>> New Revision: 322678
>> URL: https://svnweb.freebsd.org/changeset/base/322678
>>
>> Log:
>>  pw useradd: Validate the user name before creating the entry
>>
>>  Previouly it was possible to create users with spaces in the name with:
>>  pw useradd -u 1234 -g 1234 -n 'test user'
>>
>>  The "-g 1234" is relevant, without it the name was already rejected
>>  as expected:
...
>
> Usernames with passwords are permitted in some cases, e.g., AD.

I assume you meant "usernames with spaces"?

Note that this change only addresses the discrepancy between "pw
useradd 'test user'" (which was previously rejected) and "pw useradd
'test user' -g " (which was accepted prior to this change).
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r322678 - in head/usr.sbin/pw: . tests

[Ngie Cooper (yaneurabeya)]

> On Aug 18, 2017, at 17:32, Ed Maste  wrote:
> 
> Author: emaste
> Date: Sat Aug 19 00:32:26 2017
> New Revision: 322678
> URL: https://svnweb.freebsd.org/changeset/base/322678
> 
> Log:
>  pw useradd: Validate the user name before creating the entry
> 
>  Previouly it was possible to create users with spaces in the name with:
>  pw useradd -u 1234 -g 1234 -n 'test user'
> 
>  The "-g 1234" is relevant, without it the name was already rejected
>  as expected:
> 
>  [fk@test ~]$ sudo pw useradd -u 1234 -n 'test user'
>  pw: invalid character ` ' at position 4 in userid/group name
> 
>  Bug unintentionally found with a salt config without explicit name entry:
> 
>  test user:
>user.present:
>  - uid: 1234
>  - gid: 1234
>  - fullname: Test user
>  - shell: /usr/local/bin/bash
>  - home: /home/test
>  - groups:
>- wheel
>- salt
> 
>  "Luckily" salt modules rarely bother with input validation either ...
> 
>  PR:  221416
>  Submitted by:Fabian Keil
>  Obtained from:   ElectroBSD
>  MFC after:   1 week
> 
> Modified:
>  head/usr.sbin/pw/pw_user.c
>  head/usr.sbin/pw/tests/pw_useradd_test.sh

Usernames with passwords are permitted in some cases, e.g., AD.
Thanks,
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail

svn commit: r322678 - in head/usr.sbin/pw: . tests

[Ed Maste]

Author: emaste
Date: Sat Aug 19 00:32:26 2017
New Revision: 322678
URL: https://svnweb.freebsd.org/changeset/base/322678

Log:
  pw useradd: Validate the user name before creating the entry
  
  Previouly it was possible to create users with spaces in the name with:
  pw useradd -u 1234 -g 1234 -n 'test user'
  
  The "-g 1234" is relevant, without it the name was already rejected
  as expected:
  
  [fk@test ~]$ sudo pw useradd -u 1234 -n 'test user'
  pw: invalid character ` ' at position 4 in userid/group name
  
  Bug unintentionally found with a salt config without explicit name entry:
  
  test user:
user.present:
  - uid: 1234
  - gid: 1234
  - fullname: Test user
  - shell: /usr/local/bin/bash
  - home: /home/test
  - groups:
- wheel
- salt
  
  "Luckily" salt modules rarely bother with input validation either ...
  
  PR:   221416
  Submitted by: Fabian Keil
  Obtained from:ElectroBSD
  MFC after:1 week

Modified:
  head/usr.sbin/pw/pw_user.c
  head/usr.sbin/pw/tests/pw_useradd_test.sh

Modified: head/usr.sbin/pw/pw_user.c
==
--- head/usr.sbin/pw/pw_user.c  Sat Aug 19 00:19:23 2017(r322677)
+++ head/usr.sbin/pw/pw_user.c  Sat Aug 19 00:32:26 2017(r322678)
@@ -1202,7 +1202,7 @@ pw_user_add(int argc, char **argv, char *arg1)
if (arg1[strspn(arg1, "0123456789")] == '\0')
id = pw_checkid(arg1, UID_MAX);
else
-   name = arg1;
+   name = pw_checkname(arg1, 0);
}
 
while ((ch = getopt(argc, argv, args)) != -1) {
@@ -1214,7 +1214,7 @@ pw_user_add(int argc, char **argv, char *arg1)
quiet = true;
break;
case 'n':
-   name = optarg;
+   name = pw_checkname(optarg, 0);
break;
case 'u':
userid = optarg;

Modified: head/usr.sbin/pw/tests/pw_useradd_test.sh
==
--- head/usr.sbin/pw/tests/pw_useradd_test.sh   Sat Aug 19 00:19:23 2017
(r322677)
+++ head/usr.sbin/pw/tests/pw_useradd_test.sh   Sat Aug 19 00:32:26 2017
(r322678)
@@ -176,6 +176,43 @@ user_add_name_too_long_body() {
${PW} useradd name_very_vert_very_very_very_long
 }
 
+atf_test_case user_add_name_with_spaces
+user_add_name_with_spaces_body() {
+   populate_etc_skel
+   atf_check -s exit:65 -e match:"invalid character" \
+ ${PW} useradd 'test user'
+   atf_check -s exit:1 -o empty grep "^test user:.*" $HOME/master.passwd
+   # Try again with -n which uses a slightly different code path.
+   atf_check -s exit:65 -e match:"invalid character" \
+ ${PW} useradd -n 'test user'
+   atf_check -s exit:1 -o empty grep "^test user:.*" $HOME/master.passwd
+}
+
+atf_test_case user_add_name_with_spaces_and_gid_specified
+user_add_name_with_spaces_and_gid_specified_body() {
+   populate_etc_skel
+   gid=12345
+   user_name="test user"
+   # pw useradd should fail because of the space in the user
+   # name, not because the group doesn't exist.
+   atf_check -s exit:65 -e match:"invalid character" \
+ ${PW} useradd "${user_name}" -g ${gid}
+   atf_check -s exit:1 -o empty grep "^${user_name}:.*" $HOME/master.passwd
+   # Try again with -n which uses a slightly different code path.
+   atf_check -s exit:65 -e match:"invalid character" \
+ ${PW} useradd -n "${user_name}" -g ${gid}
+   atf_check -s exit:1 -o empty grep "^${user_name}:.*" $HOME/master.passwd
+   # Make sure the user isn't added even if the group exists
+   atf_check -s exit:0 ${PW} groupadd blafasel -g ${gid}
+   atf_check -s exit:65 -e match:"invalid character" \
+ ${PW} useradd "${user_name}" -g ${gid}
+   atf_check -s exit:1 -o empty grep "^${user_name}:.*" $HOME/master.passwd
+   # Try again with the -n option.
+   atf_check -s exit:65 -e match:"invalid character" \
+ ${PW} useradd -n "${user_name}" -g ${gid}
+   atf_check -s exit:1 -o empty grep "^${user_name}:.*" $HOME/master.passwd
+}
+
 atf_test_case user_add_expiration
 user_add_expiration_body() {
populate_etc_skel
@@ -415,6 +452,8 @@ atf_init_test_cases() {
atf_add_test_case user_add_password_expiration_date_month
atf_add_test_case user_add_password_expiration_date_relative
atf_add_test_case user_add_name_too_long
+   atf_add_test_case user_add_name_with_spaces
+   atf_add_test_case user_add_name_with_spaces_and_gid_specified
atf_add_test_case user_add_expiration
atf_add_test_case user_add_invalid_user_entry
atf_add_test_case user_add_invalid_group_entry