Rollup 4172 4395 4412 #4517
Closed
Rollup 4172 4395 4412 #4517
Conversation
This file contains 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
The existing thread pool code uses several layers of indirection which uses a custom lock-free stack, and offers functionality that supports features that are never used (e.g. the ability to dynamically adjust the number of threads in the pool). This refactoring aims to simplify the code, making it easier to reason about (although lock-free multi-threaded code is always tricky) what is happening, and reduce the latency of the thread pool internals.
This commit cleans up and modernizes the JobQueue but does not change the queueing logic. It focuses on simplifying the code by eliminating awkward code constructs, like "invalid jobs" and the need for default constructors. It leverages modern C++ to initialize tables and data structures at compile time and replaces `std:map` instances with directly indexed arrays. Lastly, it restructures the load tracking infrastructure and reduces the need for dynamic memory allocations by supporting move semantics and value types.
The existing code attempted to restrict the instantiation of `Coro` only to a subset of helper functions, by using the `Coro_create_t` helper structure. But the structure was public, which limited the effectiveness of this method. This commit uses a private type, fixing the issue.
This refactor was primarily aimed at reducing the size of objects derived from TimeoutCounter, by improving packing of structures. Other potential improvements also surfaced during this process and where implemented.
Even with TLS encrypted connections, it is possible for a determined attacker to mount certain types of relatively easy man-in-the-middle attacks which, if successful, could allow an attacker to tamper with messages exchanged between endpoints. The risk can be mitigated if each side has a certificate issued by a CA that the other side trusts. In the context of a decentralized and permissionless network, this is neither reasonable nor desirable. To prevent this problem all we need is to allow the two endpoints, A and B, to be able to independently verify that they are connected to each other over a single end-to-end TLS session, instead of separate TLS sessions which the attacker bridges. The protocol level handshake implements this security check by using digital signatures: each endpoint derives a fingerprint from the TLS session, which it signs with the private key associated with its own node identity. This strongly binds the TLS session to the identities of the two endpoints of the session. This commit introduces a new fingerprint derivation that uses modern and standardized TLS exporter functionality, instead of the existing derivation whch uses OpenSSL APIs that are non-standard, and derives different "incoming" and "outgoing" security cookies. Lastly, this commit refines the "self-connection" check to allow for the detection of accidental instances of node identity sharing. This check was first introduced with #4195 but was partially reverted due to a bug with #4438. By using distinct security cookies for incoming and outgoing connections, an attacker is no longer able to claim the identity of its peer by echoing its security cookie. The change is backwards compatible and servers with this commit will still generate and verify old-style fingerprints, in addition to the new style fingerprints. For a fuller discussion on this topic, please see: openssl/openssl#5509 #2413 This commit was previously introduced as #3929, which was closed. If merged, it also fixes #2413 (which had been closed as a 'WONTFIX').
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is just a rollup of #4172, #4395 and #4412, resolving merge conflicts between them. Please review the individual PRs, but merge this version instead, when ready.