d0k3 / GodMode9

GodMode9 Explorer - A full access file browser for the Nintendo 3DS console :godmode:
GNU General Public License v3.0
2.12k stars 191 forks source link

Fix exclusion of RSA key in trimmed NTR image #721

Closed SNBeast closed 3 years ago

SNBeast commented 3 years ago

With the release of nds-bootstrap v0.44.2, the issue it was having with cloneboot (a means of Download Play) was fixed. The issue was that an RSA key was not being read. This RSA key is at the end of the game data, so an extra 0x88 bytes needs to be read from NTR cartridges. Reading this extra data was implemented into GM9i v2.7.0.

Relevant code snippet (difference at the end): Before: if (trimRom) { romSize = (isDSiMode() && (ndsCardHeader.unitCode != 0) && (ndsCardHeader.twlRomSize > 0)) ? ndsCardHeader.twlRomSize : ndsCardHeader.romSize; } After: if (trimRom) { romSize = (isDSiMode() && (ndsCardHeader.unitCode != 0) && (ndsCardHeader.twlRomSize > 0)) ? ndsCardHeader.twlRomSize : ndsCardHeader.romSize + 0x88; }

lifehackerhansol commented 3 years ago

After some testing with a handful of my 24 flashcarts: the RSA key is also a requirement in Download Play on flashcarts as well. So this issue applies to NDS backup playback on hardware as a whole and not specific to nds-bootstrap.

d0k3 commented 3 years ago

Can you try with this test build?

GodMode9.zip

d0k3 commented 3 years ago

And, a question to those in the know - do we actually only need to enlarge the trimmed size by 0x88 byte for non DSi carts?

d0k3 commented 3 years ago

And, third, @lifehackerhansol - I've seen the pull request, but this is not the best place for this fix and can cause other problems there (like reading beyound cart limits). Also, you're doing it for both, DS and NDSi carts there, which doesn't seem neccessary.

SNBeast commented 3 years ago

Can you try with this test build?

Tried with Metroid Prime: Hunters USA Rev 0, still 0x88 bytes short compared to GM9i. I didn't test DLP but I'll assume you didn't shift things around.

And, a question to those in the know - do we actually only need to enlarge the trimmed size by 0x88 byte for non DSi carts?

@RocketRobz made the nds-bootstrap and GM9i fixes, so I'll ping him.

RocketRobz commented 3 years ago

And, a question to those in the know - do we actually only need to enlarge the trimmed size by 0x88 byte for non DSi carts?

DSi ROMs that use cloneboot will put the RSA key before the DSi-specific data, so yes, enlarging the trimmed size is only needed for non-DSi carts.

lifehackerhansol commented 3 years ago

Also, you're doing it for both, DS and NDSi carts there, which doesn't seem neccessary.

I thought I reverted the NDSi cart size? Or does the first NTR function also affect the DSi carts as well?

I can't seem to find any other relevant function from my reading of the gamecart section.

d0k3 commented 3 years ago

Sorry about the radio silence. @lifehackerhansol - the best place for changes such as this is here: https://github.com/d0k3/GodMode9/blob/master/arm9/source/utils/gameutil.c#L3221 Maybe you can change your pull request? And maybe you guys could test the changed pull request then?

You don't want to edit the data size directly, rather focus on the trimsize. Stuff happens, people may try bad carts and whatever. The function here checks that we never read beyound the cart limits.

lifehackerhansol commented 3 years ago

Appending 0x88 to that exact line freezes GodMode9 for some reason. With no exceptions either.

Update: It's not crashing anymore, think it was a build quirk or a console quirk. But... it doesn't report any difference in file size. I wonder what's up now.

SNBeast over on Discord, confirmed by me: Copying the trim file from the card drive excludes the key, but doing the trim operation on an existing ROM includes the key.

To avoid divided discussion I'll continue commenting on the PR.

d0k3 commented 3 years ago

Alright, here's a test build based on #722 .

Can someone test this both with dumps and carts and tell us if everything is alright now? GodMode9.zip

RocketRobz commented 3 years ago

Can someone test this both with dumps and carts and tell us if everything is alright now?

Dumping is fixed, but trimming a full dump still trims off the RSA key.

lifehackerhansol commented 3 years ago

In that case, I assume we'd need to add both my check and d0k3's check. Because mine worked for dumping carts, while d0k3's worked for trimming existing dumps.

d0k3 commented 3 years ago

Here's one with both checks: GodMode9-v2.0.0-26-g33d59f6d-20210819211335.zip

I'll merge it once you guys say it's okay.

RocketRobz commented 3 years ago

Here's one with both checks: GodMode9-v2.0.0-26-g33d59f6d-20210819211335.zip

I'll merge it once you guys say it's okay.

Both dumping and trimming are fixed now!

d0k3 commented 3 years ago

Alright, merged to master. Thanks everyone for testing and your contributions!