Proxy when module_accounting rejects the request
I noticed that accounting requests are proxied anyway even if a module of the accounting section rejected the request. I don't know whether this is the expected behaviour or not. In other words: is this a bug or feature ? :-) Perhaps there are reasons to always proxy an accounting request, but I'm thinking the action might be: Return value Proxy Drop -------------------------------------- RLM_MODULE_REJECT X RLM_MODULE_FAIL X RLM_MODULE_OK X RLM_MODULE_HANDLED X RLM_MODULE_INVALID X RLM_MODULE_USERLOCK X RLM_MODULE_NOTFOUND X RLM_MODULE_NOOP X RLM_MODULE_UPDATED X -- Nicolas Baradakis
Nicolas Baradakis <nbk@sitadelle.com> wrote:
I noticed that accounting requests are proxied anyway even if a module of the accounting section rejected the request. I don't know whether this is the expected behaviour or not. In other words: is this a bug or feature ? :-)
I would say there's a few bugs there. If "preacct" says that the request should be proxied, we probably shouldn't even run "accounting" at all. That will let people log local accounting data only for requests that are handled locally.
Perhaps there are reasons to always proxy an accounting request, but I'm thinking the action might be:
Return value Proxy Drop -------------------------------------- RLM_MODULE_REJECT X RLM_MODULE_FAIL X RLM_MODULE_OK X RLM_MODULE_HANDLED X RLM_MODULE_INVALID X RLM_MODULE_USERLOCK X RLM_MODULE_NOTFOUND X RLM_MODULE_NOOP X RLM_MODULE_UPDATED X
That sounds reasonable, except for FAIL. If we fail to log accounting data, it's even more useful to proxy it. And most of those return codes don't make sense for accounting requests. Since accounting just does logging, the return codes should be: FAIL, OK, HANDLED, INVALID, NOOP. REJECT doesn't make sense. USERLOCK doesn't make sense, and I'm not sure what UPDATED means. Alan DeKok.
Alan DeKok wrote:
If "preacct" says that the request should be proxied, we probably shouldn't even run "accounting" at all.
I think there are some cases when there is a need to do both logging and proxying. (for example if the server and the proxy belong to different ISP) In those cases logging could be done in pre-proxy section instead of accounting, but currently not all the modules have a method for both accounting and pre-proxy. (for example rlm_sql can do accounting only) I've never understood why we have pre-proxy and post-proxy for accounting requests. As it is now, everything done in pre-proxy can be done in accounting, too. And post-proxy is meaningless since Accounting-Response packets are empty.
That will let people log local accounting data only for requests that are handled locally.
For now that can be achieved using Acct-Type stanzas.
That sounds reasonable, except for FAIL. If we fail to log accounting data, it's even more useful to proxy it.
I understand your reasons. The logs of the proxy may be incoherent, but that's probably better than to have nothing at all.
And most of those return codes don't make sense for accounting requests. Since accounting just does logging, the return codes should be:
FAIL, OK, HANDLED, INVALID, NOOP.
I agree. It should be the same for preacct modules, too.
REJECT doesn't make sense. USERLOCK doesn't make sense, and I'm not sure what UPDATED means.
Comments in modules.h says UPDATED is "OK (pairs modified)". However if a module returns REJECT or USERLOCK, it just means the module is seriously broken. It's unclear whether the packet should be proxied in this case. If something that shouldn't happen actually happens, I would vote to drop the packet. -- Nicolas Baradakis
Nicolas Baradakis <nbk@sitadelle.com> wrote:
I think there are some cases when there is a need to do both logging and proxying. (for example if the server and the proxy belong to different ISP)
Sure, but we don't want to *force* that, either.
I've never understood why we have pre-proxy and post-proxy for accounting requests. As it is now, everything done in pre-proxy can be done in accounting, too. And post-proxy is meaningless since Accounting-Response packets are empty.
pre-proxy may require User-Name re-writing, which really belongs in the same "function" module for authentication && accounting. Post-proxy is pretty useless for accounting, though.
However if a module returns REJECT or USERLOCK, it just means the module is seriously broken. It's unclear whether the packet should be proxied in this case. If something that shouldn't happen actually happens, I would vote to drop the packet.
Sure. We should take a sweep through the modules to double-check their return codes. Alan DeKok.
Alan DeKok wrote:
I think there are some cases when there is a need to do both logging and proxying. (for example if the server and the proxy belong to different ISP)
Sure, but we don't want to *force* that, either.
I agree, although for now I don't know how we could make that user-defined. Perhaps this could work: if Proxy-To-Realm is set and Acct-Type is not set, then skip accounting section.
pre-proxy may require User-Name re-writing, which really belongs in the same "function" module for authentication && accounting.
Post-proxy is pretty useless for accounting, though.
Thanks for your responses: I was a little lost with all this accounting proxy stuff. -- Nicolas Baradakis
participants (2)
-
Alan DeKok -
Nicolas Baradakis