On Mon, Apr 18, 2011 at 7:24 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
> 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>
>
Yes, I did verify that auth mechs like sec=ntlm/i and sec=ntlmv2/i
do work with this patch.
Regards,
Shirish
--
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
