On Mon, 18 Apr 2011 07:16:36 -0500 Shirish Pargaonkar <[email protected]> wrote:
> On Fri, Apr 8, 2011 at 8:49 AM, Shirish Pargaonkar > <[email protected]> wrote: > > On Fri, Apr 8, 2011 at 8:40 AM, Jeff Layton <[email protected]> wrote: > >> On Wed, 6 Apr 2011 08:46:06 -0500 > >> [email protected] wrote: > >> > >>> From: Shirish Pargaonkar <[email protected]> > >>> > >>> > >>> 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 <[email protected]> > >>> Signed-off-by: Shirish Pargaonkar <[email protected]> > >>> --- > >>> 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 <[email protected]> > >> > > > > 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 <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
