Re: Patch to fix SMB browsing bugs

[Nielsen <nielsen memberwebs com>]

Alexander Larsson wrote:
> So, what is the exact issue here. You have to authenticate to the master
> browser when enumerating the workgroup? 

Yes. This is on a Windows 2K domain. A typical company domain, with 
several AD domain browsers. Authentication is needed to get a host list 
from the master browser. smbclient correctly requests this authentication.

> What happens if you authenticate as a guest? 

Doesn't work.

> Do you actually have to authenticate, or does passing the
> username/domain suffice? 

Tried this too, and no go.

> I was hoping we could avoid getting spammed
> with new login dialogs each time you click when navigating workgroups
> and smb shares.

Yes, that would be nice. I'm interested in fixing this up. Eventually 
I'd like to help integrate kerberos into this whole deal so we don't get 
prompted at all, except for a single login (which would be tied into 
gnome-keyring-manager).

In addition it's a safe assumption that multiple shares on the same 
computer use the same user/password combinations. IMO, we should cache 
this for a single smb host, rather than per share as is currently the case.

> I don't like the way your patch handles things. The list of workgroups
> is a global cache that is needed to quickly tell the type of a smb uri.

Point taken, and because the authentication code no longer needs a 
current_uri, this is no longer necessary. I'll remove this in any 
updated patches.

> It
> will also ask for (and save) the password to the server when it really
> needs the password for the master browser.

Perhaps it's not apparent from a cursory perusal of the code, but the 
patch has the correct behavior. It prompts for the password to the 
master browser (correct host noted in the prompt, no share shown), and 
then caches it properly.

> If we really have to authenticate against the master browser we should
> probably handle that specifically in the code. It should have an auth
> dialog different from the one for the server you're conencting to. 

It does. The dialog has no share shown. Or are you talking about a 
completely different dialog? I wasn't aware that the gnome 
authentication subsystem supported this. Willing to be educated :)

> We
> should save the master browser user/domain/password for later cache
> updates, so we don't keep re-asking for it at random points. Of course,
> this makes it hard to correct later if you initially type in the wrong
> username/password...

Yes, this is done, when the user checks the 'Remember password for this 
session' check box.

> -	in_args.port = ((GnomeVFSToplevelURI *)current_uri)->host_port;
> +	in_args.port = 0;
>
> Why this change?

We don't always have a current_uri to work with while in the auth code. 
In addition, not all smb traffic goes over a specific or predictable 
(between configurations) port.

> > There are still some other issues.
> >
> > There seems to be a deadlock or race in the authentication code. In many
> > cases the gnome_vfs_module_callback_invoke call in invoke_fill_auth
> > (modules/smb-method.c) never returns.
> >
> > When stracing or debugging the process (ie: gnome-vfs-daemon or
> > nautilus) all works reliably and as expected, so I suspect a
> > locking/race issue. Any ideas what this could could be?
>
> Ugh. I think i know. auth_fn() is called from libsmb when it needs
> authentication. This means we've called LOCK_SMB() to protect (thread-
> unsafe) libsmbclient. However, auth_fn then calls out various password
> related callbacks while still holding the lock. At this time the client
> getting the callback can be blocking on LOCK_SMB in a gnome-vfs call,
> and will not process the callback, leading to a deadlock.
>
> I really have no idea how to fix this. The way authentication is set up
> in libsmbclient is really targeted at a blocking console app. We can't
> drop the lock while we do the callback, because there is no guarantee
> that libsmb can handle another thread doing things while in authfn.
> There is also no way we could return FAIL through libsmbclient so we can
> sleep without having libsmbclient active, because if we return something
> wrong from authfn libsmbclient automatically tries to do an anon login.

I agree that the libsmbclient interface isn't the best. The very fact 
that no caller data can be passed to the authentication function implies 
a very hastily put together API.

However I think we can get around that, if we eliminate the main lock. 
Perhaps this sounds naive, but bear with me....

AFAICT the two mian uses for the global lock in the SMB module are:

- Locking of call state during the auth_fn
- Locking of the smb context since it's not reentrant or thread safe.

I've already elimitated most per-call-state use of global data from 
within auth_fn and it's called functions.

Secondly with the (possibly incorrect) assumption that each 
GnomeVFSContext should not be used to call in reentrantly, we can make a 
map of smb contexts to GnomeVFSContext's. Any check ins and check outs 
of this map would of course need to be thread safe.

Holding a hard lock across network operations (and especially something 
as flakey as SMB) is scary. If we could have more short lived locking 
(global vars only, for example) that would probably solve the problem.

BTW, thanks for all your work on Gnome VFS. Everything else in Gnome VFS 
works wonderfully. I understand the frustrations with SMB completely.

Cheers,
Nate