On 9/16/2022 10:07 PM, Enzo Matsumiya wrote:
The signature check will always fail for a response with SMB2
Status == STATUS_END_OF_FILE, so skip the verification of those.
Can you elaborate on this assertion? I don't see this as a protocol
requirement:
3.2.5.1.3 Verifying the Signature
The client MUST skip the processing in this section if any of the
following is TRUE:
- Client implements the SMB 3.x dialect family and decryption in
section 3.2.5.1.1.1 succeeds
- MessageId is 0xFFFFFFFFFFFFFFFF
- Status in the SMB2 header is STATUS_PENDING
[goes on to discuss action if session not found, etc]
Also, in async IO, it doesn't make sense to verify the signature
of an unsuccessful read (rdata->result != 0), as the data is
probably corrupt/inconsistent/incomplete. Verify only the responses
of successful reads.
Same question. Why would we ever want to selectively skip signing
verification? Signing protects against corrupted SMB headers, MITM,
etc etc.
Tom.
Signed-off-by: Enzo Matsumiya <ematsumiya@xxxxxxx>
---
fs/cifs/smb2pdu.c | 4 ++--
fs/cifs/smb2transport.c | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 6352ab32c7e7..9ae25ba909f5 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -4144,8 +4144,8 @@ smb2_readv_callback(struct mid_q_entry *mid)
case MID_RESPONSE_RECEIVED:
credits.value = le16_to_cpu(shdr->CreditRequest);
credits.instance = server->reconnect_instance;
- /* result already set, check signature */
- if (server->sign && !mid->decrypted) {
+ /* check signature only if read was successful */
+ if (server->sign && !mid->decrypted && rdata->result == 0) {
int rc;
rc = smb2_verify_signature(&rqst, server);
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index 1a5fc3314dbf..37c7ed2f1984 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -668,6 +668,7 @@ smb2_verify_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server)
if ((shdr->Command == SMB2_NEGOTIATE) ||
(shdr->Command == SMB2_SESSION_SETUP) ||
(shdr->Command == SMB2_OPLOCK_BREAK) ||
+ (shdr->Status == STATUS_END_OF_FILE) ||
server->ignore_signature ||
(!server->session_estab))
return 0;
