I suspect the leak was introduced in sslcontext.c rev 1.9 in
IssueTmpRSAKey. The change fixed a server crash bug (which
explains why the "leak" was never observed before).
RSA_generate_key allocates a new RSA structure and returns a
pointer to it. IssueTmpRSAKey returns that pointer but
nothing ever frees it.
The reality is that RSA_generate_key is "slow" and only
needs to be executed once per key-length (512 and 1024 seem
to be the only ones used as far as I could find). So, I
suggest that the two temporary RSA key key-lengths be
generated from nsopenssl's Ns_ModuleInit. If IssueTmpRSAKey
gets executed with any other key-lengths, we'll throw an
error -- we have no way of freeing the newly allocated RSA
structure, which would lead to a memory leak.
Other applications use a function scoped static RSA *rsaPtr
which is a bad idea because suppose the first call is for a
512-bit key, then the second call is for a 1024-bit key:
since rsaPtr isn't NULL (it points to a 512-bit RSA key),
the function erroneously returns a pointer to a 512-bit RSA
key when a 1024-bit key was requested. Modifying this
slightly to have a separate rsaPtr_512 and rsaPtr_1024 (but
still function scoped static) still has problems in a
multi-threaded app, because typically there's a check to see
if the ptr is NULL and if it isn't, generate the temp. RSA
key and store the ptr -- we COULD make this safe by adding a
mutex lock around the whole mess, but performance-wise, it's
just smarter to pre-generate the keys ahead of time and
don't use locks.
Which is, exactly what I've implemented as the "fix" for
this memory leak.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Attaching a patch which implements the fix -- the patch is a
lot bigger than the minimal change because I cleaned up the
Makefile as well as reformatted some code to make it closer
to the AOLserver Coding Style.
The meaningful changes are in sslcontext.c in IssueTmpRSAKey
and the new NsMakeTmpRSAKey, and a minor change in
nsopenssl.c in Ns_ModuleInit.
I've committed the change and have tagged nsopenssl
v3_0beta26 which contains the fix.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Logged In: YES
user_id=21885
I suspect the leak was introduced in sslcontext.c rev 1.9 in
IssueTmpRSAKey. The change fixed a server crash bug (which
explains why the "leak" was never observed before).
RSA_generate_key allocates a new RSA structure and returns a
pointer to it. IssueTmpRSAKey returns that pointer but
nothing ever frees it.
The reality is that RSA_generate_key is "slow" and only
needs to be executed once per key-length (512 and 1024 seem
to be the only ones used as far as I could find). So, I
suggest that the two temporary RSA key key-lengths be
generated from nsopenssl's Ns_ModuleInit. If IssueTmpRSAKey
gets executed with any other key-lengths, we'll throw an
error -- we have no way of freeing the newly allocated RSA
structure, which would lead to a memory leak.
Other applications use a function scoped static RSA *rsaPtr
which is a bad idea because suppose the first call is for a
512-bit key, then the second call is for a 1024-bit key:
since rsaPtr isn't NULL (it points to a 512-bit RSA key),
the function erroneously returns a pointer to a 512-bit RSA
key when a 1024-bit key was requested. Modifying this
slightly to have a separate rsaPtr_512 and rsaPtr_1024 (but
still function scoped static) still has problems in a
multi-threaded app, because typically there's a check to see
if the ptr is NULL and if it isn't, generate the temp. RSA
key and store the ptr -- we COULD make this safe by adding a
mutex lock around the whole mess, but performance-wise, it's
just smarter to pre-generate the keys ahead of time and
don't use locks.
Which is, exactly what I've implemented as the "fix" for
this memory leak.
Logged In: YES
user_id=21885
Attaching a patch which implements the fix -- the patch is a
lot bigger than the minimal change because I cleaned up the
Makefile as well as reformatted some code to make it closer
to the AOLserver Coding Style.
The meaningful changes are in sslcontext.c in IssueTmpRSAKey
and the new NsMakeTmpRSAKey, and a minor change in
nsopenssl.c in Ns_ModuleInit.
I've committed the change and have tagged nsopenssl
v3_0beta26 which contains the fix.
nsopenssl patch against HEAD (v3_0beta25)