Short read from krb5_rc_io_read

[Mike Patnode]

I'm sure it would, but I think it's pretty clear that's not the problem
and the assumption that 4 bytes will be read is incorrect.   The real
question is whether krb5_rc_io_read() should always loop until it's read
all the data requested, or whether there are cases where the buffer size
will be larger than the data.

mp

> -----Original Message-----
> From: JC Ferguson [mailto:jc at f5.com] 
> Sent: Friday, March 14, 2008 10:17 AM
> To: Mike Patnode; Kerberos developers list
> Subject: RE: Short read from krb5_rc_io_read
> 
> 
> 
> 
> line 356 also references "maxlen".
> Where is that initialized?
> 
> If you set "len2" to zero when you declare it, does the 
> warning go away?
> 
>  
> 
> > -----Original Message-----
> > From: krbdev-bounces at mit.edu 
> [mailto:krbdev-bounces at mit.edu] On Behalf 
> > Of Mike Patnode
> > Sent: Friday, March 14, 2008 12:42
> > To: Kerberos developers list
> > Subject: Short read from krb5_rc_io_read
> > 
> > 
> > I'm currently running valgrind on code build with 1.4.3, 
> but I don't 
> > see
> > any difference here in the 1.6 source base.    I'm getting 
> > the following
> > error:
> > 
> > 
> > ==7938== Conditional jump or move depends on uninitialised value(s)
> > ==7938==    at 0x72A8F7: krb5_rc_io_fetch (rc_dfl.c:356)
> > ==7938==    by 0x72ABE5: krb5_rc_dfl_recover_locked (rc_dfl.c:460)
> > ==7938==    by 0x72AF19: krb5_rc_dfl_recover_or_init (rc_dfl.c:522)
> > ==7938==    by 0x6F68B4: krb5_rc_recover_or_initialize (rcfns.c:44)
> > ==7938==    by 0x6F1671: krb5_get_server_rcache (srv_rcache.c:115)
> > ==7938==    by 0x6EC4A6: krb5_rd_req (rd_req.c:89)
> > ... (my code which looks something like this:
> > 
> >     rc = krb5_auth_con_init(m_kcontext, &serverAuthContext);
> > 
> >     ...
> > 
> >     rc = krb5_rd_req(m_kcontext,
> >                      &serverAuthContext,
> >                      &ticket,
> >                      NULL /* don't check SPN */,
> >                      &ktab,
> >                      NULL /* no options */,
> >                      ret_ticket);
> > 
> > Where we're verifying a user ticket by requesting a service 
> ticket for
> > the local host to prevent the classic rogue KDC response 
> attack.   In
> > anycase, it appears krb5_rc_io_read doesn't protect against a short
> > read:
> > 
> > In krb5_rc_io_fetch, we have:
> > 
> >     int len2;
> >     unsigned int len;
> >     krb5_error_code retval;
> > 
> >     rep->client = rep->server = 0;
> > 
> >     retval = krb5_rc_io_read(context, &t->d, (krb5_pointer) &len2,
> >                              sizeof(len2));
> >     if (retval)
> >         return retval;
> > 
> >     if ((len2 <= 0) || (len2 >= maxlen))   // line 356
> >         return KRB5_RC_IO_EOF;
> > 
> > And krb5_rc_io_read is:
> > 
> > krb5_error_code
> > krb5_rc_io_read(krb5_context context, krb5_rc_iostuff *d, 
> krb5_pointer 
> > buf,
> >                 unsigned int num)
> > {
> >     int count;
> >     if ((count = read(d->fd, (char *) buf, num)) == -1)
> >         switch(errno)
> >         {
> >         case EIO: return KRB5_RC_IO_IO;
> >         case EBADF:
> >         default:
> >             krb5_set_error_message(context, KRB5_RC_IO_UNKNOWN,
> >                                    "Can't read from replay 
> cache: %s",
> >                                    strerror(errno));
> >             return KRB5_RC_IO_UNKNOWN;
> >         }
> >     if (count == 0)
> >         return KRB5_RC_IO_EOF;
> >     return 0;
> > }
> > 
> > So there is no guarantee that we're are going to read 4 bytes.   I
> > suspect in this case we read 1, 2 or 3, and valgrind 
> complained about 
> > some of the bytes in len2 not being initialized (or fstat() didn't 
> > initialize st_size?).
> > 
> > Now I strongly suspect this was a side effect of running within 
> > valgrind
> > due to the performance issues introduced.    It only happened once
> > during a 24 hour stress test of this code path, but I 
> confess to not 
> > having a deep understanding of what's happening at this 
> point in time.
> > We could obviously pacify valgrind by initializing len2 to 0, but I 
> > don't think that's necessarily the right thing to do...
> >  Is it possible
> > the length value would be less than 4 bytes?   Any thoughts 
> on whether
> > this is a real-world problem?
> > 
> > mp
> > 
> > 
> > _______________________________________________
> > krbdev mailing list             krbdev at mit.edu
> > https://mailman.mit.edu/mailman/listinfo/krbdev
> > 
>