[vlc-devel] [PATCH] smb: try libdsm first

[Thomas Guillem]

On Thu, Oct 17, 2019, at 13:58, Alexandre Janniaux wrote:
> Would it have some meaning to try smb2 but failing if authentication is
> required, then try libdsm, and if it fails try smb2 again but handle
> authentication? Maybe a smart thing can be done with keystore to fix this in the
> first case.

But then, you favor SMBv1 or V2 if authentication is required, that should be the most common case.

> 
> It could be achieved by a dual capability in the smb2 module.
> 
> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
> On Tue, Oct 15, 2019 at 03:41:17PM +0200, Thomas Guillem wrote:
> > Some samba servers (on Windows 7) implement both SMB2 and SMB1. The problem is
> > that the SMB2 part is not configured like the SMB1 one. Only SMB1 seems to
> > reflect the user configuration (using Windows Settings, not anything
> > complicated like via powershell/regedit).
> >
> > If we try to connect to such server via libsmb2, the server will return a
> > SMB2_STATUS_ACCESS_DENIED (0xC0000022) status. Our libsmb2 module will then ask
> > the user for credentials via a dialog. The problem is that no credentials will
> > ever work since only the SMB1 part is configured.
> >
> > I tried to differentiate (via wireshark) the negotiation between such server
> > and an other working SMB2 server but could not find anything that could tell us
> > that this ACCESS_DENIED status should be ignored on this specific server (in
> > order to fallback to libdsm).
> >
> > The only possible fix is to try libdsm first. VLC will then favor the SMB1
> > protocol over SMB 2&3.
> >
> > NB1: libsmb2 is backported to VLC 3.0 for iOS and Android ports. These ports
> > are beta-testing SMB 2&3 support on mobile.
> >
> > NB2: We get a lot of angry mail/reviews about SMB1 support broken, I don't
> > think we can drop SMB1 (even if I would love to).
> >
> > NB3: We can't drop libsmb2 either for the same reason (we got a *lot* of
> > requests to support it).
> > ---
> >  modules/access/dsm/access.c | 17 ++++++++++-------
> >  modules/access/smb2.c       | 14 +++++++-------
> >  2 files changed, 17 insertions(+), 14 deletions(-)
> >
> > diff --git a/modules/access/dsm/access.c b/modules/access/dsm/access.c
> > index 776925c9eeb..186a567a72c 100644
> > --- a/modules/access/dsm/access.c
> > +++ b/modules/access/dsm/access.c
> > @@ -69,7 +69,7 @@ vlc_module_begin ()
> >      set_shortname( "dsm" )
> >      set_description( N_("libdsm SMB input") )
> >      set_help(BDSM_HELP)
> > -    set_capability( "access", 20 )
> > +    set_capability( "access", 22 )
> >      set_category( CAT_INPUT )
> >      set_subcategory( SUBCAT_INPUT_ACCESS )
> >      add_string( "smb-user", NULL, SMB_USER_TEXT, SMB_USER_LONGTEXT, false )
> > @@ -343,12 +343,6 @@ static int login( stream_t *p_access )
> >
> >      if( connect_err == EACCES )
> >      {
> > -        if (var_Type(p_access, "smb-dialog-failed") != 0)
> > -        {
> > -            /* A higher priority smb module (likely smb2) already requested
> > -             * credentials to the users. It is useless to request it again. */
> > -            goto error;
> > -        }
> >          while( connect_err == EACCES
> >              && vlc_credential_get( &credential, p_access, "smb-user", "smb-pwd",
> >                                     SMB_LOGIN_DIALOG_TITLE,
> > @@ -365,6 +359,15 @@ static int login( stream_t *p_access )
> >          if( connect_err != 0 )
> >          {
> >              msg_Err( p_access, "Unable to login" );
> > +
> > +            if (credential.i_get_order == GET_FROM_DIALOG)
> > +            {
> > +                /* Tell other smb modules (likely smb2) that we already
> > +                 * requested credential to the users and that it it useless to
> > +                 * try again.  This avoid to show 2 login dialogs for the same
> > +                 * access. */
> > +                var_Create(p_access, "smb-dialog-failed", VLC_VAR_VOID);
> > +            }
> >              goto error;
> >          }
> >      }
> > diff --git a/modules/access/smb2.c b/modules/access/smb2.c
> > index 923e6d57e04..7f9b614d006 100644
> > --- a/modules/access/smb2.c
> > +++ b/modules/access/smb2.c
> > @@ -664,6 +664,13 @@ Open(vlc_object_t *p_obj)
> >                         NULL);
> >      ret = vlc_smb2_open_share(access, smb2_url, &credential);
> >
> > +    if (ret == -1 && var_Type(access, "smb-dialog-failed"))
> > +    {
> > +        /* A higher priority smb module (likely dsm) already requested
> > +         * credentials to the users. It is useless to request it again. */
> > +        goto error;
> > +    }
> > +
> >      while (ret == -1
> >          && (!sys->error_status || VLC_SMB2_STATUS_DENIED(sys->error_status))
> >          && vlc_credential_get(&credential, access, "smb-user", "smb-pwd",
> > @@ -683,13 +690,6 @@ Open(vlc_object_t *p_obj)
> >          if (error && *error)
> >              vlc_dialog_display_error(access,
> >                                       _("SMB2 operation failed"), "%s", error);
> > -        if (credential.i_get_order == GET_FROM_DIALOG)
> > -        {
> > -            /* Tell other smb modules (likely dsm) that we already requested
> > -             * credential to the users and that it it useless to try again.
> > -             * This avoid to show 2 login dialogs for the same access. */
> > -            var_Create(access, "smb-dialog-failed", VLC_VAR_VOID);
> > -        }
> >          goto error;
> >      }
> >
> > --
> > 2.20.1
> >
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel