[vlc-devel] [PATCH] smb: try libdsm first
Simon Latapie
garf at videolan.org
Wed Oct 16 13:29:01 CEST 2019
Hello,
On Wed, Oct 16, 2019, at 12:01, Rémi Denis-Courmont wrote:
> Hi,
>
> There is already a vulnerability if there is an MITM. The point is that this patch adds a second vulnerability by using SMB1 even without an MITM.
The man-in-the-middle attack on a samba server is, as I said, completely out of scope of this patch. On that subject, if you have a solution or suggestion, please share it.
Sorry but I still do not understand where the second vulnerability comes from.
> Downgrading is also going to damage performance that already sucks badly enough with SMB inputs.
This is a complete other subject. Note that this is also completely irrelevant on most modern operating system shares, as this is now really complicated to activate SMB1 shares on supported Windows.
> And sorry but I do draw a line with backward or bug compatibility hacks. If they break or damage functionality where the hack would not be needed, then it's wrong.
The patch *is* a compatibility hack, and yes, this is probably wrong, but doing another way causes a functional regression.
Again, if you have another solution or suggestion, please share it.
--
Simon Latapie
>
> Le 16 octobre 2019 12:08:21 GMT+03:00, Simon Latapie <garf at videolan.org> a écrit :
>>
>> On Wed, Oct 16, 2019, at 10:41, Rémi Denis-Courmont wrote:
>>> Hi,
>>>
>>> A downgrade occurs if two SMB2 capable nodes end up using SMB1 between them. The classic attack is for an active MITM to block the SMB2 transactions, but with this patch the attacker needs not even be an MITM, just an eavesdropper.
>>
>> This is a generic man-in-the-middle scenario, yes. But I really doubt this changes anything in VLC case: man-in-the-middle is a matter of trust, and VLC does not verify the server identity, either with SMB1 or SMB2 protocol (I am not even sure this is possible in most of the cases - aka without an AD/kerberos infrastructure). So a man-in-the-middle attack can be achieved, in SMB1 or SMB2.
>>
>> Do not get me wrong: I think SMB1 is a protocol that *must* be avoided at all cost when publishing a share service. The problem is a lot of VLC users seems to use old operating systems or NAS.
>>
>> I just have difficulties to see how this can be blamed to VLC.
>>
>> Regards,
>>
>> --
>> Simon Latapie
>> garf at videolan.org
>>
>>
>>> Le 16 octobre 2019 10:02:54 GMT+03:00, Simon Latapie <garf at videolabs.io> a écrit :
>>>> 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
>>>>
>>>
>>> --
>>> 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
>>
>
> --
> 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/4349f0e8/attachment.html>
More information about the vlc-devel
mailing list