[WIP] Rewrite qrexec documentation (Phase 1)

[pierwill]

Greetings! 👋

I'm embarking on a project to address the qrexec docs in response to

• qrexec docs look like a big mess qubes-issues#1392;
• Qubes Google Season of Docs project idea (https://www.qubes-os.org/gsod/#rewrite-qrexec-documentation); and
• the need for this work identified by the SecureDrop team (@eloquence, @redshiftzero).

I'm opening this PR as a way to track my progress, share my work, and discuss the technical specifics of qrexec with the Qubes and SD teams. Hopefully people find this work useful!

[pierwill]

For a SecureDrop-related issue, see QubesOS/qubes-issues#3318.

[pierwill]

How does this diagram look for the Qrexec basics section?

Made using mermaid here: https://mermaidjs.github.io/mermaid-live-editor/

graph LR
    subgraph someVM
    A[qrexec-agent]
    end
    subgraph dom0
    B[qrexec-daemon]-. listens .->C[qrexec-client]
    C -->|sends request to start process| A
    B-. sends vchan details.-> C
    B-. sends vchan details.-> A
    end
    C -->|established vchan channel| A

[marmarek]

How does this diagram look for the Qrexec basics section?

I'd swap qrexec-client and qrexec-daemon places - it's qrexec-client initiating the action. Technically "sends request to start process" goes through qrexec-damon (but the data channel not).

BTW I really like this diagrams tool. Looks very clean and source is very concise.

[pierwill]

I'd swap qrexec-client and qrexec-daemon places - it's qrexec-client initiating the action. Technically "sends request to start process" goes through qrexec-damon (but the data channel not).

BTW I really like this diagrams tool. Looks very clean and source is very concise.

Cool! I think I'll do a separate PR with this and any other diagrams created for qrexec3. I still have questions... 😉

[marmarek]

Github doesn't have enough emotion icons to express how I'm happy about you doing this rewrite!

[jpouellet]

This is fantastic. Thank you so much for doing this!!

[jpouellet]

Not sure the best place to fit it in, but you might want to mention that

$ journalctl -o cat -t qrexec -f

in dom0 watches qrexec calls (to e.g. trace qrexec-policy issues, etc).

[pierwill]

@jpouellet: that's a great suggestion! I'll work on fitting that in.

[pierwill]

Hi everyone: Thanks for your feedback and enthusiasm! :)

I'd like to make a proposal for moving forward with this project. We could go about it in phases:

• Split the existing qrexec3.md doc into two pages: one for the overview etc., and another page for the "internals". I can add a commit to this branch for this.

• Make another pass to review what I've done so far, making further edits and addressing the recent feedback. At this point we could discuss merging the commits in this PR. This would end Phase 1.

• Phase 2: Start another PR for editing the remaining non-specification docs (up until "Qubes REP internals" in the current doc).

• Phase 3: Start work on the technical descriptions. This will require more discussion I'm sure, as well as more testing on my part.

If this plan makes general sense, let's rename this PR "[WIP] Rewrite qrexec documentation (Phase 1)".

[andrewdavidwong]

Hi everyone: Thanks for your feedback and enthusiasm! :)

I'd like to make a proposal for moving forward with this project. We could go about it in phases:

• Split the existing qrexec3.md doc into two pages: one for the overview etc., and another page for the "internals". I can add a commit to this branch for this.
• Make another pass to review what I've done so far, making further edits and addressing the recent feedback. At this point we could discuss merging the commits in this PR. This would end Phase 1.
• Phase 2: Start another PR for editing the remaining non-specification docs (up until "Qubes REP internals" in the current doc).
• Phase 3: Start work on the technical descriptions. This will require more discussion I'm sure, as well as more testing on my part.

If this plan makes general sense, let's rename this PR "[WIP] Rewrite qrexec documentation (Phase 1)".

Sounds good to me.

[pierwill]

I've addressed the feedback given so far, and split the docs into the overview and internals pages. Rendered locally, the links work fine.

I think this is ready for another round of review. After merging Phase 1 I can do smaller PRs for other edits to these sections as necessary.

[marmarek]

I have limited capacity this week, so I can't provide full review right now.
As for splitting docs, I vaguely remember we had it separate previously and merged it @andrewdavidwong do you remember reasoning?

[andrewdavidwong]

As for splitting docs, I vaguely remember we had it separate previously and merged it @andrewdavidwong do you remember reasoning?

I recall having that discussion about the USB docs, but not these.

[marmarek]

I've found it - it is the very same issue about qrexec doc: QubesOS/qubes-issues#1392. I wasn't great fan of a single qrexec page back then. And I still think that separation makes sense (having clearly separated sections works, but separate pages could be better).

[pierwill]

I'm happy to keep them as one page if that's what folks prefer. I've got some ideas for the outline hierarchy that would make the page easier to navigate.

I think I can easily kill de28e0f in a rebase to accomplish this..

Update: ah, I misread the comment above.

[pierwill]

Image for 98d9f12 is awaiting approval in QubesOS/qubes-attachment#22.

[andrewdavidwong]

Image for 98d9f12 is awaiting approval in QubesOS/qubes-attachment#22.

Merged.

[marmarek]

Thanks for this iteration!
I've added some comments. Note some are not necessary applicable in this iteration (for example notes about R4.1), but I've put them here anyway, since I've re-read this whole doc again.

I'm not yet sure how to handle differences in R4.1 here. There are few subtle one mentioned in comments, but also a bigger one: https://github.com/QubesOS/qubes-core-qrexec/blob/master/Documentation/multifile-policy.markdown (old policy location still works).

[marmarek]

pipes -> local sockets

[marmarek]

Technically user field is mandatory, "requests that don't specify one" means "requests that specify DEFAULT user".

[marmarek]

Local program is optional. If not specified, qrexec-client-vm's stdin/stdout are connected to the remote service endpoint.

[marmarek]

There are more options. The current qrexec-client --help output:

usage: qrexec-client [-w timeout] [-W] [-t] [-T] -d domain_name [-l local_prog|-c request_id,src_domain_name,src_domain_id|-e] remote_cmdline
-e means exit after sending cmd,
-t enables replacing problematic bytes with '_' in command output, -T is the same for stderr
-W waits for connection end even in case of VM-VM (-c)
-c: connect to existing process (response to trigger service call)
-w timeout: override default connection timeout of 5s (set 0 for no timeout)

[marmarek]

"qrexec-agent executes some_command in domX" - this is a little incomplete, it isn't always qrexec-agent executing the command directly. On Linux there are two cases:

1. target user has full GUI session running and qrexec-fork-server is running as part of it (default for user account): in this case qrexec-agent connects to qrexec-fork-server process (through /var/run/qubes/qrexec-server.<USERNAME>.sock socket) and delegate further steps to it.
2. no qrexec-fork-server is running for that user - in this case qrexec-agent creates a child process, opens a user session there (using PAM) and proceed there.

Main qrexec-agent process is involved only in control messages, actual data is always handled in a separate, usually not privileged process.
On Windows we don't have qrexec-fork-server and it's always handled like in the second point.

[pierwill]

(Force pushed a rebase onto current master.)

[pierwill]

With 7e7814f, I believe I've addressed all of Marek's comments on the main qrexec.md.

The internals feedback I'll address in a later phase of this project.

Thanks for everyone's help so far! 😃 👏

[andrewdavidwong]

@pierwill, it looks like the Travis CI log is showing that this PR would result in some broken links. Would you mind fixing those before we merge?

[pierwill]

Pushed fixes for most of those broken links. The last two are in release announcements:

- ./_site/doc/releases/3.2/release-notes/index.html
  *  linking to /doc/qrexec3/#service-argument-in-policy, but service-argument-in-policy does not exist (line 215)
     <a href="/doc/qrexec3/#service-argument-in-policy">documentation</a>
- ./_site/doc/releases/4.0/release-notes/index.html
  *  linking to /doc/qrexec3/#extra-keywords-available-in-qubes-40-and-later, but extra-keywords-available-in-qubes-40-and-later does not exist (line 222)
     <a href="/doc/qrexec3/#extra-keywords-available-in-qubes-40-and-later">documentation</a>

Should we fix those as well?

Update: I have the changes to fix the release note links standing by.

[andrewdavidwong]

Pushed fixes for most of those broken links. The last two are in release announcements:

- ./_site/doc/releases/3.2/release-notes/index.html
  *  linking to /doc/qrexec3/#service-argument-in-policy, but service-argument-in-policy does not exist (line 215)
     <a href="/doc/qrexec3/#service-argument-in-policy">documentation</a>
- ./_site/doc/releases/4.0/release-notes/index.html
  *  linking to /doc/qrexec3/#extra-keywords-available-in-qubes-40-and-later, but extra-keywords-available-in-qubes-40-and-later does not exist (line 222)
     <a href="/doc/qrexec3/#extra-keywords-available-in-qubes-40-and-later">documentation</a>

Should we fix those as well?

Update: I have the changes to fix the release note links standing by.

Yes, please do.

[pierwill]

Fixed those links and a couple misspellings pointed out by Travis CI. (The rest of the spelling errors were false positives.)

[andrewdavidwong]

Phase 1 complete! Thank you, @pierwill!

[pierwill]

Happy to help—and thank you!