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

[turcsanyip]

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:

• if withEncryptData is set to true, the library sends SMB2_GLOBAL_CAP_ENCRYPTION capability in the negotiate request to the server, and if the server also supports encryption (answers with the same capability), the client will use encryption
• if withEncryptData is set to false (default), the library does not send the encryption capability and will not use encryption (regardless of the server's response)

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):

• Samba running on Ubuntu 20.04 does not send back encryption info in this case and smbj will not use encryption
• Windows Server 2002 sends back the encryption info, and based on this smbj will turn on encryption (and the communication will be successful)
• NetApp Files sends back the encryption info, and based on this smbj will turn on encryption but the communication will fail (it seems NetApp Files does not tolerate that the client does not claim encryption capability but then tries to use encryption, which sounds reasonable from the server's perspective)

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]

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]

@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]

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

[hierynomus]

@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]

@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]

Thanks @hierynomus !