Bug 187653

Summary: pw(8): 'pw user mod' is creating users instead of changing them.
Product: Base System Reporter: tmwalaszek
Component: binAssignee: Baptiste Daroussin <bapt>
Status: Closed FIXED    
Severity: Affects Only Me CC: ngie
Priority: Normal Keywords: patch
Version: UnspecifiedFlags: ngie: mfc-stable10+
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Patch for preventing attempts to modify users other than local/NIS none

Description tmwalaszek 2014-03-17 08:40:00 UTC
In my setup the system is searching for users in 2 sources, files and ldap.
passwd: files ldap

If we have for example user 'test' in our ldap database and we try to change shell for user 'test' we will see strange behaviour:


root@ldap:~ # getent passwd | grep test
test:*:1000:1000:test:/home/test:/usr/local/bin/bash
root@ldap:~ # grep 'test' /etc/passwd
root@ldap:~ # 
root@ldap:~ # pw mod user -n test -s /bin/sh
root@ldap:~ # grep 'test' /etc/passwd
test:*:1000:1000:test:/home/test:/bin/sh
root@ldap:~ # 
root@ldap:~ # getent passwd | grep test
test:*:1000:1000:test:/home/test:/bin/sh
test:*:1000:1000:test:/home/test:/usr/local/bin/bash

The pw tool created new user instead of fail with 'no such user' message. Maybe this is desirable behaviour but in my opinion 'mod' switch should only change users not creating them.

Fix: 

This behavior is caused by getpwnam getpwuid functions. Pw uses those functions to search for the users, when we have user test in ldap those function will return it. Pw dont know anything about ldap so it will create new entry in passwd files.
Fixes:
1. Use pw with -V /etc. Pw with -V will not use getpwnam getpwuid but vgetpnam, vgetpwuid and search for the users in passwd files in /etc directory.
2. Mayebe use functions defined in struct pwf VPWF ?
How-To-Repeat: To repeat the problem we need to setup ldap server and configure our system to use it as the user source.
After that create in ldap user 'test' and try to change it shell using pw.
Comment 1 tmwalaszek 2015-05-19 21:12:29 UTC
Created attachment 156961 [details]
Patch for preventing attempts to modify users other than local/NIS

Hi,
If anyone would be interested, I'm not allowing pw to go any further if the user is not local/NIS.  

Patch is for FreeBSD 10.1 usr.sbin/pw r283124
Comment 2 commit-hook freebsd_committer freebsd_triage 2015-07-28 21:50:34 UTC
A commit references this bug:

Author: bapt
Date: Tue Jul 28 21:49:39 UTC 2015
New revision: 285989
URL: https://svnweb.freebsd.org/changeset/base/285989

Log:
  Reject usermod and userdel if the user concerned is not on the user database
  supposed to be manipulated

  This prevent pw usermod creating a new local user when requesting to usermod on
  a username is defined in LDAP.

  This issue only happens when modifying the local user database (not inpacting
  commands when -V or -R are used).

  PR:		187653
  Submitted by:	tmwalaszek@gmail.com

Changes:
  head/usr.sbin/pw/pw_user.c
Comment 3 Enji Cooper freebsd_committer freebsd_triage 2015-10-25 22:08:01 UTC
bapt MFCed the commit to stable/10 in r287084. Closing.

------------------------------------------------------------------------
r287084 | bapt | 2015-08-23 14:42:27 -0700 (Sun, 23 Aug 2015) | 18 lines

MFC: r285133,r285136,r285137,r285156,r285157,r285158,r285256,r285318,r285395,
r285396,r285398,r285401,r285403,r285405,r285406,r285408,r285409,r285411,
r285412,r285413,r285415,r285418,r285430,r285433,r285434,r285442,r285948,
r285984,r285985,r285989,r285996,r285997,r286045,r286047,r286066,r286150,
r286151,r286152,r286154,r286155,r286156,r286157,r286173,r286196,r286197,
r286198,r286199,r286200,r286201,r286202,r286203,r286204,r286210,r286211,
r286217,r286218,r286258,r286259,r286341,r286775,r286982,r286986,r286991,
r286993

Validate most pw inputs.
Rewrite the way parsing sub arguments is made to simplify code and improve
maintenability
Add -y (NIS) to userdel/usermod
pw userdel -r <rootdir> now deletes directories in the rootdir
Only parse pw.conf when needed
Reject usermod and userdel if the user concerned is not on the user database
supposed to be manipulated