[Pkg-samba-maint] Bug#443230: Enable net usershare

[Steve Langasek]

Hi guys,

On Wed, Sep 19, 2007 at 03:33:33PM -0400, Mathias Gug wrote:
> I've attached a debdiff that adds net usershare support to samba. It
> enables users part of the smbshare group to create shares using the net
> usershare command. 

> More information about net usershare can me found in the man page of the
> net command: 

>   Starting  with  version 3.0.23, a Samba server now supports the
>   ability for non-root users to add user define shares to be exported
>   using  the "net usershare" commands.

> This was discussed for Ubuntu here:
> https://bugs.launchpad.net/ubuntu/+source/samba/+bug/128548

> diff -u samba-3.0.25b/debian/rules samba-3.0.25b/debian/rules
> --- samba-3.0.25b/debian/rules
> +++ samba-3.0.25b/debian/rules
> @@ -147,6 +147,8 @@
>  	install -m 0644 debian/mksmbpasswd.8 $(DESTDIR)/usr/share/man/man8/mksmbpasswd.8
>  	install -m 0644 source/nsswitch/libnss_winbind.so $(DESTDIR)/lib/libnss_winbind.so.2
>  	install -m 0644 source/nsswitch/libnss_wins.so $(DESTDIR)/lib/libnss_wins.so.2
> +	# Create usershare directory
> +	install -m 01770 -d $(DESTDIR)/var/lib/samba/usershares
>  
>  ifeq ($(smbfs),yes)
>  	# Create the symlinks that will allow us to do "mount -t smbfs ..."

Why is this directory not world-readable and world-executable, which should
be the default policy for all directories in Debian?

> diff -u samba-3.0.25b/debian/samba-common.postinst samba-3.0.25b/debian/samba-common.postinst
> --- samba-3.0.25b/debian/samba-common.postinst
> +++ samba-3.0.25b/debian/samba-common.postinst
> @@ -113,0 +114,15 @@
> +
> +case "$1" in
> +	configure)
> +		# add the smbshare group
> +		if ! getent group smbshare > /dev/null 2>&1
> +		then
> +			addgroup --system smbshare
> +		fi
> +
> +		# update the ownership of /var/lib/samba/usershares
> +		chgrp smbshare /var/lib/samba/usershares
> +		# update the permissions
> +		chmod 01770 /var/lib/samba/usershares
> +		;;
> +esac

This code is not guarded to keep it from being invoked on every upgrade.
Should it be?  (If someone changes the permissions of this directory, why
should they be overwritten again?)

> diff -u samba-3.0.25b/debian/smb.conf samba-3.0.25b/debian/smb.conf
> --- samba-3.0.25b/debian/smb.conf
> +++ samba-3.0.25b/debian/smb.conf
> @@ -214,6 +214,15 @@
>  ;   winbind enum groups = yes
>  ;   winbind enum users = yes
>  
> +# Setup usershare options to enable non-root user to share folders
> +# with the net usershare command.
> +
> +# The path were the share definition will be stored. Only members of the group
> +# owning the path will be able to use the net usershare command.
> +   usershare path = /var/lib/samba/usershares

Could this be better addressed by fixing the built-in default in the binary
instead of requiring an override in smb.conf?  (The current default seems to
be /var/run/samba/usershares, which is simply wrong.)

> +# Maximum number of usershare. 0 (default) means that usershare is disabled.
> +   usershare max shares = 100

Why "100" as the max shares limit?  (It seems that any limit we'd choose is
arbitrary and an override of the upstream value, so I'm not particularly
bothered by the number, just wondering if there's a rationale for this
cutoff.)

> diff -u samba-3.0.25b/debian/samba-common.dirs samba-3.0.25b/debian/samba-common.dirs
> --- samba-3.0.25b/debian/samba-common.dirs
> +++ samba-3.0.25b/debian/samba-common.dirs
> @@ -5,0 +6 @@
> +var/lib/samba/usershares

I think either this or the debian/rules patch is superfluous.  I'm not sure
which, but I don't think dh_installdirs checks debian/tmp for templates,
which would seem to make the "install" bit in debian/rules the redundant
bit.

For that matter, if the permissions are being set in the postinst, why
shouldn't the directory creation be done there as well?  None of the other
directories are shipped in the packages, and samba-common's postrm already
does an rm -rf /var/lib/samba.

On Wed, Sep 19, 2007 at 10:50:01PM +0200, Christian Perrier wrote:
> A first concern comes with the dedicated group name. Should we use
> "smbshare" and then still advertise that obsolete acronym (SMB) which
> is however known by nearly everybody?

How about "sambashare" or "samba-share"?  It does, after all, have little to
do with the smb protocol, but everything to do with the samba package.

On Wed, Sep 19, 2007 at 05:14:59PM -0400, Mathias Gug wrote:
> On Wed, Sep 19, 2007 at 10:50:01PM +0200, Christian Perrier wrote:
> > A first concern comes with the dedicated group name. Should we use
> > "smbshare" and then still advertise that obsolete acronym (SMB) which
> > is however known by nearly everybody?

> Another proposal is to use a group named fileshare, that could be used
> to define a list of users that are allowed to define shared directories
> on the network (via samba, nfs, ftp or any other protocol).

I'd prefer not to use a generic name here; even supposing that some other
package would want to do something similar for other protocols, there's no
reason to assume that the privilege would be granted to the same users
regardless of protocol.  It's easier to add more groups to the list of
default permissions for single-user systems than it is to split a group
after the fact.

Anyway, I am in favor of enabling this feature, I just think the patch needs
a bit of work first.

Thanks,
-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
vorlon at debian.org                                   http://www.debian.org/