Skip to content
This repository has been archived by the owner on Jul 5, 2023. It is now read-only.

Add connlib basic implementation #18

Open
wants to merge 54 commits into
base: master
Choose a base branch
from
Open

Conversation

conectado
Copy link
Contributor

@conectado conectado commented May 18, 2023

Overview

This PRs establish a basis on which to build on for connlib, this doesn't try to be a working version of connlib.

This PR tries to:

  • Establish a way to handle egress/ingress message
  • Organize code in crates
  • Have the most basic Tunnel showing how webrtc + wireguard works
  • Establish how to handle errors
  • Establish an initial FFI API

There's still a lot to do, there are quite a few "TODO"s in the code but here are some general things

  • We assume here that the client will be running when on_error is called, so it can disconnect the Session, this will not always be the case and we need to disconnect the session from Rust.
  • Use Phoenix channel's topics and events in a way that's compatible with the portal.
  • Implement all missing functions for messages and wireguard stats.
  • Fix integration with clients
  • Unit and Integration tests

This changes are only tested to be compiled not to be working but creating the PR now so that we don't keep bloating the PR.

Crate structure

We have 2 big directories:

  • libs: libraries that have buisness logic
  • clients: are the outter shells that directly interact with outside code (or are directly a binary)
  • gateway: Contains the actual gateway binary

These are self-explainatory save for directories in libs:

  • common: have types shared by all connlib.
  • tunnel: is the actual data-plane logic.
  • client: buisness logic specific for clients.
  • gateway: buisness logic specific for gateway.

Platform specific behavior

For now, only linux and macos have an implementation and we are directly using boringtun's device although moving to our own implementation won't be hard.

Next steps

My next step is making it actually work with the portal, for that I'll fix the gateway binary and make a simple headless client.
Once that is done we can start working on client-specific ffi and be sure that it is working.

libs/tunnel/src/index.rs Outdated Show resolved Hide resolved
Copy link
Member

@jamilbk jamilbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just did a quick first pass. Looking good, just want to drop a subtle reminder to follow boringtun's licensing requirements where appropriate before this gets merged / shipped.

@conectado
Copy link
Contributor Author

Just did a quick first pass. Looking good, just want to drop a subtle reminder to follow boringtun's licensing requirements where appropriate before this gets merged / shipped.

Oh good catch 👍

This commit convert this repo into a rust workspace.
Divides into 2 types of crates, libs/clients.
Clients are thin wrappers used directly by native clients.
Libs implement the logic.
We have 4 logic crates:
- Gateway: Gateway-specific logic.
- Clients: Client-specific logic.
- Tunnel: General logic for wireguard/ice tunnels.
- Common: Types shared by all crates.
@conectado
Copy link
Contributor Author

@jamilbk could you double check if this copyright notice is enough?

@conectado conectado marked this pull request as ready for review May 25, 2023 20:35
@jamilbk jamilbk requested a review from roop May 26, 2023 17:25
@jamilbk
Copy link
Member

jamilbk commented May 26, 2023

adding @roop for visibility on the Apple FFI bridge

clients/apple/src/lib.rs Outdated Show resolved Hide resolved
clients/apple/src/lib.rs Outdated Show resolved Hide resolved
clients/apple/src/lib.rs Outdated Show resolved Hide resolved
macros/Cargo.toml Outdated Show resolved Hide resolved
gateway/Cargo.toml Outdated Show resolved Hide resolved
libs/tunnel/src/tun_darwin.rs Show resolved Hide resolved
libs/tunnel/src/tun_darwin.rs Outdated Show resolved Hide resolved
libs/tunnel/src/tun_darwin.rs Outdated Show resolved Hide resolved
libs/tunnel/src/tun_darwin.rs Outdated Show resolved Hide resolved
libs/tunnel/src/tun_darwin.rs Outdated Show resolved Hide resolved
Copy link
Member

@jamilbk jamilbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @francesca64 for the thorough review! In the main repo we use codespell + precommit to catch typos like these, noting here in case it's useful

@jamilbk jamilbk requested a review from timClicks June 7, 2023 22:36
@jamilbk
Copy link
Member

jamilbk commented Jun 7, 2023

Adding @timClicks for a review on organization/structure. @timClicks -- probably will need a call to provide more context around Connlib but it's meant to be our core networking library used across Apple/Android/Windows/Linux. This may be helpful as well:

conectado and others added 14 commits June 8, 2023 18:18
Co-authored-by: Francesca Lovebloom <[email protected]>
Signed-off-by: Gabi <[email protected]>
Co-authored-by: Francesca Lovebloom <[email protected]>
Signed-off-by: Gabi <[email protected]>
Co-authored-by: Francesca Lovebloom <[email protected]>
Signed-off-by: Gabi <[email protected]>
Co-authored-by: Francesca Lovebloom <[email protected]>
Signed-off-by: Gabi <[email protected]>
Co-authored-by: Francesca Lovebloom <[email protected]>
Signed-off-by: Gabi <[email protected]>
Co-authored-by: Francesca Lovebloom <[email protected]>
Signed-off-by: Gabi <[email protected]>
Co-authored-by: Francesca Lovebloom <[email protected]>
Signed-off-by: Gabi <[email protected]>
Co-authored-by: Francesca Lovebloom <[email protected]>
Signed-off-by: Gabi <[email protected]>
Co-authored-by: Francesca Lovebloom <[email protected]>
Signed-off-by: Gabi <[email protected]>
Co-authored-by: Francesca Lovebloom <[email protected]>
Signed-off-by: Gabi <[email protected]>
@roop
Copy link
Contributor

roop commented Jun 14, 2023

Having connect return a Result<Self, SwiftConnlibError> is nice. Presumably, it would become func connect(portal_url: String, token: String) throws -> WrappedSession in Swift (it doesn't build currently, so I can't see the generated bridge code).

With this PR, how should the Swift code tell the Ruby code about the callback handlers to be called? (Previously, we passed in a CallbackHandler struct with functions.)

Apart from that, I think we should add the following callback handlers in the FFI:

  • Log handler: When running connlib inside the network extension, stdout and stderr are not accessible, so we'll have to call the Apple logging functions to debug what happens in Rust code. So having a logging callback is important.
  • Error handler: While it's good that connect can now return an error, we should probably have a callback to tell the client in case something went wrong in the connlib side while the tunnel was up. (Edit: I see it's already added as onError)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants