I added the commit tag and cleaned up the related checkpatch warning.
I don't know of any other locations where we have to make a similar
change as this is the only place where we (on the client) forced
signing to be on for a request when the user (or server) didn't
request it (due to the requirement in MS-SMB2 that tree connect be
signed). In the other places you noted, the client or server would
have forced signing on ("sign" mount option or "server signing =
mandatory" for example).
On Thu, Mar 21, 2019 at 6:46 AM Aurélien Aptel <aaptel@xxxxxxxx> wrote:
>
> Ronnie Sahlberg <lsahlber@xxxxxxxxxx> writes:
>
> > Fix Guest/Anonymous sessions so that they work with SMB 3.11.
> >
> > In git commit 6188f28 tightened the conditions and forced signing for
> > the SMB2-TreeConnect commands as per MS-SMB2.
>
> We could add a
>
> Fixes: 6188f28bf608 ("Tree connect for SMB3.1.1 must be signed for non-encrypted shares")
>
> tag in the commit message.
> >
> > - /* 3.11 tcon req must be signed if not encrypted. See MS-SMB2 3.2.4.1.1 */
> > + /*
> > + * 3.11 tcon req must be signed if not encrypted. See MS-SMB2 3.2.4.1.1
> > + * unless it is guest or anonymous user. See MS-SMB2 3.2.5.3.1
> > + */
> > if ((ses->server->dialect == SMB311_PROT_ID) &&
> > - !smb3_encryption_required(tcon))
> > + !smb3_encryption_required(tcon) &&
> > + !(ses->session_flags & (SMB2_SESSION_FLAG_IS_GUEST|SMB2_SESSION_FLAG_IS_NULL)))
> > req->sync_hdr.Flags |= SMB2_FLAGS_SIGNED;
> >
> > memset(&rqst, 0, sizeof(struct smb_rqst));
>
> I suspect there might more (and/or better) places to check. If you grep for
> SMB2_FLAGS_SIGNED there are a couple of other places that should be
> considered:
>
> smb2_hdr_assemble() { ...
> if (tcon->ses && tcon->ses->server && tcon->ses->server->sign &&
> !smb3_encryption_required(tcon))
> shdr->Flags |= SMB2_FLAGS_SIGNED;
> ...
> }
>
> cifs_get_smb_ses() { ...
> ses->sectype = volume_info->sectype;
> ses->sign = volume_info->sign;
>
> mutex_lock(&ses->session_mutex);
> rc = cifs_negotiate_protocol(xid, ses);
> if (!rc)
> rc = cifs_setup_session(xid, ses, volume_info->local_nls);
> mutex_unlock(&ses->session_mutex);
> if (rc)
> goto get_ses_fail;
> ...
> }
>
> After negprot and before sess setup we already know the protocol version so I guess there
> could be some patch there as well.
>
> See also SMB2_logoff(), SMB2_ioctl_init(), cifs_enable_signing()
>
> Cheers,
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3
> SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
Thanks,
Steve
