Ben Laurie <[EMAIL PROTECTED]>:
[...]
> I'm still not convinced that the callbacks should be on the cert, but
> OTOH that does give maximum flexibility, so I'm not going to argue very
> hard.
> Certainly the cert callbacks should be used (if set) if we are going to
> keep them.
I think that for achieving an interface as consistent as possible, the
SSL_xxx functionality should mirror the SSL_CTX_xxx functionality as
far as it is possible without causing too much hassle. In the case of
the callbacks for temporary keys, applications could use the
flexibility provided by that approach in order to use different key
sizes (or just different exponent bit lengths, different subgroup
sizes, different generators ...) for different situations -- e.g. use
parameters optimized for efficiency if the data protected by the
protocol does not need long-time secrecy. Even if the need to have
this possibility may not be obvious, it is certainly confusing if the
data structure has entries that are never used.
>> [1] s->cert is set to s->session->cert by SSL_new, and
> ^^^^^^^ ctx, surely?
>> s->session->cert is set to s->cert during the handshake if a new
>> session is started.
Yes; s->ctx->default_cert, to be exact.
>> [2] More exactly, the first change to the SSL-object specific
>> "cert" parameters causes a fresh s->cert to be created, which
>> does not copy the values of s->ctx->default_cert; this happens in
>> the call to ssl_cert_instantiate from within ssl_rsa.c
>> (SSL_use_certificate, SSL_use_RSAPrivateKey, SSL_use_PrivateKey)
>> or from within s3_lib.c (ssl3_ctrl, which is used to implement
>> SSL_set_tmp_rsa_callback and so on).
Proposal: Let's modify ssl_cert_instantiate to copy the callback
function pointers from default_cert (if available), and change the
code that uses those callbacks (ssl3_send_server_key_exchange in file
s3_srvr.c) to use always the callbacks in s->session->cert (as is
currently the case with dh_tmp_cb) and never the ones from
s->ctx->default_cert (which is where rsa_tmp_cb is currently taken
from).
Additionaly, if s->session->cert doesn't have the needed callback
but s->ctx->default_cert does (which can happen if
SSL_CTX_set_tmp_{rsa,dh}_callback or the equivalent
SSL_CTX_ctrl(SSL_CTRL_SET_TEMP_{RSA,DH}_CB) is called too late, namely
after the ssl_cert_instantiate() took place), copy the one from
default_cert into session_cert as soon as this is noticed
(i.e., in ssl_set_cert_masks -- if this function does not find the
callback functions, the ciphers needing them won't be used anyway).
By the way, ssl_set_cert_masks relies on
s->ctx->default_cert->dh_tmp_cb for determining which ciphers can be
used, whereas ssl3_send_server_key_exchange actually uses
s->session->cert->dh_tmp_cb. Obviously, this can lead to problems
if the pointers s->ctx->default_cert and s->session->cert don't
point to the the same data object (that is, when SSL_xxx and not
SSL_CTX_xxx was used to manipulate a struct cert_st) because OpenSSL
may incorrectly conclude that DHE ciphersuites can be used when they
actually can't because the wrong callback was set -- probably the
handshake will fail in this case --, or it may incorrectly conclude
that DHE ciphersuites cannot be used when they actually could.
Earlier versions (SSLeay 0.9.0, OpensSSL 0.9.1c) operated only on one
struct cert_st, which was s->cert if this was not NULL (that's buggy
for the rsa_tmp_cb case) and s->ctx->default_cert otherwise (that's
buggy for the dh_tmp_cb case). Now this bug should have caused
programs like Apache-SSL to fail to communicate with export browsers
if the RSA host key was too long -- but it was compensated by a bug in
ssl3_choose_cipher: Even though ssl_set_cert_masks thought that the
RSA export cipher suites could not be used because the wrong
rsa_tmp_cb was checked, the lines
alg=c->algorithms&(SSL_MKEY_MASK|SSL_AUTH_MASK);
if (alg & SSL_EXPORT)
...
ok = ...
else
...
ok = ...
always went through the "else" branch because SSL_EXPORT == 0x00300000L
and SSL_MKEY_MASK|SSL_AUTH_MASK == 0x000003FFL. (OpenSSL 0.9.2b
uses the correct
if (SSL_IS_EXPORT(c->algorithms))
instead.)
Since this may be pretty confusing, here's a small summary of what I
think should be changed:
- ssl_cert_instantiate should copy rsa_tmp_cb and dh_tmp_cb, if
available.
- ssl3_send_server_key_exchange should only use callbacks in
s->session->cert, it should not look at default_cert.
- ssl_set_cert_masks could copy rsa_tmp_cb from default_cert into
the actual cert if rsa_tmp_cb is not set in the latter; similarly
for dh_tmp_cb. This is a bit awkward, but provides compatibility
with applications that set the callbacks in default_cert *after*
the specific cert structure has been created -- this works
now (in the rsa_tmp_cb case), and if such applications actually
exist, it probably wouldn't be nice to break them. (Or maybe it
is, because they mix SSL_CTX_xxx and SSL_xxx in a strange order.)
- Obviously, ssl_set_cert_masks must be changed to look at
whatever set of callback functions we decide to use -- the current
code does not match the implementation in s3_srvr.c.
Comments?
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
