1
0
mirror of https://github.com/openSUSE/osc.git synced 2024-11-10 06:46:15 +01:00

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\"")
This commit is contained in:
Marcus Huewe 2018-05-08 14:23:08 +02:00
parent 5877988c7e
commit b730f880cf

View File

@ -174,7 +174,6 @@ class mySSLContext(SSL.Context):
class myHTTPSHandler(M2Crypto.m2urllib2.HTTPSHandler): class myHTTPSHandler(M2Crypto.m2urllib2.HTTPSHandler):
handler_order = 499 handler_order = 499
saved_session = None
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
self.appname = kwargs.pop('appname', 'generic') self.appname = kwargs.pop('appname', 'generic')
@ -204,8 +203,6 @@ class myHTTPSHandler(M2Crypto.m2urllib2.HTTPSHandler):
selector = req.get_selector() selector = req.get_selector()
# End our change # End our change
h.set_debuglevel(self._debuglevel) h.set_debuglevel(self._debuglevel)
if self.saved_session:
h.set_session(self.saved_session)
headers = dict(req.headers) headers = dict(req.headers)
headers.update(req.unredirected_hdrs) headers.update(req.unredirected_hdrs)
@ -218,9 +215,6 @@ class myHTTPSHandler(M2Crypto.m2urllib2.HTTPSHandler):
headers["Connection"] = "close" headers["Connection"] = "close"
try: try:
h.request(req.get_method(), selector, req.data, headers) h.request(req.get_method(), selector, req.data, headers)
s = h.get_session()
if s:
self.saved_session = s
r = h.getresponse() r = h.getresponse()
except socket.error as err: # XXX what error? except socket.error as err: # XXX what error?
err.filename = full_url err.filename = full_url