Use openssl instead of tls in federator http2 client#3051
Merged
akshaymankar merged 21 commits intodevelopfrom Feb 28, 2023
Merged
Use openssl instead of tls in federator http2 client#3051akshaymankar merged 21 commits intodevelopfrom
akshaymankar merged 21 commits intodevelopfrom
Conversation
zebot
added
the
ok-to-test
akshaymankar
force-pushed
the
federation-openssl
branch
3 times, most recently
from
354d687 to
107fd90
Compare
akshaymankar
force-pushed
the
federation-openssl
branch
from
e300573 to
1ca0952
Compare
akshaymankar
force-pushed
the
federation-openssl
branch
from
45e5b55 to
acf427f
Compare
akshaymankar
force-pushed
the
federation-openssl
branch
2 times, most recently
from
4144d9d to
b8ab49e
Compare
Make it clear that this only works with TLS 1.2 as of now
This will prevent reloading in case the files are being updated one by one.
It was broken in a previous commit so it was not testing with a hostname with trailing dot at all.
This ensures that HTTP2 client doesn't close the connection before the response body gets consumed. In current implementation of the HTTP2 client there is a race between the part which consumes the response and "background threads". These background threads are sending and receiving data and they are not supposed to finish unless connection gets abruptly terminated, however, due to the race they get a `Async.cancel` when the response consumer function finishes executing. Before this change, `Codensity` was supposed to ensure that the consumer doesn't finish executing, but I am not sure why it didn't work, changing the code to use CPS fixes this.
…Call" This reverts commit febf71a2f00cb11aafb855e50c8d0c964af9c936. Thanks to @pcapriotti for clarifying that the test was failing because the test was exiting Codensity before making the assertion causing the test to fail.
akshaymankar
force-pushed
the
federation-openssl
branch
from
784eb19 to
80a0987
Compare
pcapriotti
approved these changes
Feb 28, 2023
This was referenced Mar 6, 2023
smatting
added a commit
that referenced
this pull request
Mar 13, 2023
akshaymankar
added a commit
that referenced
this pull request
Mar 13, 2023
battermann
pushed a commit
that referenced
this pull request
Mar 15, 2023
2 tasks
akshaymankar
added a commit
that referenced
this pull request
Mar 21, 2023
* Revert "Revert "Use openssl instead of tls in federator http2 client (#3051)" (#3148)" This reverts commit 615404b. * Ensure that when http2 wants n bytes, we give it n bytes `SSL.read ssl n` doesn't always return `n` bytes, so reading data multiple times is necessary. Upstream PR has been made to warn future users: haskell-cryptography/HsOpenSSL#81 * Add changelog * Remove the IORef read buffer as openssl never returns extra bytes * Read from openssl in tail recursion
lepsa
pushed a commit
to lepsa/wire-server
that referenced
this pull request
Nov 28, 2023
* Use openssl instead of tls in federator http2 client * changelog * Strip trailing dot for hostname validation * Move blessed ciphers close to where context is being built Make it clear that this only works with TLS 1.2 as of now * Check client certificate and private key to ensure they match This will prevent reloading in case the files are being updated one by one. * Add options to ssl context to workaround various bugs https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_options.html#SSL_OP_ALL * Remove leftover debugging code * Ensure test for testing hostname with trailing dot is correct It was broken in a previous commit so it was not testing with a hostname with trailing dot at all. * Remove commented out code for hs-tls * Remove duplicated comment * Slightly better types for CertifiateAndPrivateKeyDoNotMatch * Share code to create ssl context between test and src * Grammar Co-authored-by: Paolo Capriotti <paolo@capriotti.io> * federator: Pass response consumer continuation to discoverAndCall This ensures that HTTP2 client doesn't close the connection before the response body gets consumed. In current implementation of the HTTP2 client there is a race between the part which consumes the response and "background threads". These background threads are sending and receiving data and they are not supposed to finish unless connection gets abruptly terminated, however, due to the race they get a `Async.cancel` when the response consumer function finishes executing. Before this change, `Codensity` was supposed to ensure that the consumer doesn't finish executing, but I am not sure why it didn't work, changing the code to use CPS fixes this. * Remove `-Wno-unused-imports`, perhaps added by mistake * Federator Client: Simplify reading data from SSL * Revert "federator: Pass response consumer continuation to discoverAndCall" This reverts commit febf71a2f00cb11aafb855e50c8d0c964af9c936. Thanks to @pcapriotti for clarifying that the test was failing because the test was exiting Codensity before making the assertion causing the test to fail. * federator-integration: Avoid exiting Codensity too soon * federator: Run all code warpped in `withOpenSSL` * federator-unit-tests: Ensure assertions happen without exiting Codensity * Special handling for reading 0 bytes out of the TLS socket --------- Co-authored-by: Paolo Capriotti <paolo@capriotti.io>
lepsa
pushed a commit
to lepsa/wire-server
that referenced
this pull request
Nov 28, 2023
…#3051)" (wireapp#3148) This reverts commit fd78663.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
changelog.d