Skip to content

Comments

Fix regression with session cache use by clients#5967

Closed
kaduk wants to merge 1 commit intoopenssl:masterfrom
kaduk:sid_ctx
Closed

Fix regression with session cache use by clients#5967
kaduk wants to merge 1 commit intoopenssl:masterfrom
kaduk:sid_ctx

Conversation

@kaduk
Copy link
Contributor

@kaduk kaduk commented Apr 16, 2018

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.

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.
@kaduk kaduk added 1.1.0 branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) labels Apr 16, 2018
@mattcaswell mattcaswell added the approval: done This pull request has the required number of approvals label Apr 25, 2018
@mattcaswell mattcaswell added this to the 1.1.1 milestone Apr 25, 2018
levitte pushed a commit that referenced this pull request May 1, 2018
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.

Reviewed-by: Matt Caswell <[email protected]>
(Merged from #5967)
levitte pushed a commit that referenced this pull request May 1, 2018
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.

Reviewed-by: Matt Caswell <[email protected]>
(Merged from #5967)

(cherry picked from commit c4fa1f7)
@kaduk
Copy link
Contributor Author

kaduk commented May 1, 2018

Pushed to master and 1.1.0, and closing.
Sorry for the long delay in getting this actually committed -- I was travelling and short on time.

@kaduk kaduk closed this May 1, 2018
raspbian-autopush pushed a commit to raspbian-packages/openssl that referenced this pull request May 24, 2018
Commit d316cdcf6d8d6934663278145fe0a8191e14a8c5 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.

Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl/openssl#5967)

(cherry picked from commit c4fa1f7fc016919a5b3d4ea2aa66c77e0cc40c9d)

Gbp-Pq: Name Fix-regression-with-session-cache-use-by-clients.patch
raspbian-autopush pushed a commit to raspbian-packages/openssl that referenced this pull request May 27, 2018
Commit d316cdcf6d8d6934663278145fe0a8191e14a8c5 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.

Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl/openssl#5967)

(cherry picked from commit c4fa1f7fc016919a5b3d4ea2aa66c77e0cc40c9d)

Gbp-Pq: Name Fix-regression-with-session-cache-use-by-clients.patch
matzbot pushed a commit to ruby/ruby that referenced this pull request Aug 9, 2018
Due to a bug in OpenSSL 1.1.0h[1] (it's only in this specific version;
it was introduced just before the release and is already fixed in their
stable branch), the callback set by SSLContext#session_new_cb= does not
get called for clients, making net/http and net/ftp not attempt session
resumption.

Let's disable the affected test cases for now. Another option would be
to fallback to using SSLSocket#session as we did before r64234. But
since only a single version is affected and hopefully a new stable
version containing the fix will be released in near future, I chose not
to add such workaround code to lib/.

[1] openssl/openssl#5967

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64252 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
matzbot pushed a commit to ruby/ruby that referenced this pull request Mar 12, 2019
	net/http, net/ftp: fix session resumption with TLS 1.3

	When TLS 1.3 is in use, the session ticket may not have been sent yet
	even though a handshake has finished. Also, the ticket could change if
	multiple session ticket messages are sent by the server. Use
	SSLContext#session_new_cb instead of calling SSLSocket#session
	immediately after a handshake. This way also works with earlier protocol
	versions.

	net/http, net/ftp: skip SSL/TLS session resumption tests

	Due to a bug in OpenSSL 1.1.0h[1] (it's only in this specific version;
	it was introduced just before the release and is already fixed in their
	stable branch), the callback set by SSLContext#session_new_cb= does not
	get called for clients, making net/http and net/ftp not attempt session
	resumption.

	Let's disable the affected test cases for now. Another option would be
	to fallback to using SSLSocket#session as we did before r64234. But
	since only a single version is affected and hopefully a new stable
	version containing the fix will be released in near future, I chose not
	to add such workaround code to lib/.

	[1] openssl/openssl#5967

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_5@67237 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
hsbt pushed a commit to ruby/net-ftp that referenced this pull request Feb 13, 2020
Due to a bug in OpenSSL 1.1.0h[1] (it's only in this specific version;
it was introduced just before the release and is already fixed in their
stable branch), the callback set by SSLContext#session_new_cb= does not
get called for clients, making net/http and net/ftp not attempt session
resumption.

Let's disable the affected test cases for now. Another option would be
to fallback to using SSLSocket#session as we did before r64234. But
since only a single version is affected and hopefully a new stable
version containing the fix will be released in near future, I chose not
to add such workaround code to lib/.

[1] openssl/openssl#5967

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64252 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
hsbt pushed a commit to ruby/net-http that referenced this pull request Feb 13, 2020
Due to a bug in OpenSSL 1.1.0h[1] (it's only in this specific version;
it was introduced just before the release and is already fixed in their
stable branch), the callback set by SSLContext#session_new_cb= does not
get called for clients, making net/http and net/ftp not attempt session
resumption.

Let's disable the affected test cases for now. Another option would be
to fallback to using SSLSocket#session as we did before r64234. But
since only a single version is affected and hopefully a new stable
version containing the fix will be released in near future, I chose not
to add such workaround code to lib/.

[1] openssl/openssl#5967

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64252 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: done This pull request has the required number of approvals branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants