Re: [PATCH] smb: client: fail SMB2 responses with a bad signature

[Enzo Matsumiya <ematsumiya@suse.de>]

On 11/13, Wong Steve wrote:
>On 11/13, Enzo Matsumiya wrote:
>> However, I still think the error should only be returned if signing is
>> required (*) AND rc == -EACCES.
>>
>> Any other error should always be returned unconditionally.
>>
>> (*) Note that "signing required" at that point means that either/both
>> client and server required signing, and the other side agreed -- it
>> doesn't really matter who asked for it.
>
>Thanks for looking at it.
>
>I think the existing server->sign check already covers that.
>cifs_enable_signing() only sets server->sign when the server requires
>signing and the client allows it, or when the client requires it (mount
>option or SecurityFlags) and the server supports it. Both sides merely
>supporting signing doesn't turn it on.
>
>smb2_verify_signature() is only called under that check, so the new
>return passes on -EACCES for a mismatch, and any other error from it
>(e.g. from smb3_calc_signature()) as well.
>
>Is there a case I'm missing where server->sign is set but signing isn't
>required in your sense?

Not really, you're 100% correct, sorry.

I just remembered that there's a very specific exception to that, and it's
currently not handled;  SMB 3.1.1 tcon requests (!anonymous && !guest) are
signed unconditionally, but the responses are never verified because
they're gated by server->sign.

While your patch fix this indirectly, I couldn't find in the spec
whether such tcon responses should also fail if signing isn't required.

Anyway, I think this can be discussed later in a follow up patch.

@Paulo, @Namjae:
Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>


Cheers,

Enzo