Improve MITM detection during session establishment: #4395
Conversation
intelliot
added
Request for Comments
Feature Request
|
|
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 with 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').
|
Updated this branch to introduce directional cookies and to strongly bind the |
| It is used to secure the peer link against certain types of attack. For more | ||
| details see the section titled "Session Signature" below. |
There was a problem hiding this comment.
There is no section titled "Session Signature" anymore. Should probably be "Session Security".
|
|
||
| { | ||
| auto const sig = signDigest( | ||
| app.nodeIdentity().first, app.nodeIdentity().second, sharedValue); | ||
| h.insert("Session-Signature", base64_encode(sig.data(), sig.size())); | ||
| } | ||
|
|
||
| h.insert("FOO", to_string(ekm)); |
There was a problem hiding this comment.
Did you mean to include the raw ekm?
There was a problem hiding this comment.
This looks like some temporary/debug code that survived into the final build.
There was a problem hiding this comment.
That was my assumption, too. But it does need to be removed.
|
|
||
| This value is generated in a secure fashion and is never communicated over | ||
| the wire, even over an encrypted connection. Used properly, it can help to | ||
| detect and prevent preventing active MITM attacks. |
There was a problem hiding this comment.
Typo: prevent preventing -> prevent
|
LGTM. There's the reference to a renamed section by the old section name and the line of debug code that got left in, but other than that, no issues except: This code currently violates RFC 5705:
We can register our two labels, use experimental labels, or use existing registered labels whose uses do not conflict with ours. |
|
@nbougalis at your convenience, please respond to the suggestions by ximinez |
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 with 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.
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').
High Level Overview of Change
Context of Change
Type of Change
Test Plan
Running this against servers with and without this change and verifying that peer connections are established (e.g. by using the
peerscommand) should be sufficient. Ideally, in addition to Linux, this should also be tested against servers running on the "compiles-on-but-it's-not supported" platforms: MacOS and Windows.