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

rendezvous protocol #44

Closed
wants to merge 17 commits into from
Closed
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions rendezvous/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# Rendezvous Service

Scope:
- real-time applications that require rendezvous
- replace ws-star-rendezvous with a rendezvous service daemon and a fleet
of p2p-circuit relays.

## Rendezvous Protocol

The rendezvous protocol provides facilities for real-time peer discovery within
application specific namespaces. Peers connect to the rendezvous service and register
their presence in one or more namespaces. Registrations persist until the peer
disconnects or explicitly unregisters.

Peers can enter rendezvous and dynamically receive announcements about peer
registrations and unregistrations within their namespaces of interest.
For purposes of discovery (eg bootstrap), peers can also ask the service for
a oneof list of peers within a namespace.

### Interaction

Client peer `A` connects to the rendezvous service `R` and registers for namespace
`my-app` with a `REGISTER` message. It subsequently enters rendezvous with
a `RENDEZVOUS` message and waits for `REGISTER`/`UNREGISTER` announcements from
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sold on having an explicit Rendezvous thingy. Isnt it technically just the same as polling the Discovery protocol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a more efficient version of that, as it listens for new peers on top of the initial lookup:

  • DISCOVER returns a list of current peers and stops
  • RENDEZVOUS returns the current peers and then sends deltas (new REGISTER/UNREGISTER).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it's pull -vs- push.

the service.

```
A -> R: REGISTER{my-app, {QmA, AddressA}}
A -> R: RENDEZVOUS{my-app}
```

Client peer `B` connects, registers and enters rendezvous.
The rendezvous service immediately notifies `B` about the current namespace registrations
and emits a register notification to `A`:

```
B -> R: REGISTER{my-app, {QmB, AddressB}}
B -> R: RENDEZVOUS{my-app}

R -> B: REGISTER{my-app, {QmA, AddressA}}
R -> A: REGISTER{my-app, {QmB, AddressB}}
```

A third client `C` connections and registers:
```
C -> R: REGISTER{my-app, {QmC, AddressC}}
C -> R: RENDEZVOUS{my-app}

R -> C: REGISTER{my-app, {QmA, AddressA}}
REGISTER{my-app, {QmB, AddressB}}
R -> A: REGISTER{my-app, {QmC, AddressC}}
R -> B: REGISTER{my-app, {QmC, AddressC}}
```

A client can discover peers in the namespace by sending a `DISCOVER` message; the
service responds with the list of current peer reigstrations.
```
D -> R: DISCOVER{my-app}
R -> D: REGISTER{my-app, {QmA, AddressA}}
REGISTER{my-app, {QmB, AddressB}}
REGISTER{my-app, {QmC, AddressC}}
```

### Protobuf


```protobuf
message Message {
Copy link
Member

Choose a reason for hiding this comment

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

I think there need to be error types.
For ex. when a peer tries to register for an ID that the peer does not own.
Or when the peer tries to register a namespace that is way too long (say a big .jpg in base64).

Copy link
Contributor Author

@vyzo vyzo Apr 15, 2018

Choose a reason for hiding this comment

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

So you want a RegisterResponse?
Sure, we can do that.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. Also DiscoverResponse could include an optional error, too

Copy link
Contributor Author

@vyzo vyzo Apr 15, 2018

Choose a reason for hiding this comment

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

Ok, although it's utility would be rather limited.
What is considered an error in this case? A non-existent namespace is equivalent to empty, and namespace name that is too long will always be empty.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, although it's utility would be rather limited.

Let's not do this then.

So you want a RegisterResponse?

The errors that it would throw would be:

  • E_NAMESPACE_TOO_LONG
  • E_OWNERSHIP_NOT_TRUSTED (or similar)

What do you think of that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

E_NAMESPACE_TOO_LONG

that could be generalized to E_INVALID_NAMESPACE to convey rejected characters for some implementation. Or it could be a different error altogether.

E_OWNERSHIP_NOT_TRUSTED

Maybe a generic E_NOT_AUTHORIZED is more apt?

Another one we want is E_INVALID_MULTIADDR perhaps.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe a generic E_NOT_AUTHORIZED is more apt?

Yes

Another one we want is E_INVALID_MULTIADDR perhaps.

Good idea, although I don't know if we should validate those as the clients might have newer incompatible protocol tables

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's have E_INVALID_PEER_INFO to signal generic rejection of the peer info, and we can see what kind of validation makes sense.

enum MessageType {
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it make more sense to merge register and unregister requests? Or dropping the message type completly?
The client could simply check the length of the array for all types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we want unregister so that we can depart from a namespace without having to disconnect.
The message type is needed to disambiguate the protocol.

Copy link

Choose a reason for hiding this comment

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

For messages with enums it is easy to to create messages that accidentally say something they don't intend to. The way I've seen this happen is that if an enum field has a default value that carries some semantics (name of a message, or a status) then the reader can't distinguish between a programmer explicitly setting that value versus not setting it at all. Or at least the API I was using a few years ago didn't distinguish between the two by default. Anyway, in order to avoid accidentally having semantics for enum fields the pattern was to have the default enum value always be something like UNSET or NOT_SET so that it was clear when someone explicitly set the value versus not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can certainly see some value in defensive programming, but I am not sure I like polluting the protocol with something that protects against PEBKAC-style programming errors.

It would also be rather inconsistent as I don't think we do this anywhere else.

Copy link
Contributor Author

@vyzo vyzo Apr 21, 2018

Choose a reason for hiding this comment

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

Also note that the casual programmer, who is more likely to suffer from such PEBKAC, should never have to touch these protobufs directly.
These should be well abstracted by implementation libraries, and bugs of this type should be quickly weeded out there.

REGISTER = 0;
UNREGISTER = 1;
RENDEZVOUS = 2;
DISCOVER = 3;
}

message Peer {
optional string id = 1;
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to use the smaller non-encoded version of the peer-id instead of the b58 string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, that's just the data type -- we use string for unencoded addresses in go.

Copy link
Member

Choose a reason for hiding this comment

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

If there is no crypto-challenge for the id the only way to prove ownership is the SECIO challenge, which is only done for the current peer-id. Therefore this field seems useless to me...
Why not just drop the PeerInfo msg and use repeated addrs on register?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The registration is forwarded in the DiscoverResponse (modulo ttl) and it would thus be ambiguous without the peer id.

You also are missing the potential use case of rendezvous points sharing registrations (say in federation of daemons, using pubsub, etc).
More generally, having a peer id allows registrations to be passed around and reconstruct full PeerInfo objects from them.

Copy link
Member

Choose a reason for hiding this comment

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

You also are missing the potential use case of rendezvous points sharing registrations (say in federation of daemons, using pubsub, etc).

Then the data must be somehow authenticated, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can authenticate the sharing peer instead of the data.
Authenticating the registration itself would require a signature in them.

Copy link
Member

Choose a reason for hiding this comment

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

You can authenticate the sharing peer instead of the data.

If the data isn't authenticated, wouldn't that allow for replay attacks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, but you'd get that too if you were authenticating with a nonce.

I don't think that replay attacks are a big concern in this context, as the sharing peers can establish a trust model with each other.

Note that signatures along are not sufficient to prevent replay attacks in a shared setting either, and trying to add a signature timestamp gets complicated quick.

Copy link
Member

Choose a reason for hiding this comment

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

Ok

repeated bytes addrs = 2;
}

message Register {
optional string ns = 1;
optional Peer peer = 2;
}

message Unregister {
optional string ns = 1;
optional Peer peer = 2;
}

message Rendezvous {
optional string ns = 1;
}

message Discover {
optional string ns = 1;
}

optional MessageType type = 1;
repeated Register register = 2;
repeated Unregister unregister = 3;
repeated Rendezvous rendezvous = 4;
repeated Discover discovery = 5;
}
```