Bug 14829

Looks like we may have to revert the ducktape change for kirkstone and update master as soon as there's a new version.

I'm running several projects with polkit 0.119 and duktape on yocto 2.5 sumo and 3.1 dunfell.

I have not seen issues with polkit rule incompatibility between mozjs and duktape.

I have seen a lot of boot time and rootfs size regressions over the years due to mozjs. Thus I have proposed to backport this patch.

It would be nice to know what exactly in your rules is special which breaks on duktape but works mozjs.

If revert is needed, then it can be done. I don't see the problems and can use duktape.

FWIW, I'm using duktape and polkit 0.119 backports on both x86_64 and aarch64 ARM products from multiple SoC vendors.

Waiting for Piotr so moved to Need Info state.

Created attachment 4874 [details]
Polkit rules for binaries to be run with pkexec.

Hi all,
sorry for late response, but i haven't been available yesterday evening. OK, first of all I have uploaded the rules with we are using for our services. These services are being run from user group mgmtd and the problem which I have noticed is that I'm getting i.e. this kind of error:

Jun 07 09:46:06 eg pkexec[59699]: test: Error executing command as another user: Not authorized [USER=root] [TTY=/dev/pts/0] [CWD=/home/root] [COMMAND=/usr/sbin/nft]

And when i have switched back to mozjs-78 the problem is gone and the services are now running properly. There is no problem with running apps with the usage of pkexec.

One more thing this is the link to my discussion with Jan Rybar from polkit development team https://www.mail-archive.com/polkit-devel@lists.freedesktop.org/msg00579.html and I have also found somewhere in space:P a gitlab conversation in which there are voices that not everything works with duktape. Only from version 0.121 there is official support and in my opinion, it should be considered to switch to that release.

Can you post some more logs from polkit?

I have plenty of more complex rules for polkit and they work after switch from mozjs to duktape.

Then the blocking issue for duktape patches to polkit were the unit tests which were failing but they do pass now, or otherwise https://gitlab.freedesktop.org/polkit/polkit/-/merge_requests/35 would not have been merged.

In upstream polkit, the version 0.121 there isn't much changes for duktape, except for switch from autotools to meson build system. If there are some fixes, then I can't find them from git. That's why I was confident that backport of the polkit changes from 0.121 to 0.119 was a small patch and it does work for me.

More logs would be needed here. And possibly a reproduction with polkit side unit tests. It could also be a bug in duktape, but 2.7.0 is still the latest release there.

In my use cases and HW, startup time of polkit service is reduced with duktape from 5 to under 1 second.

Created attachment 4875 [details]
Authorization log.

Created attachment 4876 [details]
Pkexec log.

I have attached the proper log files with mozjs set as js engine. You can se in there, that pkexec starts command properly. I will also attach log files with errors when duktape is set as default js engine in a moment

What if user_app is prefixed with "var user_app"?

$ diff -up 30-sbin-mgmtd.rules-orig 30-sbin-mgmtd.rules
--- 30-sbin-mgmtd.rules-orig    2022-06-10 10:08:45.503055043 +0200
+++ 30-sbin-mgmtd.rules 2022-06-10 11:19:25.904248692 +0200
@@ -1,5 +1,5 @@
 polkit.addRule(function(action, subject) {
-    user_app = [
+    var user_app = [
     '/bin/chmod',
     '/bin/chown',
     '/bin/rm',

Created attachment 4877 [details]
Auth log with failures.

Created attachment 4878 [details]
Pkexec log with failures.

OK, log files with errors have been also added. Mikko, gimme a moment I can verify your change to my rules.

Mikko, I have verified the rule with adding var and still the same behaviour.

(In reply to comment #16)
> Mikko, I have verified the rule with adding var and still the same behaviour.

behavior

How about this change to the rule?

--- 30-sbin-mgmtd.rules-orig    2022-06-10 10:08:45.503055043 +0200
+++ 30-sbin-mgmtd.rules 2022-06-10 12:11:05.544234291 +0200
@@ -19,7 +19,7 @@ polkit.addRule(function(action, subject)
     '/usr/sbin/useradd',
     '/usr/sbin/userdel'
 ];
-    if (action.id == "org.freedesktop.policykit.exec" && subject.user == "mgmtd" && user_app.includes(action.lookup("program"))) {
+    if (action.id == "org.freedesktop.policykit.exec" && subject.user == "mgmtd" && (user_app.indexOf(action.lookup("program") >= 0)) {
         return polkit.Result.YES;
 }
 });


I played around with
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes
and
https://duktape.org/dukweb.html
and it seems that the array search includes() function is not implemented in duktape. Sadly there is no error for this but this is normal to javascript :(

It kind of makes sense for a tiny js engine to not support all latest features but this one may bite users, sorry about that.

> It kind of makes sense for a tiny js engine to not support all latest
> features but this one may bite users, sorry about that.

N/p at all :] I'm glad we are fixing this issue and also that it has been discovered. Gimme a sec i need to re-run all testing stuff to investigate with this change.

OK, I am still facing the issue with this rule. The fix you have given isn't working.

Created attachment 4879 [details]
systemctl without changes in 30-sbin-mgmtd.rules

Created attachment 4880 [details]
systemctl with changes in 30-sbin-mgmtd.rules

I have attached this to log files for systemctl, as as you can see something has changed. Now these 3 services:
- mgmtd-device@devadmin.service
- mgmtd-device@devread.service
- mgmtd-firewall@devadmin.service
are not being auto-restared, but running as supposed to, but I'm still getting the pkexec errors like before... I suspect that more of our rules needs investigation. Let me verify which that might be and i will upload them in here, so we may investigate it together.

Created attachment 4881 [details]
10-systemd-mgmtd.rules

Created attachment 4882 [details]
20-cli.rules

OK, I've uploaded 2 more rules which i suspect that may have problems. More over I have changed the mgmtd user group in /etc/passwd to /bin/bash to be able to log as mgmtd user and run 'pkexec /usr/sbin/nft flush ruleset` command and I'm still getting this error:

Error executing command as another user: Not authorized

This incident has been reported.

FYI: duktape compatibility with various javascript features include array.includes()

https://kangax.github.io/compat-table/es6/

In short, duktape doesn't support any of them.

The rules posted here all seem to use array.includes() function which is not supported by duktape.

In my small experiments https://duktape.org/dukweb.html all array.includes('item') calls could be replaced by "array.indexOf('item') >= 0"

I'm pretty sure this should work on the rules reported here.

Indeed, in my products array.includes() is not used at all.

OK, I'll be able to verify it later, because I'm currently little blocked by yocto compilation.

If polkit rules stick to ECMA script standard version 5 features, then duktape is an almost full replacement of mozjs:

https://kangax.github.io/compat-table/es5/

But if not, then of course mozjs has more features and duktape will fail to work with them. Luckily it at least denies access.

From my point of view, yocto should stick with more embedded oriented duktape.

The overhead of mozjs compared to duktape is huge.

(In reply to comment #29)
> If polkit rules stick to ECMA script standard version 5 features, then
> duktape is an almost full replacement of mozjs:
> 
> https://kangax.github.io/compat-table/es5/
> 
> But if not, then of course mozjs has more features and duktape will fail to
> work with them. Luckily it at least denies access.
> 
> From my point of view, yocto should stick with more embedded oriented
> duktape.
> 
> The overhead of mozjs compared to duktape is huge.

I totally agree with you, but - yes there are some buts:P - there should be some informations about such issues and also a possiblity to switch between duktape and mozjs in order to rule out various problems.

I guess yocto standard solution here would be PACKAGECONFIG option with default mozjs and optional of using duktape and then a documented warning about javascript feature differences between mozjs and duktape.

I don't know if polkit rule validation could be done, I think javascript is quite poor for such things...

This is definitely a must.

> I don't know if polkit rule validation could be done, I think javascript is quite poor for such things...

Everyone is currently so excited with js and this is a huge issue that it lacks of such possibilities.

Mikko, I will verify all other rules and post you with an update.

(In reply to comment #18)
> How about this change to the rule?
> 
> --- 30-sbin-mgmtd.rules-orig    2022-06-10 10:08:45.503055043 +0200
> +++ 30-sbin-mgmtd.rules 2022-06-10 12:11:05.544234291 +0200
> @@ -19,7 +19,7 @@ polkit.addRule(function(action, subject)
>      '/usr/sbin/useradd',
>      '/usr/sbin/userdel'
>  ];
> -    if (action.id == "org.freedesktop.policykit.exec" && subject.user ==
> "mgmtd" && user_app.includes(action.lookup("program"))) {
> +    if (action.id == "org.freedesktop.policykit.exec" && subject.user ==
> "mgmtd" && (user_app.indexOf(action.lookup("program") >= 0)) {
>          return polkit.Result.YES;
>  }
>  });
> 
> 
> I played around with
> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/
> Global_Objects/Array/includes
> and
> https://duktape.org/dukweb.html
> and it seems that the array search includes() function is not implemented in
> duktape. Sadly there is no error for this but this is normal to javascript :(
> 
> It kind of makes sense for a tiny js engine to not support all latest
> features but this one may bite users, sorry about that.

OK, I have finally solved the issue with duktape there is a mistake in your patch which i haven't noticed in the logs:

2022-06-10T14:40:56.987866+00:00 eg polkitd[672]: Reloading rules
2022-06-10T14:40:56.987991+00:00 eg polkitd[672]: Collecting garbage unconditionally...
2022-06-10T14:40:56.988018+00:00 eg polkitd[672]: Loading rules from directory /etc/polkit-1/rules.d
2022-06-10T14:40:56.988041+00:00 eg polkitd[672]: Loading rules from directory /usr/share/polkit-1/rules.d
2022-06-10T14:40:56.988499+00:00 eg polkitd[672]: Error compiling script /etc/polkit-1/rules.d/30-sbin-mgmtd.rules: SyntaxError: parse error (line 22)
2022-06-10T14:40:56.988868+00:00 eg polkitd[672]: Finished loading, compiling and executing 6 rules

The closing ending bracket ')' for if (action.id == "org.freedesktop.policykit.exec" && subject.user == "mgmtd" && (user_app.indexOf(action.lookup("program") >= 0)) {

Now everything works perfectly fine:]

Heh, sorry about that.

I would recommend to check if polkit with duktape is better than with mozjs for the product since the javascript change is rather small and boot time and rootfs size reductions quite significant.

I'll try to post a patch anyway to make mozjs and duktape PACKAGECONFIG features in polkit. The default would be mozjs. Testing the changes now on top of master branch.

(In reply to comment #35)
> Heh, sorry about that.

N/p.

> I would recommend to check if polkit with duktape is better than with mozjs
> for the product since the javascript change is rather small and boot time
> and rootfs size reductions quite significant.

As you have mentioned it at the beginning:

> I have seen a lot of boot time and rootfs size regressions over the years due to mozjs.

I decided to switch to duktape, besides our product is an IoT edge gateway so I think it's a must.

> I'll try to post a patch anyway to make mozjs and duktape PACKAGECONFIG
> features in polkit. The default would be mozjs. Testing the changes now on

That would be great in case some more problems will occur for others in the future.

Piort, Mikko,

Thanks so much for working out what the problems were and documenting them here in in the commit log so that others can make a decision about whether to use mozjs or duktape. 

pokkit has switched back to mozjs by default but a PACKAGECONFIG option can 
be used to select duktape:
https://git.openembedded.org/meta-openembedded/commit/?id=461ceffc48e28dc8f04b8a982b3a4a2db22b718b

One could backport this to kirkstone but it's not clear if the default should change. 

Closing the issue as resolved since master is in good shape.