Skip Menu |
 
Ticket metadata
The Basics
Id: 2167
Status: resolved
Priority: 0/
Queue: OpenSSL-Bugs

Custom Fields
Milestone: (no value)
Subsystem: (no value)
Severity: (no value)
Broken in: (no value)

People
Owner: Nobody in particular
Requestors: Pawel Kolodziej
Aleksey Samsonov
Cc:
AdminCc:

Attachments
0001-Don-t-release-read-buffer-if-read-ahead-left.patch

More about the requestors

Pawel Kolodziej

Comments about this user: No comment entered about this user
Groups this user belongs to
  • Everyone
  • Unprivileged

Aleksey Samsonov

Comments about this user: No comment entered about this user
Groups this user belongs to
  • Everyone
  • Unprivileged

New reminder:
Subject:
Owner:
Due:

Dates
Created: Tue Feb 09 15:38:17 2010
Starts: Not set
Started: Thu Sep 19 13:15:23 2013
Last Contact: Not set
Due: Not set
Closed: Wed Apr 23 08:40:37 2014
Updated: Wed Apr 23 08:40:37 2014 by Ben Laurie



Subject: openssl-1.0.0-beta5 - fails if used from multiple threads and with SSL_MODE_RELEASE_BUFFERS
Date: Mon, 08 Feb 2010 20:57:29 +0100
To: rt@openssl.org
From: Paweł Kołodziej <Pawel.Kolodziej@redefine.pl>
Download (untitled) / with headers
text/plain 1.7k
My server handles incomming SSL connection. If all connections are
handled within one threads evertyhing works fine. In multithread
configurations (pool of threads handle connections) SSL_read() fails
very often when using openssl-1.0.0-beta5 with SSL_MODE_RELEASE_BUFFERS
flag set.

When flag SSL_MODE_RELEASE_BUFFERS is _not_ set then multithread version
works fine. I think it may be some race condition in
SSL_MODE_RELEASE_BUFFERS implementation inside openssl.

Selftest report:
OpenSSL self-test report:

OpenSSL version: 1.0.0-beta5
Last change: Add new -subject_hash_old and -issuer_hash_old options ...
Options: enable-threads no-gmp no-jpake no-krb5 no-md2 no-rc5
no-rfc3779 no-shared no-store no-zlib no-zlib-dynamic static-engine
OS (uname): Linux chat221 2.6.28-11-server #42-Ubuntu SMP Fri Apr
17 02:48:10 UTC 2009 i686 GNU/Linux
OS (config): i686-whatever-linux2
Target (default): linux-elf
Target: linux-elf
Compiler: Using built-in specs.
Target: i486-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
4.3.3-5ubuntu4' --with-bugurl=file:///usr/share/doc/gcc-4.3/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr
--enable-shared --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix --enable-nls
--with-gxx-include-dir=/usr/include/c++/4.3 --program-suffix=-4.3
--enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc
--enable-mpfr --enable-targets=all --with-tune=generic
--enable-checking=release --build=i486-linux-gnu --host=i486-linux-gnu
--target=i486-linux-gnu
Thread model: posix
gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4)

Test passed.


Please let me know if you need more infomrations.
Subject: [openssl.org #2167] [PATCH 1.0.1e] openssl/ssl/s3_pkt.c SSL multiple threads and SSL_MODE_RELEASE_BUFFERS
Date: Wed, 18 Sep 2013 17:47:01 +0000
To: "rt@openssl.org" <rt@openssl.org>
From: "BensonKwok923@air-watch.com" <BensonKwok923@air-watch.com>
Download (untitled) / with headers
text/plain 1.4k
I have found a problem with SSL_MODE_RELEASE_BUFFERS and with
multithreading when using version 1.0.1e. This issue has already been
logged by someone at rt.openssl.org and the ticket # is 2167.

The issue is when the buffer is released by ssl3_release_read_buffer(),
there may still be data left in the buffer (s->s3->rbuf.left != 0). With
single threading, when another read occurs, the same buffer is reused
during a call to ssl3_setup_read_buffer() so the data is still there and
can be read and processed so it works fine. When running with multiple
threads, the buffer is shared in a pool and another thread may have gotten
that buffer already. If the call to ssl3_setup_read_buffer() returns a new
buffer, it assume the data is still there but will run into parsing error
with the record.

The fix I did was to check s->s3->rbuf.left and only call
ssl3_release_read_buffer() when s->s3->rbuf.left is zero. This delays the
release of the buffer until all the data has processed. The fix seems to
fix the issue.

Here is a patch of the changes:


--- s3_pkt_original.c 2013-02-11 10:26:04 -0500
+++ s3_pkt.c 2013-09-18 10:15:47 -0400
@@ -1055,7 +1055,7 @@
{
s->rstate=SSL_ST_READ_HEADER;
rr->off=0;
- if (s->mode & SSL_MODE_RELEASE_BUFFERS)
+ if (s->mode & SSL_MODE_RELEASE_BUFFERS && s->s3->rbuf.left
== 0)
ssl3_release_read_buffer(s);
}
}





Best Regards,

Benson Kwok
Development
Www.air-watch.com
Subject: [PATCH] fix: don't release read buffer if read-ahead left
Date: Mon, 17 Feb 2014 08:03:29 +0400
To: rt@openssl.org
From: Aleksey Samsonov <s4ms0n0v@gmail.com>
Download (untitled) / with headers
text/plain 393b
Hello,

Attached patch solving the problem with OpenSSL 1.0.0 and more recent
versions, including recent git snapshot.

Error: SSL_read() failed (SSL: error:1408F119:SSL
routines:SSL3_GET_RECORD:decryption failed or bad record mac)

See more information to reproduce error:
http://trac.nginx.org/nginx/ticket/215
http://mailman.nginx.org/pipermail/nginx-devel/2013-October/004385.html

Thanks

Message body is not shown because sender requested not to inline it.

Subject: Re: [openssl.org #3265] AutoReply: [PATCH] fix: don't release read buffer if read-ahead left
Date: Wed, 26 Mar 2014 00:29:20 +0400
To: rt@openssl.org
From: Aleksey Samsonov <s4ms0n0v@gmail.com>
Download (untitled) / with headers
text/plain 1.4k
Hello,

Could you please review this very simple patch? But patch solving real
problem: release a buffer with contained read-ahead data and after
next read (alloc new buffer) we lose a chunk of flow of read data, so
error "SSL3_GET_RECORD:decryption failed or bad record mac".


Thanks,
Aleksey


2014-02-17 12:15 GMT+04:00 The default queue via RT <rt@openssl.org>:
Show quoted text
>
> Greetings,
>
> This message has been automatically generated in response to the
> creation of a trouble ticket regarding:
> "[PATCH] fix: don't release read buffer if read-ahead left",
> a summary of which appears below.
>
> There is no need to reply to this message right now. Your ticket has been
> assigned an ID of [openssl.org #3265].
>
> Please include the string:
>
> [openssl.org #3265]
>
> in the subject line of all future correspondence about this issue. To do so,
> you may reply to this message.
>
> Thank you,
> rt@openssl.org
>
> -------------------------------------------------------------------------
> Hello,
>
> Attached patch solving the problem with OpenSSL 1.0.0 and more recent
> versions, including recent git snapshot.
>
> Error: SSL_read() failed (SSL: error:1408F119:SSL
> routines:SSL3_GET_RECORD:decryption failed or bad record mac)
>
> See more information to reproduce error:
> http://trac.nginx.org/nginx/ticket/215
> http://mailman.nginx.org/pipermail/nginx-devel/2013-October/004385.html
>
> Thanks
>
Subject: [openssl.org #2167] [PATCH 1.0.1e] openssl/ssl/s3_pkt.c SSL multiple threads and SSL_MODE_RELEASE_BUFFERS
Date: Wed, 23 Apr 2014 09:10:26 +0200
To: rt@openssl.org
From: Petter Reinholdtsen <pere@hungry.com>
Download (untitled) / with headers
text/plain 319b

Why is this issue still open? Something wrong with the patch?

The problem described is assigned CVE-2010-5298 and further described in
<URL: http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2010-5298 >
and <URL: https://security-tracker.debian.org/tracker/CVE-2010-5298 >.

--
Happy hacking
Petter Reinholdtsen
Subject: Re: [openssl.org #2167] [PATCH 1.0.1e] openssl/ssl/s3_pkt.c SSL multiple threads and SSL_MODE_RELEASE_BUFFERS
Date: Wed, 23 Apr 2014 09:30:03 +0200
To: rt@openssl.org
From: Petter Reinholdtsen <pere@hungry.com>
Download (untitled) / with headers
text/plain 462b
[Petter Reinholdtsen]
Show quoted text
> Why is this issue still open? Something wrong with the patch?

I guess not, as I just noticed it was commited today in
<URL: http://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=94d1f4b0f3d262edf1cf7023a01d5404945035d5 >.

Thank you.

This issue seem to be reported also in
<URL: https://rt.openssl.org/Ticket/Display.html?id=3265 >, I guess
these tickets should be merged before closing.

--
Happy hacking
Petter Reinholdtsen