On Wed, Aug 14, 2002 at 01:57:32PM +0100, Ben Laurie wrote:
[...]
>> But for various other potential errors, we do know what happened
>> (e.g. a buffer has insufficient size) and we do know how to continue
>> without doing significant harm (abort this one TLS/SSL connection, and
>> in a way such that we have a likelihood of seeing a useful error
>> message in some log file). These are still "internal errors".
>>
>> For example, here is a consistency check I added in April when I
>> implemented the empty fragment protocol weakness workaround
>> (ssl/s3_pkt.c):
>>
>> if (s->s3->wbuf.len < (size_t)prefix_len +
>SSL3_RT_MAX_PACKET_SIZE)
>> {
>> /* insufficient space */
>> SSLerr(SSL_F_DO_SSL3_WRITE, ERR_R_INTERNAL_ERROR);
>> goto err;
>> }
>>
>> The buffer is allocated by the SSL library, but this happens
>> elsewhere, so it's safer not to rely on buffer allocation being
>> sufficient for what I plan to do with the buffer. If the buffer is
>> not sufficient, this would be a bug in OpenSSL.
> The buffer is _supposed_ to be big enough. We know of no path through
> the code that should cause it to be not big enough. So if this condition
> occurs something we don't understand has happened.
Quite in contrary, this would be quite easy to understand: if the
condition fails, the overhead for the empty fragment taken into
account during buffer allocation apparently was an underestimate,
possibly because a new cipher suite uses a block cipher with an
unusually large block size, or because TLS padding to larger lengths
has been implemented. There is no known path through that current
OpenSSL code such that the condition can fail (as far as I know), but
inconsistencies might be introduced with future changes in the SSL
library. Inconsistencies of this type are no reason to completely
abort the application.
For most of the consistency checks that your patch added, a failure
would very likely indicate that there is an inconsistency in the
OpenSSL source code similar to the potential inconsistency discussed
above. Those checks do not appear to be targeted at testing whether
memory was somehow corrupted at run-time. (Tests of the latter kind
are possible [e.g. magic constants in structs], but no such approach
is visible in your patch.)
> Continuing under
> these circumstances as if nothing were wrong strikes me as foolhardy.
Certainly there may be conditions under which it is unwise to
continue, but I don't think the ones detected by your patch
are among them.
--
Bodo Möller <[EMAIL PROTECTED]>
PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html
* TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt
* Tel. +49-6151-16-6628, Fax +49-6151-16-6036
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
