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

[Simon Latapie]

Hello,

can you explain a bit more about a downgrade attack scenario ? This does not look obvious to me.
Downgrade attacks are usually either a problem for the server (so not VLC), or an service spoof, which does not seem to be relevant here (the patch is not modifying the server selection/address).

Regards,

-- 
Simon Latapie
garf at videolabs.io
+33 1 84 17 56 63



On Tue, Oct 15, 2019, at 17:19, Rémi Denis-Courmont wrote:
> Hi,
> 
> Looks like an obvious downgrade attack to me. You're waiting for a CVE if you merge this patch.
> 
> Le 15 octobre 2019 16:41:17 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>> 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;
>>      } 
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté. 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20191016/f0190674/attachment.html>