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

RFC JSONRPC ZeroMQServer #5098

Closed
wants to merge 1 commit into from
Closed

RFC JSONRPC ZeroMQServer #5098

wants to merge 1 commit into from

Conversation

topfs2
Copy link
Contributor

@topfs2 topfs2 commented Jul 25, 2014

I'm not sure if this is something we would want? Basically its an alternative to the web and tcp server which is much easier to use as it both provides framing and is bidirectional.

Some benefits with ZeroMQ

  • ZeroMQ supports many transports, both TCP, IPC and in memory. So it could be useful to further extend this for out of process stuff. So a big upside is that we could have IPC enabled by default
  • ZeroMQ abstract uses a composite pattern, so its usually transparent if your getting data from many clients or if your sending it to many. So this could be a great start to create a distributed xbmc were we talk lots between different xbmc machines (obviously this is possible with tcp also, its just easier with ZMQ).
  • ZeroMQ has libraries for many languages and could provide a simple base for third party were they get the benefits of the TCP server but with a very simple API to send and receive the data, essentially all they need to do is parse and handle the jsons

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone Jul 26, 2014
@Tolriq
Copy link
Contributor

Tolriq commented Jul 26, 2014

Does this support authentication ?

Http is authenticated but slow and needed for file download.
Tcp is not authenticated but fast but cannot download
If this can have all benefits with no drawback this would be a major feature at least for remote writers :)

@topfs2
Copy link
Contributor Author

topfs2 commented Jul 26, 2014

Don't think this will help with authentication and probably not download
either. It could possible allow download but doubt it. Consider it
essentially the same as TCP but with framing (don't need to count brackets)
and that you can listen to any number of xbmcs without any extra code.
On 26 Jul 2014 18:31, "Ano69" notifications@github.com wrote:

Does this support authentication ?

Http is authenticated but slow and needed for file download.
Tcp is not authenticated but fast but cannot download
If this can have all benefits with no drawback this would be a major
feature at least for remote writers :)


Reply to this email directly or view it on GitHub
#5098 (comment).

#include "interfaces/json-rpc/ITransportLayer.h"
#include "threads/CriticalSection.h"
#include "threads/Thread.h"
#include "websocket/WebSocket.h"

This comment was marked as spam.

@Montellese
Copy link
Member

I've never heard of ZeroMQ before but the concept sounds very interesting. I'll try to find some time to read more about it.

It looks like you copied a lot of code from CTCPServer (and forgot to remove some of it).

As I was contemplating rewriting/restructuring CTCPServer to make it easier to use different "services" (JSON-RPC over raw TCP, JSON-RPC over WebSockets) I was wondering if this would make sense here too. Since this is a message based system it's probably even much easier and maybe it would make sense to allow different message handlers. That way we could add support for other stuff (like file download) in addition to JSON-RPC. Not sure though if this fits into the basic concept of ZeroMQ.

@Tolriq
Copy link
Contributor

Tolriq commented Jul 29, 2014

Forget to answer about auth, but if there's no way to add some auth / security to tcp and this, would it be possible to have a new setting for all those, instead of the allow remoting from external devices ?

Or at least to change description, as the option for local does explain that this opens JSON-RPC too (well it also talks about web interface that could lead to confusion).

But the "other devices" in a "remote control" part really sounds like simple remoting and not all the power of those and most users do not understand it.

@topfs2
Copy link
Contributor Author

topfs2 commented Jul 29, 2014

@Tolriq Its good that you brought up authentication, I've never really thought of it being relevant on TCP as I mostly consider it being used on a local network. However I think we could add authentication on both a ZeroMQ and the original TCP server through one of two ways.

  1. We add authentication to the actual json-rpc, on those transports. i.e. client needs to do JSONRPC.Authenticate or similar.
  2. We just simply encrypt the entire stream always and the clients with correct passwords can decrypt.

From what I can gather ZeroMQ has added authentication in version 3 and they seem to have done it via (2), which makes sense as they are lower level than us. And IMO I think that would be the simplest way for us to do aswell. And if its available in the transport we just use their encryption/authentication

Does this sound ok to you?

@Montellese Yeah its a very quick POC, so mostly copied from the TCP server, and noticed I left a bunch hehe.

It's very close to the TCP Server so I'm sure we could combine them somewhat. I think we only would need a virtual SendReply, SendRequest, SendNotification essentially. And possibly some events like onClientAdded we could trigger in each handler.

And regarding file download, yeah we really need to find a way to do that :) Perhaps we could have an IRC discussion about it at some point, (not to hide anything, just that its quicker).

@topfs2
Copy link
Contributor Author

topfs2 commented Jul 29, 2014

Sorry, meant ZeroMQ version 4 seems to have security.

@Montellese
Copy link
Member

IMO (1) and (2) (can) solve different problems.

(1) can make sure that a specific client can only perform specific actions and deny access to other actions. It works per-request and is transport protocol independent (and also covers requests from python addons).
(2) makes sure that only a client with the proper credentials can connect to XBMC in the first place. But it only works on a per-transport-protocol basis so you need different authentication for different protocols.

IMO both concepts are valid and have their use cases and they are not mutually exclusive.

@topfs2
Copy link
Contributor Author

topfs2 commented Jul 29, 2014

Yeah thats true, so using (2) would only really bring tcp servers up to the same standard as the webserver really.

@Tolriq
Copy link
Contributor

Tolriq commented Jul 29, 2014

Well both have pro and cons.

But from my remote writer perspective 2 is not viable :(
Imagine requesting 500 full movies data, currently for memory and performance reason on phones I do handle the JSON at a Stream level to avoid dealing with mega bytes strings.
With encoding, this would mean, get a xMB string, decode it (possibly slow and needs at least 2 times the memory. Then parse the JSON string that most Library will handle by generating a big JSON object needing 3 to 4 times the memory and not allowing Stream handling for a string, needing to write a string to Stream wrapper.

This sound like a crazy solution from an optimisation point of view.

(Not talking rpi that already cries to encode to simple json, so adding encryption to big data would kinda kill those).

@Tolriq
Copy link
Contributor

Tolriq commented Jul 29, 2014

The problem with 1 is how to handle this correctly without breaking backward compatibility.
For real world use case authenticate should send back a token that you then add to every requests, since authenticating the source IP for TCP or webserver is not good in proxy or other solutions.

HTTP could use headers, but not TCP.

@Montellese
Copy link
Member

@Tolriq: Could you open a thread in the forum to discuss these issues (and post the link here) as they are not really blockers for this PR. I have already done quite some work on JSON-RPC authentication but that was 2-3 years ago.

@Tolriq
Copy link
Contributor

Tolriq commented Jul 29, 2014

http://forum.xbmc.org/showthread.php?tid=200894 will complete the post later today when more time.

@topfs2
Copy link
Contributor Author

topfs2 commented Jul 29, 2014

I think we could add 1 without breaking compatibility, I mean, if an old client would try to execute on a server with authentication that request would yield an error.

I'm fairly sure there are encodings which are streamable, I'll see if I can find one which might be suitable. Because in a proper secure world we would almost need both solutions.

@Montellese
Copy link
Member

It's called a stream cipher (like the very old RC4).

@Tolriq
Copy link
Contributor

Tolriq commented Jul 29, 2014

As long as it's optional or can be disabled by the user if low end device on any end of the protocol sounds good.

@ePirat
Copy link

ePirat commented Apr 6, 2015

Any updates on this? I am currently developing a client that uses the JSON RPC API (with TCP transport) and it's so much pain to implement, so an alternative would be awesome. (Or just some properly delimited JSON RPC over TCP…)

@topfs2
Copy link
Contributor Author

topfs2 commented Apr 6, 2015 via email

@topfs2
Copy link
Contributor Author

topfs2 commented Apr 6, 2015 via email

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

Successfully merging this pull request may close these issues.

None yet

6 participants