Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tags and features to pillar #17

Merged
merged 1 commit into from
May 17, 2024

Conversation

ben-grande
Copy link
Contributor

@marmarek
Copy link
Member

Tags are probably fine, but all features I'd be more careful about. Including them in pillar means the VM will indirectly learn them. Some are explicitly published into qubesdb (see qvm-features man page), but not all, and this is the API for any 3rd-party extension to store extra data about the qube, which may include some secrets.

@ben-grande
Copy link
Contributor Author

Then an enumeration/allow-list of features can be made. From the manpage, only vm-config features.

How to choose what is okay to be published? Are the ones listed in the manpage for qvm-features allowed to be published? If you think this will complicate too much the PR, I can separate the tags from the features on different PRs.

@marmarek
Copy link
Member

How to choose what is okay to be published?

This is a very good question. Definitely vm-config.* and service.* are safe. Other currently defined might be safe too (but it would be behavior change, as VM didn't have this ability before. For example showing net.fake-ip will tell the VM it's not seeing its "real" IP, but that can be deduced indirectly too. Or internal will tell the VM its menu entries (and some other stuff) is hidden - another thing that VM didn't know before.

So, I'd go with the safe list above, and if anybody needs something else, it can be made configurable or such, to avoid having some long/complex list hardcoded.

@marmarek
Copy link
Member

Oh, and just noticed the service.* is not really documented there. QubesOS/qubes-core-admin-client#290

@gonzalo-bulnes
Copy link

FWIW, when thinking about what features may be safe to expose to the VMs or not, I considered creating a custom namespace (as in service.*, not in the Salt sense).

That seemed to me more practical than allowlisting individual features, which felt like it would be pushing the problem one config file further.

Unless I'm wrong, the namespace implementation wouldn't require new code, only documenting the convention so that no-one accidentally creates features which name will make them be exposed to VMs. Of course, the name chosen for that namespace could also go a long way in preventing surprises.

@ben-grande
Copy link
Contributor Author

FWIW, when thinking about what features may be safe to expose to the VMs or not, I considered creating a custom namespace (as in service.*, not in the Salt sense).

That seemed to me more practical than allowlisting individual features, which felt like it would be pushing the problem one config file further.

I agree it would be better to do that directly to qubesadmin rather than modify the _pillar/qvm_features.py.

Unless I'm wrong, the namespace implementation wouldn't require new code, only documenting the convention so that no-one accidentally creates features which name will make them be exposed to VMs. Of course, the name chosen for that namespace could also go a long way in preventing surprises.

I believe it would need new code on the qubesadmin side, at least to report via a new safe_to_share_features() for example, that gather only vm-config. and service..

My latest commit only covers those two classes of features, but in the future, it makes sense to do that directly to qubesadmin.

@marmarek
Copy link
Member

I believe it would need new code on the qubesadmin side, at least to report via a new safe_to_share_features() for example, that gather only vm-config. and service..

That's interesting idea, but I think it's okay to defer adding it until there will be a need for a second user of such function.
For now, it just wants documentation which features are shared - see https://www.qubes-os.org/doc/salt/#the-qubes-pillar-module

@marmarek marmarek merged commit a5f5bdd into QubesOS:main May 17, 2024
2 checks passed
ben-grande added a commit to ben-grande/qubes-doc that referenced this pull request May 17, 2024
marmarek added a commit to marmarek/qubes-core-admin that referenced this pull request May 17, 2024
@marmarek
Copy link
Member

FYI QubesOS/qubes-core-admin#593

marmarek added a commit to marmarek/qubes-core-admin that referenced this pull request May 17, 2024
marmarek added a commit to marmarek/qubes-core-admin that referenced this pull request May 22, 2024
@ben-grande ben-grande deleted the pillar-tags-features branch June 9, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants