Skip Menu |
 

This queue is for tickets about the Net-SSLeay CPAN distribution.

Report information
The Basics
Id: 48132
Status: resolved
Worked: 20 min
Priority: 0/
Queue: Net-SSLeay

People
Owner: MIKEM [...] cpan.org
Requestors: eli [...] dvns.com
Cc: mj [...] ucw.cz
AdminCc:

Bug Information
Severity: Important
Broken in:
  • 1.31_01
  • 1.31_02
  • 1.32
  • 1.33_01
  • 1.34
  • 1.35
Fixed in: (no value)

Attachments
netssley-ssl_write_all-error_checking.patch



CC: mj [...] ucw.cz
Subject: Looping connection fix by ssl error checking
Download (untitled) / with headers
text/plain 781b
Outstanding bug (see 44170 requested by marm) where a disconnect socket that occurs during a ssl_write_all loops forever attempting to retry. However marm's patch breaks the contract of attempting again on -1 under certain conditions. After in-depth study the SSL_write and SSL_get_error man pages, under certain conditions we should continue to loop and call the same IO function again. It appears that this patch would break this by always reporting the errno if the ERR_get_errors did not have return values. The correct way is to check for SSL_get_errors via get_errors, and ignore certain errors that occur during session renegotiation that are non fatal to attempting to write the data again. I believe my attached patch should be applied instead, and marm's backed-out.
Subject: netssley-ssl_write_all-error_checking.patch
diff -r -U 3 -N Net-SSLeay-1.35/lib/Net/SSLeay.pm Net-SSLeay-1.35_ssl_write_all_checkSSLErrors/lib/Net/SSLeay.pm --- Net-SSLeay-1.35/lib/Net/SSLeay.pm 2008-07-24 17:09:26.000000000 -0500 +++ Net-SSLeay-1.35_ssl_write_all_checkSSLErrors/lib/Net/SSLeay.pm 2009-07-23 18:06:52.000000000 -0500 @@ -1924,11 +1924,122 @@ if (defined $wrote && ($wrote > 0)) { # write_partial can return -1 $written += $wrote; $to_write -= $wrote; - } + } else { + if (defined $wrote) { + # check error conditions via SSL_get_error per man page + if ( my $sslerr = get_error($ssl, $wrote) ) { + my $errstr = ERR_error_string($sslerr); + my $errname = ''; + SWITCH: { + $sslerr == constant("ERROR_NONE") && do { + # according to map page SSL_get_error(3ssl): + # The TLS/SSL I/O operation completed. + # This result code is returned if and only if ret > 0 + # so if we received it here complain... + warn "ERROR_NONE unexpected with invalid return value!" + if $trace; + $errname = "SSL_ERROR_NONE"; + }; + $sslerr == constant("ERROR_WANT_READ") && do { + # operation did not complete, call again later, so do not + # set errname and empty err_que since this is a known + # error that is expected but, we should continue to try + # writing the rest of our data with same io call and params. + warn "ERROR_WANT_READ (TLS/SSL Handshake, will continue)\n" + if $trace; + print_errs('SSL_write(want read)'); + last SWITCH; + }; + $sslerr == constant("ERROR_WANT_WRITE") && do { + # operation did not complete, call again later, so do not + # set errname and empty err_que since this is a known + # error that is expected but, we should continue to try + # writing the rest of our data with same io call and params. + warn "ERROR_WANT_WRITE (TLS/SSL Handshake, will continue)\n" + if $trace; + print_errs('SSL_write(want write)'); + last SWITCH; + }; + $sslerr == constant("ERROR_ZERO_RETURN") && do { + # valid protocol closure from other side, no longer able to + # write, since there is no longer a session... + warn "ERROR_ZERO_RETURN($wrote): TLS/SSLv3 Closure alert\n" + if $trace; + $errname = "SSL_ERROR_ZERO_RETURN"; + last SWITCH; + }; + $sslerr == constant("ERROR_SSL") && do { + # library/protocol error + warn "ERROR_SSL($wrote): Library/Protocol error occured\n" + if $trace; + $errname = "SSL_ERROR_SSL"; + last SWITCH; + }; + $sslerr == constant("ERROR_WANT_CONNECT") && do { + # according to man page, should never happen on call to + # SSL_write, so complain, but handle as known error type + warn "ERROR_WANT_CONNECT: Unexpected error for SSL_write\n" + if $trace; + $errname = "SSL_ERROR_WANT_CONNECT"; + last SWITCH; + }; + $sslerr == constant("ERROR_WANT_ACCEPT") && do { + # according to man page, should never happen on call to + # SSL_write, so complain, but handle as known error type + warn "ERROR_WANT_ACCEPT: Unexpected error for SSL_write\n" + if $trace; + $errname = "SSL_ERROR_WANT_ACCEPT"; + last SWITCH; + }; + $sslerr == constant("ERROR_WANT_X509_LOOKUP") && do { + # operation did not complete: waiting on call back, + # call again later, so do not set errname and empty err_que + # since this is a known error that is expected but, we should + # continue to try writing the rest of our data with same io + # call parameter. + warn "ERROR_WANT_X509_LOOKUP: (Cert Callback asked for in ". + "SSL_write will contine)\n" if $trace; + print_errs('SSL_write(want x509'); + last SWITCH; + }; + $sslerr == constant("ERROR_SYSCALL") && do { + # some IO error occured. According to man page: + # Check retval, ERR, fallback to errno + if ($wrote==0) { # EOF + warn "ERROR_SYSCALL($wrote): EOF violates protocol.\n" + if $trace; + $errname = "SSL_ERROR_SYSCALL(EOF)"; + } else { # -1 underlying BIO error reported. + # check error que for details, don't set errname since we + # are directly appending to errs + my $chkerrs = print_errs('SSL_write (syscall)'); + if ($chkerrs) { + warn "ERROR_SYSCALL($wrote): Have errors\n" if $trace; + $errs .= "ssl_write_all $$: 1 - ERROR_SYSCALL($wrote,". + "$sslerr,$errstr,$!)\n$chkerrs"; + } else { # que was empty, use errno + warn "ERROR_SYSCALL($wrote): errno($!)\n" if $trace; + $errs .= "ssl_write_all $$: 1 - ERROR_SYSCALL($wrote,". + "$sslerr) : $!\n"; + } + } + last SWITCH; + }; + warn "Unhandled val $sslerr from SSL_get_error(SSL,$wrote)\n" + if $trace; + $errname = "SSL_ERROR_?($sslerr)"; + } # end of SWITCH block + if ($errname) { # if we had an errname set add the error + $errs .= "ssl_write_all $$: 1 - $errname($wrote,$sslerr,". + "$errstr,$!)\n"; + } + } # endif on have SSL_get_error val + } # endif on $wrote defined + } # endelse on $wrote > 0 $vm = $trace>2 && $linux_debug ? (split ' ', `cat /proc/$$/stat`)[22] : 'vm_unknown'; warn " written so far $wrote:$written bytes (VM=$vm)\n" if $trace>2; - + # append remaining errors in que and report if errs exist $errs .= print_errs('SSL_write'); return (wantarray ? (undef, $errs) : undef) if $errs; }
Download (untitled) / with headers
text/plain 126b
Thanks for your patch. The previous patch has been backed out as suggested and yours applied. Now available in SVN. Cheers.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.