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

[Enzo Matsumiya]

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


Cheers,

Enzo