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 support for restart to bootloader via USB command #14212

Closed
wants to merge 1 commit into from

Conversation

stdvar
Copy link

@stdvar stdvar commented Aug 29, 2021

Description

This change will piggyback on existing HID descriptor used by shared EP for extra key and shared keyboard/mouse.
New feature report section in the HID descriptor is added to allow PC to send HID feature report to KB and thus instruct it to restart in bootloader.
Added code to distinguish between input/output/feature reports.
set_report buffer that used to handle led status is increased from (1+1 size) to (1+8 size).
This should work for any keyboard since handler uses generic bootloader_jump() function.

There is new USB_FEATURE_ENABLE option that enables shared ep just like EXTRAKEY_ENABLE. So only one option is needed but new option added for consistency in case user opted out of using EXTRAKEY or other shared ep options.

Report to send to KB is struct.pack("<II", 0x5AA555AA, 0xCC3300FF).

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@github-actions github-actions bot added the core label Aug 29, 2021
@tzarc
Copy link
Member

tzarc commented Aug 29, 2021

Jumping to bootloader without external intervention is a security risk and will not be accepted, I'm afraid.

The in-progress XAP development will require functionality like bootloader jump to be guarded by an unlock key combination before becoming accessible. See here for more details: https://hackmd.io/@tzarc/ryY5liviO

@zvecr zvecr changed the base branch from master to develop August 30, 2021 19:24
@zvecr
Copy link
Member

zvecr commented Aug 30, 2021

Jumping to bootloader without external intervention is a security risk and will not be accepted, I'm afraid.

#7456 was removed for exactly this reason. For now I think it would be best to defer items in this area till XAP has progressed.

@tzarc
Copy link
Member

tzarc commented Aug 30, 2021

As was #11892.

@stdvar
Copy link
Author

stdvar commented Aug 30, 2021

how different having raw_hid from having hid feature report configured? either way KB is open to accept packets. or the concern is bootloader jump itself?

@tzarc
Copy link
Member

tzarc commented Aug 30, 2021

The bootloader jump itself is an attack vector.

@stale
Copy link

stale bot commented Oct 24, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@tzarc tzarc closed this Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants