Document QubesOS-contrib procedures

[andrewdavidwong]

@marmarek (#953 (comment)):

This, and others, is blocked on setting up @QubesOS-contrib finally. To be honest, most of technical stuff is done - there are even package repository definitions. What is missing is a procedure to handle it:

• how to request package being added there
• what are inclusion criteria
• how to handle updates
  etc

Some of this was already discussed on qubes-devel, but not all. And we need to clarify all that and document it somewhere. @andrewdavidwong do you want to draft it? Anyway probably worth a separate ticket.

[jpouellet]

The qubes-devel thread @marmarek was referring to: https://groups.google.com/d/topic/qubes-devel/j5o3Fv6WOG8/discussion

[jpouellet]

The qubes-devel thread @marmarek was referring to: https://groups.google.com/d/topic/qubes-devel/j5o3Fv6WOG8/discussion

[andrewdavidwong]

Initial draft is up:

https://www.qubes-os.org/doc/package-contributions/

Please feel free to edit that page or provide feedback here (or both).

CC @marmarek, @jpouellet

[andrewdavidwong]

Initial draft is up:

https://www.qubes-os.org/doc/package-contributions/

Please feel free to edit that page or provide feedback here (or both).

CC @marmarek, @jpouellet

[marmarek]

Thanks @andrewdavidwong ! I think it should also mention package versioning - at least who is responsible for deciding when new version should be uploaded, and who adds a version tag. Should it be after each pull request (regardless of author)? Or should we have a "package maintainer" (most likely initial contributor) for this task? In the later case, should we require each pull request being reviewed (additionally) by such a person?

[marmarek]

Thanks @andrewdavidwong ! I think it should also mention package versioning - at least who is responsible for deciding when new version should be uploaded, and who adds a version tag. Should it be after each pull request (regardless of author)? Or should we have a "package maintainer" (most likely initial contributor) for this task? In the later case, should we require each pull request being reviewed (additionally) by such a person?

[jpouellet]

I've just re-read the qubes-devel thread, and it appears to me that the following points are agreed upon:

1. The contributing authors are to be members of the QubesOS-contrib org, but not have push access to the repos for their own packages (at least not to master branch).
2. All changes at some point require a PR against QubesOS-contrib/* for the purpose of having a dedicated place to comment on the proposed patch. (edit: even those from the original author and/or maintainer)
3. All merges must be fast-forwards to ease reviewing.
4. Passed reviews are proved by signed tags stating such.

And the following appear to be unanswered questions:

5. What is the intended workflow for merging changes from people other than the original author?
   @andrewdavidwong said:

   We certainly should aim for a solution that allows authors to retain ownership of and control over their own software, as well as receive credit and recognition for it, if only because this is likely
   to promote more contributions from individuals who care (quite reasonably, IMHO) about those things.

   Does this mean proposed changes from people other than the original author should necessarily be approved by the primary author first? What if that author disappears / becomes unresponsive?

6. What code review is required for a new package to be pushed to users? I feel quite strongly that meaningful code review should be required. Ideally by a trusted core dev (especially for packages with components in dom0, or somehow otherwise unavoidably in the Qubes TCB for all VMs), but at least by N of M trusted community members.

7. Where are code-review signed tags actually pushed? If we want to support code review from community members, then they can't be pushed to the QubesOS-contrib repos because those can not be pushed to directly (edit: at least not by non core-devs). If pushed to reviewers' local forks, then how are they found to be checked? Should some signed-comment system like updates-status package upload commands be used instead?

[jpouellet]

I've just re-read the qubes-devel thread, and it appears to me that the following points are agreed upon:

1. The contributing authors are to be members of the QubesOS-contrib org, but not have push access to the repos for their own packages (at least not to master branch).
2. All changes at some point require a PR against QubesOS-contrib/* for the purpose of having a dedicated place to comment on the proposed patch. (edit: even those from the original author and/or maintainer)
3. All merges must be fast-forwards to ease reviewing.
4. Passed reviews are proved by signed tags stating such.

And the following appear to be unanswered questions:

5. What is the intended workflow for merging changes from people other than the original author?
   @andrewdavidwong said:

   We certainly should aim for a solution that allows authors to retain ownership of and control over their own software, as well as receive credit and recognition for it, if only because this is likely
   to promote more contributions from individuals who care (quite reasonably, IMHO) about those things.

   Does this mean proposed changes from people other than the original author should necessarily be approved by the primary author first? What if that author disappears / becomes unresponsive?

6. What code review is required for a new package to be pushed to users? I feel quite strongly that meaningful code review should be required. Ideally by a trusted core dev (especially for packages with components in dom0, or somehow otherwise unavoidably in the Qubes TCB for all VMs), but at least by N of M trusted community members.

7. Where are code-review signed tags actually pushed? If we want to support code review from community members, then they can't be pushed to the QubesOS-contrib repos because those can not be pushed to directly (edit: at least not by non core-devs). If pushed to reviewers' local forks, then how are they found to be checked? Should some signed-comment system like updates-status package upload commands be used instead?

[andrewdavidwong]

I've significantly updated the page after taking into account the comments from @marmarek and @jpouellet above. Please review the new version at your convenience.

[andrewdavidwong]

I've significantly updated the page after taking into account the comments from @marmarek and @jpouellet above. Please review the new version at your convenience.

[rustybird]

Does this mean proposed changes from people other than the original author should necessarily be approved by the primary author first? What if that author disappears / becomes unresponsive?

One option: If someone thoroughly disagrees with the state of a package (unresponsiveness, design choices, whatever), they could always fork the upstream to their GitHub account, and then submit their fork as a new QubesOS-contrib package. Maybe the two forks will reconcile at some point, maybe they'll keep going in different directions, maybe one of them will eventually become so obviously inferior that it can be removed. This would mirror the "normal" open source workflow pretty closely. Although if this approach is chosen, there should be a rule that any new fork must keep the old git history fully intact, so that the QCR doesn't have to review everything again.

What code review is required for a new package to be pushed to users? I feel quite strongly that meaningful code review should be required. Ideally by a trusted core dev (especially for packages with components in dom0, or somehow otherwise unavoidably in the Qubes TCB for all VMs), but at least by N of M trusted community members.

Could the mandatory review of code, configuration files, etc. perhaps be limited to only the parts that have a more or less automatic effect after package installation? RPM specs would get a thorough review, as would systemd units (and any commands started by them if the unit is enabled automatically), or stuff like files dropped into /etc/sysctl.d. Whereas a standalone executable merely installed in $PATH would mostly get a review of its name, to ensure it's not invoked automagically or without real consent ("Uh, why is your script called /usr/bin/lsmod?").

dom0 is not even necessarily a special case here. For example, my Split dm-crypt package has a dom0 part that's only a single shell script, /usr/bin/qvm-block-split, which the user invokes manually.

[rustybird]

Does this mean proposed changes from people other than the original author should necessarily be approved by the primary author first? What if that author disappears / becomes unresponsive?

One option: If someone thoroughly disagrees with the state of a package (unresponsiveness, design choices, whatever), they could always fork the upstream to their GitHub account, and then submit their fork as a new QubesOS-contrib package. Maybe the two forks will reconcile at some point, maybe they'll keep going in different directions, maybe one of them will eventually become so obviously inferior that it can be removed. This would mirror the "normal" open source workflow pretty closely. Although if this approach is chosen, there should be a rule that any new fork must keep the old git history fully intact, so that the QCR doesn't have to review everything again.

What code review is required for a new package to be pushed to users? I feel quite strongly that meaningful code review should be required. Ideally by a trusted core dev (especially for packages with components in dom0, or somehow otherwise unavoidably in the Qubes TCB for all VMs), but at least by N of M trusted community members.

Could the mandatory review of code, configuration files, etc. perhaps be limited to only the parts that have a more or less automatic effect after package installation? RPM specs would get a thorough review, as would systemd units (and any commands started by them if the unit is enabled automatically), or stuff like files dropped into /etc/sysctl.d. Whereas a standalone executable merely installed in $PATH would mostly get a review of its name, to ensure it's not invoked automagically or without real consent ("Uh, why is your script called /usr/bin/lsmod?").

dom0 is not even necessarily a special case here. For example, my Split dm-crypt package has a dom0 part that's only a single shell script, /usr/bin/qvm-block-split, which the user invokes manually.

[jpouellet]

Could the mandatory review of code, configuration files, etc. perhaps be limited to only the parts that have a more or less automatic effect after package installation?

I don't really see a meaningful difference here. If you installed something, it's probably because you plan to use it, so manually executed things are probably just as inevitably executed as automatically-executed things.

I can see how your argument would apply to some obscure deep dependency with auxiliary tools, but here we're talking about qubes-specific stuff that the user explicitly installs.

[jpouellet]

Could the mandatory review of code, configuration files, etc. perhaps be limited to only the parts that have a more or less automatic effect after package installation?

I don't really see a meaningful difference here. If you installed something, it's probably because you plan to use it, so manually executed things are probably just as inevitably executed as automatically-executed things.

I can see how your argument would apply to some obscure deep dependency with auxiliary tools, but here we're talking about qubes-specific stuff that the user explicitly installs.

[jpouellet]

Some things I think are still missing:

• Some guideline(s) attempting to clarify what should be contributed as patches to the core repos, and what things belong as -contrib pkgs.
• Some clarification of how trustworthy -contrib packages should be considered compared to code under the main QubesOS org. If there is no distinction (which the current policy of no push access and indefinite delay to allow for through review by core Qubes devs suggests) then it's not clear to me why -contrib exists in the first place.

[jpouellet]

Some things I think are still missing:

• Some guideline(s) attempting to clarify what should be contributed as patches to the core repos, and what things belong as -contrib pkgs.
• Some clarification of how trustworthy -contrib packages should be considered compared to code under the main QubesOS org. If there is no distinction (which the current policy of no push access and indefinite delay to allow for through review by core Qubes devs suggests) then it's not clear to me why -contrib exists in the first place.

[jpouellet]

Also a reference from https://github.com/QubesOS/qubes-doc/blob/master/basics_user/contributing.md once it's ready.

[jpouellet]

Also a reference from https://github.com/QubesOS/qubes-doc/blob/master/basics_user/contributing.md once it's ready.

[andrewdavidwong]

One option: If someone thoroughly disagrees with the state of a package (unresponsiveness, design choices, whatever), they could always fork the upstream to their GitHub account, and then submit their fork as a new QubesOS-contrib package. [...]

This seems like it would create too much extra work for the QCR, who would then have to adjudicate between competing packages and decide which one goes into Qubes (surely not both).

Some guideline(s) attempting to clarify what should be contributed as patches to the core repos, and what things belong as -contrib pkgs.

I would think something like: "If you want to patch some code in the core repos, contribute a patch. If you want to contribute something that makes sense as its own package, contribute a package. If it's in-between or you're not sure, ask us." What do you think, @marmarek?

Some clarification of how trustworthy -contrib packages should be considered compared to code under the main QubesOS org. If there is no distinction (which the current policy of no push access and indefinite delay to allow for through review by core Qubes devs suggests) then it's not clear to me why -contrib exists in the first place.

This has already been covered in the discussion thread and in the comments here. There are many reasons (to incentivize contributions, to allow for a relaxed naming scheme, etc.).

[andrewdavidwong]

One option: If someone thoroughly disagrees with the state of a package (unresponsiveness, design choices, whatever), they could always fork the upstream to their GitHub account, and then submit their fork as a new QubesOS-contrib package. [...]

This seems like it would create too much extra work for the QCR, who would then have to adjudicate between competing packages and decide which one goes into Qubes (surely not both).

Some guideline(s) attempting to clarify what should be contributed as patches to the core repos, and what things belong as -contrib pkgs.

I would think something like: "If you want to patch some code in the core repos, contribute a patch. If you want to contribute something that makes sense as its own package, contribute a package. If it's in-between or you're not sure, ask us." What do you think, @marmarek?

Some clarification of how trustworthy -contrib packages should be considered compared to code under the main QubesOS org. If there is no distinction (which the current policy of no push access and indefinite delay to allow for through review by core Qubes devs suggests) then it's not clear to me why -contrib exists in the first place.

This has already been covered in the discussion thread and in the comments here. There are many reasons (to incentivize contributions, to allow for a relaxed naming scheme, etc.).

[andrewdavidwong]

Could the mandatory review of code, configuration files, etc. perhaps be limited to only the parts that have a more or less automatic effect after package installation? [...]

IMO, no. Should be fully reviewed by a QCR.

Some clarification of how trustworthy -contrib packages should be considered compared to code under the main QubesOS org. If there is no distinction (which the current policy of no push access and indefinite delay to allow for through review by core Qubes devs suggests)

Yes, just as trustworthy (because fully reviewed by a QCR).

Speaking to both points: All the code we sign and ship should always be thoroughly reviewed to the highest security standards of the project, or we shouldn't sign or ship it at all.

[andrewdavidwong]

Could the mandatory review of code, configuration files, etc. perhaps be limited to only the parts that have a more or less automatic effect after package installation? [...]

IMO, no. Should be fully reviewed by a QCR.

Some clarification of how trustworthy -contrib packages should be considered compared to code under the main QubesOS org. If there is no distinction (which the current policy of no push access and indefinite delay to allow for through review by core Qubes devs suggests)

Yes, just as trustworthy (because fully reviewed by a QCR).

Speaking to both points: All the code we sign and ship should always be thoroughly reviewed to the highest security standards of the project, or we shouldn't sign or ship it at all.

[jpouellet]

Speaking to both points: All the code we sign and ship should always be thoroughly reviewed to the highest security standards of the project, or we shouldn't sign or ship it at all.

Do you perhaps mean "anything with the Qubes name on it" rather than "all the code we sign and ship"? The signed and shipped Qubes installer is ~4gb, a giant majority of which has never been audited by us, nor reasonably could be.

[jpouellet]

Speaking to both points: All the code we sign and ship should always be thoroughly reviewed to the highest security standards of the project, or we shouldn't sign or ship it at all.

Do you perhaps mean "anything with the Qubes name on it" rather than "all the code we sign and ship"? The signed and shipped Qubes installer is ~4gb, a giant majority of which has never been audited by us, nor reasonably could be.

[jpouellet]

Bumping this issue since it seems qubes-contrib might potentially be the right place for @raffaeleflorio's qubes-url-redirector (discussion, code), at least initially.

[jpouellet]

Bumping this issue since it seems qubes-contrib might potentially be the right place for @raffaeleflorio's qubes-url-redirector (discussion, code), at least initially.

[andrewdavidwong]

Speaking to both points: All the code we sign and ship should always be thoroughly reviewed to the highest security standards of the project, or we shouldn't sign or ship it at all.

Do you perhaps mean "anything with the Qubes name on it" rather than "all the code we sign and ship"? The signed and shipped Qubes installer is ~4gb, a giant majority of which has never been audited by us, nor reasonably could be.

"Thoroughly reviewed to the highest security standards of the project" doesn't necessarily mean that it has to be reviewed by us, just that it has to be reviewed by someone. For this to make sense, of course, it should be someone we trust. The present nature of Qubes is that we (are forced to) trust certain upstream projects, such as Xen. I think it's fair to say that the Xen code (for example) should be reviewed to the highest standards of the (Qubes) project, even if the ones reviewing it are Xen devs (or auditors) rather than Qubes devs. That kind of review is still meaningful as long as the code is signed by keys that the Qubes devs trust.

[andrewdavidwong]

Speaking to both points: All the code we sign and ship should always be thoroughly reviewed to the highest security standards of the project, or we shouldn't sign or ship it at all.

Do you perhaps mean "anything with the Qubes name on it" rather than "all the code we sign and ship"? The signed and shipped Qubes installer is ~4gb, a giant majority of which has never been audited by us, nor reasonably could be.

"Thoroughly reviewed to the highest security standards of the project" doesn't necessarily mean that it has to be reviewed by us, just that it has to be reviewed by someone. For this to make sense, of course, it should be someone we trust. The present nature of Qubes is that we (are forced to) trust certain upstream projects, such as Xen. I think it's fair to say that the Xen code (for example) should be reviewed to the highest standards of the (Qubes) project, even if the ones reviewing it are Xen devs (or auditors) rather than Qubes devs. That kind of review is still meaningful as long as the code is signed by keys that the Qubes devs trust.

[andrewdavidwong]

It's also worth noting that "the highest security standards of the project" denotes something complex due to the nature of Qubes, namely that the appropriate security standards vary in stringency depending on where the code will run. The standards for code that runs in dom0 are higher than the standards for NetVMs, for example.

[andrewdavidwong]

It's also worth noting that "the highest security standards of the project" denotes something complex due to the nature of Qubes, namely that the appropriate security standards vary in stringency depending on where the code will run. The standards for code that runs in dom0 are higher than the standards for NetVMs, for example.

[andrewdavidwong]

Since there have been no further objections or changes for a little over two months, I believe we've reached an agreement on a stable version of this page, so I've removed the "unofficial" notice from the page and linked it from the appropriate places around the website. If you believe further changes should be made to the page, please do not hesitate to comment here, reopen this issue, edit the page, or submit a PR.

[andrewdavidwong]

Since there have been no further objections or changes for a little over two months, I believe we've reached an agreement on a stable version of this page, so I've removed the "unofficial" notice from the page and linked it from the appropriate places around the website. If you believe further changes should be made to the page, please do not hesitate to comment here, reopen this issue, edit the page, or submit a PR.