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)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Qiang.Xu, Assigned: Qiang.Xu)

Details

Attachments

(1 file)

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.
The patch looks good.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Rich: Can you help me check in the change? I have difficulty with CVS, you know. Thanks a lot, Xu Qiang
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?
Assignee: nobody → Qiang.Xu
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)
I have reviewed it, but I do not have the ability to set the review+ flag.
(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.
Attachment #398058 - Flags: review?(dmose) → review+
Comment on attachment 398058 [details] [diff] [review] Patch to fix the Malformed Packet in SASL binding trace Setting r+ on behalf of Rich based on comment 2 and comment 6.
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.

Attachment

General

Creator:
Created:
Updated:
Size: