ben-grande / qusal

Salt Formulas for Qubes OS.
19 stars 7 forks source link

upstream Qubes VM kernel #3

Closed adrelanos closed 8 months ago

adrelanos commented 8 months ago

https://github.com/ben-grande/qusal/blob/main/salt/kicksecure-minimal/install.sls

"{{ slsdotpath }}-installed":
  pkg.installed:
    - refresh: True
    - install_recommends: False
    - skip_suggestions: True
    - pkgs:
      - kicksecure-qubes-cli
      - lkrg-dkms
      - hardened-kernel
      - tirdad
      - linux-image-amd64
      - linux-headers-amd64
      - grub2
      - qubes-kernel-vm-support

Installing

  - linux-image-amd64
  - linux-headers-amd64
  - grub2
  - qubes-kernel-vm-support

should not be up to Kicksecure. Basically I am suggesting to contribute towards https://github.com/QubesOS/qubes-issues/issues/5212.

This is problematic because it derivatives from Kicksecure default kernel, which is Qubes default kernel configuration.

This is unmaintainable, because if there are Qubes VM kernel specific crashes, issues, then these would be reported by users to Kicksecure where I would have no possibility to debug these except saying "please reproduce this in Qubes Debian and report the bug to Qubes".

1) Add a PR for https://github.com/QubesOS/qubes-meta-packages to add a meta package pulling all of these dependencies.

2) Add a PR for Qubes Debian Template builder installing the kernel packages by default.

3) Other changes towards making "flipping the switch to VM kernel" more easy. Last PR would be actually changing the default.

Don't be discouraged by https://github.com/QubesOS/qubes-doc/pull/1342 not being merged for a long time. That apparently depends on Qubes internal processes, maintainer availability. It's easier to land Qubes source code pull requests than Qubes documentation pull requests.

ben-grande commented 8 months ago

This is problematic because it derivatives from Kicksecure default kernel, which is Qubes default kernel configuration.

Understood, but Qubes default kernel is from Dom0 as you already know, I thought of shipping a secure by default base and using the kernel from the VM was the choice I made at the beginning, but this can be changed.

This is unmaintainable, because if there are Qubes VM kernel specific crashes, issues, then these would be reported by users to Kicksecure where I would have no possibility to debug these except saying "please reproduce this in Qubes Debian and report the bug to Qubes".

They can reproduce these issues in Kicksecure provided formula now, which is easier than running a buch of commands.

  1. Add a PR for https://github.com/QubesOS/qubes-meta-packages to add a meta package pulling all of these dependencies.

Is there a suitable package? Can not be kicksecure-qubes-cli as these install extra tools for the minimal template. Can a new package be created called kicksecure-qubes-kernel?

  1. Add a PR for Qubes Debian Template builder installing the kernel packages by default.

I believe this is out of scope of this issue. It would change Debian and derivatives, not only Kicksecure. It is also not that simple for already existing qubes because the qube preferred kernel (qvm-prefs QUBE kernel) has to be changed according to virt_mode. This would require changing the Qubes Documentation also...

  1. Other changes towards making "flipping the switch to VM kernel" more easy. Last PR would be actually changing the default.

Yes, it would be changing the default, so I don't think this request belong to this project, but directly to qubes.

This is why I, from this issue, I will complete the number 1, as it would remove deviation from upstream/downstrean relational packages, and close after it, as the rest is out of scope.

adrelanos commented 8 months ago

This is problematic because it derivatives from Kicksecure default kernel, which is Qubes default kernel configuration.

Understood, but Qubes default kernel is from Dom0 as you already know, I thought of shipping a secure by default base and using the kernel from the VM was the choice I made at the beginning, but this can be changed.

This is unmaintainable, because if there are Qubes VM kernel specific crashes, issues, then these would be reported by users to Kicksecure where I would have no possibility to debug these except saying "please reproduce this in Qubes Debian and report the bug to Qubes".

They can reproduce these issues in Kicksecure provided formula now, which is easier than running a buch of commands.

Then Qubes can ignore or accidentally assume the issue as "Kicksecure specific issue".

From my perceptive, Qubes VM kernel isn't a Kicksecure specific feature.

dom0 tool something similar to this:

  1. Add a PR for https://github.com/QubesOS/qubes-meta-packages to add a meta package pulling all of these dependencies.

Is there a suitable package?

It's seems to be a correct package in context of upstreaming Qubes VM kernel to Qubes.

Can not be kicksecure-qubes-cli as these install extra tools for the minimal template. Can a new package be created called kicksecure-qubes-kernel?

Yes, but Qubes VM kernel should be attempted to upstream to Qubes Debian Template first.

  1. Add a PR for Qubes Debian Template builder installing the kernel packages by default.

I believe this is out of scope of this issue. It would change Debian and derivatives, not only Kicksecure.

This is how it should be. Qubes by default using VM kernel for all VMs. Because then all users (most importantly including non-Kicksecure) will run into, report Qubes VM kernel specific issues and not only blame them on Kicksecure, where these cannot be fixed by me.

It is also not that simple for already existing qubes because the qube preferred kernel (qvm-prefs QUBE kernel) has to be changed according to virt_mode. This would require changing the Qubes Documentation also...

Qubes wants this feature as per https://github.com/QubesOS/qubes-issues/issues/5212 and the most correct, clean way is to implement it at the lowest (as in closest to upstream) technical level.

I think it's a very bad idea to implement these features far away downstream though salt configuration on top because that is unmaintanable. Can create a similar mess like here: https://github.com/ben-grande/qusal/issues/14

ben-grande commented 8 months ago

They can reproduce these issues in Kicksecure provided formula now, which is easier than running a buch of commands.

Then Qubes can ignore or accidentally assume the issue as "Kicksecure specific issue".

From my perceptive, Qubes VM kernel isn't a Kicksecure specific feature.

dom0 tool something similar to this:

  • qvm-kernel vm-kernel on vm-name
  • qvm-kernel vm-kernel on vm-name

It is not a Kicksecure specific feature but it is not a Qusal specific features.

  1. Add a PR for https://github.com/QubesOS/qubes-meta-packages to add a meta package pulling all of these dependencies.

Is there a suitable package?

It's seems to be a correct package in context of upstreaming Qubes VM kernel to Qubes.

Can not be kicksecure-qubes-cli as these install extra tools for the minimal template. Can a new package be created called kicksecure-qubes-kernel?

Yes, but Qubes VM kernel should be attempted to upstream to Qubes Debian Template first.

It is not that I don't want to fix in-VM kernel, it is just not related to Qusal. Yes, we are doing in-VM kernel because Kicksecure has a hardened kernel and this is useful, but from all the ideas I have to Qusal and Qubes issue I face, fixing this Qubes issue would take a long time.

  1. Add a PR for Qubes Debian Template builder installing the kernel packages by default.

I believe this is out of scope of this issue. It would change Debian and derivatives, not only Kicksecure.

This is how it should be. Qubes by default using VM kernel for all VMs. Because then all users (most importantly including non-Kicksecure) will run into, report Qubes VM kernel specific issues and not only blame them on Kicksecure, where these cannot be fixed by me.

And neither by me.

It is also not that simple for already existing qubes because the qube preferred kernel (qvm-prefs QUBE kernel) has to be changed according to virt_mode. This would require changing the Qubes Documentation also...

Qubes wants this feature as per QubesOS/qubes-issues#5212 and the most correct, clean way is to implement it at the lowest (as in closest to upstream) technical level.

I think it's a very bad idea to implement these features far away downstream though salt configuration on top because that is unmaintanable. Can create a similar mess like here: #14

It is not a bad idea to implement downstream, it is just not how upstream does because there is not upstream Kicksecure Template, only a guide. I rather advocate the contrary, use ShellScript when necessary (Salt not installed or difficult to do with Salt), Salt every other time.

Also, salt makes it more maintainable than trying to debug shellscripts and I am not going to debate more than that, if you try to replicate all the formulas provided by the project on the command-line, you will be more prone to mistakes and hard to understand bugs.

The "similar mess" created by me is a supposition and if confirmed, it is an unsupported configuration, as explained in https://github.com/ben-grande/qusal/issues/14#issuecomment-1923361901.

If you think it is unmaintainable to use a in-VM kernel downstream, I can remove it from the project to not diverge with upstream.

Moving the custom kernel to install-developers.sls would complicate things a bit because then I'd have to support Kicksecure with in-VM kernel and without in-VM kernel.

adrelanos commented 8 months ago

They can reproduce these issues in Kicksecure provided formula now, which is easier than running a buch of commands.

Then Qubes can ignore or accidentally assume the issue as "Kicksecure specific issue". From my perceptive, Qubes VM kernel isn't a Kicksecure specific feature. dom0 tool something similar to this:

  • qvm-kernel vm-kernel on vm-name
  • qvm-kernel vm-kernel on vm-name

It is not a Kicksecure specific feature but it is not a Qusal specific features.

That however would be a good feature.

  1. Add a PR for https://github.com/QubesOS/qubes-meta-packages to add a meta package pulling all of these dependencies.

Is there a suitable package?

It's seems to be a correct package in context of upstreaming Qubes VM kernel to Qubes.

Can not be kicksecure-qubes-cli as these install extra tools for the minimal template. Can a new package be created called kicksecure-qubes-kernel?

Yes, but Qubes VM kernel should be attempted to upstream to Qubes Debian Template first.

It is not that I don't want to fix in-VM kernel, it is just not related to Qusal. Yes, we are doing in-VM kernel because Kicksecure has a hardened kernel and this is useful, but from all the ideas I have to Qusal and Qubes issue I face, fixing this Qubes issue would take a long time.

Going the shortcut can create a mess.

  1. Add a PR for Qubes Debian Template builder installing the kernel packages by default.

I believe this is out of scope of this issue. It would change Debian and derivatives, not only Kicksecure.

This is how it should be. Qubes by default using VM kernel for all VMs. Because then all users (most importantly including non-Kicksecure) will run into, report Qubes VM kernel specific issues and not only blame them on Kicksecure, where these cannot be fixed by me.

And neither by me.

So the only answer that can be given in these cases is "don't Qubes VM kernel until at least until Qubes makes VM kernel default for Debian".

It is also not that simple for already existing qubes because the qube preferred kernel (qvm-prefs QUBE kernel) has to be changed according to virt_mode. This would require changing the Qubes Documentation also...

Qubes wants this feature as per QubesOS/qubes-issues#5212 and the most correct, clean way is to implement it at the lowest (as in closest to upstream) technical level. I think it's a very bad idea to implement these features far away downstream though salt configuration on top because that is unmaintanable. Can create a similar mess like here: #14

It is not a bad idea to implement downstream, it is just not how upstream does because there is not upstream Kicksecure Template, only a guide. I rather advocate the contrary, use ShellScript when necessary (Salt not installed or difficult to do with Salt), Salt every other time.

Also, salt makes it more maintainable than trying to debug shellscripts and I am not going to debate more than that, if you try to replicate all the formulas provided by the project on the command-line, you will be more prone to mistakes and hard to understand bugs.

I didn't mean to suggest replacing salt with shellscripts. Not at all.

I just mean Qubes should not "learn" getting VM kernel by default through a downstream salt configuration but instead thorough modification of qubes-meta-packages debian/control file modifications etc. (No shell scripting involved.) So this can become the default for all users.

In other words, it seems the wrong place to do this downstream with salt (or any technology, ansible, puppet, shell, ...) instead of contributing this upstream to Qubes.

The "similar mess" created by me is a supposition and if confirmed, it is an unsupported configuration, as explained in #14 (comment).

If you think it is unmaintainable to use a in-VM kernel downstream, I can remove it from the project to not diverge with upstream.

Yes. This should be opt-in somehow or not implemented at all so the user is aware of Qubes dom0 vs Qubes VM kernel.

Moving the custom kernel to install-developers.sls would complicate things a bit because then I'd have to support Kicksecure with in-VM kernel and without in-VM kernel.

ben-grande commented 8 months ago

Yes. This should be opt-in somehow or not implemented at all so the user is aware of Qubes dom0 vs Qubes VM kernel.

Made it opt-in, only instructed for Kicksecure Developers.