Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upConsider pre-installing Salt states for USB input device support #2409
Comments
andrewdavidwong
added
the
C: mgmt
label
Oct 30, 2016
andrewdavidwong
added this to the Release 3.2 milestone
Oct 30, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
marmarek
Oct 30, 2016
Member
This all is exactly the reason why firstboot do not allow enabling qvm.sys-usb when USB keyboard is detected.
There are few cases:
- USB keyboard is connected to separate (preferably dedicated) USB controller - in this case, the best would be to create separate USB VM and configure it to allow only keyboard/mouse.
- USB keyboard is connected to the same controller (probably the only one) as other devices. In this case the only way to have USB VM is to allow it to run
qubes.InputKeyboardservice, which make it much more sensitive (will be able to sniff your input and inject keystrokes). Still better than having USB controller in dom0, but not that much.
In any case, having USB keyboard requires not blacklisting it during system startup, to enter LUKS passphrase.
The first case above requires new state to create such VM and indeed some grain listing USB controller(s) for handling keyboard (and probably mouse). The second case is very similar to the current state - it's just about allowing qubes.InputKeyboard. And unfortunately the second case (only one USB controller) is also much more common.
Your approach basically exclude selected USB controller from sys-usb. In case of single USB controller, it makes sys-usb useless. IMO much better (from security POV) would be to:
- In case of multiple USB controllers: create separate VM for handling USB keyboard.
- In case of single USB controller: allow
qubes.InputKeyboardservice fromsys-usb. And maybe name it differently to make it obvious that it's much more sensitive than "standard" USB VM.
|
This all is exactly the reason why firstboot do not allow enabling There are few cases:
In any case, having USB keyboard requires not blacklisting it during system startup, to enter LUKS passphrase. The first case above requires new state to create such VM and indeed some grain listing USB controller(s) for handling keyboard (and probably mouse). The second case is very similar to the current state - it's just about allowing Your approach basically exclude selected USB controller from
|
andrewdavidwong
added
the
wontfix
label
Oct 31, 2016
andrewdavidwong
closed this
Oct 31, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Sen-Dion
Nov 1, 2016
Thank you for taking time to look into this issue. In reply to @marmarek 's comments:
This all is exactly the reason why firstboot do not allow enabling qvm.sys-usb when USB keyboard is detected.
The Qubes OS Development Team made a right (safe) choice.
There are few cases:
- USB keyboard is connected to separate (preferably dedicated) USB controller - in this case, the best would be to create separate USB VM and configure it to allow only keyboard/mouse.
- USB keyboard is connected to the same controller (probably the only one) as other devices. In this case the only way to have USB VM is to allow it to run qubes.InputKeyboard service, which make it much more sensitive (will be able to sniff your input and inject keystrokes). Still better than having USB controller in dom0, but not that much.
Enhancement proposal
Can Qubes OS have two pre-installed states to support these two use cases? This way, the documentation may instruct user to enable one of these states based on his/her preference. It looks like a win-win situation: developer saves time by producing less documentation; user saves time by reading less, and performing less post installatuion steps.
IMO much better (from security POV) would be to:
- In case of multiple USB controllers: create separate VM for handling USB keyboard.
Better approach
This is really a good advice. I tried to follow it. Unfortunately, I hit the wall. In particular, the keyboard is unresponsive till the separate VM is up and running. Well, all VMs start only after user enters his/her password. In order to enter the password, user needs keyboard to be responsive. This chicken and egg dependency forced me to go with the plan "B":
basically exclude the selected USB controller from sys-usb
The detailed record of this endeavour:
# 1. Create sys-usb VM
dom0$ sudo qubesctl top.enable qvm.sys-usb
dom0$ sudo qubesctl state.highstate
# USB keyboard is the only keyboard that is available on this system.
# USB mouse is the only mouse that is available on this system.
# After creation of sys-usb VM, neither keyboard, nor mouse is operational.
# 2.Make them operational again by performing the following steps:
# 2.1. Detach one OHCI controller and bring it back to dom0
# 2.2. Revoke permission to pass mouse/keyboard input from sys-usb to dom0
# 2.3. Whitelist all USB controllers in dom0
# 3. At a later time, I needed to add "smallfoot" VM based on fedora-23-minimal template.
# I copied personal.sls and personal.top to smallfoot.sls and smallfoot.top, respectfully.
# After few editing touches, I run
dom0$ sudo qubes-dom0-update qubes-template-fedora-23-minimal
dom0$ sudo qubesctl top.enable qvm.smallfoot
dom0$ sudo qubesctl state.highstate
After the step 3, smallfoot VM operates as expected. However, all adjustments 2.1-2.3 are overwritten.
Deficiencies
The deficiencies (from a user prospective) are:
I. Use case: detect USB controller that communicates with keyboard/mouse, and attach it to the VM separate from sys-usb. Such state is not pre-installed.
2. Use case: detect USB controller that communicates with keyboard/mouse, and detach it from sys-usb (it will end up in dom0). Such state is not pre-installed.
3. Use case: create "smallfoot" VM based on fedora-23-minimal template. Such state is not pre-installed.
Could Qubes OS Team consider my proposal (see "General notes" section in my original submission) and "Enhancement proposal" (in this post) or come up with their own approach to address the current deficiencies?
Note that I may submit my versions of states. However, someone will have to review them, as I am not versed in saltstack/jinja/python infrastructure.
Sen-Dion
commented
Nov 1, 2016
•
|
Thank you for taking time to look into this issue. In reply to @marmarek 's comments:
The Qubes OS Development Team made a right (safe) choice.
Enhancement proposalCan Qubes OS have two pre-installed states to support these two use cases? This way, the documentation may instruct user to enable one of these states based on his/her preference. It looks like a win-win situation: developer saves time by producing less documentation; user saves time by reading less, and performing less post installatuion steps.
Better approachThis is really a good advice. I tried to follow it. Unfortunately, I hit the wall. In particular, the keyboard is unresponsive till the separate VM is up and running. Well, all VMs start only after user enters his/her password. In order to enter the password, user needs keyboard to be responsive. This chicken and egg dependency forced me to go with the plan "B":
The detailed record of this endeavour:
After the step 3, smallfoot VM operates as expected. However, all adjustments 2.1-2.3 are overwritten. DeficienciesThe deficiencies (from a user prospective) are: Could Qubes OS Team consider my proposal (see "General notes" section in my original submission) and "Enhancement proposal" (in this post) or come up with their own approach to address the current deficiencies? Note that I may submit my versions of states. However, someone will have to review them, as I am not versed in saltstack/jinja/python infrastructure. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Sen-Dion
Nov 6, 2016
Could you please reopen the bug and rename it "Create and upload few states for the user to choose from". Thank you.
Sen-Dion
commented
Nov 6, 2016
|
Could you please reopen the bug and rename it "Create and upload few states for the user to choose from". Thank you. |
andrewdavidwong
added
enhancement
help wanted
and removed
wontfix
labels
Nov 7, 2016
andrewdavidwong
modified the milestones:
Far in the future,
Release 3.2
Nov 7, 2016
andrewdavidwong
changed the title from
Saltstack overwrites local adjustments
to
Consider pre-installing Salt states for USB input device support
Nov 7, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
andrewdavidwong
Nov 7, 2016
Member
@Sen-Dion: Reopened, but I think we need a more descriptive title, so here's my best shot. If you think it's inaccurate, please let me know.
|
@Sen-Dion: Reopened, but I think we need a more descriptive title, so here's my best shot. If you think it's inaccurate, please let me know. |
andrewdavidwong
reopened this
Nov 7, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
marmarek
Nov 8, 2016
Member
Ok, so I think we need grain/pillar with USB controller(s) used for keyboard. Once the controller is disconnected from dom0 (by this state, or by user manually), dom0 have no idea whether some keyboard is attached to it, so probably it should be pillar, as grains are stateless (at least in theory).
I'm slightly for the option with separate USB VM, as it will isolate dom0 from other device types plugged there. But one may only set appropriate pillar to exclude selected controller(s) from sys-usb, then enable or not state to create new USB VM (personally I call it sys-input).
In short, @Sen-Dion I agree.
|
Ok, so I think we need grain/pillar with USB controller(s) used for keyboard. Once the controller is disconnected from dom0 (by this state, or by user manually), dom0 have no idea whether some keyboard is attached to it, so probably it should be pillar, as grains are stateless (at least in theory). I'm slightly for the option with separate USB VM, as it will isolate dom0 from other device types plugged there. But one may only set appropriate pillar to exclude selected controller(s) from sys-usb, then enable or not state to create new USB VM (personally I call it sys-input). |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
marmarek
Nov 8, 2016
Member
Clarification: keyboard is that important device. Mouse (if not on the same USB controller) can also be in sys-usb.
|
Clarification: keyboard is that important device. Mouse (if not on the same USB controller) can also be in sys-usb. |
Sen-Dion commentedOct 30, 2016
•
edited
Edited 1 time
-
Sen-Dion
edited Nov 1, 2016 (most recent)
Qubes OS version:
3.2 (R3.2)
Expected behavior:
After the step 3 (see details below), smallfoot VM operates as expected. The adjustments 2.1-2.3 shall stay.
Actual behavior:
After the step 3 (see details below), smallfoot VM operates as expected. However, all adjustments 2.1-2.3 are overwritten.
Steps to reproduce the behavior:
General notes:
I found that it is possible to disable the overwriting of adjustments 2.1-2.3 by modifying the following sections in sys-usb.sls:
2.1.a. Add pci_usb_devs_skipping_console grain, which returns all pci_usb_devs, except the one which is designated to communicate with USB keyboard/mouse.
2.2.a. Comment out
qubes-input-proxyandsys-usb-input-proxy2.3.a. Comment out
include qvm.hide-usb-from-dom0Rather then expect user to perform all these modifications, a simpler,and a much more user-friendly approach may be employed:
A) Instruct user to disconnect all USB devices except the keyboard and reboot.
B) Instruct user to create sys-usb VM by running:
sys-usb.sls should use the following approach: detect USB controller that communicates with the USB keyboard. Record it as a console_grain. Make 2.1.a-2.3.a conditional on the presence of the console_grain. User is allowed to reconnect all USB devices once
qubesctl state.highstatecompletes.On the subsequent run, the overwrite will take place only if console_grain doesn't exist.
Related issues: