[Date Prev][Date Next] [Chronological] [Thread] [Top]

I think there's a bug with p->sasl_maxbuf in cyrus.c



Hi All,

I think there's a bug with p->sasl_maxbuf in cyrus.c.
I'll explain what I think the bug is first, and then
explain how I came across it afterward.

In sb_sasl_setup of cyrus.c, the variable
p->sasl_maxbuf is set like this:

  sasl_getprop( p->sasl_context, SASL_MAXOUTBUF,
               (SASL_CONST void **) &p->sasl_maxbuf );

And it's then later used in sb_sasl_read like this:

  /* The new packet always starts at 
     p->sec_buf_in.buf_base 
   */
  ret = sb_sasl_pkt_length( p->sec_buf_in.buf_base, 
                           *p->sasl_maxbuf, 
                           sbiod->sbiod_sb->sb_debug
);

  /* Grow the packet buffer if neccessary */
  if ( ( p->sec_buf_in.buf_size < (ber_len_t) ret ) &&
       ber_pvt_sb_grow_buffer( &p->sec_buf_in, ret ) <
0 )
    {
      errno = ENOMEM;
      return -1;
    }

In other words, it's used to limit the length of a
packet a client can read from a server. I think that
using this buffer size value for this is incorrect.

The sasl_getprop call returns the value of the SASL
variable o->sasl_context->oparams.maxoutbuf.
oparams.maxoutbuf is the maximum data that can be fed
into the SASL layer for encoding. In other words, the
maximum amount of data that can be *sent to the
server* by a client. However, here in sb_sasl_read
it's instead being used to set a limit on the maximum
packet size we can *read from the server*. 

I base all of this on the following code in Cyrus Sasl
2.1.15:

function gssapi_client_mech_step in plugins/gssapi.c:

  oparams->maxoutbuf =
   (((unsigned char *) output_token->value)[1] << 16)
|
   (((unsigned char *) output_token->value)[2] << 8) |
   (((unsigned char *) output_token->value)[3] << 0);

where output_token has been unwrapped from a server
response. To further backup my claim, the only place
in the entire SASL code where oparams->maxoutbuf is
referenced is in the function sasl_encodev, which is a
function used for encoding data to be sent (in this
case, sent to the server). In that function,
oparams->maxoutbuf is checked to make sure that not
too much data is being encoded (i.e. more than the
server advertised it could handle).

Incidentally, why is there a need for incoming data to
be limited to any negotiated size, since the code just
below the call to sb_sasl_pkt_length grows the local
buffer anyhow?

Am I way off base here, or am I correct that the code
in cyrus.c is using the wrong buffer size to
unnecessarily limit the packets read from the server?

Here's how I discovered this:

I have an LDAP client that talks to Windows domain
controllers using GSSAPI authentication and
signing/sealing. It worked fine against Windows 2000
domain controllers, but then broke against Windows
2003 DCs (in fact, it caused LSASS to die on the DC
and the DC to then reboot). So I tried upgrading to
use Cyrus 2.1.15. This fixed all problems with 2003,
but now Windows 2000 no longer worked (though it
didn't crash).

I found the problem to be in in plugins/gssapi.c. The
code which reads the oparams->maxoutbuf used to look
like this in Cyrus 2.1.4:
  oparams->maxoutbuf = (*(unsigned long
*)output_token->value & 0xFFFFFF);

but in Cyrus 2.1.15, looks like this (same as above):

  oparams->maxoutbuf =
   (((unsigned char *) output_token->value)[1] << 16)
|
   (((unsigned char *) output_token->value)[2] << 8) |
   (((unsigned char *) output_token->value)[3] << 0);

Also, I found out that the output_token->value sent
from the server is different for Windows 2000 versus
2003:
  Windows 2000: 00 00 a0 07
  Windows 2003: 00 40 00 07

Note that byte 0 (0x07) is not part of the maxoutbuf.
So it seems that at some point the SASL folks realized
that they were incorrectly parsing this field, and in
Windows 2003 the Microsoft folks realized that they
were incorrectly sending this field.

In either case, the incorrect parsing of the field
resulted in a server buffer size which was lower than
the packet sizes that I was actually receiving from
the server. I didn't think that this should really
matter, since I send very small packets (requests) to
the server anyway. Researching this problem led me to
the OpenLDAP find.

__________________________________
Do you Yahoo!?
The New Yahoo! Shopping - with improved product search
http://shopping.yahoo.com