diff mbox

slirp/smbd: disable printer in smb config

Message ID 1415011930-1964-1-git-send-email-peter@lekensteyn.nl
State New
Headers show

Commit Message

Peter Wu Nov. 3, 2014, 10:52 a.m. UTC
The file sharing module should not handle printers, so disable it.
The options 'load printers' and 'printing' have been available since the
beginning (May 1996, commit 0e8fd3398771da2f016d72830179507f3edda51b).
Option 'disable spoolss' is available since Samba 2.0.4, commit
de5f42c9d9172592779fa2504d44544e3b6b1c0d).

Next, "socket address" was reported as deprecated, use a combination of
"interfaces" and "bind interfaces only" instead (available since October
1997, commit 79f4fb52c1ed56fd843f81b4eb0cdd2991d4d0f4).

Override cache directory to avoid writing to a global directory. Option
available since Samba 3.4.0, Jan 2009, commit
19a05bf2f485023b11b41dfae3f6459847d55ef7.

Set "usershare max shared=0" to prevent a global directory from being
used. Option available since Samba 3.0.23, February 2006, commit
5831715049f2d460ce42299963a5defdc160891b.

The most recently option was introduced with Samba 3.4.0, but previously
"state directory" was already added which exists in Samba 3.4.0. As
unknown parameters are ignored (while printing a warning), it should be
safe to add another option.

Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
Hi,

While trying to share a folder with a guest, I noticed that the option -net
user,smb=... would time out in the guest due to an incompatibility with Samba 4
(see also mailing list message "slirp-smb broken with Samba 4.1" from Jan Kiska
and https://bugs.debian.org/747636). FYI, the bug is fixed in newer Samba
(tested with samba-4.2.0rc1-388-ga3b333a).

While trying to fix that, I found that Samba would try to communicate with CUPS.
This patch disables that fixes some other paths as well. Looking through the
smb.conf manual for "{prefix}", it seems that no other directory is forgotten
now.

As the inetd mode is broken, I work around by starting smbd with the generated
config:

    smbd -s smb.conf -p 1337

Then I forward the ports to the guest with (newline inserted for readability):

    -user net,
        guestfwd=tcp:0.0.0.0:139-cmd:'nc 127.0.0.1 1337',
        guestfwd=tcp:0.0.0.0:445-cmd:'nc 127.0.0.1 1337'

This "works" but is certainly not optimal.

Kind regards,
Peter
---
 net/slirp.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Michael Tokarev Nov. 3, 2014, 2:15 p.m. UTC | #1
03.11.2014 13:52, Peter Wu wrote:
> The file sharing module should not handle printers, so disable it.
> The options 'load printers' and 'printing' have been available since the
> beginning (May 1996, commit 0e8fd3398771da2f016d72830179507f3edda51b).
> Option 'disable spoolss' is available since Samba 2.0.4, commit
> de5f42c9d9172592779fa2504d44544e3b6b1c0d).
> 
> Next, "socket address" was reported as deprecated, use a combination of
> "interfaces" and "bind interfaces only" instead (available since October
> 1997, commit 79f4fb52c1ed56fd843f81b4eb0cdd2991d4d0f4).
> 
> Override cache directory to avoid writing to a global directory. Option
> available since Samba 3.4.0, Jan 2009, commit
> 19a05bf2f485023b11b41dfae3f6459847d55ef7.
> 
> Set "usershare max shared=0" to prevent a global directory from being
> used. Option available since Samba 3.0.23, February 2006, commit
> 5831715049f2d460ce42299963a5defdc160891b.
> 
> The most recently option was introduced with Samba 3.4.0, but previously
> "state directory" was already added which exists in Samba 3.4.0. As
> unknown parameters are ignored (while printing a warning), it should be
> safe to add another option.

I think this all makes very good sense.  Let's apply it to -trivial too,
for now anyway, -- if we'll ever decide to factor it out to a helper
script, that script will be more complete.

BTW, I'm not sure `socket address' paraameter is relevant in this context
at all, -- smbd should not use it in inetd mode.  It'd be interesting to
know why this option is here to start with, and whenever we really need
the new interfaces/bind-interfacs-only replacement.

I picked it up for -trivial, and also Cc'ing Jan.

Thank you!

/mjt

> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
> Hi,
> 
> While trying to share a folder with a guest, I noticed that the option -net
> user,smb=... would time out in the guest due to an incompatibility with Samba 4
> (see also mailing list message "slirp-smb broken with Samba 4.1" from Jan Kiska
> and https://bugs.debian.org/747636). FYI, the bug is fixed in newer Samba
> (tested with samba-4.2.0rc1-388-ga3b333a).
> 
> While trying to fix that, I found that Samba would try to communicate with CUPS.
> This patch disables that fixes some other paths as well. Looking through the
> smb.conf manual for "{prefix}", it seems that no other directory is forgotten
> now.
> 
> As the inetd mode is broken, I work around by starting smbd with the generated
> config:
> 
>     smbd -s smb.conf -p 1337
> 
> Then I forward the ports to the guest with (newline inserted for readability):
> 
>     -user net,
>         guestfwd=tcp:0.0.0.0:139-cmd:'nc 127.0.0.1 1337',
>         guestfwd=tcp:0.0.0.0:445-cmd:'nc 127.0.0.1 1337'
> 
> This "works" but is certainly not optimal.
> 
> Kind regards,
> Peter
> ---
>  net/slirp.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index c171119..bad427b 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -523,15 +523,21 @@ static int slirp_smb(SlirpState* s, const char *exported_dir,
>      fprintf(f,
>              "[global]\n"
>              "private dir=%s\n"
> -            "socket address=127.0.0.1\n"
> +            "interfaces=127.0.0.1\n"
> +            "bind interfaces only=yes\n"
>              "pid directory=%s\n"
>              "lock directory=%s\n"
>              "state directory=%s\n"
> +            "cache directory=%s\n"
>              "ncalrpc dir=%s/ncalrpc\n"
>              "log file=%s/log.smbd\n"
>              "smb passwd file=%s/smbpasswd\n"
>              "security = user\n"
>              "map to guest = Bad User\n"
> +            "load printers = no\n"
> +            "printing = bsd\n"
> +            "disable spoolss = yes\n"
> +            "usershare max shares = 0\n"
>              "[qemu]\n"
>              "path=%s\n"
>              "read only=no\n"
> @@ -544,6 +550,7 @@ static int slirp_smb(SlirpState* s, const char *exported_dir,
>              s->smb_dir,
>              s->smb_dir,
>              s->smb_dir,
> +            s->smb_dir,
>              exported_dir,
>              passwd->pw_name
>              );
>
Michael Tokarev Nov. 3, 2014, 2:19 p.m. UTC | #2
03.11.2014 17:15, Michael Tokarev wrote:

> I picked it up for -trivial, and also Cc'ing Jan.

And also changed subject to

  slirp/smbd: modify/set several parameters in generated smbd.conf

-- a bit too generic, but at least it does not hide that many other
parameters are being changed.

Thanks,

/mjt
Peter Wu Nov. 3, 2014, 5:59 p.m. UTC | #3
On Monday 03 November 2014 17:15:24 Michael Tokarev wrote:
> BTW, I'm not sure `socket address' paraameter is relevant in this context
> at all, -- smbd should not use it in inetd mode.  It'd be interesting to
> know why this option is here to start with, and whenever we really need
> the new interfaces/bind-interfacs-only replacement.

The socket option is unused when QEMU invokes the command directly. The
only (weak) reason why it is still there is to ease testing, such that
you can simply use:

    smbd -s smb.conf -p 1337

Without the socket option, I am afraid that you will accidentally expose
the very permissive share to the network.

So either append a comment explaining the above or just remove it. I am
fine with removing the interfaces option from this patch (or in a future
patch if you prefer that).
diff mbox

Patch

diff --git a/net/slirp.c b/net/slirp.c
index c171119..bad427b 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -523,15 +523,21 @@  static int slirp_smb(SlirpState* s, const char *exported_dir,
     fprintf(f,
             "[global]\n"
             "private dir=%s\n"
-            "socket address=127.0.0.1\n"
+            "interfaces=127.0.0.1\n"
+            "bind interfaces only=yes\n"
             "pid directory=%s\n"
             "lock directory=%s\n"
             "state directory=%s\n"
+            "cache directory=%s\n"
             "ncalrpc dir=%s/ncalrpc\n"
             "log file=%s/log.smbd\n"
             "smb passwd file=%s/smbpasswd\n"
             "security = user\n"
             "map to guest = Bad User\n"
+            "load printers = no\n"
+            "printing = bsd\n"
+            "disable spoolss = yes\n"
+            "usershare max shares = 0\n"
             "[qemu]\n"
             "path=%s\n"
             "read only=no\n"
@@ -544,6 +550,7 @@  static int slirp_smb(SlirpState* s, const char *exported_dir,
             s->smb_dir,
             s->smb_dir,
             s->smb_dir,
+            s->smb_dir,
             exported_dir,
             passwd->pw_name
             );