[openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
This should be fixed in 0.9.8 . As we don't want to backport the necessary changes to 0.9.7 I close this ticket. Cheers, Nils __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
[openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
OK, what's the status on this ticket? [bodo - Tue Feb 4 17:30:23 2003]: Arne Ansper [EMAIL PROTECTED]: Like I say, they should only do this if there was an error reported, surely? No. Take a look at the SSL_CTX_use_certificate_chain_file: ret=SSL_CTX_use_certificate(ctx,x); if (ERR_peek_error() != 0) ret = 0; /* Key/certificate mismatch doesn't imply ret==0 ... */ Actually I think this is a bug in SSL_CTX_use_certificate() -- if it intentionally ignores an error returned by X509_check_private_key(), it should call ERR_clear_error(). The reason why I did not fix this when I looked at this some time ago is some rather weird code in ssl_set_cert(), the function used by SSL_CTX_use_certificate() from which X509_check_private_key() is called. (If you look at ssl_set_cert(), you'll see that it switches from SSL_PKEY_DH_RSA to SSKL_PKEY_DH_DSA and the other way around, which does not appear to make much sense.) Investigating this has been on my to do list for a while. Once this has been resolved, the lines if (ERR_peek_error() != 0) ret = 0; /* Key/certificate mismatch doesn't imply ret==0 ... */ can be removed from SSL_CTX_use_certificate_chain_file(). -- Richard Levitte [EMAIL PROTECTED] __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
Arne Ansper [EMAIL PROTECTED]: Like I say, they should only do this if there was an error reported, surely? No. Take a look at the SSL_CTX_use_certificate_chain_file: ret=SSL_CTX_use_certificate(ctx,x); if (ERR_peek_error() != 0) ret = 0; /* Key/certificate mismatch doesn't imply ret==0 ... */ Actually I think this is a bug in SSL_CTX_use_certificate() -- if it intentionally ignores an error returned by X509_check_private_key(), it should call ERR_clear_error(). The reason why I did not fix this when I looked at this some time ago is some rather weird code in ssl_set_cert(), the function used by SSL_CTX_use_certificate() from which X509_check_private_key() is called. (If you look at ssl_set_cert(), you'll see that it switches from SSL_PKEY_DH_RSA to SSKL_PKEY_DH_DSA and the other way around, which does not appear to make much sense.) Investigating this has been on my to do list for a while. Once this has been resolved, the lines if (ERR_peek_error() != 0) ret = 0; /* Key/certificate mismatch doesn't imply ret==0 ... */ can be removed from SSL_CTX_use_certificate_chain_file(). -- Bodo Möller [EMAIL PROTECTED] PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html * TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt * Tel. +49-6151-16-6628, Fax +49-6151-16-6036 __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
In message [EMAIL PROTECTED] on Fri, 31 Jan 2003 15:50:06 +0100 (MET), Bodo Moeller via RT [EMAIL PROTECTED] said: rt A second theory is that OpenSSL should always clear the error queue by rt calling ERR_clear_error() if stuff left in the error queue might cause rt confusion later. The problem is finding out what it's appropriate. Let's not forget that some OpenSSL functions are called from other OpenSSL functions, so this might be tricky. Besides, I'm not sure I agree with that theory in any case. Would libc functions clear errno all the time? -- Richard Levitte \ Spannvägen 38, II \ [EMAIL PROTECTED] Redakteur@Stacken \ S-168 35 BROMMA \ T: +46-8-26 52 47 \ SWEDEN \ or +46-708-26 53 44 Procurator Odiosus Ex Infernis-- [EMAIL PROTECTED] Member of the OpenSSL development team: http://www.openssl.org/ Unsolicited commercial email is subject to an archival fee of $400. See http://www.stacken.kth.se/~levitte/mail/ for more info. __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
[openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
Any more thoughts on this issue? -- Richard Levitte __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
On Thu, Jan 30, 2003 at 10:09:22PM +0100, Richard Levitte via RT wrote: Any more thoughts on this issue? The problem is not yet solved. Using the global error stack as error indicator instead of correctly passing state back via return values is a design flaw. It happend to make problems in the past. I am currently busy as hell, so I will probably not be able to fix it over the next days. Best regards, Lutz -- Lutz Jaenicke [EMAIL PROTECTED] http://www.aet.TU-Cottbus.DE/personen/jaenicke/ BTU Cottbus, Allgemeine Elektrotechnik Universitaetsplatz 3-4, D-03044 Cottbus __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
[openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
OK... [jaenicke - Thu Jan 30 22:21:50 2003]: On Thu, Jan 30, 2003 at 10:09:22PM +0100, Richard Levitte via RT wrote: Any more thoughts on this issue? The problem is not yet solved. Using the global error stack as error indicator instead of correctly passing state back via return values is a design flaw. It happend to make problems in the past. I am currently busy as hell, so I will probably not be able to fix it over the next days. Best regards, Lutz -- Richard Levitte __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
[openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
There was no time to solve this problem before the release of 0.9.7. The ticket is therefore moved forward to 0.9.7a. Best regards, Lutz __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
Re: [openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
Adding ERR_clear_errors() into SSL_read() etc seems to be the correct approach. It is already handled this way in _accept(), _connect(), but not that obvious, because it is found e.g. in ssl3_accept() which is called depending on the method selected. You will often find ERR_clear_errors() combined with clear_sys_error() but obviously not in all occasions. I just checked. Seems that SSL_CTX_use_certificate_chain_file has a same problem. Other uses of ERR_peek_error seem to be immune to the old entries in error stack. Unfortunately it is not obvious enough to simply add it without some further investigation. I will thus put this issue into the 0.9.7 queue and will not consider it for 0.9.6h anymore. 0.9.7 is fine for me. Arne __ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
[openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
[[EMAIL PROTECTED] - Sun Nov 24 19:52:37 2002]:
Hi!
I have a small problem with SSL_get_error. This function starts like
this:
int SSL_get_error(SSL *s,int i)
{
int reason;
unsigned long l;
BIO *bio;
if (i 0) return(SSL_ERROR_NONE);
/* Make things return SSL_ERROR_SYSCALL when doing
SSL_do_handshake
* etc, where we do encode the error */
if ((l=ERR_peek_error()) != 0)
{
if (ERR_GET_LIB(l) == ERR_LIB_SYS)
return(SSL_ERROR_SYSCALL);
else
return(SSL_ERROR_SSL);
}
Now, if I have something in the error stack from previous operations
and I
call SSL_read or SSL_write on non-blocking socket and read or write
returns -1 and sets errno to EAGAIN, then SSL_get_error will return
SSL_ERROR_SSL which makes this error look like an error in SSL
statemachine but it is really not.
Since BIO_f_ssl does not set retry flag for SSL_ERROR_SSL connection
aborts.
Ooops.
One solution to this problem is to make sure that ERR_clear_errors
gets
called every time before you do SSL_read or SSL_write (or
BIO_read/BIO_write). But I think that this is not a very good
approach,
because it relies on the good habbits of the application programmer.
Calling ERR_clear_errors automatically at the start of SSL_read and
SSL_write might hurt the performance and is therefore probably not a
very
good solution.
Adding ERR_clear_errors() into SSL_read() etc seems to be the correct
approach. It is already handled this way in _accept(), _connect(), but
not that obvious, because it is found e.g. in ssl3_accept() which is
called depending on the method selected.
You will often find ERR_clear_errors() combined with clear_sys_error()
but obviously not in all occasions.
Unfortunately it is not obvious enough to simply add it without some
further investigation. I will thus put this issue into the 0.9.7 queue
and will not consider it for 0.9.6h anymore.
I suggest that if the exact kind of the error is important we should
add
an explicit parameter to the lower level functions to return this
information. Or additional status fields to strucutres like SSL.
Even though this seems to be a reasonable proposal, I am afraid that it
would require significant changes to the error handling concept...
Best regards,
Lutz
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
[openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error
Hi!
I have a small problem with SSL_get_error. This function starts like this:
int SSL_get_error(SSL *s,int i)
{
int reason;
unsigned long l;
BIO *bio;
if (i 0) return(SSL_ERROR_NONE);
/* Make things return SSL_ERROR_SYSCALL when doing SSL_do_handshake
* etc, where we do encode the error */
if ((l=ERR_peek_error()) != 0)
{
if (ERR_GET_LIB(l) == ERR_LIB_SYS)
return(SSL_ERROR_SYSCALL);
else
return(SSL_ERROR_SSL);
}
Now, if I have something in the error stack from previous operations and I
call SSL_read or SSL_write on non-blocking socket and read or write
returns -1 and sets errno to EAGAIN, then SSL_get_error will return
SSL_ERROR_SSL which makes this error look like an error in SSL
statemachine but it is really not.
Since BIO_f_ssl does not set retry flag for SSL_ERROR_SSL connection
aborts.
One solution to this problem is to make sure that ERR_clear_errors gets
called every time before you do SSL_read or SSL_write (or
BIO_read/BIO_write). But I think that this is not a very good approach,
because it relies on the good habbits of the application programmer.
Calling ERR_clear_errors automatically at the start of SSL_read and
SSL_write might hurt the performance and is therefore probably not a very
good solution.
I think that the good long term solution is do not rely on ERR_peek_error
inside library, because application can call OpenSSL functions with an
error stack that is in an arbitrary state.
I grepped the (almost) current source tree for ERR_peek_error and
ERR_get_error and found that it's fortunately used in just a very few
places:
apps/req.c: if ((ERR_GET_REASON(ERR_peek_error()) ==
apps/rsa.c: while ((e = ERR_peek_error()) != 0
apps/rsa.c: ERR_get_error(); /* remove e from error stack
*/
apps/rsa.c: if (r == -1 || ERR_peek_error() != 0) /* should happen only if
r == -1 */
ssl/ssl_lib.c: if ((l=ERR_peek_error()) != 0)
ssl/ssl_rsa.c: if (ERR_peek_error() != 0)
ssl/ssl_rsa.c: err = ERR_peek_error();
ssl/ssl_rsa.c: (void) ERR_get_error();
test/bntest.c: while ((l=ERR_get_error()))
test/rsa_test.c:while ((l = ERR_get_error()) != 0)
crypto/asn1/a_d2i_fp.c: e=ERR_GET_REASON(ERR_peek_error());
crypto/asn1/a_d2i_fp.c: ERR_get_error(); /* clear error */
crypto/bn/bntest.c: while ((l=ERR_get_error()))
crypto/objects/obj_dat.c: ERR_get_error();
crypto/pem/pem_info.c: error=ERR_GET_REASON(ERR_peek_error());
crypto/pem/pem_lib.c: if(ERR_GET_REASON(ERR_peek_error()) ==
crypto/pkcs7/bio_ber.c: (ERR_GET_REASON(ERR_peek_error()) ==
ASN1_R_TOO_LONG))
crypto/pkcs7/bio_ber.c: ERR_get_error(); /* clear the error */
crypto/rand/md_rand.c: err = ERR_peek_error();
crypto/rand/md_rand.c: (void)ERR_get_error();
crypto/rsa/rsa_test.c:while ((l = ERR_get_error()) != 0)
crypto/x509/by_file.c: if ((ERR_GET_REASON(ERR_peek_error())
==
crypto/x509/by_file.c: if ((ERR_GET_REASON(ERR_peek_error())
==
I suggest that if the exact kind of the error is important we should add
an explicit parameter to the lower level functions to return this
information. Or additional status fields to strucutres like SSL.
Arne
__
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
