Add VKB and Virpil, generalize Thrustmaster#51
Add VKB and Virpil, generalize Thrustmaster#51kakra wants to merge 1 commit intoValveSoftware:masterfrom
Conversation
This adds generic hidraw support for Thrustmaster because at least T16000m throttle and joystick also need hidraw access. Also it add Virpil and VKB as mentioned here: - ValveSoftware/wine#197 - ValveSoftware/Proton#6839
|
Hello @kakra, we need to have the question at #36 (comment) answered before we can know how to proceed here. In general, the current understanding of what's in-scope for udev rules in 60-steam-input.rules are gamepad-like devices that get used with Steam Input, and the intent needs to be clarified when it comes to inclusion of udev rules for other peripherals that don't go through Steam Input. |
|
I'm not sure distros will be happy with non-Valve vendors being covered. We've kept it to specific VID/PIDs so far outside of Valve devices. We should add the PIDs of the throttle you're aware of though. |
|
I'm using this to ignore joysticks for most games because some games confuse them with gamepads: VID I leave it up to you to decide whether or how this should be shipped to distributions. But we should ensure that such devices do not become mapped as gamepads - be it via evdev, SDL, or hidraw... Otherwise some games won't accept input from your real gamepad, and actually use very weird mappings from the joystick axes. Since |
|
I think the right spot for those is this other project, right? https://github.com/denilsonsa/udev-joystick-blacklist |
|
Oh, I didn't know about that. This looks promising... EDIT: But well, the logic is different. The topic here is joysticks becoming detected as gamepads in Proton. The linked project is about traditional input devices (keyboard, mouse) or non-input devices (RGB controllers) wrongly being detected as joysticks. I think what's missing in SDL or Proton (where Proton already fixed this for some known devices) is to not detect real joysticks as gamepads. I'm seeing the problem like this: Proton maps all "gamepads" to XINPUT devices, which is good because not only genuine Xbox input devices will become XINPUT devices. But the definition of "gamepad" seems to be too fuzzy: It seems to map everything with enough buttons or axes to XINPUT, and that includes joysticks. And through the logic inside wine, those devices probably also appear as DINPUT devices, affecting some older games, especially because DINPUT doesn't seem to know analog triggers and thus such games see movement axes instead (making the camera going in circles unless you hold one or both triggers at 50%). I'm not sure how the classification should work in Windows. But I've seen my Thrustmaster (and later VKB) joysticks appear as Xbox gamepads in Elite Dangerous. This is probably not how it should work. And I'm not sure where this classification comes from in Proton. In the logs, I see there is a bool Could we use udev to classify joysticks and gamepads into two distinct classes? Currently, it looks like udev only knows For the future, it's probably better to not hard-code that in the wine code, it should come from udev or some other device database. The thing is: Only hidraw joysticks work properly in Proton. As soon as they go through evdev or SDLbus, they become gamepads in Proton... |
|
@ivyl what's your take on this? Is it just about doing more work in wine hid/dinput/rawinput/etc so that Thrustmater HOTAS are recognized as they are on Windows? I assume either way the udev rules to grant hidraw access will be needed, right? |
|
Cc: @rbernon
If the device is handled by our SDL backend and exposed as a controller we treat it as a gamepad. If not we have roughly the same comparison across the backends (including SDL non-game-controllers): if (axis_count == 6 && button_count >= (impl->hat_count ? 10 : 14)) desc.is_gamepad = TRUE;It's indeed a bit fuzzy and I don't think we can reasonably tighten this in way that won't hit regular controllers users. Except maybe having a conservative upper cap on button count. I think the safest way would be treating all thrustmaster/vkbsim/fanatec/etc. devices as non-gamepads. That would be a separate check just like the hidraw enablement.
We definitely need the udev rules for hidraw access somewhere. Not having that is extremely problematic. HID -> evdev -> fake HID is lossy. There's a lot of games that when they see vid/pid straight up assume certain input report format. Even SDL does that for some of the Sony controllers for example. Getting raw HID descriptors on Windows is impossible and you have to go through their "preparsed data" and various related helpers, which is a pain. Also we are currently allowing hidraw access for some of the devices (e.g. Sony gamepads, Thrustmaster HOTAS, some Virpil stuff) and since we cannot ship udev rules in Proton we have to have a fallback for non-hidraw backend for people who didn't set this up themselves. It's been problematic with Proton 9 and proves to be still problematic with Proton 10 in very unexpected ways - ValveSoftware/Proton#9034 which I'm currently looking at. I've seen you've accepted #36, shall I prepare a PR that adds uaccess for other devices from https://github.com/ValveSoftware/wine/blob/proton-wine-10.0-2e/dlls/winebus.sys/main.c#L505 ? I think those may be problematic as it's basically a whole vendor id: |
I think XINPUT has a hard limit on button count anyways: It's a word, so 16 bits. XINPUT Gamepad defines 14 buttons currently. So the upper limit should be 14 or 16 buttons. This would automatically exclude a lot of HOTAS already which usually have more than 20 buttons, sometimes more than 30 (where "buttons" includes hat switches). Maybe we could go a bit further: A gamepad should have only one hat switch. Old gamepads or current models with DINPUT mode have the triggers as buttons, not axes (so that makes 4 axes only but 2 more buttons). Some gamepads added more buttons (e.g. for screenshots). But all gamepads should have one property in common (at least from my experience with developing xpadneo):
Maybe it could help heuristics to put some more info about the structure of the HID reports into detection logic... Eg. "primary bitmap has 16 buttons max" (default buttons according to XINPUT spec), "secondary button bitmaps are less than 5" (hats and function buttons)...
Then it should be shipped at least as part of the documentation, so people can just drop the rules in
Not about that specific issue but generally, Proton 10 filters out all
The discussion in the issues concluded that those VIDs likely never ship anything else than HOTAS or other specific simulation hardware which needs hidraw anyways. The problem is even worse: the programming software of Virpil and VKB (likely others, too) supports setting arbitrary VID/PID for your hardware. And while people usually let the VID alone, they tend to program another PID so games can tell sticks or other simulation hardware for different purposes properly apart, even if such devices are the exact same model. |
|
I don't think we have a problem with whole VIDs if there's a justification, I don't know if distros will accept it without trying to modify it behind our back. Maybe put a comment about how the PID is configurable there. For vendors where it's not, I'd think that trying to enumerate all PIDs we've seen is a safer bet. This will always work on SteamOS and other like-minded distros focusing on desktop/local use, as we ship rules allowing hidraw to any devices from the local logged in user there. This is what we'd ideally see on any distro, but we've gotten pushback on that before. |
No, it's probably not - I consider that a security risk: The local user should not be given read access to all input devices - some are sensitive like your keyboard: You are entering passwords here. With hidraw read permissions, any process in your session scope could log any key-press. Even mouse movement may be sensitive. However, I agree: specialized gaming hardware should probably be okay. This is also why I see potential security risks with QMK and OpenRGB which simply do essentially
Well, no distribution cared when OpenRGB and QMK opened world read/write access for all hidraw devices... So I wonder if anyone would even notice... :-\ |
|
If you think there's arbitrary actively malicious code running as your local user outside of a sandbox, you have bigger problems than it covertly monitoring input devices, IMO. |
This is not about the Proton sandbox, and it's not about how such a scenario could be possible, and it's not about the idea thinking this is currently happening. It's about the possibility itself: A user could have made a drive-by download, and now a process could read key presses without root privileges. The sandbox here is that Wayland does not allow this, while Xorg allows keylogging, and root privileges probably allow it anyways, no matter what. Opening hidraw read access other than privileged processes now opens multiple possible attack vectors which other software in the system (e.g. Wayland) tries to prevent:
You could argue that the target audience of Steam/OpenRGB/QMK is single user desktop systems. But you cannot guarantee that some user doesn't set it up in another way. And you cannot guarantee that the user didn't unwillingly downloaded and executed malicious code, and usually such code is designed in a way to be not easily detected and way work for week, logging keys, without the user even noticing. It's only now, that the user has bigger problems, because keylogging is possible, not the other way around. Again, this is not about software that might run inside the Proton sandbox and getting hidraw access. That's a risk everyone should decide for themselves. I'm pretty sure we accept that risk if we play games downloaded from Steam, and thus trust Steam. It's about unwanted software that the user might not know about, and which didn't come from games, or from Proton, or the sandbox. |
|
I didn't mention Proton here - I'm just pointing out that if you're running unwanted software you might not know about, as your user, outside of any sandbox, there is an incalculable amount of things that are way more harmful and obvious to exploit that it could do immediately (and still covertly), instead of trying to monitor input devices. |
This is true, and it’s always true. If everyone only ran trusted software by trusted users, we wouldn’t have to care about exploits or security issues - we probably wouldn’t even know about them. But reality doesn’t work this way, and udev rules should not ship with obvious attack vectors. After all, it’s the user who trusts the distribution - whether that’s the Linux distro itself or whatever Valve ships. But I don’t think we need to discuss this any further. We’re just viewing the same topic from different angles: you’re saying that a malicious program could do worse things than logging keys and - I’m implying here - therefore we shouldn’t care about this. I say we shouldn’t open any opportunities, regardless of what other components are doing or not doing. And don’t get me wrong: I don’t think this is Valve’s responsibility here, and I didn’t even want to digress. I’m just questioning how such things could slip into distributions via OpenRGB or QMK when obviously - as you wrote - Valve had problems in the past shipping such or similar rules:
I just wanted to point out that this is neither “ideal”, nor do I see why your suggestions got pushed back when other software like OpenRGB or QMK just unconditionally opened hidraw world-readable and -writable... I think that opening hidraw rw access for a user session, for one specific VID covering a very well-defined class of devices, is much more focused, safe, and secure than just making the entire hidraw class world-rw (like other software did). That just doesn’t seem logically consistent to me. ;-) |
That's true on single-user devices, but the failure mode for overly-permissive raw HID udev rules is a shared computer:
This is why there has been movement towards not letting applications open raw HID devices directly, but instead making them go through some intermediary (logind? compositor? I'm not sure what the state of the art is here), which can use Until that interface has settled and is reasonably ubiquitous, the workaround is to give out raw HID access to relatively non-sensitive input devices (game controllers, but not keyboards), which is what steam-devices has generally done so far. |
|
For USB or Bluetooth game controllers, a workaround for the scenario I described is: if you don't trust the other users of a shared computer, unplug and reconnect the game controller before you use it to do anything that you don't want logged (for example entering passwords into an on-screen keyboard). The disconnection will invalidate the old raw HID device, and the re-connection will create a new one, which the other user never had permission to record (because they aren't the active local user any more). |
Yeah, I'm sure Valve will be careful with this (although an above statement said "we ship rules allowing hidraw to any devices from the local logged in user"). However, I've seen this pattern spilling over into other software like OpenRGB or QMK which do something like this in their udev rules: I didn't test how sessions and seats work with the modern solutions (logind, systemd, compositors, etc), I think they may dynamically revoke and grant access via ACLs, which mitigates the issue somewhat. I'm fine with opening the permissions for known gaming devices but keyboards should be explicitly excluded (as you pointed out). But cool, I didn't know about And sorry, I didn't want to digress so far from the original topic, but I think it may be important because the discussion started with "distributions may not accept that" (certainly for good reasons) but OTOH, they seem to just wave through software which does |
I think this is as simple as: sometimes distro maintainers make mistakes, and they won't always notice immediately when something is not consistent with the security policy that they intend to have. |
This adds generic hidraw support for Thrustmaster because at least T16000m throttle and joystick also need hidraw access.
Also it add Virpil and VKB as mentioned here:
Closes: #36