Closed
Bug 511817
Opened 15 years ago
Closed 15 years ago
Malformed Packet appears in network trace of SASL binding
Categories
(Directory :: LDAP C SDK, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Qiang.Xu, Assigned: Qiang.Xu)
Details
Attachments
(1 file)
(deleted),
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.2; WOW64; SV1)
Build Identifier: 6.0.6
Using Mozilla LDAP library, I can do SASL binding successfully. But the network trace shows it is not perfect.
As you can see, the Malformed Packet is in the 2nd round of binding interaction with the server:
========================================
32 17.839052 13.198.98.107 13.198.98.35 LDAP bindRequest(1) "<ROOT>" sasl
33 17.917608 13.198.98.35 13.198.98.107 LDAP bindResponse(1) saslBindInProgress
35 17.919333 13.198.98.107 13.198.98.35 LDAP bindRequest(2) "<ROOT>" [Malformed Packet]
Lightweight-Directory-Access-Protocol
LDAPMessage bindRequest(2) "<ROOT>"
messageID: 2
protocolOp: bindRequest (0)
bindRequest
version: 3
name:
authentication: sasl (3)
sasl
mechanism: GSSAPI
credentials: <MISSING>
[Malformed Packet: LDAP]
36 17.919637 13.198.98.35 13.198.98.107 LDAP bindResponse(2) saslBindInProgress
37 17.920316 13.198.98.107 13.198.98.35 LDAP bindRequest(3) "<ROOT>" sasl
38 17.920691 13.198.98.35 13.198.98.107 LDAP bindResponse(3) success
========================================
I am not sure if packet 35 is normal or not? After all, it says the packet is malformed. Luckily, it will not affect the result of SASL binding.
In contrast, a trace captured with OpenLDAP ldapsearch utility does not have this malformat packet:
========================================
20 24.622555 13.198.98.190 13.198.98.35 LDAP bindRequest(1) "<ROOT>" sasl
22 24.805633 13.198.98.35 13.198.98.190 LDAP bindResponse(1) saslBindInProgress
28 26.616093 13.198.98.190 13.198.98.35 LDAP bindRequest(2) "<ROOT>" sasl
Lightweight-Directory-Access-Protocol
LDAPMessage bindRequest(2) "<ROOT>"
messageID: 2
protocolOp: bindRequest (0)
bindRequest
version: 3
name:
authentication: sasl (3)
sasl
mechanism: GSSAPI
[Response In: 29]
29 26.616459 13.198.98.35 13.198.98.190 LDAP bindResponse(2) saslBindInProgress
31 26.616705 13.198.98.190 13.198.98.35 LDAP bindRequest(3) "<ROOT>" sasl
32 26.633134 13.198.98.35 13.198.98.190 LDAP bindResponse(3) success
========================================
When I compare the content of packet 35 in MozLDAP trace and packet 28 in OpenLDAP trace, it is noted that the MozLDAP packet has extra bytes "04 00" after "mechanism: GSSAPI". These extra bytes are interpreted as "<MISSING> credentials" by WireShark. In contrast, although the OpenLDAP packet doesn't have any credential information as well, it doesn't have these extra bytes. That's why packet 35 in MozLDAP trace is marked as Malformed Packet, while packet 28 in OpenLDAP trace is not.
Reproducible: Always
Steps to Reproduce:
1. Set up DNS correctly.
2. Set up Kerberos server accordingly.
3. Use WireShare to capture network trace.
4. Do an SASL binding, either with the client tool ldapsearch, or write a simple program calling the library funcs.
Actual Results:
In the captured network trace, you will find some LDAP SASL packets are marked with "Malformed Packet".
Expected Results:
There should be no Malformed Packet.
Looking into the code of MozLDAP, I found the following is suspicous:
========================================
int
LDAP_CALL
ldap_sasl_bind(
LDAP *ld,
const char *dn,
const char *mechanism,
const struct berval *cred,
LDAPControl **serverctrls,
LDAPControl **clientctrls,
int *msgidp
)
{
...
if ( cred == NULL ) {
rc = ber_printf( ber, "{it{ist{s}}", msgid,
LDAP_REQ_BIND, ldapversion, dn, LDAP_AUTH_SASL,
mechanism );
} else {
rc = ber_printf( ber, "{it{ist{so}}", msgid,
LDAP_REQ_BIND, ldapversion, dn, LDAP_AUTH_SASL,
mechanism, cred->bv_val,
cred->bv_len );
}
...
}
========================================
At the first look, it seem innocent. But...
Let's have a look at OpenLDAP's code as well:
========================================
int
ldap_sasl_bind(
LDAP *ld,
LDAP_CONST char *dn,
LDAP_CONST char *mechanism,
struct berval *cred,
LDAPControl **sctrls,
LDAPControl **cctrls,
int *msgidp )
{
...
if( mechanism == LDAP_SASL_SIMPLE ) {
/* simple bind */
rc = ber_printf( ber, "{it{istON}" /*}*/,
id, LDAP_REQ_BIND,
ld->ld_version, dn, LDAP_AUTH_SIMPLE,
cred );
} else if ( cred == NULL || cred->bv_val == NULL ) {
/* SASL bind w/o credentials */
rc = ber_printf( ber, "{it{ist{sN}N}" /*}*/,
id, LDAP_REQ_BIND,
ld->ld_version, dn, LDAP_AUTH_SASL,
mechanism );
} else {
/* SASL bind w/ credentials */
rc = ber_printf( ber, "{it{ist{sON}N}" /*}*/,
id, LDAP_REQ_BIND,
ld->ld_version, dn, LDAP_AUTH_SASL,
mechanism, cred );
}
...
}
========================================
Comparing both snippets of MozLDAP and OpenLDAP, I am wondering whether MozLDAP should add the condition "|| cred->bv_val == NULL" to the already existing one "if ( cred == NULL )" to decide whether to do SASL binding without explicit credential.
So, the proposed fix is in the following:
========================================
int
LDAP_CALL
ldap_sasl_bind(
LDAP *ld,
const char *dn,
const char *mechanism,
const struct berval *cred,
LDAPControl **serverctrls,
LDAPControl **clientctrls,
int *msgidp
)
{
...
#if 0
if ( cred == NULL ) {
#else
if ( cred == NULL || cred->bv_val == NULL || cred->bv_len == 0) {
#endif
rc = ber_printf( ber, "{it{ist{s}}", msgid,
LDAP_REQ_BIND, ldapversion, dn, LDAP_AUTH_SASL,
mechanism );
} else {
rc = ber_printf( ber, "{it{ist{so}}", msgid,
LDAP_REQ_BIND, ldapversion, dn, LDAP_AUTH_SASL,
mechanism, cred->bv_val,
cred->bv_len );
}
...
}
========================================
Thank Rich, for his suggestion of adding the last condition of credential buffer's value len.
Yet, I cannot directly change the code, which is managed by OS team in Xerox. I will pass the change to them, and try to let them build a new library for me to try out.
At the same time, if any of you guys have a chance to verify the fix, I will be grateful to you.
In libldap folder, type "patch -p0 < saslbind.c.diff" to apply the patch. Do SASL binding before and after the patch is applied, and compare the network traces captured, then you'll notice the difference.
Rich:
Can you help me check in the change? I have difficulty with CVS, you know.
Thanks a lot,
Xu Qiang
Comment 4•15 years ago
|
||
I think patch should be marked as review+ and commit is doing by someone from Mozilla if I not mistaken.
Ludo any ideas who should we ping for review and commit?
Updated•15 years ago
|
Assignee: nobody → Qiang.Xu
Comment 5•15 years ago
|
||
Comment on attachment 398058 [details] [diff] [review]
Patch to fix the Malformed Packet in SASL binding trace
According to http://www.mozilla.org/hacking/reviewers.html we can ask dmose. If not he'll know how to ask.
Attachment #398058 -
Flags: review?(dmose)
Comment 6•15 years ago
|
||
I have reviewed it, but I do not have the ability to set the review+ flag.
Comment 7•15 years ago
|
||
(In reply to comment #6)
> I have reviewed it, but I do not have the ability to set the review+ flag.
You need to be a peer for that. Let's wait for dan to have a look or for him to tell us who to ping.
Updated•15 years ago
|
Attachment #398058 -
Flags: review?(dmose) → review+
Comment 8•15 years ago
|
||
Comment 9•15 years ago
|
||
Checking in saslbind.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/saslbind.c,v <-- saslbind.c
new revision: 5.6; previous revision: 5.5
done
Assuming this bug is now fixed.
I'm looking at getting comm-central to pick it up in bug 520476.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•