Bug#895035: osc: crashes with memory corruption when using new libssl1.1

[Simon McVittie]

Control: tags -1 + patch fixed-upstream

On Wed, 02 May 2018 at 17:19:20 +0100, Simon McVittie wrote:
> * https://github.com/openSUSE/osc/issues/398
> 
>   """
>   - m2crypto 0.29 does no SSL_free(...) (which is fixed in 0.30)
> (that's why this bug is not triggered with m2crypto 0.29)
>   - there's a "bug" in ssl_update_cache cache in openssl 1.1.0h (in
> short: the session is not put in the session cache...)
>   - osc currently relies on the fact that the session is in the session
> cache (or more precisely, that there are at least two references to
> the SSL_SESSION), which is, of course, a bug in osc
>   """

The osc crash appears to be fixable by the patch that was applied
upstream (see attached Disable-ssl-session-resumption.patch). If I'm
understanding the commit message correctly, strictly speaking this was
already an osc bug (it made bad assumptions about session caching),
but the regression in openssl changed its impact from "might crash in
rare cases" to "crashes every time".

> * https://github.com/openssl/openssl/pull/5967
> 
>   """
>   Commit d316cdc introduced some extra
>   checks into the session-cache update procedure, intended to prevent
>   the caching of sessions whose resumption would lead to a handshake
>   failure, since if the server is authenticating the client, there needs to
>   be an application-set "session id context" to match up to the authentication
>   context. While that change is effective for its stated purpose, there
>   was also some collatoral damage introduced along with the fix -- clients
>   that set SSL_VERIFY_PEER are not expected to set an sid_ctx, and so
>   their usage of session caching was erroneously denied.
> 
>   Fix the scope of the original commit by limiting it to only acting
>   when the SSL is a server SSL.
>   """

Sorry for the delay in testing this.

Applying openssl commit c84f7d9251446bf76c179bf5da31f25944f4b975
(and reverting osc to the version that crashes) seems
to be another way to address this. See attached
c84f7d9251446bf76c179bf5da31f25944f4b975.patch.

Regards,
smcv
From: Marcus Huewe 
Date: Tue, 8 May 2018 14:23:08 +0200
Subject: Disable ssl session resumption

The old code could potentially yield to a use-after-free situation,
which results in UB. For this, consider the following scenario, where
osc performs several HTTPS requests (assumption: the server supports
ssl session resumption):

- HTTPS Request 1:
  * a new SSL *s connection is established, which also creates a new
SSL_SESSION *ss => ss->references == 1
  * once the handshake is done, the ss is put into the session cache
(see ssl_update_cache) => ss->references == 2
  - osc saves the session ss in a class variable
  - s is SSL_free()d, which calls SSL_SESSION_free => ss->references == 1

- HTTPS Request 2:
  * setup a new SSL *s connection that reuses the saved session ss
=> ss->references == 2
  * once the handshake is done, ssl_update_cache is called, which is a
NOP, because s->hit == 1 (that is, the session was resumed)
  * osc saves the session ss in a class variable
  * s is SSL_free()d, which calls SSL_SESSION_free => ss->references == 1

...

> 2 hours later (see tls1_default_timeout)

...

- HTTPS Request 256:
  * setup a new SSL *s connection that reuses the saved session ss
=> ss->references == 2
  * once the handshake is done, ssl_update_cache is called, but is
_no_ NOP anymore
  * ssl_update_cache flushes the session cache (this is done every
255/256 (depending on the way we count) connections) => ss is
SSL_SESSION_free()d => ss->references == 1
  * osc saves the session ss in a class variable
  * s is SSL_free()d, which calls SSL_SESSION_free:
since ss->references == 1, ss is eventually free()d

- HTTPS Request 257:
  * setup a new SSL *s connection that reuses the saved session ss

Since ss does not exist anymore, the remaining program execution is UB.

(Note: SSL_free(...) is _NOT_ called, if M2Crypto 0.29 is used.
M2Crypto 0.30 calls SSL_free(...) again.)

Due to a bug in OpenSSL_1_1_0h (see openssl commit 8e405776858) the
scenario from above can be triggered with exactly 2 HTTPS requests (the
SSL_SESSION is not cached, because we configured SSL_VERIFY_PEER, but
no sid_ctx was set). This is fixed in openssl commit c4fa1f7fc01.

In order to reliably reuse a session, we probably need to listen to the
session cache changes. Such callbacks could be registered via
SSL_CTX_sess_set_new_cb and/or SSL_CTX_sess_set_remove_cb, but both
functions are not provided by M2Crypto. Another idea is to directly utilize
the session cache, but this also has to be implemented in M2Crypto first.
Yet another approach is to retrieve the session via SSL_get1_session, which
increases the session's refcnt, but this also needs to be implemented in
M2Crypto first (if we choose to use this approach, we also have to make
sure that we eventually free the session manually...).

Fixes: #398 ("SIGSEGV on \"osc commit\"")

Bug#895035: osc: crashes with memory corruption when using new libssl1.1

[Simon McVittie]

On Wed, 02 May 2018 at 18:34:35 +0200, Kurt Roeckx wrote:
> On Wed, May 02, 2018 at 05:19:20PM +0100, Simon McVittie wrote:
> > * https://github.com/openssl/openssl/pull/5967
>
> Is it urgunt to fix this in testing/unstable?

Probably not, unless it makes other packages regress? The upstream osc
bug (if it's the same thing) suggests that osc is at least partially at
fault here.

I'll try to test the openssl patch tomorrow and see whether that fixes
osc, then you'll have that data point available at least.

smcv

Bug#895035: [Pkg-openssl-devel] Bug#895035: osc: crashes with memory corruption when using new libssl1.1

[Kurt Roeckx]

On Wed, May 02, 2018 at 07:26:02PM +0200, Sebastian Andrzej Siewior wrote:
> On 2018-05-02 18:34:35 [+0200], Kurt Roeckx wrote:
> > On Wed, May 02, 2018 at 05:19:20PM +0100, Simon McVittie wrote:
> > > * https://github.com/openssl/openssl/pull/5967
> > > 
> > >   """
> > >   Commit d316cdc introduced some extra
> > >   checks into the session-cache update procedure, intended to prevent
> > >   the caching of sessions whose resumption would lead to a handshake
> > >   failure, since if the server is authenticating the client, there needs 
> > > to
> > >   be an application-set "session id context" to match up to the 
> > > authentication
> > >   context. While that change is effective for its stated purpose, there
> > >   was also some collatoral damage introduced along with the fix -- clients
> > >   that set SSL_VERIFY_PEER are not expected to set an sid_ctx, and so
> > >   their usage of session caching was erroneously denied.
> > > 
> > >   Fix the scope of the original commit by limiting it to only acting
> > >   when the SSL is a server SSL.
> > >   """
> > 
> > Is it urgunt to fix this in testing/unstable?
> 
> If he is sure that this fixes his issue then I don't mind doing an
> upload. I can even prepare a 1.1.0h-2 with this patch included.
> [unless upstream plans a release soon]

There are no plans, currently I think 1.1.1 will be the next release.


Kurt

Bug#895035: [Pkg-openssl-devel] Bug#895035: osc: crashes with memory corruption when using new libssl1.1

[Sebastian Andrzej Siewior]

On 2018-05-02 18:34:35 [+0200], Kurt Roeckx wrote:
> On Wed, May 02, 2018 at 05:19:20PM +0100, Simon McVittie wrote:
> > * https://github.com/openssl/openssl/pull/5967
> > 
> >   """
> >   Commit d316cdc introduced some extra
> >   checks into the session-cache update procedure, intended to prevent
> >   the caching of sessions whose resumption would lead to a handshake
> >   failure, since if the server is authenticating the client, there needs to
> >   be an application-set "session id context" to match up to the 
> > authentication
> >   context. While that change is effective for its stated purpose, there
> >   was also some collatoral damage introduced along with the fix -- clients
> >   that set SSL_VERIFY_PEER are not expected to set an sid_ctx, and so
> >   their usage of session caching was erroneously denied.
> > 
> >   Fix the scope of the original commit by limiting it to only acting
> >   when the SSL is a server SSL.
> >   """
> 
> Is it urgunt to fix this in testing/unstable?

If he is sure that this fixes his issue then I don't mind doing an
upload. I can even prepare a 1.1.0h-2 with this patch included.
[unless upstream plans a release soon]

> Kurt

Sebastian

Bug#895035: [Pkg-openssl-devel] Bug#895035: osc: crashes with memory corruption when using new libssl1.1

[Kurt Roeckx]

On Wed, May 02, 2018 at 05:19:20PM +0100, Simon McVittie wrote:
> * https://github.com/openssl/openssl/pull/5967
> 
>   """
>   Commit d316cdc introduced some extra
>   checks into the session-cache update procedure, intended to prevent
>   the caching of sessions whose resumption would lead to a handshake
>   failure, since if the server is authenticating the client, there needs to
>   be an application-set "session id context" to match up to the authentication
>   context. While that change is effective for its stated purpose, there
>   was also some collatoral damage introduced along with the fix -- clients
>   that set SSL_VERIFY_PEER are not expected to set an sid_ctx, and so
>   their usage of session caching was erroneously denied.
> 
>   Fix the scope of the original commit by limiting it to only acting
>   when the SSL is a server SSL.
>   """

Is it urgunt to fix this in testing/unstable?


Kurt

Bug#895035: osc: crashes with memory corruption when using new libssl1.1

[Simon McVittie]

On Sat, 28 Apr 2018 at 13:03:14 +0200, Harald Welte wrote:
> Package: osc
> Version: 0.162.1-1
> Followup-For: Bug #895035
> 
> I also see "double free or corruption" 100% reproducible when using osc on 
> unstable
> for the past few weeks

I tried rebuilding older openssl versions, and the regression seems to be
between openssl 1.1.0g-2 and 1.1.0h-1.

Perhaps related to one or both of these:

* https://github.com/openSUSE/osc/issues/398

  """
  - m2crypto 0.29 does no SSL_free(...) (which is fixed in 0.30)
(that's why this bug is not triggered with m2crypto 0.29)
  - there's a "bug" in ssl_update_cache cache in openssl 1.1.0h (in
short: the session is not put in the session cache...)
  - osc currently relies on the fact that the session is in the session
cache (or more precisely, that there are at least two references to
the SSL_SESSION), which is, of course, a bug in osc

  Fixing...
  """

* https://github.com/openssl/openssl/pull/5967

  """
  Commit d316cdc introduced some extra
  checks into the session-cache update procedure, intended to prevent
  the caching of sessions whose resumption would lead to a handshake
  failure, since if the server is authenticating the client, there needs to
  be an application-set "session id context" to match up to the authentication
  context. While that change is effective for its stated purpose, there
  was also some collatoral damage introduced along with the fix -- clients
  that set SSL_VERIFY_PEER are not expected to set an sid_ctx, and so
  their usage of session caching was erroneously denied.

  Fix the scope of the original commit by limiting it to only acting
  when the SSL is a server SSL.
  """

Regards,
smcv

Bug#895035: osc: crashes with memory corruption when using new libssl1.1

[Kurt Roeckx]

On Fri, Apr 06, 2018 at 07:54:44PM +0100, Simon McVittie wrote:
> On Fri, 06 Apr 2018 at 19:44:18 +0200, Kurt Roeckx wrote:
> > On Fri, Apr 06, 2018 at 01:58:03PM +0100, Simon McVittie wrote:
> > > This is probably a bug in libssl1.1 or in python-m2crypto, but I'm
> > > reporting it against osc for now, because that's the only place I know
> > > how to reproduce it at the moment. X-Debbugs-Cc'd to the lower-level
> > > packages' maintainers.
> > 
> > Can you run it under valgrind?
> 
> There's a lot of Python-related noise, but yes. It looks like a
> use-after-free in m2crypto, maybe?

It at least doesn't look like an OpenSSL issue at first look.


Kurt

Bug#895035: [Pkg-openssl-devel] Bug#895035: osc: crashes with memory corruption when using new libssl1.1

[Kurt Roeckx]

On Fri, Apr 06, 2018 at 01:58:03PM +0100, Simon McVittie wrote:
> Package: osc
> Version: 0.162.1-1
> Severity: grave
> Justification: osc tool becomes mostly unusable
> 
> This is probably a bug in libssl1.1 or in python-m2crypto, but I'm
> reporting it against osc for now, because that's the only place I know
> how to reproduce it at the moment. X-Debbugs-Cc'd to the lower-level
> packages' maintainers.
> 
> Steps to reproduce:
> 
> * have an account on any OBS instance (I used :
>   anyone can register there, but an account is required to use the API)
> * be in a temporary directory
> * rm -fr binaries
> * osc -A https://api.opensuse.org getbinaries openSUSE:Leap:15.0 \
>   hello standard x86_64
>   (or some project/package combination that exists on your OBS)
> 
> Expected result: osc downloads hello into ./binaries
> 
> Actual result: osc usually segfaults in glibc malloc-related functions,
> probably due to memory corruption; sometimes glibc detects the memory
> corruption itself and aborts instead.
> 
> Workaround: Downgrading libssl1.1 to 1.1.0f-3+deb9u2 from stable-security
> makes osc work correctly, so presumably this is a behaviour change
> between 1.1.0f and 1.1.0h, either a regression or something that triggers
> a pre-existing bug in python-m2crypto (or possibly osc).

Can you run it under valgrind?


Kurt

Bug#895035: osc: crashes with memory corruption when using new libssl1.1

[Simon McVittie]

Package: osc
Version: 0.162.1-1
Severity: grave
Justification: osc tool becomes mostly unusable

This is probably a bug in libssl1.1 or in python-m2crypto, but I'm
reporting it against osc for now, because that's the only place I know
how to reproduce it at the moment. X-Debbugs-Cc'd to the lower-level
packages' maintainers.

Steps to reproduce:

* have an account on any OBS instance (I used :
  anyone can register there, but an account is required to use the API)
* be in a temporary directory
* rm -fr binaries
* osc -A https://api.opensuse.org getbinaries openSUSE:Leap:15.0 \
  hello standard x86_64
  (or some project/package combination that exists on your OBS)

Expected result: osc downloads hello into ./binaries

Actual result: osc usually segfaults in glibc malloc-related functions,
probably due to memory corruption; sometimes glibc detects the memory
corruption itself and aborts instead.

Workaround: Downgrading libssl1.1 to 1.1.0f-3+deb9u2 from stable-security
makes osc work correctly, so presumably this is a behaviour change
between 1.1.0f and 1.1.0h, either a regression or something that triggers
a pre-existing bug in python-m2crypto (or possibly osc).

Other file-downloading operations like `osc co` have a similar crash.
Perhaps notably, `osc ls` does not.

Backtrace for memory corruption detected by glibc (with MALLOC_CHECK_=2):

#0  0x7fa978092e7b in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:51
#1  0x7fa978094231 in __GI_abort () at abort.c:79
#2  0x7fa9780d57b7 in __libc_message (action=action@entry=do_abort, 
fmt=fmt@entry=0x7fa9781de0f3 "%s\n")
at ../sysdeps/posix/libc_fatal.c:181
#3  0x7fa9780dbd5a in malloc_printerr (str=str@entry=0x7fa9781dc2fe 
"free(): invalid pointer")
at malloc.c:5350
#4  0x7fa9780dfc0e in free_check (mem=, caller=) at hooks.c:274
#5  0x7fa9774999a9 in SSL_SESSION_free (ss=0x5561d9428070) at 
../ssl/ssl_sess.c:780
#6  0x7fa977499daf in ssl_get_new_session (s=s@entry=0x5561d9430d60, 
session=session@entry=0)
at ../ssl/ssl_sess.c:315
#7  0x7fa97749e05a in tls_construct_client_hello (s=0x5561d9430d60) at 
../ssl/statem/statem_clnt.c:705
#8  0x7fa97749c556 in write_state_machine (s=0x5561d9430d60) at 
../ssl/statem/statem.c:773
#9  0x7fa97749c556 in state_machine (s=0x5561d9430d60, server=0) at 
../ssl/statem/statem.c:404
#10 0x7fa977494c91 in SSL_do_handshake (s=0x5561d9430d60) at 
../ssl/ssl_lib.c:3220
#11 0x7fa96e0cb0f2 in ssl_connect (ssl=ssl@entry=0x5561d9430d60, 
timeout=-1) at SWIG/_m2crypto_wrap.c:8255
#12 0x7fa96e0cb20b in _wrap_ssl_connect (self=, 
args=)
at SWIG/_m2crypto_wrap.c:21441
#13 0x5561d73e4e5a in call_function (oparg=, 
pp_stack=0x7ffcd796bcd0)
at ../Python/ceval.c:4372
#14 0x5561d73e4e5a in PyEval_EvalFrameEx (f=, 
throwflag=)
at ../Python/ceval.c:3009
#15 0x5561d73e241a in PyEval_EvalCodeEx (co=, 
globals=, locals=, args=, 
argcount=, kws=, kwcount=0, defs=0x0, defcount=0, 
closure=0x0) at ../Python/ceval.c:3604
#16 0x5561d73ea661 in fast_function (nk=0, na=, n=, pp_stack=0x7ffcd796beb0, func=) at ../Python/ceval.c:4467

Backtrace for a segfault:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x7f34e77351c8 in _int_malloc (av=av@entry=0x7f34e7a68c40 , 
bytes=bytes@entry=32)
at malloc.c:4028
#1  0x7f34e773659c in __GI___libc_malloc (bytes=32) at malloc.c:3057
#2  0x7f34e6795469 in CRYPTO_zalloc (num=32, file=file@entry=0x7f34e6816be0 
"../crypto/asn1/tasn_new.c", line=line@entry=122) at ../crypto/mem.c:107
#3  0x7f34e66ce817 in asn1_item_embed_new (pval=pval@entry=0x7ffc4a4b75e0, 
it=it@entry=0x7f34e6a9ef40 , embed=embed@entry=0) at 
../crypto/asn1/tasn_new.c:122
#4  0x7f34e66cea97 in ASN1_item_ex_new (pval=pval@entry=0x7ffc4a4b75e0, 
it=it@entry=0x7f34e6a9ef40 ) at ../crypto/asn1/tasn_new.c:39
#5  0x7f34e66cc291 in asn1_item_embed_d2i (pval=pval@entry=0x7ffc4a4b75e0, 
in=in@entry=0x7ffc4a4b75d8, len=, it=0x7f34e6a9ef40 
, tag=, tag@entry=-1, aclass=,
aclass@entry=0, opt=0 '\000', ctx=0x7ffc4a4b77e0, depth=2) at 
../crypto/asn1/tasn_dec.c:305
#6  0x7f34e66cc9a8 in asn1_template_noexp_d2i (val=0x7ffc4a4b77d8, 
in=0x7ffc4a4b7820, len=, tt=tt@entry=0x7f34e6aa71e0 
, opt=, ctx=0x7ffc4a4b77e0, depth=1)
at ../crypto/asn1/tasn_dec.c:591
#7  0x7f34e669 in asn1_template_ex_d2i (val=val@entry=0x7ffc4a4b77d8, 
in=in@entry=0x7ffc4a4b7820, inlen=, tt=0x7f34e6aa71e0 
, opt=opt@entry=0 '\000', 
ctx=ctx@entry=0x7ffc4a4b77e0, depth=1) at ../crypto/asn1/tasn_dec.c:498
#8  0x7f34e66cc251 in asn1_item_embed_d2i (pval=pval@entry=0x7ffc4a4b77d8, 
in=0x7ffc4a4b7820, len=, it=it@entry=0x7f34e6a9ef00 
, tag=tag@entry=-1, aclass=aclass@entry=0, opt=0 '\000', 
ctx=0x7ffc4a4b77e0, depth=1) at ../crypto/asn1/tasn_dec.c:177
#9  0x7f34e66cce0d in ASN1_item_ex_d2i (pval=pval@entry=0x7ffc4a4b77d8, 
in=, len=, it=0x7f34e6a9ef00 
, tag=tag@entry=-1,