-
Notifications
You must be signed in to change notification settings - Fork 82
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
sdbusplus + polkit integration #71
Comments
There isn't really any problem with doing this, in general except for a few considerations: a. sd_bus doesn't allow you to call back into your own daemon. You will get an ELOOP. Doesn't sound applicable here.
I'm a little perplexed as to why you need the original message. The generated bindings already unpack all the parameters from the original message into function arguments for you. What is left in the original message that you need out of it? Assuming you've written the YAML with the right
Oh. You're not actually looking for message parameters. You're looking for meta-data from within the message object about who sent the request to you. I think I understand now. Currently the bindings don't allow the message to be visible anywhere, but that could be changed. I had considered adding an interface like that as a hack to add async support to the bindings (if the message were erased by the handler then assume that the handler turned it into an async operation) but it seemed like a hack and co-routines have come along now. These are the two locations where the message is saved prior to calling the handler (for properties and message handlers respectively):
I don't see any reason we couldn't turn the
I'm not sure exactly what you're doing with polkit, but are you sure this is even the right thing to do? Are you trying to protect dbus interfaces or something else? If you're trying to protect dbus interfaces then dbus has ACLs already, which I think leverage polkit, and is handled by the broker without any additional support by you. There was work someone else in the community was driving to run daemons as non-root and they had to add some bbclass support (and examples) for supplying these ACLs in order to get that working.
Unfortunately I can't really change the API of the pure-virtual functions without breaking all the existing implementations, so I can't just have the bindings feed it in as a new argument. The "expose it by a backdoor method that looks it up from a TLS" I mentioned above is probably the only way we can do it in the existing implementation. If this is valuable work to have, as I work on the co-routine support, I can put that on the table as well. I'm already planning some C++20 concept-based detection to figure out if the handler is sync/async by checking the return type for a co-routine or value. It wouldn't be too hard to also check the first argument for an optional
As I mentioned above, the ASIO code requires all your handlers to be written to be "by hand"; they don't perform any validation against YAML/XML dbus schema nor is there any capability to generate stubs. I was on hiatus from the project during the time that ASIO support was added but there was obviously some difference of opinion on how dbus interfaces should be written... but the net is that they're completely different in how to make a server than the generated bindings you're currently looking at. |
Thanks for the tips @williamspatrick. Much appreciated. Just some quick background. We're looking to integrate polkit into SDBusPlus so that we can provide an Authorization layer to dbus services implemented in sdbusplus. As an example, Michael Brown provided the following: polkit-test. It demonstrates the connection. And, it appears to be synchronous. I think we've settled on using SDBusPlus because of the YAML support. So, I'm trying to duplicate this using the SDBusPlus framework. I'll take a look at some of the ideas you provided. I'm still stuck on keeping this synchronous if I can. But, I also understand the pitfalls I'll likely face. Michael and I had already started looking at the message_t you're pointing to and how to expose it. Glad to hear we may be going the right direction. I'll let you know what I find out. |
I'm still slightly curious what polkit gives you over the busconfig ACLs. Especially considering we don't currently have a whole lot of applications running as non-root. Something you're also going to take into account is that external interfaces, such as bmcweb, might be running as one user (root) but the operation is on behalf of some other user (ex. admin). The dbus-message is going to have the daemon's effective UID and not the authenticated requester. I guess that is perhaps something we could work on also. It does look like systemd itself does do some amount of async polkit-based authentication. You might want to glance at that. I don't see a whole lot of APIs that are readily exposed, such that we could make sdbusplus wrappers around them, but it does appear that they do most of the polkit calls asynchronously. I did see this little tidbit in the dbus-daemon manpage also (https://dbus.freedesktop.org/doc/dbus-daemon.1.html) :
|
The controls that dbus gives you out of the box for access control are only granular down to the method, interface, path, bus, or property level and can use the calling user, group, or local-console-logged-in to differentiate. However, if you want to do access control based on arguments to a method, internal state of a daemon, the time of day or phase of the moon, you cannot. Example: if you have a daemon (configd) that has the API: GetConfig( key string ), SetConfig( key string, value string) and you have the following 'keys': And you have the following daemons that use configd: Using standard dbus access controls, there is no method to ensure that the RestWebService CAN NEVER read or write the PrivateKey and LicenseServerAddress. Using standard dbus access controls you cannot enforce that licenced can only write the private key if in manufacturing mode. You can do this with polkit. (and more) You dont always need polkit, but when you need it, you need it. |
To implement the above access control scheme, the configd daemon just does this: sender = sd_bus_message_get_sender() The properties passed in third argument dictionary are completely arbitrary and can be made up to suite whatever a policy engine might need to make a decision. Next, you can then write a polkit rule that matches the action ID "com.example-SetConfig", and it can pull out all the annotations, in this case 'config-key', and can then make a decision. Polkit rules are written in javascript, and run in secure environment inside polkit. The rules can do any arbitrary tests and return an answer. |
based on the above check added to the hypothetical 'configd', each daemon can drop in policy rules to grant itself access to just the things it needs. Then, finally, we can also implement user-level access controls using the same method. |
We're attempting to recreate the following example in sdbusplus wrt the dbus call to polkit's "CheckAuthorization":
Creating a D-Bus Service with dbus-python and Polkit Authentication
We need to make a “recursive” method call to another dbus method from within the handler of one dbus method. The key missing piece is that we need to pull out parameters from the original “sd_bus_message *” struct that is a parameter to the original call. The C++ wrapper stuff hides this parameter, and makes this difficult when in the context of the class-based handler.
This the sd_bus call to the polkit “CheckAuthorization” method is done from the handler of another dbus method handler. The sdbusplus class-based handler does not appear to have a way to get a reference to the “sd_bus_message *” that is needed to call sd_bus_message_get_sender(). The python example code has access to this parameter.
The sdbusplus/example/calculator-server.cpp example code. The “conceptual” thing we are trying to do is add a polkit check to the “multiply()” method (line 21) in the Calculator class. The multiply() method is called by generated code and part of the method call chain is the callback called by dbus, which is generated wrapper code that gets a sd_bus_message * parameter, but it does not appear to expose a method to get a reference to that parameter in the multiply() codepath.
In the other example in sdbusplus/example/asio-example.cpp, the “methodWithMessage(sdbusplus::message_t&)” is there. The “message_t” parameter appears to be EXACTLY what we need to be able to call sd_bus_message_get_sender(). Is there a way to get this parameter in a class-based call?
The text was updated successfully, but these errors were encountered: