hierynomus / smbj

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

Non-encrypted case should be negotiated consistently when using dialect 3.1.1 #747

Closed turcsanyip closed 1 year ago

turcsanyip commented 1 year ago

SMBJ 0.11.x introduced SMB 3.x support with (optionally enabled) encryption feature, configurable via SmbConfig.Builder.withEncryptData(boolean).

When using 3.0 dialect, the encryption is negotiated properly with the server:

So the library uses encryption only if both sides claimed that they support encryption during the protocol negotiation, which I believe the proper behaviour.

In case of 3.1.1 dialect, the library decides based on the servers response only and does not take into account whether the encryption was set via withEncryptData(true) and the capability was sent to the server, or not.

It works for the withEncryptData(true) case but leads to inconsistent behaviour with some servers when withEncryptData(false) (and therefore no encryption capability was sent from the client side):

The first 2 cases are just inconsistent but the 3rd one is a concrete failure in the communication between the client and the server.

For these reasons, I would suggest taking into account also the client side encryption setting (not just the server's response) when encryption is determined in case of 3.1.1 (similar to 3.0): ConnectionContext. supportsEncryption()

    public boolean supportsEncryption() {
        SMB2Dialect dialect = negotiatedProtocol.getDialect();
        if (dialect == SMB2Dialect.SMB_3_1_1) {
            return cipherId != null; // clientCapabilities.contains(SMB2GlobalCapability.SMB2_GLOBAL_CAP_ENCRYPTION) should be checked here too
        } else {
            return clientCapabilities.contains(SMB2GlobalCapability.SMB2_GLOBAL_CAP_ENCRYPTION)
                && supports(SMB2GlobalCapability.SMB2_GLOBAL_CAP_ENCRYPTION);
        }
    }

Error stack trace (no useful info related to the root cause though):

12:02:09.704 [main] DEBUG com.hierynomus.smbj.connection.Connection - Granted 1 (out of 128) credits to Encrypted[SMB2_CREATE with message id << 4 >>]
12:02:09.704 [main] DEBUG com.hierynomus.smbj.transport.tcp.direct.DirectTcpTransport - Writing packet Encrypted[SMB2_CREATE with message id << 4 >>]
12:02:09.705 [main] DEBUG com.hierynomus.protocol.commons.concurrent.Promise - Awaiting << 4 >>
12:02:09.813 [Packet Reader for localhost] DEBUG com.hierynomus.smbj.transport.tcp.direct.DirectTcpPacketReader - Received packet Encrypted for session id << -7976719365003134876 >>
12:02:09.813 [Packet Reader for localhost] DEBUG com.hierynomus.smbj.connection.packet.SMB3DecryptingPacketHandler - Decrypting packet Encrypted for session id << -7976719365003134876 >>
12:02:09.814 [Packet Reader for localhost] DEBUG com.hierynomus.smbj.connection.packet.SMB3DecryptingPacketHandler - Decrypted packet Encrypted for session id << -7976719365003134876 >> is packet SMB2_CREATE with message id << 4 >>.
12:02:09.814 [Packet Reader for localhost] DEBUG com.hierynomus.smbj.connection.packet.SMB2SignatureVerificationPacketHandler - Passthrough Signature Verification as packet is decrypted
12:02:09.814 [Packet Reader for localhost] DEBUG com.hierynomus.smbj.connection.packet.SMB2CreditGrantingPacketHandler - Server granted us 1 credits for SMB2_CREATE with message id << 4 >>, now available: 128 credits
12:02:09.814 [Packet Reader for localhost] DEBUG com.hierynomus.protocol.commons.concurrent.Promise - Setting << 4 >> to `SMB2_CREATE with message id << 4 >>`
Exception in thread "main" com.hierynomus.mssmb2.SMBApiException: STATUS_ACCESS_DENIED (0xc0000022): Create failed for \\localhost\netapp-vol2
    at com.hierynomus.smbj.share.Share.receive(Share.java:380)
    at com.hierynomus.smbj.share.Share.sendReceive(Share.java:359)
    at com.hierynomus.smbj.share.Share.createFile(Share.java:156)
    at com.hierynomus.smbj.share.DiskShare.createFileAndResolve(DiskShare.java:75)
    at com.hierynomus.smbj.share.DiskShare.access$100(DiskShare.java:55)
    at com.hierynomus.smbj.share.DiskShare$2.apply(DiskShare.java:109)
    at com.hierynomus.smbj.share.DiskShare$2.apply(DiskShare.java:105)
    at com.hierynomus.smbj.paths.PathResolver$1.resolve(PathResolver.java:32)
    at com.hierynomus.smbj.paths.SymlinkPathResolver.resolve(SymlinkPathResolver.java:62)
    at com.hierynomus.smbj.share.DiskShare.resolveAndCreateFile(DiskShare.java:105)
    at com.hierynomus.smbj.share.DiskShare.open(DiskShare.java:65)
    at com.hierynomus.smbj.share.DiskShare.openDirectory(DiskShare.java:151)
    at ...
hierynomus commented 1 year ago

Unfortunately it seems that NetApp is inconsistent in the implementation of the MS-SMB2 spec. The reason it's implemented as it is now stems from these lines in the specification for the 3.1.1 dialect:

Processing the SMB2_ENCRYPTION_CAPABILITIES negotiate context

The client MUST return an error to the calling application in the following cases:

The DataLength of the negotiate context is less than the size of SMB2_ENCRYPTION_CAPABILITIES structure.

CipherCount is not 1.

Ciphers[0] is not 0 and not one of the ciphers that the client specified in its negotiate request.

The client MUST set Connection.CipherId to Ciphers[0] and if Server.CipherId is empty, set Server.CipherId to Ciphers[0].

If Connection.CipherId is nonzero, the client MUST set Connection.SupportsEncryption to TRUE. Otherwise, it MUST be set to FALSE.

Source: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/3b29f3af-86f9-4962-8cf3-43471cb59363

So, though I understand the confusion, this is actually pretty well specced out, and is in line with how it should behave.

I think the error here is on the server side, as that specifies the following behaviour:

If the server received an SMB2_ENCRYPTION_CAPABILITIES negotiate context in the client's negotiate request, the server MUST add an SMB2_ENCRYPTION_CAPABILITIES negotiate context to the response's NegotiateContextList. Note that the server MUST send an SMB2_ENCRYPTION_CAPABILITIES context even if the client and server have no common cipher. This is done so that the client can differentiate between a server that does not support encryption (no SMB2_ENCRYPTION_CAPABILITIES context in the response's NegotiateContextList) and a server that supports encryption but does not share a cipher with the client (an SMB2_ENCRYPTION_CAPABILITIES context in the response's NegotiateContextList that indicates a cipher of 0).

SMBJ will correctly interprete a 0 CipherId, however it seems that NetApp does not send a 0, but rather either 1 or 2. Can you check which Cipher IDs are sent? Samba also seems to misbehave in this sense, as the spec says they should, but luckily we just don't need to distinguish, so their implementation works.

turcsanyip commented 1 year ago

@hierynomus Thanks for your detailed answer and also for the pointers to the related sections of the SMB specification (I read it carefully). Now I understand that in case of 3.1.1, the encryption negotiation is based on the SMB2_ENCRYPTION_CAPABILITIES negotiate context solely. Earlier it was not clear for me if SMB2_GLOBAL_CAP_ENCRYPTION capability also matters or not when 3.1.1 is used (so it should not). Maybe Samba and NetApp implement the protocol improperly due to the same misunderstanding. So it is fine, thanks for the clarification.

However, I still have a question regarding smbj: Why does the library always send the SMB2_ENCRYPTION_CAPABILITIES negotiate context and this way turns encryption on (supposed the server also supports it), even when withEncryptData = false? I think the proper behaviour would be to send SMB2_ENCRYPTION_CAPABILITIES negotiate context only if withEncryptData has been set to true by the app using the library (that is the app is requesting encryption by setting the flag on the api).

hierynomus commented 1 year ago

Let me check, I don't think we should indeed if that's the case.

hierynomus commented 1 year ago

@turcsanyip Finally had some time to do some work on the lib again! Could you check whether the version from branch #752 works correctly for you?

hannosgit commented 1 year ago

@hierynomus The fix from your branch seems to be working. I tested it with an on-premise NetApp. I am using following SMB config:

private static final SmbConfig cfg = SmbConfig
        .builder()
        .withDialects(SMB2Dialect.SMB_3_1_1)
        .withEncryptData(false)
        .build();

With the current main branch I get:

Exception in thread "main" com.hierynomus.mssmb2.SMBApiException: STATUS_ACCESS_DENIED (0xc0000022): Create failed for \\10.180.1.10\shr\.
    at com.hierynomus.smbj.share.Share.receive(Share.java:380)
    at com.hierynomus.smbj.share.Share.sendReceive(Share.java:359)
    at com.hierynomus.smbj.share.Share.createFile(Share.java:156)
    at com.hierynomus.smbj.share.DiskShare.createFileAndResolve(DiskShare.java:75)
    at com.hierynomus.smbj.share.DiskShare.access$100(DiskShare.java:55)
    at com.hierynomus.smbj.share.DiskShare$2.apply(DiskShare.java:109)

But on the feature branch I do not get any exceptions and connecting is successful. The log shows that a null cipher is used for encryption.

08:26:47.783 [main] INFO com.hierynomus.smbj.connection.PacketEncryptor - Initialized PacketEncryptor with Cipher << null >>
08:26:47.791 [main] INFO com.hierynomus.smbj.connection.Connection - Successfully connected to: 10.180.1.10
turcsanyip commented 1 year ago

Thanks @hierynomus !

SaurabhNITK1 commented 1 month ago

@hierynomus @turcsanyip

If we enable encryption on the SMBJ client side by setting .withEncryptData(true). At the same time on the server side executing "Get-SmbServerConfiguration" tells that EncryptData : False means server has not enabled encryption. In this scenario, will the packets be encrypted? If not will the transfer happen without encryption or will it fail ?

I tried this configuration and while debugging I found out that during client - server negotiation, even if the server side encryption is off it was sending the encryption key and during the negotiation client -server were successfully able to negotiate encryption. I want to know will this be true for all use cases if the server supports SMB3.0 ?