[CFR] OpenSSL ENGINE fix

[Peter Pentchev]

Hi,

The stunnel port had been badly broken on -CURRENT for some time, and
today I seem to have tracked it down.  There is a problem in
OpenSSL's ENGINE code, which seems to depend on realloc()'s initializing
unused memory with zeroes.  Since this is certainly not true with
malloc's 'J' option, the ENGINE code never actually runs out of RAND
routines to look for, and dumps core on attempting to dereference a
structure at 0xd0d0d0d0.

The following simple patch seems to fix this particular problem, but I
think there are others lurking close beneath the surface.  When I built
stunnel with this patch, it did not immediately segfault on
initialization, but it did later, when it attempted to get a couple of
random bytes in order to actually establish an SSL connection.  A quick
examination shows that the ENGINE code had "successfully" loaded and
initialized all built-in engines and then tried to use the last one
loaded - in this case, the 4758cca one - which promptly invoked a
function pointed to by the static randomNumberGenerate variable, which,
as you may have guessed by this point, held a NULL value.  Apparently,
something else is rotten in ENGINE initialization land, and (at least)
the 4758cca driver cannot properly detect that it has not properly
detected its hardware :(

Still, I think the attached patch should be committed and MT5'd before
5.3-RELEASE, to fix at least one of the flaws.  Also here is a simple C
program that illustrates the problem - just compile it and run it on a
stock 5.x or 6.x system, and I'd be, well, somewhat surprised if it gets
to the printf's at all.

So.. the patch itself:

Index: src/crypto/openssl/crypto/engine/eng_table.c
===================================================================
RCS file: /home/ncvs/src/crypto/openssl/crypto/engine/eng_table.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 eng_table.c
--- src/crypto/openssl/crypto/engine/eng_table.c	28 Jan 2003 21:22:30 -0000	1.1.1.1
+++ src/crypto/openssl/crypto/engine/eng_table.c	2 Sep 2004 14:40:52 -0000
@@ -287,7 +287,7 @@
 		}
 trynext:
 	ret = sk_ENGINE_value(fnd->sk, loop++);
-	if(!ret)
+	if(!ret || loop == sk_ENGINE_num(fnd->sk))
 		{
 #ifdef ENGINE_TABLE_DEBUG
 		fprintf(stderr, "engine_table_dbg: %s:%d, nid=%d, no "

And the test program:

#include <openssl/lhash.h>
#include <openssl/ssl.h>
#include <openssl/err.h>
#include <openssl/crypto.h> /* for CRYPTO_* and SSLeay_version */
#include <openssl/rand.h>
#if SSLEAY_VERSION_NUMBER >= 0x00907000L
#include <openssl/engine.h>
#else
#error weird openssl version
#endif

int main(void)
{
	ENGINE_load_builtin_engines();
	ENGINE_register_all_complete();
	if (RAND_status()) {
		printf("RAND_status said ok\n");
	} else {
		printf("RAND_status kinda sorta failed\n");
	}
	return (0);
}

G'luck,
Peter

-- 
Peter Pentchev	roam at ringlet.net    roam at cnsys.bg    roam at FreeBSD.org
PGP key:	http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint	FDBA FD79 C26F 3C51 C95E  DF9E ED18 B68D 1619 4553
If there were no counterfactuals, this sentence would not have been paradoxical.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20040902/fe70dff8/attachment.bin