cyphernet-labs / rust-internet2

Set of rust crates for software supporting Internet2 (Tor, Noise protocol, lightning network-style messaging)
Apache License 2.0
18 stars 9 forks source link

Handle max frame size #25

Closed TheCharlatan closed 2 years ago

TheCharlatan commented 2 years ago

We've been repeatedly running into the message frame size restrictions of internet-2 on our project. Between our running microservices we want to send messages that exceed the maximum frame size, while still respecting it for any p2p comms.

@dr-orlovsky would you be open to a contribution from us to either remove it or making it configurable? I wanted to gauge what your preference for handling this is before diving into a potential patch. Could this restriction be moved a layer up to the lnp crates?

dr-orlovsky commented 2 years ago

Just last week I had already add support for this in 0.8 beta version by introducing new version of the session called "Brontozaur" (the lightning-style limited version has a name "Brontide"). You can already use it from the master. An example how to use it can be found in https://github.com/Internet2-WG/rust-internet2/blob/master/tests/noise_xk.rs; you just need to replace BrontideSession with BrontozaurSession.

The max message size for Brontozaur session is increased from 64kb to 16MB

TheCharlatan commented 2 years ago

Thank you for the speedy answer. As I can see on the latest branch has a RpcSession type defined with the Brontozaur transcoder. I additionally saw that there a bunch of TODO's with regards to replacing the LocalSession with the RpcSession in the esb Controller. If this replacement is still planned I could try to understand what is blocking the remaining implementation of the RpcSession. I have not yet grasped what challenges the zmq socket types introduced there.

dr-orlovsky commented 2 years ago

Ok, I think we are talking about different connections and probably I didn't understand your request in full.

To clarify, there are two types of connections provided by this library: (1) on top of TCP sockets and (2) using ZMQ sockets.

TCP socket connection is used exclusively to establish P2P sessions, in lightning-style. ZMQ is not used in these sessions, and these sessions always use encryption and messsage size limits. Before, it was just a 64kb limit (Brontide protocol as described in BOLT-8), now with v0.8 I have added Brontozaur encryption having 16 MB size limit. These two sessions can be created with BrontideSession and BrontozaurSession types respectively, used by PeerConnection type in upstream rust-microservices. To establish this type of connection NodeAddr is used from inet2_addr, providing full information about both peer TCP socket and node key used in establishing encrypted session.

ZMQ socket sessions are used only for RPC interface and inter-service ESB messaging - and not for P2P protocols. By default, they are unencrypted - and this type of ZMQ session is named LocalSession and the address used to establish this session named ServiceAddr. It MUST NOT restrict size of the messages - and if it does, it is a bug. The name LocalSession implies that it is safe to use this type of connection only if two services are running on the same machine - i.e. as a part of the same process (inproc ZMQ socket), as different processes (IPC socket) or via local TCP interface. If the services communicate across non-loopback network, the communication must be encrypted with Brontozaur protocol (with 16MB message size limit) - and this connection will be named RpcSession. For obvious reasons it will support only tcp:// ZMQ sockets and again use NodeAddr (since we need both IP address, TCP port and node key).

TheCharlatan commented 2 years ago

This clarifies everything neatly, so I'll focus on the LocalSession now, since I indeed only meant the internal, non-P2P communication. LocalSession uses the PlainTranscoder and zeromq::Connection types. Connection uses an implementation of WrappedSocket for sending and receiving and in this implementation there are size checks that seem to restrict the size of messages. Is this a bug then and these checks can be removed, or am I looking at the wrong source of our errors here?

dr-orlovsky commented 2 years ago

This is a bug, probably coming from copy-pasting code from TCP version. It has to be removed. Will be happy if you can do the PR

TheCharlatan commented 2 years ago

Will get to it soon, thank you for hashing everything out.

dr-orlovsky commented 2 years ago

Closed by #26.

Released to crates.io as 0.8.0-rc.3