search

videolabs / libdsm

Defective SMb: A minimalist implementation of a client library for SMBv1 using Plain'Ol C
http://videolabs.github.io/libdsm
Other
208 stars 86 forks source link

Support SMB2 and SMB3 when browsing shares #110

Closed L6Xv3kWu closed 2 years ago

L6Xv3kWu commented 7 years ago

Currently the library does not support browsing and opening shares which enforce the SMB2 or later protocol. This is kinda a dupe for https://github.com/videolabs/libdsm/issues/80. Microsoft officially recommends that file servers disable the SMB1 protocol, so it would be nice if SMBv2 and later is supported.

sahlberg commented 3 years ago

I'll just try to summarize and just walk away, maybe someday someone will realize what I'm talking about and will fix the issue.

mDNS browsing any SMB3 share that forces encryption won't work, you can get prompted to enter authentication details and see the initial root directory but nothing further. Adding ?seal will work, but it is explicit, not implicit/auto, breaking mDNS access through the UI. But that's just hurting the easy-access of clicking on an mDNS-visible share, not a big deal.

But no one knowing about ?seal ends up not being able to browse any encrypted SMB3-ONLY shares. This makes VLC pointless to any average user. It does not work from any usability standpoint. You're forced to add a server to Favorites and manually add a non-standard ?seal string to the folder path, something I'm betting most users won't have a clue to do.

I don't know about the mDNS part since I never use it and it is not used from libsmb2. I have no idea how VLC uses mDNS or how it affects other parts of the code or functionality.

I agree that if it is hard for users to figure out how to use the software and its features then that is a problem. But it sounds like this should be easy to solve in VLC by someone creating a wikipage or similar.

OdinVex commented 3 years ago

I'll just try to summarize and just walk away, maybe someday someone will realize what I'm talking about and will fix the issue. mDNS browsing any SMB3 share that forces encryption won't work, you can get prompted to enter authentication details and see the initial root directory but nothing further. Adding ?seal will work, but it is explicit, not implicit/auto, breaking mDNS access through the UI. But that's just hurting the easy-access of clicking on an mDNS-visible share, not a big deal. But no one knowing about ?seal ends up not being able to browse any encrypted SMB3-ONLY shares. This makes VLC pointless to any average user. It does not work from any usability standpoint. You're forced to add a server to Favorites and manually add a non-standard ?seal string to the folder path, something I'm betting most users won't have a clue to do.

I don't know about the mDNS part since I never use it and it is not used from libsmb2. I have no idea how VLC uses mDNS or how it affects other parts of the code or functionality.

I agree that if it is hard for users to figure out how to use the software and its features then that is a problem. But it sounds like this should be easy to solve in VLC by someone creating a wikipage or similar.

I refreshed before unsubscribing, thought it relevant to reply: I don't recall most client software not being able to switch to encryption when the server mandates it, but I have seen some clients with checkboxes/bool-confs used to assume encryption and refuse to connect if it doesn't have it. I do not think this needs a wikipage, I think it needs a usability fix, in the library. If server is mandating, encrypt. Add a checkbox to 'assume', at least, not require knowledge of some obscure flag, for something so easily accessible to the mass public on something like an 'app store'.

Edit: ... Adding a Favorite just to do this results in duplicates in the "Local Network" subsection. Using ?seal gets changed in the Favorited entry as soon as you browse a Share, instead of sticking to the root dir as specified. That's an issue for VLC, though. Just adding here because it is also relevant to libdsm as for the folder path being destroyed upon first use.

Edit: Typo correction on ?seal.

computator commented 3 years ago

I was not aware of the ?seal parameter until just now, and have previously tried quite a number of things to get this to work in the past (and I am saying this as a software engineer). From a usability standpoint specifying this on a wiki page, although useful, is definitely not optimal. Parameters like that should be to configure optional features, not for something that is required to make it work in the first place. If VLC/libdsm fail to connect without the 'seal' option they should be smart enough to try again with it. Even better would be to, if possible, detect that it's necessary and use the option right away without having to fail first.

jbkempf commented 3 years ago

First, there is ABSOLUTELY no reason to have this kind of tone, on a ticket. So please calm down, especially when @sahlberg is trying to explain what he did.

Then, sure, there are bugs and limitations in VLC, notably with SMB, but it's getting better, since before it did only SMB, not 2/3.

And yes, it's even more complex, because of the multiple discovery methods (Netbios, mdns, DB, manual).

I had never heard of he ?seal parameter either, so the big question is "How do we detect that we need this param?" Is there a mDNS flag to parse to know that it is required?

computator commented 3 years ago

I apologize if I came across the wrong way. I too only took @sahlberg's words as explaining how things work currently. My remarks were in response to the current situation and looking to improve it, not intended as criticism of @sahlberg.

sahlberg commented 3 years ago

I had never heard of he ?seal parameter either, so the big question is "How do we detect that we need this param?" Is there a mDNS flag to parse to know that it is required?

Ideally it should not be needed and should be automatically picked up by libsmb2. Though you can programatically do the same thing as ?sign by calling smb2_set_seal()

But that is not what I wanted to talk about.

I think at least for mobile devices it could well be argued you want to "always use encryption, when available, and only fallback to no-encryption when encryption failed" or at least a tickbox in the UI to "force encryption". To use encryption regardless of whether the server requires it or not.

I am thinking of the use case when using a mobile device on an free/open hotspot at McD/StarBucks/... where you never know who might be running wireshark on a nearby device.

CPUs in modern mobile devices are so powerful today that the encryption overhead should be acceptable.

J0K3R-07 commented 2 years ago

VLC 3.5.1 installed from Google Play Store supports SMB V3 Encryption. I have the below settings on my server SMB Signing - Required SMB 3 Encryption - Required from all clients (others are rejected) VLC working pretty good. Looking for a good file manager for Android that also supports SMB V3 with encryption. Have tested few so far all failed to login.

jbkempf commented 2 years ago

This lib supports will only support SMBv1.