Re: [PATCH] ksmbd: do not sign response to session request for guest login

[Tom Talpey <tom@xxxxxxxxxx>]

On 1/11/2023 11:39 AM, Marios Makassikis wrote:
> If ksmbd.mountd is configured to assign unknown users to the guest account
> ("map to guest = bad user" in the config), ksmbd signs the response.
>
> This is wrong according to MS-SMB2 3.3.5.5.3:
>     12. If the SMB2_SESSION_FLAG_IS_GUEST bit is not set in the SessionFlags
>     field, and Session.IsAnonymous is FALSE, the server MUST sign the
>     final session setup response before sending it to the client, as
>     follows:
>      [...]
>
> This fixes libsmb2 based applications failing to establish a session
> ("Wrong signature in received").

I can definitely see why! The reason for requiring the SESSION_IS_GUEST
flag to be false is because with guest there's no secret, and therefore
no key to use for signing!

But, I'd expect the code in smb3_set_sign_rsp to determine this. Before
signing it checks the following:

	if (conn->binding == false &&
	    le16_to_cpu(hdr->Command) == SMB2_SESSION_SETUP_HE) {
		signing_key = work->sess->smb3signingkey;
	} else {
		read_lock(&work->sess->chann_lock);
		chann = lookup_chann_list(work->sess, work->conn);
		if (!chann) {
			read_unlock(&work->sess->chann_lock);
			return;
		}
		signing_key = chann->smb3signingkey;
		read_unlock(&work->sess->chann_lock);
	}

	if (!signing_key)
		return;

So, the work->sess->smb3signingkey is obviously non-null.

What value is being set? Addressing that seems to be the more
general and appropriate fix here.

Tom.

> Signed-off-by: Marios Makassikis <mmakassikis@xxxxxxxxxx>
> ---
>   fs/ksmbd/smb2pdu.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 38fbda52e06f..d681f91947d9 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -8663,6 +8663,7 @@ int smb3_decrypt_req(struct ksmbd_work *work)
>   bool smb3_11_final_sess_setup_resp(struct ksmbd_work *work)
>   {
>   	struct ksmbd_conn *conn = work->conn;
> +	struct ksmbd_session *sess = work->sess;
>   	struct smb2_hdr *rsp = smb2_get_msg(work->response_buf);
>   
>   	if (conn->dialect < SMB30_PROT_ID)
> @@ -8672,6 +8673,7 @@ bool smb3_11_final_sess_setup_resp(struct ksmbd_work *work)
>   		rsp = ksmbd_resp_buf_next(work);
>   
>   	if (le16_to_cpu(rsp->Command) == SMB2_SESSION_SETUP_HE &&
> +	    sess->user && !user_guest(sess->user) &&
>   	    rsp->Status == STATUS_SUCCESS)
>   		return true;
>   	return false;