> On Feb. 5, 2016, 9:36 a.m., David Faure wrote: > > autotests/http/httpauthenticationtest.cpp, line 73 > > <https://git.reviewboard.kde.org/r/126990/diff/1/?file=442723#file442723line73> > > > > What if key.size() > 64? (this goes out of bounds, then). Or is this > > always ensured by the caller? > > (I would add a Q_ASSERT then).
I've added a Q_ASSERT as suggested. I've also added a qMin() for the key size to make sure it also works reliably with QT_NO_DEBUG. Since the code was taken directly out of kntlm.cpp I've also updated the original implementation. > On Feb. 5, 2016, 9:36 a.m., David Faure wrote: > > autotests/http/httpauthenticationtest.cpp, line 84 > > <https://git.reviewboard.kde.org/r/126990/diff/1/?file=442723#file442723line84> > > > > If opad was a QByteArray from the start, this copying wouldn't be > > needed (you could just append to opad instead in the next line) I've rewritten the implementation to use QByteArray instead of C arrays. As with the change above I've ported the changes to the original implementation in kntlm.cpp. > On Feb. 5, 2016, 9:36 a.m., David Faure wrote: > > autotests/http/httpauthenticationtest.cpp, line 93 > > <https://git.reviewboard.kde.org/r/126990/diff/1/?file=442723#file442723line93> > > > > Maybe this can be optimized on little endian platforms? Not sure if > > it's worth having two code paths though; depends on the typical string > > length I guess. > > > > Something like > > #if Q_BYTE_ORDER == Q_LITTLE_ENDIAN > > memcpy(unicode.data(), target.unicode(), target.length() * 2); > > #else > > // current code > > #endif The strings handled here are short (usernames, passwords or domain names). I don't think it's worth the extra complexity. - Krzysztof ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126990/#review92074 ----------------------------------------------------------- On Feb. 5, 2016, 12:17 p.m., Krzysztof Nowicki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126990/ > ----------------------------------------------------------- > > (Updated Feb. 5, 2016, 12:17 p.m.) > > > Review request for KDE Frameworks and Dawit Alemayehu. > > > Repository: kio > > > Description > ------- > > Some IIS servers seem to be configured to reject NTLMv1 authentication by > refusing to reply to a NTLM stage 1 if the NTLMv2 flag is not set. If such a > thing happens try to send another stage 1 message with the NTLMv2 flag set > and if the server accepts this continue with NTLMv2. > > This also fixes invese logic when determining if the authentication needs a > password (it needs it during stage 3 response and not stage 1). > > As a bonus this includes a test case for verifying NTLMv2 authentication and > a fix for one of the existing test cases which contained wrong expected data > (the expected response was generated without use of username and password due > to the inverse logic bug above). > > > Diffs > ----- > > autotests/http/httpauthenticationtest.h > 35b822a0d400959d97e11473d48bc94352e8e5b3 > autotests/http/httpauthenticationtest.cpp > 719f7a9856194003cfba52848e0a6c5ea6324531 > src/ioslaves/http/httpauthentication.h > a74565e253ad489fed6c82116c72244386ebaaf2 > src/ioslaves/http/httpauthentication.cpp > dcc86c276fa4422fb08904b5cf6d3d2db6bb4c0d > src/kntlm/kntlm.cpp ed6f3881f3dfd0b78069368db22f7cd865261738 > > Diff: https://git.reviewboard.kde.org/r/126990/diff/ > > > Testing > ------- > > Tested on an IIS 7.5 server with NTLMv1 blacklisted. Additionally executed > automatic tests without regressions. > > > Thanks, > > Krzysztof Nowicki > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
