QubesOS / qubes-issues

The Qubes OS Project issue tracker
https://www.qubes-os.org/doc/issue-tracking/
526 stars 46 forks source link

[Contribution] qubes-remote-desktop #5358

Open fepitre opened 4 years ago

fepitre commented 4 years ago

Repository: https://github.com/fepitre/qubes-remote-desktop Description : SystemD service for creating VNC server session in dom0 or any qube. Related to https://github.com/QubesOS/qubes-issues/issues/5051

marmarek commented 4 years ago

Besides a little comment it looks fine. Reviewed as of https://github.com/fepitre/qubes-remote-desktop/commit/a91e9a2eb54576dea56ae79368c5a6b95e1a740e

Technically, according to criteria, it does weaken security, because it adds an interface to control dom0 from outside. But it does so in a controlled way, through qrexec policy, so it may be ok in a contrib repository. Allowing this service makes dom0 isolated only as hard as VMs allowing to use this service. This should be clearly stated in the package description.

fepitre commented 4 years ago

Thank you for you review. If you are ok, I will reuse your comment in the README.

marmarek commented 4 years ago

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

It is ok as of https://github.com/fepitre/qubes-remote-desktop/commit/eafdae658f2a2d6123863a28f7a13ebf229792b1 -----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEAGRCj0VUUbPr54p/Bjk4ukLPpyQFAl2Q6V8ACgkQBjk4ukLP pySnIg//fT43DEMTGMJTRVdhmOeUpGdX5wr1qTPH2HTiXfpPZyNGYqVzQ3mpVACh 8i8lEQvZrP/3CWVNUabVbzlnHlrC9bxd3Hk7lWwIjP3dU9Ez/oDqCtGUu1ANFAKt nZIs898qt6lEpauz41ohYLVTjO35a6j2poqihLKnUB/2xiynm/TruaUBDkIMC7EX 3O2pYM+YFxpLgahQ9TwI3deHDRcTaZ3kzCtDDwsCsgjI9k3tYeIYczMJcmli/Hlh S1UtreTmm4uhWRz2ETY44dGR/6ouF2BApOqZB4ia73BmJVY8IeRGhso3EHbmhMR/ 6CMFi0DlYnp8gbke0rL7qA0ZdBmZsA4RsSkM4kTIzVfKVJwXgIDNkyTiRA4lDQRO OXwX6bhBs5CwNJIwGlQoMpUiIdGMMS5lpzy+vDPyDbUYr+zojpf1oQ0zDv5WlfxX jCAdvLRBfEVnm4xzkw5AddGZL8KQceIAf3fyCt0m/6mhIEZrWexeYrh5U9jA8nw2 vDdNz89pMoAdautjWKaDT5CnlM4urtQTJycz0J1B2PQNenZhv1P4+jqxQd0hd22R BPuYCtSF4aSwlRVYPLkVCBkhdwV9OyNCcj1xBIuyOHxe8ORluKIH3Tm7Y0wVl4o0 wX/VjxMg2uh0jfdy3iWoK8tHHABN0juXoRF8h/Po/bSvkQYL9zo= =TMi9 -----END PGP SIGNATURE-----

fepitre commented 4 years ago

A new version is available in repo. The dom0 part as been adapted for a real use case: connecting to current dom0 desktop and gui-daemon DISPLAY=:0. That's what a remote admin would expect of such tool.

fepitre commented 4 years ago

@andrewdavidwong: what about current issue knowing that package is now available? (same for https://github.com/QubesOS/qubes-issues/issues/5359)

andrewdavidwong commented 4 years ago

@andrewdavidwong: what about current issue knowing that package is now available? (same for #5359)

Good question. In accordance with our contributed package review procedure, I see that @marmarek, as the Package Maintainer (PM), has added a signed comment stating that https://github.com/fepitre/qubes-remote-desktop/commit/eafdae658f2a2d6123863a28f7a13ebf229792b1 passed review.

Now it is up to a Qubes Core Reviewer (QCR) to review the pull request. (Also @marmarek in this case?) If the pull request passes the QCR’s review, the QCR pushes a signed tag to the HEAD commit stating that it has passed review and fast-forward merges the pull request. If the pull request does not pass the QCR’s review, the QCR leaves a comment on the pull request explaining why not, and the QCR may decide to close the pull request.

A new version is available in repo.

Since a new version has been added since the last PM review, I suppose it is up to @marmarek whether to first review the latest version or proceed with the procedure described above.