On Mon, Oct 23, 2006, Andreas Tscharner wrote:
> Johannes Stezenbach wrote:
> >Package: cvsnt
> >Version: 2.5.03.2382-2
> >Severity: normal
> >Tags: patch
> >
> >NTLMSSP authentication against a Windows cvsnt server fails.
> >
>
> [snip]
> >
> >The attached patch fixes this, and makes NTLM auth work for me.
> >
>
> Thank you for the report and the patch. Upstream is preparing for a new
> release (RC1 is already out). I will send the patch to upstream and
> hopefully it gets in for the new version. If it does not make it, I'll
> include it in the Debian package of the new version.
The attached might be a better patch (replaces my previous patch).
Maybe you could also forward it to upstream and let them decide which
one they like better.
The old working cvsnt version I have has the following code,
which sets header.offset to a bogus value:
AddBytes(ptr, header, buf, count) \
{ \
if (buf && count) \
{ \
SSVAL(&ptr->header.len,0,count); \
SSVAL(&ptr->header.maxlen,0,count); \
SIVAL(&ptr->header.offset,0,((ptr->buffer - ((uint8*)ptr)) + ptr->bufIndex));
\
memcpy(ptr->buffer+ptr->bufIndex, buf, count); \
ptr->bufIndex += count; \
} \
else \
{ \
ptr->header.len = \
ptr->header.maxlen = 0; \
SIVAL(&ptr->header.offset,0,ptr->bufIndex); \
} \
}
Initially I thought it's better not to touch this as it is difficult
for me to see what the consequences are for other NTLMSSP messages, but
now I think it's better to solve the problem at the root.
Thanks,
Johannes
--- cvsnt-2.5.03.2382/protocols/ntlm/smbutil.c.orig 2006-10-21
03:30:43.000000000 +0200
+++ cvsnt-2.5.03.2382/protocols/ntlm/smbutil.c 2006-10-24 21:41:12.000000000
+0200
@@ -63,13 +63,26 @@ char versionString[] = PACKAGE_STRING;
/* I am not crazy about these macros -- they seem to have gotten
* a bit complex. A new scheme for handling string/buffer fields
* in the structures probably needs to be designed
+ *
+ * The special handling for zero length items is necessary so the offset
+ * does not point past end of message if the last item of the message
+ * has zero length (otherwise server responds with "The parameter is
+ * incorrect."). However if the item length is zero it doesn't matter
+ * where exactly the offset points to, I think. We just set it to zero.
*/
#define AddBytes(ptr, header, buf, count) \
{ \
ptr->header.len = ptr->header.maxlen = UI16LE(count); \
- ptr->header.offset = UI32LE((ptr->buffer - ((uint8*)ptr)) + ptr->bufIndex); \
- memcpy(ptr->buffer+ptr->bufIndex, buf, count); \
- ptr->bufIndex += count; \
+ if (buf && count) \
+ { \
+ ptr->header.offset = UI32LE((ptr->buffer - ((uint8*)ptr)) +
ptr->bufIndex); \
+ memcpy(ptr->buffer+ptr->bufIndex, buf, count); \
+ ptr->bufIndex += count; \
+ } \
+ else \
+ { \
+ ptr->header.offset = 0; \
+ } \
}
#define AddString(ptr, header, string) \
