On Mon, 18 Apr 2011 07:16:36 -0500
Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
> On Fri, Apr 8, 2011 at 8:49 AM, Shirish Pargaonkar
> <shirishpargaonkar@xxxxxxxxx> wrote:
> > On Fri, Apr 8, 2011 at 8:40 AM, Jeff Layton <jlayton@xxxxxxxxxxxxxxx> wrote:
> >> On Wed, Â6 Apr 2011 08:46:06 -0500
> >> shirishpargaonkar@xxxxxxxxx wrote:
> >>
> >>> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> >>>
> >>>
> >>> Fix authentication failures using extended security mechanisms.
> >>> cifs client does not take into consideration extended security bit
> >>> in capabilities field in negotiate protocol response from the server.
> >>>
> >>> Please refer to Samba bugzilla 8046.
> >>>
> >>>
> >>> Reported-and-tested by: Werner Maes <Werner.Maes@xxxxxxxxxxxxxxxx>
> >>> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> >>> ---
> >>> Âfs/cifs/cifssmb.c | Â 17 ++++++-----------
> >>> Â1 files changed, 6 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> >>> index 3291770..e119d70 100644
> >>> --- a/fs/cifs/cifssmb.c
> >>> +++ b/fs/cifs/cifssmb.c
> >>> @@ -570,18 +570,10 @@ CIFSSMBNegotiate(unsigned int xid, struct cifs_ses *ses)
> >>> Â Â Â if (pSMBr->EncryptionKeyLength == CIFS_CRYPTO_KEY_SIZE) {
> >>> Â Â Â Â Â Â Â memcpy(ses->server->cryptkey, pSMBr->u.EncryptionKey,
> >>> Â Â Â Â Â Â Â Â Â Â ÂCIFS_CRYPTO_KEY_SIZE);
> >>> - Â Â } else if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC)
> >>> - Â Â Â Â Â Â Â Â Â Â && (pSMBr->EncryptionKeyLength == 0)) {
> >>> + Â Â } else if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC ||
> >>> + Â Â Â Â Â Â Â Â Â Â server->capabilities & CAP_EXTENDED_SECURITY) &&
> >>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â (pSMBr->EncryptionKeyLength == 0)) {
> >>> Â Â Â Â Â Â Â /* decode security blob */
> >>
> >> This looks wrong to me. CAP_EXTENDED_SECURITY just means that the
> >> server supports extended security, not that it's in use, right? Aren't
> >> we just working around server brokenness here. Why isn't it setting
> >> SMBFLG2_EXT_SEC if it's using extended security?
> >>
> >> Are there cases where the server might set EncryptionKeyLength to 0,
> >> and *not* be using extended security? If not, then why bother to check
> >> the flags or capabilities at all?
> >>
> >>> - Â Â } else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
> >>> - Â Â Â Â Â Â rc = -EIO; /* no crypt key only if plain text pwd */
> >>> - Â Â Â Â Â Â goto neg_err_exit;
> >>> - Â Â }
> >>> -
> >>> - Â Â /* BB might be helpful to save off the domain of server here */
> >>> -
> >>> - Â Â if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC) &&
> >>> - Â Â Â Â Â Â (server->capabilities & CAP_EXTENDED_SECURITY)) {
> >>> Â Â Â Â Â Â Â count = get_bcc(&pSMBr->hdr);
> >>> Â Â Â Â Â Â Â if (count < 16) {
> >>> Â Â Â Â Â Â Â Â Â Â Â rc = -EIO;
> >>> @@ -624,6 +616,9 @@ CIFSSMBNegotiate(unsigned int xid, struct cifs_ses *ses)
> >>> Â Â Â Â Â Â Â Â Â Â Â } else
> >>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â rc = -EOPNOTSUPP;
> >>> Â Â Â Â Â Â Â }
> >>> + Â Â } else if (server->sec_mode & SECMODE_PW_ENCRYPT) {
> >>> + Â Â Â Â Â Â rc = -EIO; /* no crypt key only if plain text pwd */
> >>> + Â Â Â Â Â Â goto neg_err_exit;
> >>> Â Â Â } else
> >>> Â Â Â Â Â Â Â server->capabilities &= ~CAP_EXTENDED_SECURITY;
> >>>
> >>
> >>
> >> --
> >> Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
> >>
> >
> > I checked in ms-cifs and ms-smb, and there is no mention of server needing
> > to set extended security bit in flags2 in smb header but ms-smb does mention
> > about server setting extended security bit in capabilities field in
> > negotiate protocol response (there is an example of the exchange in ms-smb).
> > When server is indicating it supports extended security, it should set
> > encryptionkeylength to 0 since the ensuing exchange will involve an
> > encryption key (challenge).
> >
>
> Jeff, any further comments on this?
>
No. As long as you're fairly sure that this won't cause regressions
when we don't intend to use extended security then I'm ok with it.
--
Jeff Layton <jlayton@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
