Skip to content

Conversation

@maskit
Copy link
Member

@maskit maskit commented Jan 12, 2022

ATS crashes if origin session cache is used with BoringSSL, because BoringSSL doesn't allow you to call SSL_set_session after starting handshake.

The crash is not 100% reproducible and I was only able to reproduce it on 9.2.x, but the change should make sense on master as well.

https://www.openssl.org/docs/man1.1.1/man3/SSL_set_session.html

If there is already a session set inside ssl (because it was set with SSL_set_session() before or because the same ssl was already used for a connection), SSL_SESSION_free() will be called for that session. If that old session is still open, it is considered bad and will be removed from the session cache (if used). A session is considered open, if SSL_shutdown(3) was not called for the connection (or at least SSL_set_shutdown(3) was used to set the SSL_SENT_SHUTDOWN state).

https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_set_session

It is an error to call this function after the handshake has begun.

The change is very small if you hide whitespace (indent) changes.

ATS crahses if origin session cache is used with BoringSSL, because BoringSSL
doesn't allow you to call SSL_set_session after starting handshake.
@maskit maskit added this to the 10.0.0 milestone Jan 12, 2022
@maskit maskit requested a review from duke8253 January 12, 2022 02:16
@maskit maskit self-assigned this Jan 12, 2022
@maskit maskit requested a review from bryancall as a code owner January 12, 2022 02:16
@maskit maskit merged commit 53f5869 into apache:master Feb 1, 2022
zwoop pushed a commit that referenced this pull request Feb 2, 2022
ATS crahses if origin session cache is used with BoringSSL, because BoringSSL
doesn't allow you to call SSL_set_session after starting handshake.

(cherry picked from commit 53f5869)
@zwoop
Copy link
Contributor

zwoop commented Feb 2, 2022

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.0 Feb 2, 2022
moonchen pushed a commit to moonchen/trafficserver that referenced this pull request Mar 17, 2022
* asf/9.2.x:
  Updated ChangeLog
  Prevent calling SSL_set_session in the middle of handshake (apache#8600)
  Change the function signature of safe_[get|set]sockopt (apache#8331)
  Revert fixes for apache#8539 (apache#8637)
  Update descriptions of sni.yaml.default (apache#8568)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants