search

hierynomus / smbj

Server Message Block (SMB2, SMB3) implementation in Java
Other
713 stars 180 forks source link

SMBRuntimeException: Unknown SMB2 Message Command type: SMB2_CANCEL #572

Closed zhanghai closed 4 years ago

zhanghai commented 4 years ago

This happens every time if I call Future.cancel() for the future returned for SMB2_CHANGE_NOTIFY response, causing the client to send a SMB2_CANCEL request to server and server somehow responds with a SMB2_CANCEL message. If I comment out Future.cancel(), the crash never happens and SMB2_CHANGE_NOTIFY does work, just that it can't be cancelled once user navigates away. The server I'm testing against is Windows 10's built-in file sharing.

I'm not sure the response is for the cancelled request, which according to the spec should fail with a status of STATUS_CANCELLED instead of a message of SMB2_CANCEL, or the cancel request itself, which according to the spec doesn't seem to have a response at all.

Stacktrace:

E/AndroidRuntime: FATAL EXCEPTION: Packet Reader for x.x.x.x
    Process: x.x.x.x, PID: xxxxx
    com.hierynomus.smbj.common.SMBRuntimeException: Unknown SMB2 Message Command type: SMB2_CANCEL
        at com.hierynomus.mssmb2.SMB2MessageConverter.getPacketInstance(SMB2MessageConverter.java:71)
        at com.hierynomus.mssmb2.SMB2MessageConverter.readPacket(SMB2MessageConverter.java:77)
        at com.hierynomus.smbj.connection.Connection.handle(Connection.java:423)
        at com.hierynomus.smbj.connection.Connection.handle(Connection.java:72)
        at com.hierynomus.smbj.transport.PacketReader.readPacket(PacketReader.java:72)
        at com.hierynomus.smbj.transport.PacketReader.run(PacketReader.java:48)
        at java.lang.Thread.run(Thread.java:764)

Note that I had to create my own SMB2ChangeNotifyRequest because com.hierynomus.mssmb2.messages.SMB2ChangeNotifyRequest isn't an SMB2MultiCreditPacket and server closes the connection upon receive such a packet.

zhanghai commented 4 years ago

My implementation for SMB2_CHANGE_NOTIFY is at zhanghai/MaterialFiles/app/src/main/java/me/zhanghai/android/files/provider/smb/client/DirectoryExtensions.kt.

hierynomus commented 4 years ago

It is pretty weird, indeed there is no SMB2_CANCEL response packet expected according to the spec.

However according to the Samba mailing list, there is one caveat (https://lists.samba.org/archive/cifs-protocol/2013-December/002521.html):

Actually, that is incorrect, but only for a very unlikely condition. The SMB2_CANCEL command can generate a server response if the SMB2 request itself is invalid or cannot be processed by the server. For example, if the request were improperly constructed (3.3.5.2.6), or if signing failed (3.3.5.2.4), etc. In such cases, standard server error response processing will happen regardless of the actual request in the payload. Of course, these would be from broken clients, but the responses are valid protocol.

hierynomus commented 4 years ago

Can you check whether PR #573 fixes this issue?

hierynomus commented 4 years ago

@zhanghai Does the PR fix your issue?

zhanghai commented 4 years ago

Thank you for creating the PR! I didn't have the time to locally compile your changes and link it with my app to test, but I believe it will likely suppress the error.

The SMB2_CANCEL command can generate a server response if the SMB2 request itself is invalid or cannot be processed by the server.

However according to this statement, it seems the real bug isn't fixed with #573. Something is likely wrong with the async command cancel request so that the server responded with SMB2_CANCEL. What do you think?

I'll reply again when I have the time to locally build this library with your changes, and maybe send my SMB2_NOTIFY imolememtation as a PR as well.

hierynomus commented 4 years ago

Correct, there is probably still something wrong with cancel. But now it will be reported correctly.

Op wo 5 aug. 2020 08:43 schreef Hai Zhang notifications@github.com:

Thank you for creating the PR! I didn't have the time to locally compile your changes and link it with my app to test, but I believe it will likely suppress the error.

The SMB2_CANCEL command can generate a server response if the SMB2 request itself is invalid or cannot be processed by the server.

However according to this statement, it seems the real bug isn't fixed with #573 https://github.com/hierynomus/smbj/pull/573. Something is likely wrong with the async command cancel request so that the server responded with SMB2_CANCEL. What do you think?

I'll reply again when I have the time to locally build this library with your changes, and maybe send my SMB2_NOTIFY imolememtation as a PR as well.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/hierynomus/smbj/issues/572#issuecomment-669011979, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA4XIYDLOAM3CHQZON2LYTR7D5PLANCNFSM4PHVZD5Q .

zhanghai commented 4 years ago

I've sent my CHANGE_NOTIFY implementation as a PR #575, and will start testing a local build with your fix #573.

zhanghai commented 4 years ago

I tested a local build of SMBJ with #573 and #575. If I call future.cancel() for Future<SMB2ChangeNotifyResponse>, I got the following error and session was logged off. If I don't call future.cancel(), my PR #575 alone works fine, just that the request is left dangling.

2020-08-08 16:13:47.072 9910-9982/me.zhanghai.android.files I/PacketReader: PacketReader error, got exception.
    com.hierynomus.protocol.transport.TransportException: Received response with unknown sequence number <<61>>
        at com.hierynomus.smbj.connection.Connection.handle(Connection.java:398)
        at com.hierynomus.smbj.connection.Connection.handle(Connection.java:72)
        at com.hierynomus.smbj.transport.PacketReader.readPacket(PacketReader.java:72)
        at com.hierynomus.smbj.transport.PacketReader.run(PacketReader.java:48)
        at java.lang.Thread.run(Thread.java:764)
2020-08-08 16:13:47.073 9910-9982/me.zhanghai.android.files I/c*.h*.s*.s*.Session: Logging off session 989560464998405 from host 192.168.1.106
2020-08-08 16:13:47.074 9910-9966/me.zhanghai.android.files E/c*.h*.p*.c*.c*.Promise: << 97 >> woke to: {}
    com.hierynomus.smbj.common.SMBRuntimeException: com.hierynomus.protocol.transport.TransportException: Received response with unknown sequence number <<61>>
        at com.hierynomus.smbj.common.SMBRuntimeException$1.wrap(SMBRuntimeException.java:27)
        at com.hierynomus.smbj.common.SMBRuntimeException$1.wrap(SMBRuntimeException.java:21)
        at com.hierynomus.protocol.commons.concurrent.Promise.deliverError(Promise.java:95)
        at com.hierynomus.smbj.connection.OutstandingRequests.handleError(OutstandingRequests.java:88)
        at com.hierynomus.smbj.connection.Connection.handleError(Connection.java:465)
        at com.hierynomus.smbj.transport.PacketReader.run(PacketReader.java:54)
        at java.lang.Thread.run(Thread.java:764)
     Caused by: com.hierynomus.protocol.transport.TransportException: Received response with unknown sequence number <<61>>
        at com.hierynomus.smbj.connection.Connection.handle(Connection.java:398)
        at com.hierynomus.smbj.connection.Connection.handle(Connection.java:72)
        at com.hierynomus.smbj.transport.PacketReader.readPacket(PacketReader.java:72)
        at com.hierynomus.smbj.transport.PacketReader.run(PacketReader.java:48)
        at java.lang.Thread.run(Thread.java:764) 
2020-08-08 16:13:47.074 9910-9969/me.zhanghai.android.files E/c*.h*.p*.c*.c*.Promise: << 95 >> woke to: {}
    com.hierynomus.smbj.common.SMBRuntimeException: com.hierynomus.protocol.transport.TransportException: Received response with unknown sequence number <<61>>
        at com.hierynomus.smbj.common.SMBRuntimeException$1.wrap(SMBRuntimeException.java:27)
        at com.hierynomus.smbj.common.SMBRuntimeException$1.wrap(SMBRuntimeException.java:21)
        at com.hierynomus.protocol.commons.concurrent.Promise.deliverError(Promise.java:95)
        at com.hierynomus.smbj.connection.OutstandingRequests.handleError(OutstandingRequests.java:88)
        at com.hierynomus.smbj.connection.Connection.handleError(Connection.java:465)
        at com.hierynomus.smbj.transport.PacketReader.run(PacketReader.java:54)
        at java.lang.Thread.run(Thread.java:764)
     Caused by: com.hierynomus.protocol.transport.TransportException: Received response with unknown sequence number <<61>>
        at com.hierynomus.smbj.connection.Connection.handle(Connection.java:398)
        at com.hierynomus.smbj.connection.Connection.handle(Connection.java:72)
        at com.hierynomus.smbj.transport.PacketReader.readPacket(PacketReader.java:72)
        at com.hierynomus.smbj.transport.PacketReader.run(PacketReader.java:48)
        at java.lang.Thread.run(Thread.java:764) 
hierynomus commented 4 years ago

@zhanghai Can you check again? I've fixed the error in the SMB2_CANCEL