[vlc-devel] [PATCH] smb: try libdsm first
Thomas Guillem
thomas at gllm.fr
Wed Oct 16 12:53:57 CEST 2019
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.
>
> Downgrading is also going to damage performance that already sucks badly enough with SMB inputs.
>
> 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.
We had lot of SMB2 issues for quite some time, I never wanted to put libdsm in high priority to quickly fix thoses issues. I did my best to solve lot of libsmb2 issues that was preventing users to access any SMB servers. But the issue I'm now facing is not solvable via libsmb2.
I agree that SMBv1 should never be put in high priority. On the other hand, we got a lot of angry custumers that are insulting us daily because of that.
cf. https://aka.ms/stillneedssmb1 sadly, smb1 is still used.
What I'm now proposing: add an option in VLC-Android and VLC-ios to enable SMBv1 compat mode. We will explain in that option that users should not do that and change their SMB servers instead.
Do you agree with that ?
>
> 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/1d478580/attachment.html>
More information about the vlc-devel
mailing list