Add missing .adoc files to the tarball

[Cropi]

If one attempted to build usbguard from the tarball, some documentation related files would be left out. The patch resolves this issue.

[hartwork]

Hi @Cropi I see two things here: adding 3 files for tarball generation and dropping sudo at 3 places. Dropping sudo seems unrelated to the official mission of the pull request and the commit, and it is also something I would like to discuss and understand the motivation, e.g. because I need sudo to run these commands in practice over here. Regarding the build system fix, I am wondering why the error does not show up in CI (where we do cover make distcheck) and hence maybe we should extend CI to produce this error, so that we can see that this is a complete fix and that we won't have regressions. How can the issue be reproduced that's being fixed here? Thanks!

[Cropi]

Hi @Cropi I see two things here: adding 3 files for tarball generation and dropping sudo at 3 places. Dropping sudo seems unrelated to the official mission of the pull request and the commit, and it is also something I would like to discuss and understand the motivation, e.g. because I need sudo to run these commands in practice over here. Regarding the build system fix, I am wondering why the error does not show up in CI (where we do cover make distcheck) and hence maybe we should extend CI to produce this error, so that we can see that this is a complete fix and that we won't have regressions. How can the issue be reproduced that's being fixed here? Thanks!

Hi @hartwork, thank you for the review. The issue can be reproduced when you download the tarball from e.g. from https://github.com/USBGuard/usbguard/releases/tag/usbguard-1.1.2. The mentioned files were not included because were not part of the make dist command. I've built usbguard from the tarball many times but no error was shown during that process even though some files were not present. It seems like asciidoc(a2x) simply ignores missing files when creating those particular man pages.
You are right that dropping sudo does not have to do anything with the mission of the PR. I though it would be better to remove it. In general, you should be able to generate initial policy and change the state of any device using the usbguard CLI as long as you have enough permission specified inside the IPC access control file(s).

[hartwork]

Hi @hartwork, thank you for the review. The issue can be reproduced when you download the tarball from e.g. from https://github.com/USBGuard/usbguard/releases/tag/usbguard-1.1.2. The mentioned files were not included because were not part of the make dist command. I've built usbguard from the tarball many times but no error was shown during that process even though some files were not present. It seems like asciidoc(a2x) simply ignores missing files when creating those particular man pages.

@Cropi I was now able to verify the issue as following:

# cd "$(mktemp -d)"
# wget https://github.com/USBGuard/usbguard/releases/download/usbguard-1.1.2/usbguard-1.1.2.tar.gz
# tar xf usbguard-1.1.2.tar.gz
# cd usbguard-1.1.2/
# A2X=a2x ./configure --with-bundled-catch  # note A2X=a2x!
# make -j$(nproc) |& tee build.log
# grep -F 'include file not found' build.log | awk '{print $NF}' | sort -u
/tmp/tmp.vXptX2zvUW/usbguard-1.1.2/doc/man/example-allow-device.adoc
/tmp/tmp.vXptX2zvUW/usbguard-1.1.2/doc/man/example-initial-policy.adoc
/tmp/tmp.vXptX2zvUW/usbguard-1.1.2/doc/man/footer.adoc

It seems that we should add A2X=a2x to configure calls in the CI somewhere (or everywhere) so that documentation is built in CI and that we'll have protection against this issue coming back as a regression in the future.

If there is no way of making asciidoc fail on missing include files, maybe our CI should grep build logs for 'include file not found'. That would make the CI bullet-proof.

This very pull request seems like the perfect place to make that change, to me. What do you think?

You are right that dropping sudo does not have to do anything with the mission of the PR. I though it would be better to remove it. In general, you should be able to generate initial policy and change the state of any device using the usbguard CLI as long as you have enough permission specified inside the IPC access control file(s).

I would like to vote against dropping sudo, assuming that:

• Some people copy-n-paste whatever we advise and the version with sudo will work and is not "wrong per se"
• Mention of sudo will hopefull make people think more what they are doing, if they care at all
• Dropping sudo will only help those that have additional file magic set up, but it's universal across distros and I would argue taking sudo away is something people can help with themselves.

To summarize, I believe we would lose a feature and add new trouble by dropping sudo. Please let's keep it, okay? 🙏

[Cropi]

Sorry @hartwork for the late answer. It seems like that I did not receive a notification from the comment you added.

I agree that we should extend the CI to not face similar issues in the future. I have not found a way to force asciidoc to error out. However, we could check the output and search for common mistakes as you suggested, e.g. whether build logs contain "include file not found" or maybe "asciidoc: WARNING" to capture more problems.

If I take a look at github actions, I can find the mentioned warning message inside the distcheck section:

asciidoc: WARNING: usbguard-daemon.conf.5.adoc: line 197: include file not found: /home/runner/work/usbguard/usbguard/build/usbguard-1.1.2/doc/man/footer.adoc

Regarding the sudo part, I am fine with removing that part of the commit. However, in an ideal situation, you do not need root privileges to alter the devices as long as the usbguard IPC permissions are configured properly.

[hartwork]

Sorry @hartwork for the late answer. It seems like that I did not receive a notification from the comment you added.

I agree that we should extend the CI to not face similar issues in the future. I have not found a way to force asciidoc to error out. However, we could check the output and search for common mistakes as you suggested, e.g. whether build logs contain "include file not found" or maybe "asciidoc: WARNING" to capture more problems.

If I take a look at github actions, I can find the mentioned warning message inside the distcheck section:

asciidoc: WARNING: usbguard-daemon.conf.5.adoc: line 197: include file not found: /home/runner/work/usbguard/usbguard/build/usbguard-1.1.2/doc/man/footer.adoc

@Cropi I have made the CI do that in pull request #567 now. Your fixes to Makefile.am are included.

Regarding the sudo part, I am fine with removing that part of the commit. However, in an ideal situation, you do not need root privileges to alter the devices as long as the usbguard IPC permissions are configured properly.

I see the point in avoiding root permissions but let's not drop sudo please before there is a well-documented approach on how to set-up a working alternative, in detail. Are there tried-and-tested docs on that somewhere?

[Cropi]

Sorry @hartwork for the late answer. It seems like that I did not receive a notification from the comment you added.
I agree that we should extend the CI to not face similar issues in the future. I have not found a way to force asciidoc to error out. However, we could check the output and search for common mistakes as you suggested, e.g. whether build logs contain "include file not found" or maybe "asciidoc: WARNING" to capture more problems.
If I take a look at github actions, I can find the mentioned warning message inside the distcheck section:

asciidoc: WARNING: usbguard-daemon.conf.5.adoc: line 197: include file not found: /home/runner/work/usbguard/usbguard/build/usbguard-1.1.2/doc/man/footer.adoc

@Cropi I have made the CI do that in pull request #567 now. Your fixes to Makefile.am are included.

Okay, let's continue the discussion there.

Regarding the sudo part, I am fine with removing that part of the commit. However, in an ideal situation, you do not need root privileges to alter the devices as long as the usbguard IPC permissions are configured properly.

I see the point in avoiding root permissions but let's not drop sudo please before there is a well-documented approach on how to set-up a working alternative, in detail. Are there tried-and-tested docs on that somewhere?

The usbguard-daemon.conf man page describes it in the IPC access control section. Let me know if you think something is missing from there or it's not obvious at first sight. Thanks.

[hartwork]

The usbguard-daemon.conf man page describes it in the IPC access control section. Let me know if you think something is missing from there or it's not obvious at first sight. Thanks.

@Cropi interesting pointer, thanks!
Based on the man page and…

sudo usbguard add-user "${USER}" --policy list,modify --devices list,listen,modify --exceptions listen --parameters list,listen,modify
sudo /etc/init.d/usbguard restart  # openrc

…I was able to elevate my unprivileged user to what I'd normally need root for with USBguard now, e.g. running usbguard list-devices without root. I'm not sure that's an approach we can and should expect from all users operating USBGuard. Maybe let me think and sleep about that more.