[EMAIL PROTECTED] (Bodo Moeller): >> I [...] came across what appears to be a bug in the enc_read.c routine. >> I added a few lines of code so that it would work [...] > Was that really a bug? I would not think so, and certainly the > proposed fix is not correct (the recursive call to des_enc_read can > return 0 because of either EOF, some real error or EWOULDBLOCK). It turns out that EWOULDBLOCK is not an issue here because des_enc_read() and des_enc_write() are not designed to work with non-blocking sockets, anyway. > The original version of des_enc_read (with > if (len > MAXWRITE) len=MAXWRITE; > ) makes complete sense if you insist that the caller check the return > value; and on that you have to insist for obvious reasons. Actually, it makes only a bit of sense, not complete sense. I should first explain a bit about des_enc_read() and des_enc_write(), because their behaviour is less than obvious and (as usual) poorly documented. These two functions (which are not called from anywhere else in the library) use a specific data format on files (including sockets): The data stream is a sequence of length-prefixed, encrypted data chunks. des_enc_write() allows to write any number of bytes: It uses recursive calls to itself and finally a write() loop to output that data to the file (in one or more chunks). des_enc_read() will read one chunk at a time. It may return less data than the caller requested (if the chunk is smaller). It uses an internal state to handle situations where the reader demands less than the chunk size. (This state uses static variables, so des_enc_read() can be used on only one file at a time). The big problem is that the des_enc_write()/des_enc_read() data format uses the same IV on each chunk, when actually it should update the IV. The only available modes of operation are CBC and PCBC. Thus, the cryptographic weakness means that an attacker will be able to see for any two chunks whether they start with the same plaintext block(s) or not. It would be much worse with CFB or even OFB, but this is bad enough. The current documentation to des_enc_write()/des_enc_read(), which is located in line 64 of enc_read.c, should be updated with the appropriate warnings: /* IMPORTANT NOTES: * * - The data format used by des_enc_write and des_enc_read * has a cryptographic weakness: When asked to write more * than MAXWRITE bytes, des_enc_write will split the data * into several chunks that are all encrypted * using the same IV. * * - This code cannot handle non-blocking sockets. */ Additionally for des_enc_read: * - This function uses an internal state and thus cannot be * used on multiple files. It might make sense, as there are people who want to use functions like those with a simple interface, to create imcompatible variants of des_enc_write() and des_enc_read() that do update the IV. Bodo M"oller <[EMAIL PROTECTED]> ______________________________________________________________________ OpenSSL Project http://www.openssl.org User Support Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
