Implement mTLS#21
Draft
cblgh wants to merge 20 commits into
Draft
Conversation
mTLs implementation does not follow spec yet but is the state of mTLS prior to it being removed from the previous sprint. most of the work has consisted in spec work so far :)
… connection flow the QR code flow has been disabled and removed from being visible. however QR code logic has been kept in case we reintroduce it at a later stage during this work (still in the air?) this commit has started to add changes to the connection flow as reflected in the new designs, including the new spinner modal graphic. this commit acts as a checkpoint before more widespread changes are attempted to be implemented
60b624c to
c4aac16
Compare
…npm audit warnings
If needed we can always retrieve the logic by looking back at previous commits and picking out the parts we need :) But until that day it is better to remove all of the logic relating to it to decrease the attack surface of hash verification.
note: in this commit the behaviour is almost exceedingly strict - once a request has come in which has a certificate, we this certificate's fingerprint as the fingerprint candidate and ignore + disallow any other requests from being considered to bring a certificate. benefit: it limits some attacks by swooping in and sending a malicious register request right when the other party press confirm & connect drawback: it opens up an attack where an active malicious attacker on the network could prohibit any other legitimate connections from being established by virtue of spamming the ping route with their own certificate (which ofc will not present a match when sender + reciever verify the sender fingerprint candidate). -- also in this commit: fixed a couple glaring interface issues that are completely unrelated
…hosen In order to minimize the attack surface of sender certificate hashes being either maliciously denied (attacker spamming known routes with their own certificate that would not match sender) or maliciously timed (timing a request with stolen credentials to make the receiver pin the wrong certificate) we in this commit implement a locking scheme. It works as follows: In each request that comes in pre-mTLS being established, and if the request has a certificate attached, then its fingerprint is calculated and saved as a potential candidate for a sender certificate hash. This candidate can be chosen many times. However, on successfully processing an authorised register request (PIN valid, nonce unseen), the candidate for sender certificate hash is locked and can no longer be changed to a different hash. This happens in such a way that the logic for getting the sender certificate hash to display in the desktop interface initiates the lock on the most recent fingerprint candidate. This candidate can now not be changed by future requests (unless connection is discarded and HTTPS server restarted). Since fingerprint candidate locking only happens after PIN and nonce have been checked to be valid, attacks to deny authorised request are prevented. Preventing the fingerprint candidate from being changed after a sender fingerprint has been displayed stops attacks that try to time sending a malicious request with stolen credentials. (Note for reviewers: As with all other commits by my user to date, this change and commit message has been fully authored by a human; no LLMs used.)
Currently this is mostly unused, but now it exists and can be attached to
'nearby-sharing-error' events emitted from the backend.
However, it is currently invoked inside of the
CertificateVerificationModal and might be better extracted elsewhere /
live alongside CertificateVefificationModal.
For example these connect-centric modals conditionally à la:
if !err then verifModal else errModal
7c9235b to
19c0f68
Compare
if we receive a register request with an incorrect PIN (e.g. sender typing a 1 instead of a 2 in a PIN). we now reset the channel we use to 'lock' the ping response to a single ping request. this means a sender can try anew especially if the sender goes through the whole ping flow nad again and doesn't immediately send a register request with the correct PIN this change also allows the frontend the ability (not in this commit though!) to selectively hide the verification modal on waiting for the sender, and effectively creates the ability for the receiving user to 'manually unlock the ping channel' if that makes sense.
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
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 PR once merged will implement mTLS in the manner described in the ongoing revisions of the Tella P2P Protocol
WIP :)