I think I am experiencing a bug in the implementation of SMB2 keepalives. As
libsmbclient does not set the session for the echo request, the server used in
my environment responds with STATUS_USER_SESSION_DELETED, which is the
expected behavior (see [1,2]). As the server response indicates an
error, the client disconnects. Ultimately, this leads to a new
connection for every operation, which introduces painful delays.
I first experienced this behavior using libsmbclient 4.7.0 on a Fedora Linux 27. It looks like older versions would behave the same, but 4.7 was the first using SMB2 in my setup.
Unfortunately, I am not able to share PCAPs publicly.
SMB2 does not have keep avlives at all. We still have a very old but I think that we send SMBv1 keep alives on port 445, where keep alive pacakges from smbd to a windows client lead to connection terminations. We should probably fix that one also one day.
You could send a pcap from a test system, won't you? Without a pcap we can only make guesses what kind of issue you actually see in your case.
the SMBv1 keepalive bug is https://bugzilla.samba.org/show_bug.cgi?id=6558
OK, I've followed up on this. smbd as a server doesn't require a valid sessionid to process an incoming keepalive packet, which is why the libsmbclient code doesn't bother to send one.
What server are you talking to ?
Samba torture test source4/torture/smb2/session.c:test_session_expire2i()
shows that a sessionid is not required to process an SMB2_ECHO (called SMB2_OP_KEEPALIVE in the samba code).
There's a simple fix for this I can add to libsmbclient, but I think your server shouldn't be returning STATUS_USER_SESSION_DELETED for an SMB2 echo request.
Can you run all the Samba smb2.session.expire tests against your server ?
Remember these tests (smb2.session.expire) pass against Windows, which show that SMB2_ECHO requests don't require a valid sessionid, unless the transport is encrypted I think.
Created attachment 15676 [details]
git-am fix for master.
Can you test this fix against your server and let me know if it fixes the problem ?
Created attachment 15677 [details]
Corrected version that doesn't change the error return from 1 -> -1.
(In reply to Jeremy Allison from comment #7)
It will take me some time to do the testing. Thanks a lot for having a look!
Jan: what were the results of your tests with the patch ?
I am very sorry but I haven't found the time to test the patch in the environment in which I could verify it works as intended.