search

rabbitmq / rabbitmq-auth-backend-cache

Authorisation result caching plugin (backend) for RabbitMQ
Other
17 stars 10 forks source link

vhost requests not cached #20

Closed BenTalagan closed 5 years ago

BenTalagan commented 5 years ago

Hi all,

Today I've been integrating a rabbitmq (3.7.12) with an HTTP authentication backend, with a backend cache. The http server is a rails server. I've noticed that everything seems to work well, every requests are cached except the vhost ones, strangely.

All four web services (user,vhost,resource,topic) just reply http status 200 and content 'allow' .

I use mosquitto_sub to connect to the rabbitmq server, and have enabled the mqtt plugin. This is actually my complete conf : 

mqtt.vhost            = /
mqtt.exchange         = mqttexchange
# 24 hours by default
mqtt.subscription_ttl = 86400000
mqtt.prefetch         = 10

auth_backends.1 = internal
auth_backends.2 = cache

auth_cache.cached_backend = http
auth_cache.cache_ttl      = 120000
auth_cache.cache_refusals = true

auth_http.http_method   = post
auth_http.user_path     = http://rabbitmq:password@localhost:5000/rabbitmq_auth/user
auth_http.vhost_path    = http://rabbitmq:password@localhost:5000/rabbitmq_auth/vhost
auth_http.resource_path = http://rabbitmq:password@localhost:5000/rabbitmq_auth/resource
auth_http.topic_path    = http://rabbitmq:password@localhost:5000/rabbitmq_auth/topic

The parameters received on the vhost service look like :

{"username"=>"acfef9c2157f", "vhost"=>"/", "ip"=>"::ffff:127.0.0.1"}

When performing the connection, quite a number of requests are sent to the rails auth backend (user,vhost,resource,topic), all are correctly cached by the cache plugin - except the vhost ones.

I'm not familiar to the erlang language, so I don't have a clue about a potential bug. Any idea?

lukebakken commented 5 years ago

When performing the connection, quite a number of requests are sent to the rails auth backend (user,vhost,resource,topic), all are correctly cached by the cache plugin - except the vhost ones.

"except the vhost ones" - can you provide a couple complete HTTP request / responses for these non-cached requests? Please attach in a file. Thanks.

BenTalagan commented 5 years ago

Sure, this is a serie of vhost requests sniffed with wireshark between RMQ and the rails server. Please tell me if it suits your needs and if you want any other info. Thanks a lot for your very fast answer by the way!

uncached_requests.txt

lukebakken commented 5 years ago

Thanks, that helps a lot. I'll check it out.

michaelklishin commented 5 years ago

Is there a way for you to share the requests with timestamps? It should be possible to only export a range of frames in Wireshark.

BenTalagan commented 5 years ago

Okay, I'm not very familiar with wireshark, here's a complete capture of the flow (wireshark format).

vhost_capture.pcapng.zip

Requests are made repeatedly almost every second by mosquitto here (the connection is refused because I've denied the authorization at topic level, so mosquitto will retry over and over again, fully restarting the authentication/authorization process, on user / vhost / resource / topic services - the cache plugin does it job well for every service except for vhost, so the final result is that RMQ will only send the vhost requests, not the other ones, and as such, the flow is only made from vhost requests).

michaelklishin commented 5 years ago

This plugin performs caching within the context of a connection. If a client reconnects over and over, those are new connections and their operations will not be cached. Can you please share server logs, which will make it clear whether that's the case? I don't see any MQTT connections in the capture file above.

I don't see how authorization failure at topic level would refuse connection. The guide on Connections describes the flow. Before access to virtual host is allowed, a client cannot perform any operation, so topic authorization restrictions are not relevant. If virtual host access is allowed, and topic authorization fails, in case ofo MQTT the connection must be closed. If Mosquitto client reacts to that by reconnecting, we have the scenario above where caching is not meant to work because it's a new connection.

@BenTalagan please provide a way to reproduce. So far it seems to be that the plugin works as expected but the dump doesn't include any MQTT client activity so we are guessing.

michaelklishin commented 5 years ago

I wasn' completely correct in the comment above. There are more nuances: this plugin doesn't cache refused operations by default. auth_cache.cache_refusals can be set to true.

With ETS caching backend then cache results should be kept around for a while between connections, as long as all authentication context values are identical and cache invalidation hasn't happened.

I don't see any explicit cache invalidation for closing connections.

BenTalagan commented 5 years ago

@BenTalagan please provide a way to reproduce. So far it seems to be that the plugin works as expected but the dump doesn't include any MQTT client activity so we are guessing.

Ok, this is gonna be a bit long to reduce to a simple case. I think the best way would be for me to write a very small auth server with almost nothing inside, it'd be easier that way :) . It should take me a little while to provide it, but just let me react to your other answers.

I don't see how authorization failure at topic level would refuse connection. The guide on Connections describes the flow. Before access to virtual host is allowed, a client cannot perform any operation, so topic authorization restrictions are not relevant. If virtual host access is allowed, and topic authorization fails, in case ofo MQTT the connection must be closed. If Mosquitto client reacts to that by reconnecting, we have the scenario above where caching is not meant to work because it's a new connection. I wasn't completely correct in the comment above. There are more nuances: this plugin doesn't cache refused operations by default. auth_cache.cache_refusals can be set to true. With ETS caching backend then cache results should be kept around for a while between connections, as long as all authentication context values are identical and cache invalidation hasn't happened. I don't see any explicit cache invalidation for closing connections.

cache_refusals is set to true in my configuration, so this is the context you are evoking in your second answer, and it looks more to what I've observed (your first answer was really surprising compared to what I'm observing : for example, when killing a mosquitto , and relaunching it, I can clearly see the cache involved). You're right, mosquitto does not 'disconnect' on topic denial, but the behaviour seen from the HTTP server is that the full cycle of authorizations will be done again (it can be seen when disabling the cache).

This is what's printed by mosquitto : 

Client toto received CONNACK (0)
Client toto sending SUBSCRIBE (Mid: 698, Topic: /acfef9c217f/uplink/#, QoS: 1)
Client toto sending CONNECT
Client toto received CONNACK (0)
Client toto sending SUBSCRIBE (Mid: 699, Topic: /acfef9c217f/uplink/#, QoS: 1)
Client toto sending CONNECT
Client toto received CONNACK (0)
Client toto sending SUBSCRIBE (Mid: 700, Topic: /acfef9c217f/uplink/#, QoS: 1)

So, the flow I'm observing, and caching disabled :

And with caching enabled (with a clear cache) :

That's what I mean by all requests are cached, except vhost requests. Killing mosquitto and relaunching it will not change the cache behaviour.

@BenTalagan please provide a way to reproduce. So far it seems to be that the plugin works as expected but the dump doesn't include any MQTT client activity so we are guessing.

Sure, and now I have too much activity in my captures so it's almost unreadable. The best thing to do is probably to give me a little while to write a small server, cause it's even harder and harder for me to just write clear comments! :-)

michaelklishin commented 5 years ago

There's a bunch of example services that you can modify and share in a separate repo. We'll try to reproduce either way, thank you for the details.

BenTalagan commented 5 years ago

Yup, seen that already. No ruby example yet (sinatra or rails), could be a good occasion!

BenTalagan commented 5 years ago

Ok, I can reproduce it with a very small rails server. Here is the source :

rmq_auth_backend.zip

Here is my rabbitmq conf file :

rabbitmq.conf.zip

And here is the command to launch mosquitto :

mosquitto_sub -p 1883 -h 127.0.0.1 -u user -P password -i client_id -t topic -d -v -q 1

I hope you're a bit familiar with ruby/rvm ; with a recent install of ruby, you should be able to go to the server directory, and install the needed components with : 

cd rmq_auth_backend
gem install bundler
bundle

Then launch the server like this : 

rails s

As you can see the authentication controller is dead simple : 

class RabbitmqAuthController < ApplicationController

  def user
    render status: :ok, html: 'allow'
  end

  def vhost
    render status: :ok, html: 'allow'
  end

  def resource
    render status: :ok, html: 'allow'
  end

  def topic
    render status: :ok, html: 'deny'
  end

end

With these elements you should be in the same configuration as I am. This produces the behavior I've tried to describe (with pain!). If you need anything, don't hesitate to ask!

BenTalagan commented 5 years ago

Oh, one more useful info I can add is that the problematic behavior remains the same if I change the cache module from auth_cache.cache_module = rabbit_auth_cache_ets to auth_cache.cache_module = rabbit_auth_cache_dict.

lukebakken commented 5 years ago

Hi @BenTalagan I finally had time to investigate and yes, I can reproduce this in my environment. I plan on starting my day tomorrow with this.

BenTalagan commented 5 years ago

Hi @lukebakken, awesome. Thanks a lot for the update, wish you good luck for your investigations!

lukebakken commented 5 years ago

OK here's the reason why VHost requests aren't cached as you might expect. There are three pieces of data here that are used to uniquely identify a cached authz request. Notice that the socket itself is used as part of this unique identifier. When the mosquitto_sub client makes a new connection, more than likely its TCP port is going to change, as you can see in this output where the first request is 50988 and the second is 50990:

2019-03-20 14:07:33.470 [debug] <0.1011.0> rabbit_auth_backend_cache:check_vhost_access args: [{auth_user,<<"user">>,[],none},<<"/">>,{authz_socket_info,{{0,0,0,0,0,65535,32512,1},1883},{{0,0,0,0,0,65535,32512,1},50988}}]

2019-03-20 14:07:34.522 [debug] <0.1048.0> rabbit_auth_backend_cache:check_vhost_access args: [{auth_user,<<"user">>,[],none},<<"/">>,{authz_socket_info,{{0,0,0,0,0,65535,32512,1},1883},{{0,0,0,0,0,65535,32512,1},50990}}]

Since the 4-tuple of source IP, source port, destination IP, destination port is what uniquely identifies a TCP connection, there won't be cache hits in this case. In situations where the port remains stable, cache hits will occur.

I don't believe there to be an issue to address at this point and will close this issue.

michaelklishin commented 5 years ago

FWIW @lukebakken's findings are consistent with the "per connection caching" scenario I had in mind.

lukebakken commented 5 years ago

This plugin performs caching within the context of a connection

Oh, and there it is 😄

BenTalagan commented 5 years ago

Hi Luke and Michael, thanks for your answers! This is far clearer to me now. What's a bit misleading thus is the mismatch between the http auth backend plugin documentation and what's the cache plugin effectively does : reading the doc, one would tend to think that the vhost authorization is IP based only, but the cache plugin extends it to the full socket. Also, the cache plugin documentation does not state explicitly that its behavior is guaranteed "per connection", and it actually does more for all services - except vhost (which is the only one that really caches the full socket info, the other services are thus not, in reality, per-connection based). I don't say it's a questionnable choice, but it just feels a bit ... weird (or non-homegeneous). Is it what's really wanted here? :)

lukebakken commented 5 years ago

reading the doc, one would tend to think that the vhost authorization is IP based only, but the cache plugin extends it to the full socket

Software that implements the HTTP authz backend for vhosts don't necessarily care about the full 4-tuple representing the TCP connection - they should really only care about the client's IP address, which is why only that data is passed. The client's port is ephemeral and shouldn't be used for making auth decisions.

Also, the cache plugin documentation does not state explicitly that its behavior is guaranteed "per connection"

I'm guessing because most people don't care about that level of detail. They expect caching to "just work", so each kind of authn/authz request is cached using whatever data is necessary to identify a "unique client session". In the case of vhost access, we have to use TCP connection information.

Maybe this level of detail isn't actually necessary and we can cache vhost access based on AuthUser and VHostPath only ... @michaelklishin ??? I have checked the included auth plugins (HTTP, LDAP, and AMQP) and only the HTTP plugin makes any use of Sock, and then it's only the client IP address -

$ find . -type f -name '*.erl' -exec egrep '\<check_vhost_access\(.*Sock' {} +
./deps/rabbitmq_auth_backend_http/src/rabbit_auth_backend_http.erl:check_vhost_access(#auth_user{username = Username, tags = Tags}, VHost, Sock) ->
./deps/rabbit/src/rabbit_auth_backend_internal.erl:check_vhost_access(#auth_user{username = Username}, VHostPath, _Sock) ->
./deps/rabbit/src/rabbit_reader.erl:    ok = rabbit_access_control:check_vhost_access(User, VHost, Sock),
./deps/rabbit_common/src/rabbit_auth_backend_dummy.erl:check_vhost_access(#auth_user{}, _VHostPath, _Sock) -> true.
./deps/rabbitmq_auth_backend_cache/src/rabbit_auth_backend_cache.erl:check_vhost_access(#auth_user{} = AuthUser, VHostPath, Sock) ->
./deps/rabbitmq_management/src/rabbit_mgmt_util.erl:          case catch rabbit_access_control:check_vhost_access(User, V, Sock) of
./deps/rabbitmq_auth_backend_amqp/src/rabbit_auth_backend_amqp.erl:check_vhost_access(#auth_user{username = Username}, VHost, _Sock) ->

You're probably the first non-RabbitMQ core engineer to investigate the interaction between the HTTP backend and caching. I know I haven't had the occasion to do so, myself, until now.

lukebakken commented 5 years ago

Re-opening because, perhaps, rabbitmq-auth-backend-cache should only use the client IP as part of the unique cache identifier, and not the entire 4-tuple for the TCP connection.

BenTalagan commented 5 years ago

You're probably the first non-RabbitMQ core engineer to investigate the interaction between the HTTP backend and caching. I know I haven't had the occasion to do so, myself, until now.

This is pure chance really, since I was porting a mqtt architecture to rabbitmq with mqtt support. I thought at first that I had misconfigured the plugin before realizing the odd behavior dissected here. If mosquitto wasn't involved I would probably have missed it.

The client's port is ephemeral and shouldn't be used for making auth decisions. [...] Re-opening because, perhaps, rabbitmq-auth-backend-cache should only use the client IP as part of the unique cache identifier, and not the entire 4-tuple for the TCP connection.

Agreed, you're completely pointing out what I'd felt weird : involving the client's port in the auth process (which is probably always random) might not be totally pertinent (imho) :-)

Thanks anyway for spending much of your time on this issue (which is, honestly, a minor one !).

lukebakken commented 5 years ago

@michaelklishin how about this for a resolution -

michaelklishin commented 5 years ago

We can add a map of options for check_vhost_access/3 that would include just the client IP address for starters. 👍 on the v3.7.x change as the current behavior is confusing and I can see why many users would consider it to be a bug.

BenTalagan commented 5 years ago

Many thanks for this and for your reactivity! 👍

michaelklishin commented 5 years ago

Not all PRs are merged, so reopening.

rushikesh1 commented 5 years ago

If I need this changes along with server version 3.7.14. what steps do I need to perform ?

michaelklishin commented 5 years ago

@rushikesh1 you can't use a version built from source with 1.7.14 as it depends on core internal API changes such as https://github.com/rabbitmq/rabbitmq-common/commit/7dab9f6f60dccecfb6a1a9afff12db60fa69f463.

rushikesh1 commented 5 years ago

Thanks Michael.