[openssl.org #359] Calling SSL_read and SSL_write with non-empty error stack may cause an error

2005-04-07 Thread Nils Larsch via RT

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

2003-09-27 Thread Richard Levitte via RT

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

2003-02-03 Thread Bodo Moeller
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

2003-01-31 Thread Bodo Moeller via RT

On Tue, Nov 26, 2002 at 10:44:15PM +0200, Arne Ansper wrote:

 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.

One theory is that applications should not call arbitrary OpenSSL
functions while there is stuff in the error queue.

A second theory is that OpenSSL should always clear the error queue by
calling ERR_clear_error() if stuff left in the error queue might cause
confusion later.

The second theory is nicer, but until someone has patched OpenSSL
appropriately, unfortunately the first theory remains true.


-- 
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

2003-01-31 Thread Richard Levitte - VMS Whacker via RT

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

2003-01-30 Thread Richard Levitte via RT

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

2003-01-30 Thread Lutz Jaenicke via RT

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

2003-01-30 Thread Richard Levitte via RT

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

2002-12-30 Thread Lutz Jaenicke via RT

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

2002-11-26 Thread Arne Ansper via RT



 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

2002-11-25 Thread Lutz Jaenicke via RT

[[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

2002-11-24 Thread Arne Ansper via RT



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]