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

Disable virtual serial port by default #203

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rossy
Copy link

@rossy rossy commented May 21, 2017

It seems dangerous to leave this on. On Windows, at least, an unprivileged process can write to the serial port, and the debug interface could allow it to send privileged input or act as a keylogger.

Edit: Whoops. Somehow I missed the related issue #159 and pull request #160. I still believe the serial port should be off by default, at least until its security can be improved. It's important to note that keyloggers aren't the only threat. Sending fake input through the keyboard could be a problem too.

It seems dangerous to leave this on. If an unprivileged process can
write to serial ports, it could send privileged input (like magic SysRq
sequences on Linux or a UAC bypass on Windows) or it could act as a
keylogger.
@haata
Copy link
Member

haata commented May 21, 2017

Yeah, I agree with having the serial port disabled by default. At least for production keyboards.

I'm going to keep it enabled at least until I have the API working. Contemplating between rawhid and vendor specific. I know vendor specific will require a small driver (so it should be possible to harden on Windows), not sure about rawhid (which is the preferred interface when doing stuff like this).
Do you have any thoughts on this?

@rossy
Copy link
Author

rossy commented May 22, 2017

That's a good question. I don't know if Windows allows unprivileged processes to send raw HID reports to vendor-defined devices, but I think it might. I have three vendor-defined devices on this PC at the moment and according to WinObj, their device objects allow write access for all users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants