| Message ID | 1302097566-11576-1-git-send-email-shirishpargaonkar@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 6 Apr 2011 08:46:06 -0500 shirishpargaonkar@gmail.com wrote: > From: Shirish Pargaonkar <shirishpargaonkar@gmail.com> > > > 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@icts.kuleuven.be> > Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com> > --- > 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; >
On Fri, Apr 8, 2011 at 8:40 AM, Jeff Layton <jlayton@poochiereds.net> wrote: > On Wed, 6 Apr 2011 08:46:06 -0500 > shirishpargaonkar@gmail.com wrote: > >> From: Shirish Pargaonkar <shirishpargaonkar@gmail.com> >> >> >> 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@icts.kuleuven.be> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com> >> --- >> 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@poochiereds.net> > 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). -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */ - } 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;