Re: Error Stack Question

2002-02-01 Thread Bodo Moeller

Verdon Walker [EMAIL PROTECTED] in epsilon.openssl.bugs:

 We ran into a small piece of code in ssl_rsa.c that is confusing us. In
 SSL_CTX_use_certificate_chain_file(), the following code fragment
 exists:
  
 ret=SSL_CTX_use_certificate(ctx,x);
 if (ERR_peek_error() != 0)
 ret = 0;  /* Key/certificate mismatch doesn't imply ret==0 ... */
 if (ret)
 ...
  
 Isn't this a little strange to ignore the return code of the function
 called and instead peek at the error stack?

The return code isn't exactly ignored, this is an alternative
condition that will be treated like a 0 return value.

 There is certainly no
 guarantee that any errors on the stack are from the call to
 use_certificate

True.  There are many places where OpenSSL should call
ERR_clear_error() before doing other things in order to avoid having
old error codes left in the queue and cause confusion.  The theory is
that the application is responsible for clearing the error queue,
but in that case ERR_clear_error() wouldn't hurt either.

 and even so, why check the stack rather than the return
 code.
  
 This looks like a hack to me. Is there a specific reason it is needed?

Yes: As the comment tries to explain, under certain conditions
SSL_CTX_use_certificate() will indicate an error on the error queue,
but still return 1.

Actually this appears to be a bug in SSL_CTX_use_certificate() (in
ssl_set_cert(), more precisely): It should call ERR_clear_error() if
it intentionally ignores an error returned by X509_check_private_key().
(Functions should not leave behind error codes without indicating
an error in their return value.  Otherwise how is the application to
know that it should look at the error queue?)

But I can't make too much sense of the logic in ssl_set_cert() -- if
X509_check_private_key() fails the first time, does it ever make sense
to switch from SSL_PKEY_DH_RSA to SSL_PKEY_DH_DSA or the other way
around?  Here's the strange part:

static int ssl_set_pkey(CERT *c, EVP_PKEY *pkey)
{
[...]
i=ssl_cert_type(NULL,pkey);
[...]

if (c-pkeys[i].x509 != NULL)
{
EVP_PKEY *pktmp;
pktmp = X509_get_pubkey(c-pkeys[i].x509);
EVP_PKEY_copy_parameters(pktmp,pkey);
EVP_PKEY_free(pktmp);
ERR_clear_error();

[...]
if (!X509_check_private_key(c-pkeys[i].x509,pkey))
{
if ((i == SSL_PKEY_DH_RSA) || (i == SSL_PKEY_DH_DSA))
{
i=(i == SSL_PKEY_DH_RSA)?
SSL_PKEY_DH_DSA:SSL_PKEY_DH_RSA;

if (c-pkeys[i].x509 == NULL)
ok=1;
else
{
if (!X509_check_private_key(
c-pkeys[i].x509,pkey))
bad=1;
else
ok=1;
}
}
else
bad=1;
}
else
ok=1;
}
[...]
}

No matter how this is resolved, the weird two lines in
SSL_CTX_use_certificate_chain_file() certainly can be omitted.
__
OpenSSL Project http://www.openssl.org
Development Mailing List   [EMAIL PROTECTED]
Automated List Manager   [EMAIL PROTECTED]



Error Stack Question

2002-01-28 Thread Verdon Walker



We ran into a small piece of code in ssl_rsa.c that is confusing us. In 
SSL_CTX_use_certificate_chain_file(), the following code fragment exists:

ret=SSL_CTX_use_certificate(ctx,x);if (ERR_peek_error() != 
0) ret = 0; /* Key/certificate mismatch doesn't 
imply ret==0 ... */if (ret)...

Isn't this a little strange to ignore the return code of the function 
called and instead peek at the error stack? There is certainly no guarantee that 
any errors on the stack are from the call to use_certificate and even so, why 
check the stack rather than the return code.

This looks like a hack to me. Is there a specific reason it is 
needed?

Thanks for any help.


Verdon Walker(801) 861-2633[EMAIL PROTECTED]Novell Inc., the 
leading provider of Net Services Softwarewww.novell.com