Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add a metadata table to the generated DFU file #34

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

hughsie commented Aug 24, 2016

This extra metadata allows flashing tools know that the firmware is already
obfuscated with an offset. Tools that don't understand the metadata table will
just ignore the extra data.

For more details, see https://github.com/hughsie/fwupd/blob/master/docs/dfu-metadata-store.md

hughsie commented Aug 24, 2016

I've also left the the Copyright and License fields commented out, please tell me if you want actual values there. Also, I've not tested with with the Walkera flash tool (software is supposed to read the footer len, not just assume a set size), so someone with the actual hardware will have to test that. Thanks!

Contributor

mwm commented Aug 25, 2016

The resulting DFU files don't work in either the Walkera dfuse tool - it complains that the format is invalid - or the devention Uploader, which quietly fails to load the dfu files.

If you have a Windows box, you can download the Walkera dfuse tool from https://drive.google.com/drive/folders/0B6SupsT8-3_BYXNQM1dOUlRYcGM. Just try loading one of the dfu files, and it will complain - no need to connect a transmitter to it.

For use on Unix or Unix-like systems, you can get the Deviation Uploader from http://www.deviationtx.com/downloads-new/category/161-dfu-usb-tool. It's a java binary, and needs both libusb and some java wrappers for it; it's known to work on ubuntu and OSX. Switch to the DFU tab and try opening the dfu files. It'll just fail (hey, it's beta software).

As for your questions: deviation doesn't do copyright assignment, so a Copyright field wouldn't be appropriate. The binary is distributed under the terms of the GPL 3 or later, so the license field is ok.

Also, I don't think we want to add the CipherKind field if the offset is 0, as that value is a no-op, so you should get an unencrypted dfu file.

Add a metadata table to the generated DFU file
This extra metadata allows flashing tools know that the firmware is already
obfuscated with an offset. Tools that don't understand the metadata table will
just ignore the extra data.

For more details, see https://github.com/hughsie/fwupd/blob/master/docs/dfu-metadata-store.md

hughsie commented Aug 25, 2016

The resulting DFU files don't work in either the Walkera dfuse tool

Okay, that's disappointing. I couldn't get the java version to work on my system at all, but I did verify that the windows tool is rejecting the files as invalid. I don't suppose the source code for that is somewhere? I've made the other two changes, but it it breaks with the uploader (either one) this change probably isn't a good idea.

As a fallback (urgh) is there a known signature that's mangled as part of the encryption? Just so we can use a heuristic and find out if the firmware is already "encrypted" or not. Thanks.

Contributor

mwm commented Aug 25, 2016

No source for the Walkera tools. The Java uploader is in the bitbucket repository I sent you a link to.

I don't know of such a signature, but I didn't write the uploader. We might be able to live with breaking the Walkera tool, but there Java uploader is required for the newer transmitters.

hughsie commented Aug 25, 2016

So looking at the DfuFile.java source file I see all the offsets from the end of the DfuSe section being a positive number. DFU actually specifies them the other way around, from the last byte in the file counting upwards. For instance,

    final byte [] ucDfuSignature = new byte[] {'U', 'F', 'D'};
    if (! Arrays.equals(Arrays.copyOfRange(data, start+8, start+11), ucDfuSignature)) {

should really be:

    final byte [] ucDfuSignature = new byte[] {'U', 'F', 'D'};
    if (! Arrays.equals(Arrays.copyOfRange(data, end-7, end-4), ucDfuSignature)) {

I guess because you're consuming only files that you wrote it's not a big deal, but it's going to break if the DFU group ever increase the size of the header. I don't know how much you care about this -- "not at all" would be perfectly valid.

Being logical about things, the only projects creating DFU files for the hardware is you guys and the vendor, so if you both crypt the firmware with the DEVO algorithm then the flashing tools can just be dumb and just flash the provided image without introducing an offset at flashing time.

I'm guessing you can't just flash a 7E image onto a 8S, and so we'd never have to deal with this in the flash tool. I'm fine if you want to close this PR.

Contributor

F-D-R commented Aug 25, 2016

Hi!

I've already suggested in the forums, that IMO we should hardcode the Walkera vendor ID to enable the obfuscation.
Walkera probably won't stop obfuscating, because that would break the compability with their own DfuSe tool, and if we used a different vendor ID for the not obfuscated DFUs (i.e. the ones for transmitters with a non-Walkera bootloader), it would at least prevent the use of the Walkera DfuSe Tool, which would ruine it anyway.

BTW I aggree, that the DFU parsing and checking need to be as flexible as we can, however if the DFU group would change the specification we (and Walkera as well) would need to change our implementations anyway...

hughsie commented Aug 25, 2016

we should hardcode the Walkera vendor ID

Yes, this is a very good idea. Any non-Walkera bootloader should have a different VID:PID for sure.

Contributor

victzh commented Sep 27, 2016

So, what's the resolution for this pull request? Decline as currently unusable?

Contributor

victzh commented Nov 11, 2016

Closing as no way to merge it without disrupting usability for current users.

@victzh victzh closed this Nov 11, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment