[PATCH] Fix policy.Replace changing the file mode

[Ben Grande]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Without this, it defaults to what unmask allows, normally 644.
Without being group owned, editing the policy manually leads to a RO
file and if the user force writes, will change the ownership to
user:user.

Signed-off-by: Ben Grande <ben.gr...@gmail.com>
- ---
qrexec/policy/admin.py | 1 +
1 file changed, 1 insertion(+)

diff --git a/qrexec/policy/admin.py b/qrexec/policy/admin.py
index da5bd9f..c5bfc23 100644
- --- a/qrexec/policy/admin.py
+++ b/qrexec/policy/admin.py
@@ -201,6 +201,7 @@ class PolicyAdmin:

temp_path = path.with_name(RENAME_PREFIX + path.name)
temp_path.write_bytes(data)
+ temp_path.chmod(0o664)
temp_path.rename(path)

# Remove
- --
Benjamin Grande <ben.gr...@gmail.com>
-----BEGIN PGP SIGNATURE-----

iNUEARYKAH0WIQRklnEdsUUe50UmvUUbcxS/DMyWhwUCZHJZQl8UgAAAAAAuAChp
c3N1ZXItZnByQG5vdGF0aW9ucy5vcGVucGdwLmZpZnRoaG9yc2VtYW4ubmV0NjQ5
NjcxMURCMTQ1MUVFNzQ1MjZCRDQ1MUI3MzE0QkYwQ0NDOTY4NwAKCRAbcxS/DMyW
h4WuAQC50a9ABkqPcJk/Skl4SdKqTjrN9Y6A3BoGW0HeQyK2QgEA4+yDPtdjVrJe
GzvpRXRruti11oPj72CIaatLIrXMOQA=
=WUKS
-----END PGP SIGNATURE-----

[Ben Grande]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 23-05-27 19:25:54, Ben Grande wrote:
> Without this, it defaults to what unmask allows, normally 644.
> Without being group owned, editing the policy manually leads to a RO
> file and if the user force writes, will change the ownership to
> user:user.
>
> Signed-off-by: Ben Grande <ben.gr...@gmail.com>

> ---
> qrexec/policy/admin.py | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/qrexec/policy/admin.py b/qrexec/policy/admin.py
> index da5bd9f..c5bfc23 100644

> --- a/qrexec/policy/admin.py
> +++ b/qrexec/policy/admin.py
> @@ -201,6 +201,7 @@ class PolicyAdmin:
>
> temp_path = path.with_name(RENAME_PREFIX + path.name)
> temp_path.write_bytes(data)
> + temp_path.chmod(0o664)
> temp_path.rename(path)
>
> # Remove
> --

> Benjamin Grande <ben.gr...@gmail.com>

Perhaps it should also set the ownership to root:qubes?

- --
Benjamin Grande
-----BEGIN PGP SIGNATURE-----

iNUEARYKAH0WIQRklnEdsUUe50UmvUUbcxS/DMyWhwUCZHJbIV8UgAAAAAAuAChp
c3N1ZXItZnByQG5vdGF0aW9ucy5vcGVucGdwLmZpZnRoaG9yc2VtYW4ubmV0NjQ5
NjcxMURCMTQ1MUVFNzQ1MjZCRDQ1MUI3MzE0QkYwQ0NDOTY4NwAKCRAbcxS/DMyW
h6/kAQDpolXlt7PC8+oCZxKR4wBxjyq68XTTgrKITHhkC87aCQEA4uF5HVH3TQhG
u2TTdg0ag8LelGDYi6LWSXsMZdrUyAQ=
=oEII
-----END PGP SIGNATURE-----

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Sat, May 27, 2023 at 07:33:53PM +0000, Ben Grande wrote:
> On 23-05-27 19:25:54, Ben Grande wrote:
> > Without this, it defaults to what unmask allows, normally 644.
> > Without being group owned, editing the policy manually leads to a RO
> > file and if the user force writes, will change the ownership to
> > user:user.
> >
> > Signed-off-by: Ben Grande <ben.gr...@gmail.com>
> > ---
> > qrexec/policy/admin.py | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/qrexec/policy/admin.py b/qrexec/policy/admin.py
> > index da5bd9f..c5bfc23 100644
> > --- a/qrexec/policy/admin.py
> > +++ b/qrexec/policy/admin.py
> > @@ -201,6 +201,7 @@ class PolicyAdmin:
> >
> > temp_path = path.with_name(RENAME_PREFIX + path.name)
> > temp_path.write_bytes(data)
> > + temp_path.chmod(0o664)
> > temp_path.rename(path)
> >
> > # Remove
> > --
> > Benjamin Grande <ben.gr...@gmail.com>
>
> Perhaps it should also set the ownership to root:qubes?

Yes, I think so.

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmRya6sACgkQ24/THMrX
1ywTKAgAhiA0mnNSdHQNdcC07kujGCQAoliWmA4xLZ62eC/puXoeMPfIpAQDbDrJ
nH9V9Ep2OaERUBRO0+/gcmnGWxRmeMYD1WGt/NlGlrCXRhpJjxxBZx5xSeIZRt6b
DAtrVADJr650spRO/WuxmyStaPnZkXvefcz89Wl4qJUscrmNNnGGC1E1zmbh1V49
onJJNMemDalFyTPkA0uajh7CeSwEdpml4G+tBVh8T11k0F6DBVT0BdlsZyFrEHF6
KjB5Vlv2xVog13z7KNvWxfYn2s/Om4sTJMqCCCQr0vexpG0RXZBAEgMdI/xf5und
Y3/0vyvY2vmrnE+UkuLUzDM0Yk32fQ==
=dHmf
-----END PGP SIGNATURE-----

[Ben Grande]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Enforce file mode and ownership for replaced files.

Signed-off-by: Ben Grande <ben.gr...@gmail.com>
- ---

qrexec/policy/admin.py | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/qrexec/policy/admin.py b/qrexec/policy/admin.py
index da5bd9f..5f80070 100644
- --- a/qrexec/policy/admin.py
+++ b/qrexec/policy/admin.py
@@ -19,6 +19,8 @@

from typing import Optional
from pathlib import Path
+from pwd import getpwnam
+from grp import getgrnam
import contextlib
import fcntl
import os
@@ -201,6 +203,10 @@ class PolicyAdmin:

temp_path = path.with_name(RENAME_PREFIX + path.name)
temp_path.write_bytes(data)
+ temp_path.chmod(0o664)

+ uid = getpwnam("root").pw_uid
+ gid = getgrnam("qubes").gr_gid
+ os.chown(temp_path, uid, gid)

temp_path.rename(path)

# Remove
- --
Benjamin Grande <ben.gr...@gmail.com>
-----BEGIN PGP SIGNATURE-----

iNUEARYKAH0WIQRklnEdsUUe50UmvUUbcxS/DMyWhwUCZHMh0l8UgAAAAAAuAChp
c3N1ZXItZnByQG5vdGF0aW9ucy5vcGVucGdwLmZpZnRoaG9yc2VtYW4ubmV0NjQ5
NjcxMURCMTQ1MUVFNzQ1MjZCRDQ1MUI3MzE0QkYwQ0NDOTY4NwAKCRAbcxS/DMyW
h9FEAP0aSKtG3Oj72+/BgRwrLkXSROzNzCsWKeYYGkyDenXzDgEAzXey2JZVfztn
FGyruOXoy5RCQvUsqgcfzjKq4US/+g8=
=gaed
-----END PGP SIGNATURE-----

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Sun, May 28, 2023 at 09:41:37AM +0000, Ben Grande wrote:
> Enforce file mode and ownership for replaced files.
>
> Signed-off-by: Ben Grande <ben.gr...@gmail.com>

> ---
> qrexec/policy/admin.py | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/qrexec/policy/admin.py b/qrexec/policy/admin.py
> index da5bd9f..5f80070 100644

> --- a/qrexec/policy/admin.py
> +++ b/qrexec/policy/admin.py
> @@ -19,6 +19,8 @@
>
> from typing import Optional
> from pathlib import Path
> +from pwd import getpwnam
> +from grp import getgrnam
> import contextlib
> import fcntl
> import os
> @@ -201,6 +203,10 @@ class PolicyAdmin:
>
> temp_path = path.with_name(RENAME_PREFIX + path.name)
> temp_path.write_bytes(data)
> + temp_path.chmod(0o664)
> + uid = getpwnam("root").pw_uid
> + gid = getgrnam("qubes").gr_gid
> + os.chown(temp_path, uid, gid)

Just in case, I'd wrap it in try/except to not fail the whole operation
if chown fails (if the thing is running as non-root user for example).

> temp_path.rename(path)
>
> # Remove
> --
> Benjamin Grande <ben.gr...@gmail.com>
>

> --
> You received this message because you are subscribed to the Google Groups "qubes-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to qubes-devel...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/qubes-devel/ZHMh0fQxcKHG70gP%40personal-mutt.

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmRzWZEACgkQ24/THMrX
1yxnvAf+NJ5PSy0IKlHduJiMSsuGSmE1QarvNp6hOMZqxGgqjRg9pKwS2tF69StW
1zOM5xNmls888CKkxKeS7SsSOAMAlRt8gf1+mhS3SGGPYrDiZZcuvzClBw+JOmn7
moAbokIL5qBtTZ57X4eBC5e7iOisA1n1VIqCqwLxnQOsb2aP5BoItcpIKfilIh+I
5RI3rs/58fQfKVsLfb5IsLqolGh4PX3OKqDq8rCwABPmMYfyqfGAJ5ywiiV01LRf
lgsKVJHM1mQuSYUaazEmOVFLWvc73CeSbk+Blz08cYuogxnMK25iMSW78mPm5DiL
xo4w3oq4DvvjPW6wJcgOAN987AulDQ==
=utmA
-----END PGP SIGNATURE-----

[Ben Grande]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Enforce file mode and ownership for replaced files.

Signed-off-by: Ben Grande <ben.gr...@gmail.com>

- ---
qrexec/policy/admin.py | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/qrexec/policy/admin.py b/qrexec/policy/admin.py
index da5bd9f..d575a6e 100644
- --- a/qrexec/policy/admin.py

+++ b/qrexec/policy/admin.py
@@ -19,6 +19,8 @@

from typing import Optional
from pathlib import Path
+from pwd import getpwnam
+from grp import getgrnam
import contextlib
import fcntl
import os

@@ -201,6 +203,13 @@ class PolicyAdmin:

temp_path = path.with_name(RENAME_PREFIX + path.name)
temp_path.write_bytes(data)
+ temp_path.chmod(0o664)
+ uid = getpwnam("root").pw_uid
+ gid = getgrnam("qubes").gr_gid

+ try:
+ os.chown(temp_path, uid, gid)
+ except PermissionError:
+ pass

temp_path.rename(path)

# Remove
- --
Benjamin Grande

-----BEGIN PGP SIGNATURE-----

iNUEARYKAH0WIQRklnEdsUUe50UmvUUbcxS/DMyWhwUCZHPXMl8UgAAAAAAuAChp
c3N1ZXItZnByQG5vdGF0aW9ucy5vcGVucGdwLmZpZnRoaG9yc2VtYW4ubmV0NjQ5
NjcxMURCMTQ1MUVFNzQ1MjZCRDQ1MUI3MzE0QkYwQ0NDOTY4NwAKCRAbcxS/DMyW
h+qtAQC8hkeTwZuB3xROy8UwU6iR74xkTNOmh2EOJcgV++27pwD9F8bg2iI6JRLA
2qdytdaSB786w3uPNhkxgWLCH/9Gkw4=
=pQvi
-----END PGP SIGNATURE-----

[Ben Grande]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 23-05-28 22:35:31, Ben Grande wrote:
> Enforce file mode and ownership for replaced files.
>
> Signed-off-by: Ben Grande <ben.gr...@gmail.com>

> ---
> qrexec/policy/admin.py | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/qrexec/policy/admin.py b/qrexec/policy/admin.py
> index da5bd9f..d575a6e 100644

> --- a/qrexec/policy/admin.py
> +++ b/qrexec/policy/admin.py
> @@ -19,6 +19,8 @@
>
> from typing import Optional
> from pathlib import Path
> +from pwd import getpwnam
> +from grp import getgrnam
> import contextlib
> import fcntl
> import os
> @@ -201,6 +203,13 @@ class PolicyAdmin:
>
> temp_path = path.with_name(RENAME_PREFIX + path.name)
> temp_path.write_bytes(data)
> + temp_path.chmod(0o664)
> + uid = getpwnam("root").pw_uid
> + gid = getgrnam("qubes").gr_gid
> + try:
> + os.chown(temp_path, uid, gid)
> + except PermissionError:
> + pass
> temp_path.rename(path)
>
> # Remove
> --

> Benjamin Grande
>

Reminding of unreviewed patch.

- --
Benjamin Grande
-----BEGIN PGP SIGNATURE-----

iNUEARYKAH0WIQRklnEdsUUe50UmvUUbcxS/DMyWhwUCZNZByV8UgAAAAAAuAChp
c3N1ZXItZnByQG5vdGF0aW9ucy5vcGVucGdwLmZpZnRoaG9yc2VtYW4ubmV0NjQ5
NjcxMURCMTQ1MUVFNzQ1MjZCRDQ1MUI3MzE0QkYwQ0NDOTY4NwAKCRAbcxS/DMyW
h2+pAQChwHR8kUTBFK8+znPCUM9D5JSYu59Z02pzAru7c6njFAD/bmjnZq3R9X80
JGtb3Q3s3I9mQdFytLStO/3JSGqsHwk=
=4qjs
-----END PGP SIGNATURE-----

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Thanks, applied.

I also changed it to handle missing 'qubes' group, because that doesn't
exist in CI.

- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmTh/+oACgkQ24/THMrX
1yzYIQf/Tg2vIz3J6CvqEd3mfqrf5KfEI3l+bC/NQOJxcg+09anC+VeTN/gA1hVw
kmmTBHdfTXJCq6f4KOMHRfNMPRdtarFCdz6UOIfqkHfWlmY8LAokRc/+BqCiJxgQ
/lR2y2Pv2/zNHys67p6k4ZN1Xqxp/dI0NjsnJH55NTeME4EFVPu8d3ZW7OPHqKfm
Ex8m9QYzzIJmrxDTgjKmogv7mMiEz1T0awjzwwDJrnprgnfKvT3LuIj6WIm0e8QO
VC9a4o6sCoxTMWLIasmvQTjWT45Vpi04F0MFlRGJJAa0MumccLkFm7qQvOouP2bu
R60o4/tlfE/qwvYed8JXbdwGf3rZew==
=zZwV
-----END PGP SIGNATURE-----