On Mon, Jan 24, 2000 at 12:38:17PM +0100, Richard Levitte - VMS Whacker wrote:
[ssl/s2_clnt.c, get_server_hello]
> if (s->session->peer != NULL)
> X509_free(s->session->peer);
>
> #if 0 /* What is all this meant to accomplish?? */
> /* hmmm, can we have the problem of the other session with this
> * cert, Free's it before we increment the reference count. */
> CRYPTO_w_lock(CRYPTO_LOCK_X509);
> s->session->peer=s->session->sess_cert->key->x509;
> /* Shouldn't do this: already locked */
> /*CRYPTO_add(&s->session->peer->references,1,CRYPTO_LOCK_X509);*/
> s->session->peer->references++;
> CRYPTO_w_unlock(CRYPTO_LOCK_X509);
> #else
> s->session->peer = s->session->sess_cert->peer_key->x509;
> /* peer_key->x509 has been set by ssl2_set_certificate. */
> CRYPTO_add(&s->session->peer->references, 1, CRYPTO_LOCK_X509);
> #endif
> Personally, I'd like to do the then clause rather than the else
> clause, for MT safety's sake.
If a second thread exists that uses the same X509 structure, then it
owns a second reference count anyway, and
CRYPTO_add(&s->session->peer->references, 1, CRYPTO_LOCK_X509)
leaves the reference count at 3 or more, so there's no problem in this
case.
A problem exists if you think it should be possible to have two
clients trying to reuse the same session using identical s->session
pointers. If so, the "then" branch is just a more complicated way to
implement essentially the same bug: If s->hit is set and s->session is
shared by two processes,
X509_free(s->session->peer)
is badly wrong; we must not modify anything in s->session. (Thus if
s->hit != 0, there's no need for any locking in the first place.)
Probably all of the above code should be moved into the "s->hit == 0"
branch; I hope it is not needed if s->hit != 0 (when reusing a
session, "s->session->peer == s->session->sess_cert->key" should hold
anyway, or we have real problems).
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
