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

Jump to bottom

FS#3804 - rpcd file Plugin ACLs can be bypassed when used via uhttpd JSON/RPC #8849

Closed
openwrt-bot opened this issue May 12, 2021 · 2 comments
Closed

FS#3804 - rpcd file Plugin ACLs can be bypassed when used via uhttpd JSON/RPC #8849

openwrt-bot opened this issue May 12, 2021 · 2 comments
Labels
flyspray

Comments

@openwrt-bot
Copy link

spfendtner:

Hi,

I've configured the rcpd file plugin together with the uhttpd ubus plugin to put the ubus on the network and to me there seems to be a security problem when used in this combination.
I think it is some quite exotic experiment by myself and I hope nobody had done this on a live field system on the internet.

Let me elaborate the situation little bit.

The uhttpd will first check the ACLs of the call. In this case whether the user is allowed to call the ubus file object and the read/write/... methods. Lets assume it is allowed.
Further down the call hierarchy the rpcd file plugin will then check its file ACLs if the given session contains the permissions to execute, read, write or list the specified file or directory path.

To accomplish this task the rpcd file plugin needs a session id and as the primary session id was not passed down by uhttpd together with the call it is now not there any more. That's why there is a secondary session id within the ubus file methods payload parameters. The content can be either the same sid or another one, that doesn't mater in this example.

Now if we call a file.exec() via JSON/RPC (uhttpd/ubus) we have to specify the SID two times.
The Problem: If I leave out the inner secondary sid completely I get passed all file ACLs!
The function rpc_file_access() in rpcd will return true at file.c:180 if the sid is NULL!

There is no enforcement of the ubus file methods policy field "ubus_rpc_session". On none of the methods! Only the "path" parameter is enforced.

Either the "return true" should be removed or the methods should enforce a sid. Otherwise one can not use this rpcd file plugin securely as a authenticated users can bypass all file rpcd file ACLs easily by simply dropping the parameter field.

Regards,
Steffen
@openwrt-bot
Copy link
Author

jow-:

To accomplish this task the rpcd file plugin needs a session id and as the primary session id was not passed down by uhttpd together with the call it is now not there any more.

This sounds like a bug. It is not supposed to work this way.

That’s why there is a secondary session id within the ubus file methods payload parameters.

The parameter ubus_rpc_session is automatically set by uhttpd-mod-ubus when forwarding the HTTP request to ubus. For security reasons it should not be allowed to set this parameter in the HTTP request. See the code here:

https://git.openwrt.org/?p=project/uhttpd.git;a=blob;f=ubus.c;h=619135ce3912f3b728c698f4fe4d6018cade6422;hb=15346de8d3ba422002496526ee24c62a3601ab8c#l561

First it ensures that the method call parameters received in the HTTP request do not contain "ubus_rpc_session" (otherwise the request is rejected with an invalid parameter error). Afterwards it sets the "ubus_rpc_session" parameter to the currently authenticated session ID.

Now if we call a file.exec() via JSON/RPC (uhttpd/ubus) we have to specify the SID two times.
The Problem: If I leave out the inner secondary sid completely I get passed all file ACLs!

Do you have an actual HTTP reproducer (as in curl call) for this or are these just assumptions you made after reading the code and/or trying the ubus calls directly from the cli (via ubus call file ...), bypassing the ubus_rpc_session enforcement of uhttpd-mod-ubus ?

The function rpc_file_access() in rpcd will return true at file.c:180 if the sid is NULL!

Yes, this is by design - ubus call file ... via CLI, or rarther an ubus request without a session ID, grants access to all methods. When invoked via uhttpd-mod-ubus, the ubus_rpc_session is forcibly set though, there is no way for an HTTP client to produce a request that lacks this field, as it is internally added by uhttpd itself. Unless of course there is an actual bug in uhttpd-mod-ubus that prevents the addition of "ubus_rpc_session" which I can't rule out at the moment as this code was recently refactored and I didn't look into it since then.

@openwrt-bot
Copy link
Author

spfendtner:

OK, thanks for the hint inside uhttpd.
My uhttpd/ubus code was not quite identical with upstream.

Sorry for bothering I will try to reproduce things first with upstream next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
flyspray
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@openwrt-bot