According to pkg_create(1) and the porters-handbook/users-and-groups.html it is expected groups/users are created in a early install state. In further steps these accounts can be used to set permissions etc. Even pkg_add should stop if the requested accounts cannot be created. This is true as long the port is installed from the source, the resulting package creates the accounts in one of the last steps. The workaround is at the moment to create the groups/users via the pkg-install script. The following simple patch solve this issue. Fix: The following patch is also here affable: http://people.freebsd.org/~ohauer/diffs/patch-Mk__bsd.port.mk How-To-Repeat: A small demo port is here available: http://people.freebsd.org/~ohauer/shar/guid-package-test.shar
Responsible Changed From-To: freebsd-ports-bugs->portmgr bsd.port.mk is portmgr territory (via the GNATS Auto Assign Tool)
This patch is a way better, tested against some ports which have USERS and GROUPS defined in Makefile (manual build and tinderbox). For a quick test pick a port with USERS/GROUPS in Makefile, build with and without the patch and compaire .PLIST.mktmp or the output of pkg_info -f $portname. --- patch_ports-MK__bsd.port.mk.txt begins here --- Index: Mk/bsd.port.mk =================================================================== RCS file: /home/pcvs/ports/Mk/bsd.port.mk,v retrieving revision 1.665 diff -u -r1.665 bsd.port.mk --- Mk/bsd.port.mk 22 Dec 2010 20:05:40 -0000 1.665 +++ Mk/bsd.port.mk 27 Dec 2010 01:42:19 -0000 @@ -2482,6 +2482,7 @@ PKGMESSAGE?= ${PKGDIR}/pkg-message TMPPLIST?= ${WRKDIR}/.PLIST.mktmp +TMPGUCMD?= ${WRKDIR}/.PLIST.gucmd .for _CATEGORY in ${CATEGORIES} PKGCATEGORY?= ${_CATEGORY} @@ -4247,6 +4248,18 @@ .endif .endif +# XXX Make sure the commands to create group(s) +# and user(s) are the first in pkg-plist +.if !target(fix-plist-sequence) +fix-plist-sequence: ${TMPPLIST} +.if defined(GROUPS) || defined(USERS) + @${ECHO_CMD} "===> Correct pkg-plist sequence to create group(s) and user(s)" + @${EGREP} -e '^@exec.*${PW}' -e '^@exec ${INSTALL} -d -g' ${TMPPLIST} > ${TMPGUCMD} + @${EGREP} -v -e '^@exec.*${PW}' -e '^@exec ${INSTALL} -d -g' ${TMPPLIST} >> ${TMPGUCMD} + @${MV} -f ${TMPGUCMD} ${TMPPLIST} +.endif +.endif + .if !defined(DISABLE_SECURITY_CHECK) .if !target(security-check) .if !defined(OLD_SECURITY_CHECK) @@ -4434,7 +4447,7 @@ install-desktop-entries install-license \ post-install post-install-script add-plist-info \ add-plist-docs add-plist-examples add-plist-data \ - add-plist-post install-rc-script compress-man \ + add-plist-post fix-plist-sequence install-rc-script compress-man \ install-ldconfig-file fake-pkg security-check _PACKAGE_DEP= install _PACKAGE_SEQ= package-message pre-package pre-package-script \ --- patch_ports-MK__bsd.port.mk.txt ends here ---
Please forget about the first two patches and discuss this one. This patch has the following additions: - user and groups are no longer silent created during install from package - bring in a new variable USERS_DEL to display a hint during port remove (how to remove the unused user account). The feature to display a message during deinstall should be discussed - should USERS_DEL be a list or only yes/true? - should it be generally displayed for every user created during install or only to a selected subset (remove the new var) - different instruction if $home != /var/empty|/nonexistent Durig my research I found many ports where the pkg-install script can be removed or the account creation can be removed if this patch is implemented. Hoever I found some ports where - user/group is hardcoded and not registerd in UIDs/GIDs (example mail/assp) - new conflicting uid's, for example smokeping <==> ventrilo-server - ports wich use a user during install which is not created by the framework, pkg-install or somewhere else (astro/boinc-*) Is there an interest to solve such issues? I can add for the hardcoded but unregisterd and non conflicting ports ID entries to UIDs/GIDs --- patch_ports-MK__bsd.port.mk.txt begins here --- Index: Mk/bsd.port.mk =================================================================== RCS file: /home/pcvs/ports/Mk/bsd.port.mk,v retrieving revision 1.666 diff -u -r1.666 bsd.port.mk --- Mk/bsd.port.mk 29 Dec 2010 07:14:56 -0000 1.666 +++ Mk/bsd.port.mk 31 Dec 2010 01:06:06 -0000 @@ -1046,6 +1046,8 @@ # have a corresponding entry in ${UID_FILES}. # GROUPS - List of groups to create at install time. Each group must # have a corresponding entry in ${GID_FILES}. +# USERS_DEL - List of users where we display a message how to remove the +# user during uninstall. # # DESKTOPDIR - Name of the directory to install ${DESKTOP_ENTRIES} in. # Default: ${PREFIX}/share/applications @@ -2482,6 +2484,7 @@ PKGMESSAGE?= ${PKGDIR}/pkg-message TMPPLIST?= ${WRKDIR}/.PLIST.mktmp +TMPGUCMD?= ${WRKDIR}/.PLIST.gucmd .for _CATEGORY in ${CATEGORIES} PKGCATEGORY?= ${_CATEGORY} @@ -4178,6 +4181,7 @@ .endif .endfor @${ECHO_MSG} "===> Creating users and/or groups." + @${ECHO_CMD} "@exec echo \"===> Creating users and/or groups.\"" >> ${TMPPLIST} .for _group in ${GROUPS} # _bgpd:*:130: @if ! ${GREP} -h ^${_group}: ${GID_FILES} >/dev/null 2>&1; then \ @@ -4192,7 +4196,9 @@ else \ ${ECHO_MSG} "Using existing group \`$$group'."; \ fi; \ - ${ECHO_CMD} "@exec if ! ${PW} groupshow $$group >/dev/null 2>&1; then ${PW} groupadd $$group -g $$gid; fi" >> ${TMPPLIST}; \ + ${ECHO_CMD} "@exec if ! ${PW} groupshow $$group >/dev/null 2>&1; then \ + echo \"Creating group '$$group' with gid '$$gid'.\"; \ + ${PW} groupadd $$group -g $$gid; else echo \"Using existing group '$$group'.\"; fi" >> ${TMPPLIST}; \ done .endfor .endif @@ -4220,7 +4226,10 @@ else \ ${ECHO_MSG} "Using existing user \`$$login'."; \ fi; \ - ${ECHO_CMD} "@exec if ! ${PW} usershow $$login >/dev/null 2>&1; then ${PW} useradd $$login -u $$uid -g $$gid $$class -c \"$$gecos\" -d $$homedir -s $$shell; fi" >> ${TMPPLIST}; \ + ${ECHO_CMD} "@exec if ! ${PW} usershow $$login >/dev/null 2>&1; then \ + echo \"Creating user '$$login' with uid '$$uid'.\"; \ + ${PW} useradd $$login -u $$uid -g $$gid $$class -c \"$$gecos\" -d $$homedir -s $$shell; \ + else echo \"Using existing user '$$login'.\"; fi" >> ${TMPPLIST}; \ case $$homedir in /nonexistent|/var/empty) ;; *) ${ECHO_CMD} "@exec ${INSTALL} -d -g $$gid -o $$uid $$homedir" >> ${TMPPLIST};; esac; \ done .endfor @@ -4233,17 +4242,38 @@ list=`${PW} usershow $${_login} -P | ${SED} -ne 's/.*Groups: //p'`; \ ${ECHO_MSG} "Setting \`$${_login}' groups to \`$$list$${list:+,}${_group}'."; \ ${PW} usermod $${_login} -G $$list$${list:+,}${_group}; \ - ${ECHO_CMD} "@exec list=\`${PW} usershow $${_login} -P | ${SED} -ne 's/.*Groups: //p'\`; ${PW} usermod $${_login} -G \$${list},${_group}" >> ${TMPPLIST}; \ + ${ECHO_CMD} "@exec list=\`${PW} usershow $${_login} -P | ${SED} -ne 's/.*Groups: //p'\`; \ + echo \"Setting '$${_login}' groups to '$$list$${list:+,}${_group}'.\"; ${PW} usermod $${_login} -G \$${list},${_group}" >> ${TMPPLIST}; \ done; \ done .endfor .endif +.if defined(USERS) && defined(USERS_DEL) +.for _user in ${USERS_DEL} + @if [ ${USERS:M${_user}} ]; then \ + ${ECHO_CMD} "@unexec if ${PW} usershow ${USERS:M${_user}} >/dev/null 2>&1; then \ + echo \"To remove user ${USERS:M${_user}} permanent use command '${PW} userdel ${USERS:M${_user}}'\"; fi" >> ${TMPPLIST}; \ + fi +.endfor +.endif .endif .else @${DO_NADA} .endif .endif +# XXX Make sure the commands to create group(s) +# and user(s) are the first in pkg-plist +.if !target(fix-plist-sequence) +fix-plist-sequence: ${TMPPLIST} +.if defined(GROUPS) || defined(USERS) + @${ECHO_CMD} "===> Correct pkg-plist sequence to create group(s) and user(s)" + @${EGREP} -e '^@exec echo.*Creating users and' -e '^@exec.*${PW}' -e '^@exec ${INSTALL} -d -g' ${TMPPLIST} > ${TMPGUCMD} + @${EGREP} -v -e '^@exec echo.*Creating users and' -e '^@exec.*${PW}' -e '^@exec ${INSTALL} -d -g' ${TMPPLIST} >> ${TMPGUCMD} + @${MV} -f ${TMPGUCMD} ${TMPPLIST} +.endif +.endif + .if !defined(DISABLE_SECURITY_CHECK) .if !target(security-check) .if !defined(OLD_SECURITY_CHECK) @@ -4431,7 +4461,7 @@ install-desktop-entries install-license \ post-install post-install-script add-plist-info \ add-plist-docs add-plist-examples add-plist-data \ - add-plist-post install-rc-script compress-man \ + add-plist-post fix-plist-sequence install-rc-script compress-man \ install-ldconfig-file fake-pkg security-check _PACKAGE_DEP= install _PACKAGE_SEQ= package-message pre-package pre-package-script \ --- patch_ports-MK__bsd.port.mk.txt ends here ---
Ok, this patch will honor additional an issue which is present in the Framework but until now no one was hitting it. The patch solves the described problem about the time when a user is created by package install plus additional a problem when a user is added to a group which has other members not defined in the port. Such a problem can be for example the postfix port postfix/Makefile: USERS= postfix GROUPS= mail maildrop postfix ports/GIDs: mail:*:6:postfix,clamav Without the patch the Framework tries to add the user clamav to the group mail but clamav is not defined in the port! This will fail if the user does not exist on the system. Solution: The patch filters only users defined in USERS. To see what is filtered I haven't removed the line "DEBUG ..." If "USERS_DEL" is not wanted, it will be nice to display for every created use a generic message during uninstall. If the patch was mangled, then it is also aviable here: http://people.freebsd.org/~ohauer/diffs/Mk/PR_152498_ports__Mk__bsd.ports.mk_guid_followup_03.txt --- patch_ports-MK__bsd.port.mk.txt begins here --- Index: Mk/bsd.port.mk =================================================================== RCS file: /home/pcvs/ports/Mk/bsd.port.mk,v retrieving revision 1.666 diff -u -r1.666 bsd.port.mk --- Mk/bsd.port.mk 29 Dec 2010 07:14:56 -0000 1.666 +++ Mk/bsd.port.mk 8 Jan 2011 23:07:47 -0000 @@ -1046,6 +1046,8 @@ # have a corresponding entry in ${UID_FILES}. # GROUPS - List of groups to create at install time. Each group must # have a corresponding entry in ${GID_FILES}. +# USERS_DEL - When the port is uninstalled, display a message describing +# how to manually remove the users named in this list # # DESKTOPDIR - Name of the directory to install ${DESKTOP_ENTRIES} in. # Default: ${PREFIX}/share/applications @@ -2482,6 +2484,7 @@ PKGMESSAGE?= ${PKGDIR}/pkg-message TMPPLIST?= ${WRKDIR}/.PLIST.mktmp +TMPGUCMD?= ${WRKDIR}/.PLIST.gucmd .for _CATEGORY in ${CATEGORIES} PKGCATEGORY?= ${_CATEGORY} @@ -4178,6 +4181,7 @@ .endif .endfor @${ECHO_MSG} "===> Creating users and/or groups." + @${ECHO_CMD} "@exec echo \"===> Creating users and/or groups.\"" >> ${TMPPLIST} .for _group in ${GROUPS} # _bgpd:*:130: @if ! ${GREP} -h ^${_group}: ${GID_FILES} >/dev/null 2>&1; then \ @@ -4192,7 +4196,9 @@ else \ ${ECHO_MSG} "Using existing group \`$$group'."; \ fi; \ - ${ECHO_CMD} "@exec if ! ${PW} groupshow $$group >/dev/null 2>&1; then ${PW} groupadd $$group -g $$gid; fi" >> ${TMPPLIST}; \ + ${ECHO_CMD} "@exec if ! ${PW} groupshow $$group >/dev/null 2>&1; then \ + echo \"Creating group '$$group' with gid '$$gid'.\"; \ + ${PW} groupadd $$group -g $$gid; else echo \"Using existing group '$$group'.\"; fi" >> ${TMPPLIST}; \ done .endfor .endif @@ -4220,30 +4226,64 @@ else \ ${ECHO_MSG} "Using existing user \`$$login'."; \ fi; \ - ${ECHO_CMD} "@exec if ! ${PW} usershow $$login >/dev/null 2>&1; then ${PW} useradd $$login -u $$uid -g $$gid $$class -c \"$$gecos\" -d $$homedir -s $$shell; fi" >> ${TMPPLIST}; \ + ${ECHO_CMD} "@exec if ! ${PW} usershow $$login >/dev/null 2>&1; then \ + echo \"Creating user '$$login' with uid '$$uid'.\"; \ + ${PW} useradd $$login -u $$uid -g $$gid $$class -c \"$$gecos\" -d $$homedir -s $$shell; \ + else echo \"Using existing user '$$login'.\"; fi" >> ${TMPPLIST}; \ case $$homedir in /nonexistent|/var/empty) ;; *) ${ECHO_CMD} "@exec ${INSTALL} -d -g $$gid -o $$uid $$homedir" >> ${TMPPLIST};; esac; \ done .endfor .if defined(GROUPS) .for _group in ${GROUPS} -# _bgpd:*:130: +# mail:*:6:postfix,clamav @IFS=":"; ${GREP} -h ^${_group}: ${GID_FILES} | head -n 1 | while read group foo gid members; do \ gid=$$(($$gid+${GID_OFFSET})); \ IFS=","; for _login in $$members; do \ - list=`${PW} usershow $${_login} -P | ${SED} -ne 's/.*Groups: //p'`; \ - ${ECHO_MSG} "Setting \`$${_login}' groups to \`$$list$${list:+,}${_group}'."; \ - ${PW} usermod $${_login} -G $$list$${list:+,}${_group}; \ - ${ECHO_CMD} "@exec list=\`${PW} usershow $${_login} -P | ${SED} -ne 's/.*Groups: //p'\`; ${PW} usermod $${_login} -G \$${list},${_group}" >> ${TMPPLIST}; \ + for _user in ${USERS}; do \ + if [ "x$${_user}" = "x$${_login}" ]; then \ + list=`${PW} usershow $${_login} -P | ${SED} -ne 's/.*Groups: //p'`; \ + ${ECHO_MSG} "Setting \`$${_login}' groups to \`$$list$${list:+,}${_group}'."; \ + ${PW} usermod $${_login} -G $$list$${list:+,}${_group}; \ + ${ECHO_CMD} "@exec list=\`${PW} usershow $${_login} -P | ${SED} -ne 's/.*Groups: //p'\`; \ + echo \"Setting '$${_login}' groups to '$$list$${list:+,}${_group}'.\"; \ + ${PW} usermod $${_login} -G $${list},${_group}" >> ${TMPPLIST}; \ + else \ + ${ECHO_MSG} "==> DEBUG skip login $${_login} => not defined in USERS \"( ${USERS} )\""; \ + fi; \ + done; \ done; \ done .endfor .endif +#.if defined(USERS) && defined(USERS_DEL) +#.for _user in ${USERS_DEL} +.if defined(USERS) +.for _user in ${USERS} + @if [ ${USERS:M${_user}} ]; then \ + ${ECHO_CMD} "@unexec if ${PW} usershow ${USERS:M${_user}} >/dev/null 2>&1; then \ + echo \"To remove user ${USERS:M${_user}} permanent use command '${PW} userdel ${USERS:M${_user}}'\"; fi" >> ${TMPPLIST}; \ + fi +.endfor +.endif .endif .else @${DO_NADA} .endif .endif +# PR ports/152498 +# XXX Make sure the commands to create group(s) +# and user(s) are the first in pkg-plist +.if !target(fix-plist-sequence) +fix-plist-sequence: ${TMPPLIST} +.if defined(GROUPS) || defined(USERS) + @${ECHO_CMD} "===> Correct pkg-plist sequence to create group(s) and user(s)" + @${EGREP} -e '^@exec echo.*Creating users and' -e '^@exec.*${PW}' -e '^@exec ${INSTALL} -d -g' ${TMPPLIST} > ${TMPGUCMD} + @${EGREP} -v -e '^@exec echo.*Creating users and' -e '^@exec.*${PW}' -e '^@exec ${INSTALL} -d -g' ${TMPPLIST} >> ${TMPGUCMD} + @${MV} -f ${TMPGUCMD} ${TMPPLIST} +.endif +.endif + .if !defined(DISABLE_SECURITY_CHECK) .if !target(security-check) .if !defined(OLD_SECURITY_CHECK) @@ -4431,7 +4471,7 @@ install-desktop-entries install-license \ post-install post-install-script add-plist-info \ add-plist-docs add-plist-examples add-plist-data \ - add-plist-post install-rc-script compress-man \ + add-plist-post fix-plist-sequence install-rc-script compress-man \ install-ldconfig-file fake-pkg security-check _PACKAGE_DEP= install _PACKAGE_SEQ= package-message pre-package pre-package-script \ --- patch_ports-MK__bsd.port.mk.txt ends here ---
I don't like USERS_DEL. Ideally, there should be a ref count for USERS/GROUPS and pkg_delete would print this message when the count reaches 0. The DEBUG else clause can go away. The rest looks good to me. -- Florent Thoumie flz@FreeBSD.org FreeBSD Committer
On 2011-01-09 01:22, Florent Thoumie wrote: > I don't like USERS_DEL. Ideally, there should be a ref count for > USERS/GROUPS and pkg_delete would print this message when the count > reaches 0. > > The DEBUG else clause can go away. > > The rest looks good to me. I agree, the question is where to implement this? a) the ports framework (ports/Mk) b) src/use.sbin/pkg_install What do you think about the following? Instead of USERS_DEL use a blacklist of predefined accounts from src/etc/master.passwd and present only a generic message. This way we do not present the message to ports which have for example the user "www" defined in the Makefile. I guess now the question about an automatic sync of this blacklist accounts will come up ;) I suspect the list will not change that often, last change is five years ago and the created blacklist is against HEAD. The following diff replace the USERS_DEL part, compares _user against a blacklist and displays a very generic message during deinstall. I guess this is a fair starting point and better then display nothing. If someone has a better message to display then please change the text. A full diff is available here: http://people.freebsd.org/~ohauer/diffs/Mk/PR_152498_ports__Mk__bsd.ports.mk_guid_followup_04.txt --- patch_ports-Mk__bsd.port.mk_incremental.txt begins here --- Index: Mk/bsd.port.mk =================================================================== RCS file: /home/pcvs/ports/Mk/bsd.port.mk,v retrieving revision 1.666 diff -u -r1.666 bsd.port.mk --- Mk/bsd.port.mk 29 Dec 2010 07:14:56 -0000 1.666 +++ Mk/bsd.port.mk 9 Jan 2011 17:43:45 -0000 @@ -1282,6 +1282,10 @@ UID_OFFSET?= 0 GID_OFFSET?= 0 +# predefined accounts from src/etc/master.passwd +# alpha numeric sort order +USERS_BLACKLIST= _dhcp _pflogd bin bind daemon games kmem mailnull man news nobody operator pop proxy root smmsp sshd toor tty uucp www + LDCONFIG_DIR= libdata/ldconfig LDCONFIG32_DIR= libdata/ldconfig32 @@ -4238,6 +4242,14 @@ done .endfor .endif +.if defined(USERS) +.for _user in ${USERS} + @if [ ! ${USERS_BLACKLIST:M${_user}} ]; then \ + ${ECHO_CMD} "@unexec if ${PW} usershow ${_user} >/dev/null 2>&1; then \ + echo \"==> You should manually remove the \\\"${_user}\\\" user. \"; fi" >> ${TMPPLIST}; \ + fi +.endfor +.endif .endif .else @${DO_NADA} --- patch_ports-Mk__bsd.port.mk_incremental.txt ends here ---
Patch looks good to me (except for the DEBUG bit that shouldn't be committed). -- Florent Thoumie flz@FreeBSD.org FreeBSD Committer
State Changed From-To: open->analyzed Take for -exp.
State Changed From-To: analyzed->closed Committed. Thanks!