Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase connection-level security #1897

Closed
Tracked by #1902
leblowl opened this issue Oct 3, 2023 · 10 comments
Closed
Tracked by #1902

Increase connection-level security #1897

leblowl opened this issue Oct 3, 2023 · 10 comments
Assignees
Labels

Comments

@leblowl
Copy link
Collaborator

leblowl commented Oct 3, 2023

See: https://github.com/TryQuiet/quiet/blob/1fe325e798585c727c21766085d17b636871394d/packages/backend/src/libp2p/websocketOverTor/index.ts

Historically, we used TLS mutual auth on websocket connections between peers using the owner as the CA. So two peers could not connect unless they each had a cert signed by the owner. It also somewhat discourages a member pretending to be the owner and signing another member's cert, since if that happened, that other member would be unable to connect to most people (and so it would sort of be a community fork).

When invited you would first connect to the owner. If someone gave you an invalid invite link, you would get a cert signed by the wrong owner and then you wouldn't be able to connect to anyone.

We are no longer doing this, because we are not issuing username certificates until after they have joined (when the owner comes online next).

  • Can a PSK be used in place of TLS mutual auth?

    One difference is a PSK is non revokable, whereas a cert is revokable. If someone shares the PSK with someone else, they can join the network. If someone shares their cert/key with someone else they can also join, but it's easier to trace who is leaking their credentials and revoke that certificate.

  • Can we use a chain of trust with TLS mutual auth?

    Another alternative is a chain of trust, if an existing member of the community signs a user's cert and that member's cert was signed by the owner, then we can use that chain for TLS mutual auth. This way, nobody can connect unless they share a cert signed by a mutally trusted party.

    • How would this work with unique usernames?

      There are a variety of options, including:

      1. The owner can re-sign the cert and only certs that are signed directly by the owner (not by a member of a chain) can be considered to contain unique details (like a username).
      2. The owner can simply have a orbitDB key/value store that only they can write to which connects usernames to public keys. The cert itself can (but doesn't doesn't need to) contain a user's username.
@holmesworcester
Copy link
Contributor

holmesworcester commented Oct 3, 2023

The hierarchy of certificates approach is incompatible with being able to add and remove multiple devices, since if someone uses one of their devices to add a second device, they can't use their second device to remove the first device without also removing the second device. This is an edge case but I think there are others too.

I think the revocation problem with the psk is fine because once we have a system we like for group message encryption we can also use it to rotate the psk, provided there is some way for users to learn the new secret, such as some network or endpoint that does not require the psk to connect but does not have sensitive data on it.

To me this seems like something we should tackle before releasing if we have a clear plan that we feel good about. If not, let's release and investigate further.

@leblowl
Copy link
Collaborator Author

leblowl commented Oct 3, 2023

Discussing more, it seems that using a cert chain for TLS mutual auth has complexities:

  1. If userA signs userB's certificate and userA's certificate gets revoked, now userB's certificate is invalid. One solution to this case is that when revoking userA's certificate, the individual performing the revocation re-signs any certificates signed by userA.

  2. When revoking userA's certificate, because revocation can take time to propagate, unless there is an additional layer of encryption, messages can still leak to userA until all other peers process the revocation correctly.

  3. It seems like TLS mutual auth is pretty tied to TCP and we may want to use other protocols in the future. UDP is connectionless.

And perhaps, ultimately, we should handle this sort of authentication at a higher layer, e.g. allow anyone to connect, however authenticate transfer of data (like any regular web API does).

We want to guard any data sent to connected peers, which currently means any data sent via gossip protocols and orbitDB (read permissions).

For now, perhaps the best approach is using a PSK. With a PSK, it's likely that only users who are invited can connect to other users (since everyone needs the same PSK). It is possible to leak a PSK on the internet. The same could be said about an invitation, however invites can be further restricted to be single-use and I think they can be tied to the issuer of the invite so that it's possible to track who invited who.

The PSK can help prevent against trivial bridging of communities (#1896) and we can include stronger authentication in the application layer.

Perhaps the PSK could also be related to community-level group encryption which could help with the leaking data issue on revocation.

@holmesworcester
Copy link
Contributor

Notes:

  • We'll try a PSK and make sure it works
  • We may run into length limits with linkifying

@leblowl leblowl moved this from Sprint to In progress in Quiet Oct 5, 2023
@leblowl
Copy link
Collaborator Author

leblowl commented Oct 5, 2023

We can add the PSK to the invite link (there should be enough room in the link with 4 peers), and on joining a network, we can send the PSK to the backend and store it in LevelDB. The PSK can then be used in initializing the libp2p layer.

According to Emi's research, we have 512 characters after 'https://tryquiet.org/?codes=' to include in an invite link while still allowing the link to be "linkified" by common messaging apps.

Each peer address is 104 characters:

  onionAddress -> 56 characters
  peerId -> 46
  "=" -> 1
  "&" -> 1

And we currently include 4 in the link... (4 * 104 = 416), leaving (512 - 416 = 96) characters left. The PSK is 256 bits/8 = 32 bytes which encodes to 44 characters base64:

  > let crypto = require('crypto')
  undefined
  > let rand = crypto.randomBytes(32)
  undefined
  > rand.toString('base64')
  '8onHGo2u5j35OBUGyLUEHD3WkZkGfqTBZtghnCiZ938='
  > rand.toString('base64').length
  44

So we should have plenty of room to add the PSK to the current invite link.

Future considerations: how can we tell if a PSK is invalid? Is there a specific error?

@EmiM
Copy link
Contributor

EmiM commented Oct 12, 2023

#1961

@holmesworcester
Copy link
Contributor

Is there any early indication of how well this works?

@EmiM
Copy link
Contributor

EmiM commented Oct 24, 2023

There are two types of 'invalid psk' errors:

  1. Invalid psk format - this is easy to handle because libp2p throws error on initialization:
backend:Libp2pService:err Create libp2p: Error: Your private shared key is invalid
    at decodeV1PSK (/home/emi/dev/monorepo/packages/backend/node_modules/libp2p/src/pnet/crypto.ts:65:11)

I validate invitation code data before psk reaches libp2p constructor so technically this should not happen (with regular application usage).

  1. PSK that has valid format but does not belong to the community user tries to join

This option is a bit more complicated, maybe we could think of a solution as part of the next task.

  • Joiner connecting to community with non-matching psk sees following logs:
  backend:libp2p:websockets connect a4czjiaktx7dyiir6olndep2sajhbvwg544a7e3ps3jod32ds5owgzad.onion:80 +38s
  backend:libp2p:websockets /dns4/iaywmod32kkp3i6p3bfm6xvf6gyslpbep244pw4uj5ie5djhdvgse2id.onion/tcp/80/ws/p2p/QmPbtanAXJrfPhYoq1YtcFUhh3GaseWvrxzt9s1mpbX1cd connected /dns4/a4czjiaktx7dyiir6olndep2sajhbvwg544a7e3ps3jod32ds5owgzad.onion/tcp/80/ws/p2p/QmVacrLR7iN5PwkqUPJmzhkHqyPfEeYz1p3kAZmaMq3wLn +4s
  backend:libp2p:websockets new outbound connection /dns4/a4czjiaktx7dyiir6olndep2sajhbvwg544a7e3ps3jod32ds5owgzad.onion/tcp/80/ws/p2p/QmVacrLR7iN5PwkqUPJmzhkHqyPfEeYz1p3kAZmaMq3wLn +0ms
  backend:libp2p:websockets:err error upgrading outbound connection /dns4/a4czjiaktx7dyiir6olndep2sajhbvwg544a7e3ps3jod32ds5owgzad.onion/tcp/80/ws/p2p/QmVacrLR7iN5PwkqUPJmzhkHqyPfEeYz1p3kAZmaMq3wLn. Details: stream ended before 1 bytes became available +44s
  backend:libp2p:websockets connect do6ebi6vh6x66tweytblwl76bv6vrgiijun3vnwoujdhvcou6mwd3jyd.onion:80 +30s
  backend:libp2p:websockets:err connection error: Unexpected server response: 404 +5s
  backend:libp2p:websockets:err error connecting to /dns4/do6ebi6vh6x66tweytblwl76bv6vrgiijun3vnwoujdhvcou6mwd3jyd.onion/tcp/80/ws/p2p/QmVDoEb2ie7352YPDx9qTP29PK3CsLvVSFNpjiBsURrVZU. Details: Unexpected server response: 404 +0ms
  • Peer that joiner is trying to connect to has more explicit information:
ackend:libp2p:websockets:socket timeout closing stream to iaywmod32kkp3i6p3bfm6xvf6gyslpbep244pw4uj5ie5djhdvgse2id.onion:80 after 2001ms, destroying it manually +0ms
  backend:libp2p:websockets server connecting with /dns4/iaywmod32kkp3i6p3bfm6xvf6gyslpbep244pw4uj5ie5djhdvgse2id.onion/tcp/80/ws/p2p/QmPbtanAXJrfPhYoq1YtcFUhh3GaseWvrxzt9s1mpbX1cd +17s
  backend:libp2p:websockets new inbound connection /dns4/iaywmod32kkp3i6p3bfm6xvf6gyslpbep244pw4uj5ie5djhdvgse2id.onion/tcp/80/ws/p2p/QmPbtanAXJrfPhYoq1YtcFUhh3GaseWvrxzt9s1mpbX1cd +1ms
  backend:libp2p:websockets:err inbound connection failed to upgrade AbortError: The operation was aborted
    at nextAbortHandler (webpack://@quiet/backend/./node_modules/libp2p/node_modules/abortable-iterator/dist/src/index.js?:34:32)
    at EventTarget.abortHandler (webpack://@quiet/backend/./node_modules/libp2p/node_modules/abortable-iterator/dist/src/index.js?:21:17)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:731:20)
    at EventTarget.dispatchEvent (node:internal/event_target:673:26)
    at abortSignal (node:internal/abort_controller:308:10)
    at TimeoutController.abort (node:internal/abort_controller:338:5)
    at TimeoutController.abort (webpack://@quiet/backend/./node_modules/timeout-abort-controller/index.js?:26:18)
    at eval (webpack://@quiet/backend/./node_modules/timeout-abort-controller/index.js?:16:38)
    at Retimer._timerWrapper (webpack://@quiet/backend/./node_modules/retimer/retimer.js?:21:18)
    at listOnTimeout (node:internal/timers:564:17) {
  type: 'aborted',
  code: 'ERR_ENCRYPTION_FAILED'
} +45s

@holmesworcester
Copy link
Contributor

holmesworcester commented Oct 24, 2023

Let's create a backlog issue for showing an error. #2014

@EmiM
Copy link
Contributor

EmiM commented Oct 31, 2023

I noticed that the target peer throws ERR_ENCRYPTION_FAILED a few times even if psk is correct, then approves connection.

@holmesworcester
Copy link
Contributor

I found this issue: ipfs/js-ipfs#3702

This is thought to be fixed in a newer libp2p.

How much of a delay does this cause in connecting?

@EmiM EmiM moved this from Waiting for review to Merged (develop) in Quiet Nov 14, 2023
@Kacper-RF Kacper-RF moved this from Merged (develop) to Ready for QA in Quiet Nov 14, 2023
@siepra siepra moved this from Ready for QA to Done in Quiet Nov 14, 2023
@siepra siepra closed this as completed Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

4 participants