usb: dfu: disable USB DFU uploading by default #42424
Conversation
It is possible that the actual data stage length of the control transfer is zero, in that case we do not need an additional ZLP packet. This fixes a problem with USB DFU, where after an upload the device is no longer responsive if upload size is multiple of control endpoint MPS. Signed-off-by: Johann Fischer <[email protected]>
Rename confusing UPLOAD_FLASH_AREA_ID to DOWNLOAD_FLASH_AREA_ID as it is area where firmware image is downloaded. Signed-off-by: Johann Fischer <[email protected]>
jfischer-no
added
bug
jfischer-no
force-pushed
the
pr-usb-dfu-fixes-for-300
branch
from
d9d35a5 to
af2f936
Compare
subsys/usb/class/dfu/Kconfig
Outdated
There was a problem hiding this comment.
Hi, this description is not very clear
There was a problem hiding this comment.
Enabling this option allows to download the image by the host. Be aware that the primary image (executable one) is always plain-text despite the image encryption is enabled.
There was a problem hiding this comment.
Maybe do the Security Risk: Enabling this option ... to make it more clear
There was a problem hiding this comment.
It is upload / uploading, device uploads the firmware to the host, downloading is from host to device. Clearly described according to the USB DFU spec.
There was a problem hiding this comment.
@jfischer-no Yes, let use upload word:
Security Risk: Enabling this option allows to upload the image to the host. Be aware that the primary image (executable one) is always plain-text despite the image encryption is enabled.
There was a problem hiding this comment.
Decrypted firmware is correct term in this case, not plain-text.
What about single slot?
There was a problem hiding this comment.
Single slot update works with mcuboot serial recovery and allows decryption in place on serial upload. This means that allowing download is risk in application running from single slot if it has been uploaded as encrypted.
There was a problem hiding this comment.
Unfortunately we do not, yet, have a method to get information from on mcuboot configuration to resolve this at application runtime. Application also does not know whether it has been uploaded encrypted.
There was a problem hiding this comment.
Decrypted firmware is correct term in this case, not plain-text.
OK
There was a problem hiding this comment.
Updated description
There was a problem hiding this comment.
Good thing this is being fixed :)
Note that the device can let the host know it is not capable of uploading. See https://github.com/zephyrproject-rtos/zephyr/blob/af2f936d8d0000e143faa32e5cf997160082554c/subsys/usb/class/dfu/usb_dfu.c#L104
And USB DFU Specification page 13
subsys/usb/class/dfu/usb_dfu.c
Outdated
There was a problem hiding this comment.
Might be a good idea to not even compile in the upload code. Then an upload is impossible even if someone manages to bypass this check. Altho the compiler will probably optimize away the unused code, that does require those optimizations to be enabled.
There was a problem hiding this comment.
No, my wish would be that underlying flash API simply has no access to this area if not enabled.
Thanks for the hint, totally forgot about DFU attributes. |
Firmware uploading to the host may not always be desired, disable it by default. Enable it for existing USB DFU sample to keep the usual behavior, add note about new option to README.rst. Signed-off-by: Johann Fischer <[email protected]>
jfischer-no
force-pushed
the
pr-usb-dfu-fixes-for-300
branch
from
af2f936 to
d6e20de
Compare
jfischer-no
requested review from
nvlsianpu
and removed request for
utzig and
mbolivar-nordic
|
@nvlsianpu can you please take a look? |
usb: do not add ZLP if lenght to be transmitted is just zero
It is possible that the actual data stage length of
the control transfer is zero, in that case we do not
need an additional ZLP packet.
This fixes a problem with USB DFU, where after an upload
the device is no longer responsive if upload size is
multiple of control endpoint MPS.
usb: dfu: disable USB DFU uploading by default
Firmware uploading to the host may not always be desired,
disable it by default.
Enable it for existing USB DFU sample to keep
the usual behavior, add note about new option to README.rst.