crossbario / crossbar

Crossbar.io - WAMP application router
https://crossbar.io/
Other
2.05k stars 275 forks source link

Router-side warning log for missing dynamic authenticator #649

Open sehoffmann opened 8 years ago

sehoffmann commented 8 years ago

In case dynamic authentication is used but the authenticator is actually not registered, the current behaviour is bad in my opinion.

How the current behaviour looks like:

1) The peer trying to authenticate gets back an error e.g wamp.error.authentication_failed (" Object { reason: "wamp.error.authentication_failed", message: "dynamic authenticator failed: ApplicationError(error=<wamp.error.no_such_procedure>, args=[u'no callee registered for procedure <replay.internal.authenticate.user.wampcra>'], kwargs={}, enc_algo=None) 2) There is no log in the router process

I believe that a missing authenticatior should not be considered a regular case. It should be considered a bug in the application. Thus, there must be a log of such events, when someone tries to authenticates but the authenticator is missing. Furthermore I consider it questionable, that internal information (i.e which authenticator is used) is exposed to a peer.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/30842750-missing-authenticator-questionable-behaviour?utm_campaign=plugin&utm_content=tracker%2F462544&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F462544&utm_medium=issues&utm_source=github).
oberstet commented 8 years ago

Yes, I guess I agree with both of the issues you raise.

The client should probably get something like a HTTP 500 - that is a generic "internal server" error. The client should only get a generic error, and not leak app internal information.

And the server log should contain a log message at error level. However, not every instance of calling into a missing procedure should log at level error, or even log at all. Because there is a DoS attack vector here too: make the app drown in bad requests and accompanied logging. But that means, there would need to be an upfront list of "required procedure" - that is procedures which when missing should be treated as app errors.

That would be possible with the new, more flexible namespace configuration in Crossbar.io. Something roughly like this:

{
   "uri": "com.example.authenticate",
   "allow": {
      "call": true,
      "register": true
   },
   "max-concurrent": {
      "caller": 100,
      "overflow": "queue"
   },
   "min-concurrent": {
      "callee": 1,
      "underflow": "error"
   }
}
sehoffmann commented 8 years ago

Interessting is, that yonder behaviour is already implemented (log + "internal error" message), but if e.g the authenticator returns illformed values.

rafzi commented 8 years ago

Related: #560