Add authentication support

[adieu]

I'm not quite sure if this patch should be merged. But I'd like to bring up a discussion about whether should zerorpc have authentication support built-in.

The need of authentication support start from day one when I use zerorpc to build some internal service since I have to expose this service to the internet.

When I heard the release of stack.io, I though I might use stack.io as a gateway doing the authentication work. But I couldn't find the authentication method for clients connected through zerorpc.

So I begin to add authentication support by myself.

This patch consists two commits. The first one patches current zerorpc code base to support custom Event class for Server and Client. And the second one add signed requests based authentication using custom Event class and proper middleware.

When I was modifying current zerorpc code base, I tried to change as less as possible and I put authentication related code in a separate file as kind of extension.

Here are some example code shows about how to use authentication:

# server
import time
import zerorpc
from zerorpc.auth import SignedEvent, SharedSecretMiddleware

zerorpc.Context.get_instance().register_middleware(SharedSecretMiddleware('some_random_key'))
server = zerorpc.Server(time, event_class=SignedEvent)
server.bin("tcp://0.0.0.0:4242")
server.run()

# client
import zerorpc
from zerorpc.auth import SignedEvent, SharedSecretMiddleware

zerorpc.Context.get_instance().register_middleware(SharedSecretMiddleware('some_random_key'))
client = zerorpc.Client(event_class=SignedEvent)
client.connect('tcp://127.0.0.1:4242')
print client.strftime('%Y/%m/%d')

I don't know the design decisions about authentication. If we decided not to include authentication in zerorpc, could we just merge the first commit and give the developers ability to change the default behavior of events so that they could implement their own authentication mechanism in their own code base?

Btw, I really enjoyed writing services with zerorpc. Thank you for this wonderful project.

[bombela]

Hey glad to hear that you like zerorpc :) To confirm your last statement: yes it is preferable to keep auth outside of zerorpc for now. A middleware is perfectly fine. For the customization of the Event class, give me some time to review it. Thanks for contributing!

[adieu]

Let's discuss the custom Event class patch here. I could send in another pull request when it's ready to be merged.

[bombela]

Thanks for putting None everywhere, it reduces the number of files modified, and simply keep the new modifications as local as possible to Events. And I like that Core.py stays away from any strong knowledge about Events/Event.

Btw that was a really dumb idea to name one class Events and the other one Event! We should definitively rename that. Lets rename Events into EventsTransport. That makes more sense.

In term of design, I think that rather than providing en Event class to be used by the Events class. It would be better to provide an instance of a class replacing Events itself. This class could then provide its one Event class as well. Both customized Events and Event class should respect some well defined interface in order to work properly with zerorpc.

It will allow people to customize even more what Events and Event are doing, so you could change the transport of zerorpc. Also, pay attention that there is one instance of Events per zerorpc Client or Server object, but Event can be recursively encapsulated! (this is not used by the official branch of zerorpc right now, but there is some experiments in the subcontext branch).

I will try to look at this problem in detail during the weekend, especially how you implement the authentication, and see if I can come up with some satisfying ideas in order to solve your initial problem: adding auth to zerorpc.

[adieu]

Thank you!

I think the custom transport idea is great. Actually, I was trying to go that way. But I found it may bring some big changes to the current code base. And I wanted the patch to be less invasion as possible since people use zerorpc in production and such changes might break things.

I'd love to contribute for the custom transport feature if you haven't started to do that yet. Just let me know.

[bombela]

I did not started any implementation yet, and patches are welcome! I would like to talk about the different possible solutions first though.

I was reading your code in detail in order to pinpoint what zerorpc is missing for your use case. I was considering extending the different uses of load_task_context/get_task_context that you already used to store your message signature, but I realized that it does not deserve the same purpose:

A zerorpc.server, upon load_task_context() could load and verify the event header by raising an exception. But this is not the best logical place to put a check, and it becomes awkward when load_task_context() is used during a coroutine fork. Also, right now the client will not call load_task_context() when it receives messages from the server, since logically, you already know in which task you are, and so re-loading it would be redundant.

The goal of load_task_context() and get_task_context() is initially to build a distributed tracer. For example, each time a new event is created, a trace_id is created or copied from the current task context. This permit passing the trace_id from a zerorpc.Server to another via zerorpc.Client.

Your use case is different: you do not want to forward anything between services. You want to authenticate ALL the messages between a given zerorpc.Server and a zerorpc.Client. In order to do so, you want the Server and Client to inject a signature in every single event, that both side will verify against a given shared secret. But I am not teaching you anything here.

The signature do not escape the boundaries of the Client/Server communication. The signatures are exchanged between a Server and a Client directly. By extension and to makes zerorpc implementation easier, we can assume that all the zerorpc.Client/Server attached to the same zerorpc.Context share the same secret (as you did in your proposal). Some people might prefer to have a different secret for every zerorpc.Client for example, without having to create a context for each of them. Feel free to share any idea if you think this problem should be solved.

I see two direction were we could head in order to solve your problem:

• add hooks in zerorpc.context focused on authentication. You probably need two hooks:
  • sign_event(mutable_event) -> inject anything you want in the header
  • verfify_signed_event(readonly_event) -> check anything you want, either it returns true. or simply raise an exception. (to be determined).
• add one hook in zerorpc.context that return a customized Events class. I can picture the hook function to look like: create_transport() -> returning an Events class. We have to make sure that events are created by the Events class itself so zerorpc does not try to create its own events. If you want to re-use the default Events class from zerorpc, you can simply inherit/compose from/with zerorpc.Events.

Let me know what you think, and feel free to criticize and share any other idea.

Thanks for reading up to this point ;)

[lopter]

I/We'll have to look at this with the new middleware API I'm trying to finalize (I'm just committing it now, but it was actually a background work for the past few months).

[adieu]

Great news!

I've been busy with other work recently. Now I have some time working on zerorpc again. I'll port my authentication related code to 0.4 after I figure out how to play with the new middleware api nicely.

[bombela]

I am not sure that you will be able to retrofit the current hooks to your
needs, the new hooks are not so much different for authentication. You
might want to add some middleware hooks dedicated to authentication.
On Jan 8, 2013 11:24 PM, "Adieu" notifications@github.com wrote:

Great news!

I've been busy with other work recently. Now I have some time working on
zerorpc again. I'll port my authentication related code to 0.4 after I
figure out how to play with the new middleware api nicely.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/46#issuecomment-12034246.

[adieu]

I just pushed my authentication related code modified against the new middleware api and I managed to add all I want through middleware so I reverted my changes made to the core part of zerorpc.

I found the latest middleware api brings much more possibilities.

With the old version, I can't properly inject signature to the event to be sent because the old load_task_context hook took event_header as the argument and I need the event object to generate the signature. And as bombela explained, the load_task_context hook is designed for the distributed tracer so it's not the proper hook to be used for authentication purposes. So I added custom Event class support in order to inject signature to the header.

With the new version, the server_before_exec and client_before_request hook just gave me what I want. I could inject signature in client_before_request and check signature in server_before_exec which makes the whole authentication process tidy and clean.

When it comes to authentication, there are many ways to do it. In my code, I demonstrated two simple ways. One is to use shared secret to sign the request and the other is oauth like request signing with client private key. Anyone with similar needs might find it useful. And one can easily add more authentication methods to meet their own needs.

I need some advice on the way I used middleware to add authentication. If it's proper, I think we can consider this issue closed and just leave the auth.py in my repo as kind of example for people with similar needs. If not and there's better way to do it, I'll continue to improve my code along the better way.

Thanks a lot!

[bombela]

I am really glad to hear that the new middleware api solved your problem!
Many thanks to Louis Opter who drove the design and implemented it! I will
take a look when I have a chance and tell you what I think about. when we
are both satified by the solution to you problem, I will be glad to put a
link to your middleware in the readme so others can discover it easily.
On Jan 9, 2013 1:31 AM, "Adieu" notifications@github.com wrote:

I just pushed my authentication related code modified against the new
middleware api and I managed to add all I want through middleware so I
reverted my changes made to the core part of zerorpc.

I found the latest middleware api brings much more possibilities.

With the old version, I can't properly inject signature to the event to be
sent because the old load_task_context hook took event_header as the
argument and I need the event object to generate the signature. And as
bombela explained, the load_task_context hook is designed for the
distributed tracer so it's not the proper hook to be used for
authentication purposes. So I added custom Event class support in order to
inject signature to the header.

With the new version, the server_before_exec and client_before_requesthook just gave me what I want. I could inject signature in
client_before_request and check signature in server_before_exec which
makes the whole authentication process tidy and clean.

When it comes to authentication, there are many ways to do it. In my code,
I demonstrated two simple ways. One is to use shared secret to sign the
request and the other is oauth like request signing with client private
key. Anyone with similar needs might find it useful. And one can easily add
more authentication methods to meet their own needs.

I need some advice on the way I used middleware to add authentication. If
it's proper, I think we can consider this issue closed and just leave the
auth.py in my repo as kind of example for people with similar needs. If
not and there's better way to do it, I'll continue to improve my code along
the better way.

Thanks a lot!

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/46#issuecomment-12037091.

[adieu]

Thank you! I'm looking forward to it.

[andresriancho]

Don't really understand why this code isn't merged into the main repository.

Having authentication is an important feature many users will want today, or will require tomorrow (after a security assessment of some type).

The default should be to expose ZeroRPC without any authentication, but developers should be pulling from a third party repository to get authentication features. Also, having that important feature in a different repository will only bring problems: a change in this repository can break the authentication code, and nobody will notice it (unless there is some complex cross-project continuous integration setup).

So, to sum up, I believe its best to merge making sure that this is disabled by default and is 100% back-compatible.

[jpetazzo]

The code hasn't been merged for a few reasons:

• thanks to Louis' work, it is now possible to implement authentication (and other features) through pluggable middlewares; in other words, you can add authentication without changing ZeroRPC's core;
• authentication with ZeroRPC also requires some transport-level security (just signing requests will only be useful in some contexts, because ZMQ sockets should not be publicly exposed), so even if authentication would of course be nice, it would not be useful for everybody; it might even be misleading, because people would think "oh, nice, I just have to enable this authentication middleware and I'm safe!" but that would not be true;
• last but not least, this pull request (as it is right now) is based on an older ZeroRPC version, so it can't be merged as-is, and would need to be heavily rebased. So we can't even merge it "disabled by default", it would require a lot of work first.

I hope you understand why it is not merged; but don't hesitate to ask questions if you need more details!

Cheers,

[bombela]

Additionally, zmq>=4 supports authentication. It exposes few different authentication method, all of that nicely integrated in zmq (so you get proper handshakes and so on).

import zerorpc
c = zerorpc.Client('tcp://....')
c.setsockopt(...)  # see http://api.zeromq.org/4-0:zmq-setsockopt

[adieu]

authentication support from zmq>=4 looks promising. I'm happy to rebase my code with the latest master when it comes out.

I think maybe we could put middlewares like authentication and tracing in a contrib sub module just like what Django did for admin and flatpages. Users could enable these additional features by themselves and the zerorpc core keeps clean.

[jpetazzo]

Sounds good!