D16299: RFC fallback to dnssd service discovery if smb listDir failed on root

[Harald Sitter]

sitter created this revision.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
This elevates a problem with newer SMB protocol versions and smbclient
not supporting discovery/browsing. when not using older (as in:
ancient) protocol versions discovery doesn't work and smb:/ gives no
results.

By falling back to DNSSD based discovery we can ensure discovery of DNSSD
remotes (namely linux and osx) is always working. Windows unfortunately
does not support DNSSD and as such will not benefit from this mode of
discovery and continue to be unlisted when using a protocol version
without browsing support.

CCBUG: 392447
CCBUG: 390551

TEST PLAN
smb.conf:

[global]
client min protocol = SMB2

Lists devices

REPOSITORY
R320 KIO Extras

BRANCH
smb-discovery

REVISION DETAIL
https://phabricator.kde.org/D16299

AFFECTED FILES
smb/CMakeLists.txt
smb/kio_smb.h
smb/kio_smb_browse.cpp

To: sitter
Cc: kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp

[Nathaniel Graham]

ngraham added reviewers: Frameworks, Dolphin.
ngraham added a comment.


Wow, awesome work. What more is needed to support Windows? Support for the `WS-Discovery` protocol?

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: ngraham, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp

[Harald Sitter]

sitter added a comment.


This diff does somewhat depend on D16298 <https://phabricator.kde.org/D16298> as currently KDNSSD can easily lock indefinitely on resolving services. We could kinda bypass this with a timeout on service resolution, that would however have the disadvantage of either having to repeat the resolution or not showing the device. Both of which suck a bit. My plan is to simply have the fallback only run with KDNSSD 5.52 (assuming that gets the fix landed).
Besides that this is working nicely for me.
I would love to have a way to actually get the workgroup from a share, but from some research that seems simply unobtainable (via smbclient anyway) unless SMB1 is used (as in: if you use the smbclient CLI it would transparently drop down to SMB1 for the workgroup query but even that doesn't work if the minimum version is set >=SMB2). So we could drop to SMB1, but honestly I fail to appreciate the use in that in particular since the maintainer of SMB within Microsoft is very insistent on people not using a 30 year old insecure protocol anymore (rightfully, I should add ;))

Post by Nathaniel Graham
Wow, awesome work. What more is needed to support Windows? Support for the `WS-Discovery` protocol?

Yes. I am not sure we have a production quality lib for that anywhere though. I haven't had much of a look since it's tricky to try in my network. There is however a good chance windows 10 already does, or possibly will at some point in the future, embrace dnssd like the rest of the world https://channel9.msdn.com/Events/Build/2015/3-79

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: ngraham, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp

[Nathaniel Graham]

ngraham added a comment.


FWIW, in practice the workgroup is much less prominent to even unnecessary in a macOS or Linux shared file environment. I've only ever seen shares on multiple workgroups used in a Windows environment.

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: ngraham, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp

[Harald Sitter]

sitter added a comment.


Done some research on ws-discovery. gsoap is the only free c implementation I could find. There are some python, js and java implementations but they'll probably not help us nearly as much (unless someone wants to write a daemon or helper binary to bridge the language divide without much hassle). Alternative approach would be writing our own implementation using KDAB's kdsoap library, but then that'd need a maintainer which I am confident we don't have :)

I have no idea how stable or lovely gsoap is to use though. It has only a hand full of users (as in: software using it) that is packaged in debian and the way gsoap is packaged is using an aggressively versioned soname in debian (which is often an indication of either poor soname maintenance or super lax API stability policy; both of which are not very good). So I am not too confident here.

What I'd really like to know is if windows 10 actually has dnssd nowadays, and if it is enabled by default, and whether smb is published if/when dnssd is enabled.

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: ngraham, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp

[Nathaniel Graham]

ngraham added a comment.

Post by Harald Sitter
What I'd really like to know is if windows 10 actually has dnssd nowadays, and if it is enabled by default, and whether smb is published if/when dnssd is enabled.

https://www.ctrl.blog/entry/windows-mdns-dnssd and https://stackoverflow.com/a/30769404/2934226seem to indicate that DNS-SD is used for discovery in Windows 10.

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: ngraham, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp

[Harald Sitter]

sitter added a comment.


It's available as an API, but it is not used to publish **shares** it seems. In fact, if I am reading Microsoft's arcticle <https://support.microsoft.com/en-us/help/4034314/smbv1-is-not-installed-by-default-in-windows> on smb1 correctly then ws-discovery is also not enabled by default. Finding other shares in the network seems simply not supported out of the box. I've gotten a device which has an up to date windows 10, albeit installed before the 2017 Fall update, so it has smb1 still enabled. As expected the discovery service also mentioned in the article isn't started by default. Starting it gives me some ws-discovery chatter on the network.

I've also tried to get gsoap to work but I feel somewhat defeated. It's sending out a probe event but gets nothing back, getting it to even build properly is also fairly mental because it expects symbols to be visible. It all feels fairly terrible. While I eventually got some Probe events multicasted, the windows machine then proceeded to not reply to those and instead kept spouting Resolve requests which AFAIK don't need handling by clients at all. I'm probably gonna download a development VM to stand a better chance at debugging this, but overall ws-discovery supports seems incredibly unreasonable to add at this time.

I've also tried kdsoap and have it auto generate a proxy class from the ws-discovery wsdl, but somehow the generator didn't like the fact that the wsdl didn't define any bindings and decided to qAbort instead of generating anything...

So, all in all a big fat waste of time. Assuming Windows 10 indeed doesn't have ws-discovery enabled by default I really really think everyone would be better off if we got Microsoft to either add a DNSSD discovery service OR we could just do something like that ourselves that the user can download. In the end the user would have to manually do something if they want to discover Windows 10 hosts it seems.

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: ngraham, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp

[Ben Cooksley]

bcooksley added a comment.


It might be worth getting in touch with the Samba developers, as I believe they've worked with the people at Microsoft who look after the SMB parts of Windows - and they'd know best what is done in regards to discovery of shares these days.
Turning to Google though, it looks like this is simply broken in current versions of Windows 10 (by default at least) - https://answers.microsoft.com/en-us/windows/forum/windows_10-files/homegroup-removed-how-to-get-network-sharing-work/01277332-2916-4a68-853a-116696b20743

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: bcooksley, ngraham, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp

[Harald Sitter]

sitter added a comment.


Yep, so, by default Windows 10 does no service publication. The only way to make a host discoverable is by manually starting a publication service (which the user could switch to autostart, so it starts automatically on subsequent boots). With that service Windows will advertise itself over ws-discovery. ws-discovery as far as I understand is only solving the finding device problem, not finding a service, nor name resolution (all of which dnssd can do). Iff we were to add ws-discovery support we'd still need to smb-poke found devices to actually determine if they are smb hosts and also obtain a name from smb_* or netbios or something (I am not sure about speed implications of either).

As for actually supporting ws-discovery. I am really not fond of it and would possibly just ignore it given it doesn't work out of the box anyway.

I've eventually gotten gsoap's wsd support <https://www.genivia.com/doc/wsdd/html/wsdd_0.html> to "work" in a sample program by randomly switching sockets *shrug*.

soap->master = soap->socket;
soap_wsdd_listen(soap, 5);

But there's more! gsoap's facilities for changing network interfaces are compile-time ipv4-only or ipv6-only ifdef'd which makes for weird scenarios where you want to probe on ethernet and wifi but you can potentially not switch to one or the other because of how gsoap was built (it is as strange as it sounds).

All in all I continue to feel meh about ws-discovery support and think lobbying for a dnssd based solution is way more useful in the long run.

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: bcooksley, ngraham, kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp

[Nathaniel Graham]

ngraham added a comment.


OK, I guess that's the best we can do for now. Can we push forward on this?

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: bcooksley, ngraham, kde-frameworks-devel, kfm-devel, alexde, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

[Harald Sitter]

sitter added a comment.


It's on my todo, unfortunately I am at a sprint this week so probably nothing much happening on this until next week.

Meanwhile I'd still like some comments on the presented diff (:

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: alexde, bcooksley, ngraham, kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

[Nathaniel Graham]

ngraham added a comment.


Is this logic correct? Just because we found any entries, we don't use DNSSD? Is there ever a case where we found some entries but might find even more by using DNSSD?

Also, I want to compliment your code. It's so nice and clean and well commented.

INLINE COMMENTS

kio_smb.h:282
bool workaroundEEXIST(const int errNum) const;
+ void listDNNSD(UDSEntry &udsentry, const QUrl &url, const uint direntCount);

Shouldn't this be `listDNSSD`?

kio_smb_browse.cpp:459
+ listDNNSD(udsentry, url, direntCount);
+

Shouldn't this be `listDNSSD`?

kio_smb_browse.cpp:504
+void SMBSlave::listDNNSD(UDSEntry &udsentry, const QUrl &url, const uint direntCount)
+{

Shouldn't this be `listDNSSD`?

kio_smb_browse.cpp:508
+
+ // This entire method act as fallback logic iff SMB discovery is not working
+ // (for example when using a protocol version that doesn't have discovery).

iff -> if

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: alexde, bcooksley, ngraham, kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

[Harald Sitter]

sitter updated this revision to Diff 45938.
sitter marked 4 inline comments as done.
sitter added a comment.


- check kdnssd version; do not use dnssd discovery if it too old and suffers from signal racing (also see D16298 <https://phabricator.kde.org/D16298>)
- fix typo in function name
- no longer use debug names for udsentry
- convert resolution from fixme to todo, I am unsure if we should even care though. input welcome
- add note about lack of workgroup resolution, it's also unclear if workgroup resolution in its current form has any future. specifically LLMNR (the future replacement of NBS) does not have a concept of SMB workgroups, so the way I see it workgroup discovery is a dead-end anyway. in the future the only way to inspect workgroups would be to actually talk to the SMB host (which may require auth) and as such is impossible to do consistently AFAICT

REPOSITORY
R320 KIO Extras

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D16299?vs=43869&id=45938

BRANCH
smb-discovery

REVISION DETAIL
https://phabricator.kde.org/D16299

AFFECTED FILES
CMakeLists.txt
config-runtime.h.cmake
smb/CMakeLists.txt
smb/kio_smb.h
smb/kio_smb_browse.cpp

To: sitter, #frameworks, #dolphin
Cc: alexde, bcooksley, ngraham, kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

ngraham wrote in kio_smb_browse.cpp:508
iff -> if

https://en.wikipedia.org/wiki/If_and_only_if

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: alexde, bcooksley, ngraham, kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

[Harald Sitter]

sitter retitled this revision from "RFC fallback to dnssd service discovery if smb listDir failed on root" to "fallback to dnssd service discovery if smb listDir failed on root".

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: alexde, bcooksley, ngraham, kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

[Harald Sitter]

sitter edited the summary of this revision.

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: alexde, bcooksley, ngraham, kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

[Harald Sitter]

sitter added a comment.


// TODO: it may be better to resolve the host to an ip address. dnssd
// being able to find a service doesn't mean name resolution is
// properly set up for its domain. So, we may not be able to resolve
// this without help from avahi. OTOH KDNSSD doesn't have API for this
// and from a platform POV we should probably assume that if avahi
// is functional it is also set up as resolution provider.
// Given the plugin design on glibc's libnss however I am not sure
// that assumption will be true all the time. ~sitter, 2018

If someone has a thought to offer on this please do.
Personally I am somewhat leaning towards assuming that systems should be properly configured. The problem with setting the URL to the ip address is that it messes with the visual look in the address bar. So, if we want reliable resolution and good looks we'd need to resolve each dnssd address (e.g. `foobar.local`) to their ip address (e.g. 172.17.0.1) via kdnssd and then resolve that back to a hostname via glibc/qhostinfo (e.g. `foobar.sky.net`). That way we'd know that the name we have in the end is in fact resolvable by the system without aid from dnssd. Speed impact of this aside it also adds 3 additional points of failure (and KDNSSD currently doesn't implement the resolver API it seems).

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: alexde, bcooksley, ngraham, kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

[Andrew Crouthamel]

acrouthamel added a comment.

Post by Harald Sitter
If someone has a thought to offer on this please do.
Personally I am somewhat leaning towards assuming that systems should be properly configured. The problem with setting the URL to the ip address is that it messes with the visual look in the address bar. So, if we want reliable resolution and good looks we'd need to resolve each dnssd address (e.g. `foobar.local`) to their ip address (e.g. 172.17.0.1) via kdnssd and then resolve that back to a hostname via glibc/qhostinfo (e.g. `foobar.sky.net`). That way we'd know that the name we have in the end is in fact resolvable by the system without aid from dnssd. Speed impact of this aside it also adds 3 additional points of failure (and KDNSSD currently doesn't implement the resolver API it seems).

I would recommend assuming DNS and systems are configured properly. I say that as someone with ~15 years IT experience. Modern IT networks are so heavily reliant on it (ever set up a VMware cluster? phew!), almost nothing works if it is not set up properly.

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: acrouthamel, alexde, bcooksley, ngraham, kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

sitter wrote in kio_smb_browse.cpp:508
https://en.wikipedia.org/wiki/If_and_only_if

How about writing that out in English, then? :) "iff" just looks like a typo in this context.

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D16299

To: sitter, #frameworks, #dolphin
Cc: acrouthamel, alexde, bcooksley, ngraham, kde-frameworks-devel, kfm-devel, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov