#14 Rework the logic for looking up disc key

Merged
necklace merged 2 commits from clee/iso-lookup-rework into necklace/master 4 months ago
clee commented 4 months ago

I saw TODO: clean up this logic and, well, I took a crack at it.

Notes:

  • I did re-enable the CRC32 fallback
  • also I disabled the "look up IRD from website" fallback because the server appears to be down. I have another patch here that uses ps3.aldostools.org/ird.html instead, but I haven't reached out to them yet to ask if they would be okay with their site being used in libray.
  • I added a single-line change to core.py to fix lookup for FFXIII (game ID MRTC00003 for the US release, super weird)

Figured I'd open the PR for discussion here first and see how that goes.

I saw `TODO: clean up this logic` and, well, I took a crack at it. Notes: - I did re-enable the CRC32 fallback - also I disabled the "look up IRD from website" fallback because the server appears to be down. I have another patch here that uses ps3.aldostools.org/ird.html instead, but I haven't reached out to them yet to ask if they would be okay with their site being used in libray. - I added a single-line change to core.py to fix lookup for FFXIII (game ID `MRTC00003` for the US release, super weird) Figured I'd open the PR for discussion here first and see how that goes.

Generally I think it'd be wise to first create an issue on the TODO(s) rather than do and submit work that might get rejected. But you know, yolo, etc.

  1. You'll unfortunately have to disable or completely remove the CRC32 again. It was disabled because Python is slow and especially when reading directly from disc drive it just takes way too long. I don't think identification should take many minutes if there's a chance it'll fail. I could maybe allow some sort of logic where it'll first figure out how long it would take in theory to calculate the CRC32, and then if it takes too long it just skips it. Maybe not.

  2. I thought that the website lookup was disabled already, maybe I dreamt that. Was it just failing silently/loudly all this time? Jonnysp has been down for like years at this point.

  3. Regarding core.py; very nice find if true, but I have to verify. What's the hash on your copy of FFXIII / BLUS-30416 / MRTC00003?

Also, did you test this on all your games and it behaved the same way? We might want to write some tests.

Generally I think it'd be wise to first create an issue on the TODO(s) rather than do and submit work that might get rejected. But you know, yolo, etc. 1. You'll unfortunately have to disable or completely remove the CRC32 again. It was disabled because Python is slow and especially when reading directly from disc drive it just takes way too long. I don't think identification should take many minutes if there's a chance it'll fail. I could maybe allow some sort of logic where it'll first figure out how long it would take in theory to calculate the CRC32, and then if it takes too long it just skips it. Maybe not. 2. I thought that the website lookup was disabled already, maybe I dreamt that. Was it just failing silently/loudly all this time? Jonnysp has been down for like years at this point. 3. Regarding core.py; very nice find if true, but I have to verify. What's the hash on your copy of FFXIII / BLUS-30416 / MRTC00003? Also, did you test this on all your games and it behaved the same way? We might want to write some tests.
gmipf commented 4 months ago

I'm not sure if aldostools ird database is the most up-to-date. But the following one gets still updates since it is connected to ManaGunz ird creation and upload feature: http://ps3ird.free.fr/

I'm not sure if aldostools ird database is the most up-to-date. But the following one gets still updates since it is connected to ManaGunz ird creation and upload feature: http://ps3ird.free.fr/
clee commented 4 months ago
Poster

@necklace Hey, thanks for the feedback! (And yes, I probably should have started by asking, but the workflow here went: try to decrypt my PS3 games, have issues, fix issues locally, then way later, hey maybe I should try to clean up and share some of these changes)

  1. About CRC32: it takes just under 6 seconds here to calculate the CRC32 for my copy of MGS4, which is over 30GB. But that's with an NVMe SSD and a pretty fast CPU. I completely forgot that some folks might use libray directly with their disc drives. Perhaps starting a 10-second timer thread and killing the CRC32 calculation if it takes longer than 10 seconds? Or some other value than 10, just thinking out loud.
  2. Ha, I think you might have done that one in a dream. The code was trying and failing to download IRDs from jonnysp for me when it couldn't find a match in the keys.db.
  3. md5 'Final Fantasy XIII.enc.iso' reports e15938be490a8884134094f53806a2b8 and crc32 is bf883f4e, both which match the keys.db data, but it didn't even have to fall back to CRC32 for me once I added that change to core.py.

Also, did you test this on all your games and it behaved the same way? We might want to write some tests.

The only real change is that some of my ISOs (MGS4, Legend of Spyro, Splinter Cell Trilogy, Ratchet & Clank Future - Tools of Destruction, Killzone 3…) fail to automatically decrypt without the CRC32 fallback lookup, and FFXIII specifically crashes because of ValueError('Unknown country?!') without the core.py change.

Tests sound like a great idea! Doing it without the tests having access to all the encrypted ISOs might be a bit challenging.

@necklace Hey, thanks for the feedback! (And yes, I probably should have started by asking, but the workflow here went: try to decrypt my PS3 games, have issues, fix issues locally, then way later, _hey maybe I should try to clean up and share some of these changes_) 1. About CRC32: it takes just under 6 seconds here to calculate the CRC32 for my copy of MGS4, which is over 30GB. But that's with an NVMe SSD and a pretty fast CPU. I completely forgot that some folks might use libray directly with their disc drives. Perhaps starting a 10-second timer thread and killing the CRC32 calculation if it takes longer than 10 seconds? Or some other value than 10, just thinking out loud. 2. Ha, I think you might have done that one in a dream. The code was trying and failing to download IRDs from jonnysp for me when it couldn't find a match in the keys.db. 3. `md5 'Final Fantasy XIII.enc.iso'` reports `e15938be490a8884134094f53806a2b8` and `crc32` is `bf883f4e`, both which match the keys.db data, but it didn't even have to fall back to CRC32 for me once I added that change to core.py. > Also, did you test this on all your games and it behaved the same way? We might want to write some tests. The only real change is that some of my ISOs (MGS4, Legend of Spyro, Splinter Cell Trilogy, Ratchet & Clank Future - Tools of Destruction, Killzone 3…) fail to automatically decrypt without the CRC32 fallback lookup, and FFXIII specifically crashes because of `ValueError('Unknown country?!')` without the core.py change. Tests sound like a great idea! Doing it without the tests having access to all the encrypted ISOs might be a bit challenging.

Yo @gmipf, long time no see.

@clee

  1. Damn your system is fast. It takes me 3 minutes to calculate MGS4 (BLES00246), though that is from an older storage HDD and on an older CPU. It's bad, but it would have been okay if it had a 100% success rate, but as of now it does not. That's why I don't think is acceptable from a user perspective, I'd rather they report it. I'm sure there are people with worse hardware than me as well. Maybe we could settle on it asking if the user wishes to try calculating crc32, though that might break some scripts I know people have written to go decrypt their entire library automatically. Maybe it should be a flag like "--crc32" or "--force", then.

  2. So, loudly failing, heh.

    The real solution to both 1. and 2. in my opinion is to just gather as many keys and .irds and put them in a new database (which maybe also can be hosted here or some other place and automatically downloaded by libray if it's outdated). The redump keys for example are not linked to the title_ids of the games, which is why that logic existed in the first place. In contrast, the irds are linked to title_id, so for the games you mention there the identification could be as simple as checking title_id + size in a database with .ird data. When I say .irds I don't mean the full .ird as they are somewhat large, only the relevant data and the keys. This has been my goal for 1.0.0 for a while now, I just haven't had any time.

    I don't think that is something that should be done right now in this PR, though.

  3. Very interesting, perhaps you've found an instance where someone manually were supposed to type "BLUS" but they typed "BLTS"? A typo since the T-key neighbors the U-key? Should definitely be added.

It is a little weird that MGS4 needs a CRC32, it looks very identifiable with name + (country) + (version) + size in the database. I guess that isn't implemented though, should definitely implement that as well, at least until 1.0.0.

For tests we could for now write some manually with all the cases we know about.

Yo @gmipf, long time no see. @clee 1. Damn your system is fast. It takes me 3 minutes to calculate MGS4 (BLES00246), though that is from an older storage HDD and on an older CPU. It's bad, but it would have been okay if it had a 100% success rate, but as of now it does not. That's why I don't think is acceptable from a user perspective, I'd rather they report it. I'm sure there are people with worse hardware than me as well. Maybe we could settle on it asking if the user wishes to try calculating crc32, though that might break some scripts I know people have written to go decrypt their entire library automatically. Maybe it should be a flag like "--crc32" or "--force", then. 2. So, loudly failing, heh. The real solution to both 1. and 2. in my opinion is to just gather as many keys and .irds and put them in a new database (which maybe also can be hosted here or some other place and automatically downloaded by libray if it's outdated). The redump keys for example are not linked to the title_ids of the games, which is why that logic existed in the first place. In contrast, the irds are linked to title_id, so for the games you mention there the identification could be as simple as checking title_id + size in a database with .ird data. When I say .irds I don't mean the full .ird as they are somewhat large, only the relevant data and the keys. This has been my goal for 1.0.0 for a while now, I just haven't had any time. I don't think that is something that should be done right now in this PR, though. 3. Very interesting, perhaps you've found an instance where someone manually were supposed to type "BLUS" but they typed "BLTS"? A typo since the T-key neighbors the U-key? Should definitely be added. It is a little weird that MGS4 needs a CRC32, it looks very identifiable with name + (country) + (version) + size in the database. I guess that isn't implemented though, should definitely implement that as well, at least until 1.0.0. For tests we could for now write some manually with all the cases we know about.
clee commented 4 months ago
Poster

Very interesting, perhaps you've found an instance where someone manually were supposed to type "BLUS" but they typed "BLTS"? A typo since the T-key neighbors the U-key? Should definitely be added.

I just assumed the title[2] was coming from the MRTC title ID, since libray -v reports this while decrypting FFXIII:

Game ID: MRTC-00003
Key: <redacted>
Info from ISO:
Unencrypted regions: 4

Related, it looks like FFXIII was not the only game with an MRTC-prefix title ID. Others include Star Ocean Last Hope international, Lost Planet 2, and Cabela's Dangerous Hunts 2011 (lol, what).

> Very interesting, perhaps you've found an instance where someone manually were supposed to type "BLUS" but they typed "BLTS"? A typo since the T-key neighbors the U-key? Should definitely be added. I just assumed the `title[2]` was coming from the `MRTC` title ID, since `libray -v` reports this while decrypting FFXIII: ``` Game ID: MRTC-00003 Key: <redacted> Info from ISO: Unencrypted regions: 4 ``` Related, it looks like FFXIII was not the only game with an MRTC-prefix title ID. Others include Star Ocean Last Hope international, Lost Planet 2, and Cabela's Dangerous Hunts 2011 (lol, _what_).
clee commented 4 months ago
Poster

Okay, updated the code. New arguments in libray --help:

  -c, --checksum        Allow fallback to CRC32 checksum (disabled by default)
  -t CHECKSUM_TIMEOUT, --checksum-timeout CHECKSUM_TIMEOUT
                        How many seconds to wait for CRC32 checksum (default 15)

Does that look better? Happy to change names to --crc32/--crc32-timeout or whatever if you feel strongly about it.

If this looks decent enough, I'll get started on some tests next.

Okay, updated the code. New arguments in `libray --help`: ``` -c, --checksum Allow fallback to CRC32 checksum (disabled by default) -t CHECKSUM_TIMEOUT, --checksum-timeout CHECKSUM_TIMEOUT How many seconds to wait for CRC32 checksum (default 15) ``` Does that look better? Happy to change names to `--crc32`/`--crc32-timeout` or whatever if you feel strongly about it. If this looks decent enough, I'll get started on some tests next.

I just assumed the title[2] was coming from the MRTC title ID, since libray -v reports this while decrypting FFXIII:

Ah! Well in that case it was a legitimate bug, I don't think I have any MRTC games. That part of the code was just based on the region codes found here: https://www.psdevwiki.com/ps3/Template:TITLE_ID_for_Physical_Media which do not mention MRTC at all

Either way, looks great to me! I'd be happy to merge, you don't have to write tests if you don't want to. I don't think there are any working tests now anyway lol.

> I just assumed the title[2] was coming from the MRTC title ID, since libray -v reports this while decrypting FFXIII: Ah! Well in that case it was a legitimate bug, I don't think I have any MRTC games. That part of the code was just based on the region codes found here: https://www.psdevwiki.com/ps3/Template:TITLE_ID_for_Physical_Media which do not mention MRTC at all Either way, looks great to me! I'd be happy to merge, you don't have to write tests if you don't want to. I don't think there are any working tests now anyway lol.
clee commented 4 months ago
Poster

Tests added. I had to learn some new tricks to get these working, but I feel pretty confident that they're testing the right code paths at this point.

Let me know if you have any questions or if there's anything I can clean up before you merge!

Tests added. I had to learn some new tricks to get these working, but I feel pretty confident that they're testing the right code paths at this point. Let me know if you have any questions or if there's anything I can clean up before you merge!

Amazing!

Thank you very much. Merging.

Amazing! Thank you very much. Merging.
Nichlas Severinsen referenced this issue from a commit 1 month ago
This pull request has been merged successfully!
Sign in to join this conversation.
No Milestone
No assignee
3 Participants
Loading...
Cancel
Save
There is no content yet.