On Wed, Apr 05, 2006 at 09:58:56PM -0400, Justin Pryzby wrote: > For the record, I like the intent of this patch, but I think it is a > little too long for inclusion in the Developers reference. Perhaps a > reference to the "Securing Debian" section where it will be included > will be sufficient? It's just that I this somehow out of context in the "Securing Debian" manual as it's a document more oriented towards system adminstrators setting up Debian boxes. But I do agree that if the maintainer believes this is a patch that is too long > Do you intend to have so many links to the Cross_site_scripting wiki > page? No. That was a mistake. > The user adding bit is useful; I wonder if you would consider > contributing a debhelper bit, dh_adduser? See #81697 and #118787. Ummm.. when we talked about dh_adduser in -devel there were few that liked the idea. Please also see #291177 and the thread under http://lists.debian.org/debian-policy/2005/01/msg00024.html > + if [ "$FIRST_SYSTEM_UID" -le "$USERID" ] && \ > + [ "$USERID" -le "$LAST_SYSTEM_UID" ]; then > + echo "The user $SERVER_USER already exists as a non sys > tem user!" >&2 > + echo "Aborting package installation" >&2 > + exit 1 > + fi > This is broken; you must use ||, and the comparisons are backwards. Hmmmm, you are correct this says that if first_system_uid <= userid <= last_system_uid then it's a non-system user, when it's the other way around. > + if ! groups $SERVER_USER | grep -q $ADDGROUP; then > This needs to be grep -qw to avoid substring matches, and should > really have |cut -d: -f2 also: Noted. > +Does not run if either the user or the group do not exist: > +<example> > + if getent passwd | grep -q "^<var>server_user</var>:"; then > + echo "Server user does not exist. Aborting" >&2 > + exit 1 > + fi > Backwards test. What do you mean by this? As for the other typos, I will fix them if either the maintainer wishes me to submit a new patch for inclusion in the Developer's Reference or if I get some spare time to add this to the Securing Debian Manual. FWIW, I've reused some of the content here in my Debconf6 talk, available at http://people.debian.org/~jfs/debconf6/#security Regards Javier
Attachment:
signature.asc
Description: Digital signature