#1249 OpenSSL not used correctly?

closed-fixed
openssl (9)
6
2013-06-23
2013-06-22
Nach M. S.
No

I just upgraded to 7.31.0, and testing my software against it, I see some of my SSL/TLS unit tests are now failing with:
* SSL read: error:00000000:lib(0):func(0):reason(0), errno 0
* Closing connection 1

This doesn't look right, as it seems to be failing with no error whatsoever.

I bisected the issue to this commit: https://github.com/bagder/curl/commit/520833cbe1601feed1c6473bd28c4c894e7ee63e

As we all know, OpenSSL documentation is woefully incomplete, and the library has many bizare issues. But after looking at whatever documentation I can find, the best I can come up with is that in some cases, OpenSSL's SSL_read() returns zero, because the underlying read() or recv() returned 0, and that this is not an error, yet curl's existing code does not handle this.

I'm including a test case where in previous versions, curl seems to work fine, and in newer versions, curl fails.

I also whipped up a patch which I'm including for curl's OpenSSL code to actually check that errno from OpenSSL is indeed reporting an error instead of assuming it. With this patch, I no longer have SSL/TLS problems with my test case.

curl 7.31.0 (x86_64-unknown-linux-gnu) libcurl/7.31.0 OpenSSL/1.0.1e zlib/1.2.8
Protocols: file http https
Features: IPv6 Largefile NTLM NTLM_WB SSL libz TLS-SRP

1 Attachments

Discussion

  • Nach M. S.

    Nach M. S. - 2013-06-22

    Sorry, forgot my test case.

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-06-23

    Thanks for your report. However, if SSL_read() returned a negative number we really shouldn't just return that number if ERR_get_error() for any reason wouldn't say it is an error!

    I suggest we do this minor variation of your patch. (I also whitespace edited your patch as checksrc.pl complained)

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-06-23
    • labels: --> openssl
    • assigned_to: Daniel Stenberg
    • Priority: 5 --> 6
     
  • Nach M. S.

    Nach M. S. - 2013-06-23

    I very much agree with your variation of the patch.

    I applied your patch and rebuilt curl, and now all my SSL/TLS unit tests are passing.

     
  • Nach M. S.

    Nach M. S. - 2013-06-23

    Okay, I upgraded curl on all our platforms with this patch included, and all our unit tests which use curl pass everywhere.

     
  • Daniel Stenberg

    Daniel Stenberg - 2013-06-23
    • status: open --> closed-fixed
     
  • Daniel Stenberg

    Daniel Stenberg - 2013-06-23

    Thanks, merged and pushed as commit 8a7a277c086199b3. Case closed!

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks