Skip to content
  1. Aug 16, 2017
  2. Aug 15, 2017
  3. Aug 14, 2017
  4. Aug 13, 2017
  5. Aug 12, 2017
  6. Aug 11, 2017
  7. Aug 10, 2017
  8. Aug 09, 2017
    • Benjamin Kaduk's avatar
      Don't modify resumed session objects · e3743355
      Benjamin Kaduk authored
      
      
      If s->hit is set, s->session corresponds to a session created on
      a previous connection, and is a data structure that is potentially
      shared across other SSL objects.  As such, there are thread-safety
      issues with modifying the structure without taking its lock (and
      of course all corresponding read accesses would also need to take
      the lock as well), which have been observed to cause double-frees.
      
      Regardless of thread-safety, the resumed session object is intended
      to reflect parameters of the connection that created the session,
      and modifying it to reflect the parameters from the current connection
      is confusing.  So, modifications to the session object during
      ClientHello processing should only be performed on new connections,
      i.e., those where s->hit is not set.
      
      The code mostly got this right, providing such checks when processing
      SNI and EC point formats, but the supported groups (formerly
      supported curves) extension was missing it, which is fixed by this commit.
      
      However, TLS 1.3 makes the suppported_groups extension mandatory
      (when using (EC)DHE, which is the normal case), checking for the group
      list in the key_share extension processing.  But, TLS 1.3 only [0] supports
      session tickets for session resumption, so the session object in question
      is the output of d2i_SSL_SESSION(), and will not be shared across SSL
      objects.  Thus, it is safe to modify s->session for TLS 1.3 connections.
      
      [0] A psk_find_session callback can also be used, but the restriction that
      each callback execution must produce a distinct SSL_SESSION structure
      can be documented when the psk_find_session callback documentation is
      completed.
      
      Reviewed-by: default avatarAndy Polyakov <appro@openssl.org>
      (Merged from https://github.com/openssl/openssl/pull/4123)
      e3743355
    • Tomas Mraz's avatar