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]

Reply via email to