Re: [PATCH] cifs: verify signature only for valid responses

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/17/2022 12:28 PM, Enzo Matsumiya wrote:
On 09/17, Tom Talpey wrote:
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]

Yeah I didn't find anything in the spec either. I woke up this morning
thinking about this actually, and it might actually be a miscalculation
on our side. My initial assumption, and debugging target now, is the
1-byte cropping done on some odd-sized structs, but I haven't deepened
on that so far.

I'll reply back with my findings later.

Good, because there are definitely some tricky rules regarding what
parts of the payload are included in the signing. Padding, especially,
is easy to get wrong.

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.

The problem here is actually different because rdata->result can
contain an internal (kernel) error code when an underlying problem
occurred (think EIO, EINTR, ECONNABORTED (not sure if possible this one),
ENOMEM maybe?). But in between "mid set with MID_RESPONSE_RECEIVED state"
and "verify the signature", the SMB2 header/message itself might be
correct/valid, but our internal processing failed somewhere, so, the

Wait, we process the message *before* we check the signature??? Apart
from inspecting the MID and verifying it's a response to a request we
made, there isn't a lot to cause such an error. See 3.2.5.1.3.

Also, if we process a bogus response, or drop a valid one, that's a
seriously important issue. It's not a server/protocol/network error
but we trashed the operation!

Not quoting the ENOCOFFEE part of the rest of your message. :)

Tom.



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux