Implement XDVD security challenge responses #1659

Ryzee119 · 2024-06-10T12:12:34Z

This PR implements the DVD security challenge/response mechanism to the best of my knowledge used on a Xbox console between the DVD drive and the Xbox.

This allows loading retail Xbox titles both in 'redump' format and xiso format with an unmodified retail BIOS.

For this to work, you need the security info from your original game disk. There is two methods:

SS.bin from a compatible 'Kreon' drive using the method on redump.org http://wiki.redump.org/index.php?title=Microsoft_Xbox_and_Xbox_360_Dumping_Guide,
dvd_layout.bin from your Xbox and original disk using these homebrew apps https://github.com/Ryzee119/OGX_CHALLENGE_TABLE/releases/tag/build-202310141001 or https://gist.github.com/Ernegien/bef054a43213c52c2bef8d94bf9f51cb

These two dumps have the same info but in a slightly different format. As I understand SS.bin represents what is physically on the DVD, dvd_layout.bin is the packet that is formatted and sent by the DVD drive firmware. This PR detects either and converts accordingly.

To provide this file to xemu, there is currently no GUI element and is done by command line

xemu.exe -dvd_security_path "SS.bin". The same arg is used for "dvd_layout.bin".

Notes from my testing:

I had pretty good success using one common SS.bin/.dvd_layout.bin dump with many games. However later bioses seemed to only accept SS.bin dumps from that specific title. This could be improved security or an issue in my implementation?
The Xbox didn't seem to care if the DVD is 'redump' or XISO style aslong as the challenges are responded to correctly. Your mileage may vary.
Using a retail BIOS comes with the restrictions of the retail system such as region restrictions, and you wont be able to run homebrew (FTP etc)

References

https://gist.github.com/Ernegien/bef054a43213c52c2bef8d94bf9f51cb (Major shout out the xbox7887 for this!)
https://wayback-api.archive.org/web/20240316195746/https://multimedia.cx/eggs/xbox-sphinx-protocol/
https://xboxdevwiki.net/DVD_Drive
https://xboxdevwiki.net/Xbox_Game_Disc

This process just follows the text book challenge response process from the Xbox. There may be some unusual aspects that are missed and I only have a smallish selection of disks to test with. I pulled in some public domain RC4 and SHA1 libraries, not sure if they should live in the xdvd folder or not.

Future work suggestions

Generate SS.bin automatically?
Add to GUI and somehow link to specific titles

Fixes #1036

mborgerson

Wow, nice work! Left some initial comments.

hw/ide/atapi.c

hw/xbox/xdvd/WjCryptLib_Rc4.c

mborgerson · 2024-06-10T21:06:27Z

hw/ide/atapi.c

+#ifdef XBOX
+    // We need to prevent small XISOs from being reported as CD-ROMs
+    // If the DVD security path is set, we can assume it is an XISO.
+    const char *dvd_security_path = g_config.sys.files.dvd_security_path;


We should probably make this a qdev property, like disc file path, media type, etc. so it can play nicely with other systems

hw/ide/atapi.c

hw/xbox/xdvd/xdvd.c

mborgerson · 2024-06-10T22:34:05Z

include/hw/ide/internal.h

@@ -457,6 +461,11 @@ struct IDEState {
    uint8_t *smart_selftest_data;
    /* AHCI */
    int ncq_queues;
+#ifdef XBOX
+    uint8_t xdvd_challenges_encrypted[XDVD_STRUCTURE_LEN];


Do any of these fields need to be saved in vm state to support snapshots?

mborgerson · 2024-06-10T22:44:31Z

hw/xbox/xdvd/xdvd.c

+
+bool xdvd_is_redump(uint64_t total_sectors)
+{
+    return total_sectors == XDVD_REDUMP_SECTOR_CNT;


Is it possible for this to misclassify if a non-redump image happens to be this same size?

Youre right it would be technically possible; although I dont think any retail xbox title in XISO format could possibly have this size but maybe some homebrew could have.

Any suggestions? I presume there is some magic bytes in the redump style image somewhere, Id need to research it

You should be able to check the volume descriptor magic fields at the appropriate offset: https://xboxdevwiki.net/XDVDFS#Volume_descriptor

hw/xbox/xdvd/xdvd.c

mborgerson · 2024-06-10T22:49:55Z

hw/xbox/xdvd/xdvd.c

+bool xdvd_get_encrypted_challenge_table(uint8_t *xdvd_challenge_table_encrypted)
+{
+    bool status = true;
+    // Check if we have already read it from the file


Where is this checked?

Removed the comment. Left over from old code

hw/ide/atapi.c

PatrickvL

A few code review remarks

PatrickvL · 2024-06-13T18:00:46Z

hw/ide/atapi.c

@@ -1206,8 +1338,12 @@ static void cmd_read_dvd_structure(IDEState *s, uint8_t* buf)
        }
    }

+     // Still some important info we don't want to lose so we disable clearing


Since this is one of the few "not defined" conditionals, it's perhaps nicer to change it into an #ifdef XBOX then the comment lines, and then an #else line before the code that has to be skipped under an Xbox build.

PatrickvL · 2024-06-13T18:02:40Z

hw/ide/atapi.c

+#endif
+
+// #define DEBUG
+#ifdef DEBUG


This could be moved inside the above #ifdef XBOX conditional block.

PatrickvL · 2024-06-13T18:06:08Z

hw/ide/atapi.c

+
+            ide_atapi_cmd_reply(s, sizeof(XBOX_DVD_SECURITY), max_len);
+#endif
+        break;


This break must reside in the above conditional block, not after it.

PatrickvL · 2024-06-13T18:10:20Z

hw/xbox/xdvd/xdvd.c

+
+bool xdvd_get_encrypted_challenge_table(uint8_t *xdvd_challenge_table_encrypted)
+{
+    bool status = true;


Either move this variable declaration down to reside along with the others below (since it's not referred to until after those), or (if this project still requires all variable declarations to reside at the start of a function) move all below variable declarations towards here.

mborgerson · 2024-06-13T18:09:55Z

hw/xbox/xdvd/xdvd.c

+        (PXBOX_DVD_CHALLENGE)(&xdvd_challenge_table_decrypted[CR_ENTRIES_OFFSET]);
+
+    for (int i = 0; i < challengeEntryCount; i++) {
+        if (challenges[i].type != 1)


We should name this constant. Supposedly 1 means the entry is valid.

nit: Braces around condition body, same with below

mborgerson · 2024-06-13T18:12:26Z

hw/xbox/xdvd/xdvd.c

+    memcpy(xdvd_security, &s, sizeof(s));
+};
+
+bool xdvd_is_redump(uint64_t total_sectors)


I'd like to avoid using the term "redump" in xemu code. Although we use the term informally to mean a complete disc backup, it really is just the name of the metadata archival project and not tied directly to the layout of the disc image. So instead of using the term redump here, let's use something like xdvd_has_video_partition.

mborgerson · 2024-06-13T18:22:37Z

hw/ide/atapi.c

+    if (max_len == XDVD_SECURITY_PAGE_LEN && buf[8] == MODE_PAGE_XBOX_SECURITY)
+    {
+        memcpy(&s->xdvd_security, buf, XDVD_SECURITY_PAGE_LEN);
+        XBOX_DPRINTF("Authenticated: %d, Partition: %d\n", s->xdvd_security.page.Authenticated, s->xdvd_security.page.Partition);


My understanding is that the drive requires the full unlock challenge-response sequence to happen before it will unlock and we are not yet fully modeling this state machine. If this is correct and you don't plan to implement this, can you leave a comment so we can remember to come back to it?

mborgerson · 2024-06-13T18:24:40Z

hw/xbox/xdvd/xdvd.c

+uint32_t xdvd_get_challenge_response(const uint8_t *xdvd_challenge_table_decrypted,
+                            uint8_t challenge_id)
+{
+    int challengeEntryCount =


nit: challenge_entry_count

mborgerson reviewed Jun 10, 2024

View reviewed changes

Ryzee119 force-pushed the dvd_pr branch from 490d12e to dee570f Compare June 11, 2024 02:22

mborgerson reviewed Jun 11, 2024

View reviewed changes

hw/ide/atapi.c Outdated Show resolved Hide resolved

mborgerson reviewed Jun 11, 2024

View reviewed changes

hw/ide/atapi.c Outdated Show resolved Hide resolved

Ryzee119 mentioned this pull request Jun 12, 2024

Move SHA1 and RC4 functions from eeprom specific file to utils #1660

Merged

Ryzee119 added 4 commits June 13, 2024 11:49

xdvd: Add command line option for dvd security file path

99dd9dd

xdvd: Implement XDVD security library

36c97d3

xdvd: Add security structs to IDE device

f039a21

atapi: Implement atapi mode selection callback

43c2173

Ryzee119 force-pushed the dvd_pr branch from dee570f to acf91dd Compare June 13, 2024 02:59

Ryzee119 added 4 commits June 13, 2024 13:20

xdvd: Implement MODE_PAGE_XBOX_SECURITY in atapi controller

c0d833f

xdvd: Handle redump and xiso style dvd images

f7fe9e2

xdvd: Force small XISOs to report as DVD roms

c68b546

xdvd: Reset security data on disc change

4fabcf1

Ryzee119 force-pushed the dvd_pr branch from acf91dd to 4fabcf1 Compare June 13, 2024 03:54

PatrickvL reviewed Jun 13, 2024

View reviewed changes

mborgerson reviewed Jun 13, 2024

View reviewed changes

mborgerson mentioned this pull request Jul 10, 2024

Audio CD Support #435

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement XDVD security challenge responses #1659

Implement XDVD security challenge responses #1659

Ryzee119 commented Jun 10, 2024 •

edited by mborgerson

Loading

mborgerson left a comment

mborgerson Jun 10, 2024 •

edited

Loading

mborgerson Jun 10, 2024

mborgerson Jun 10, 2024

Ryzee119 Jun 10, 2024

antangelo Jun 10, 2024

mborgerson Jun 10, 2024

Ryzee119 Jun 11, 2024

PatrickvL left a comment

PatrickvL Jun 13, 2024

PatrickvL Jun 13, 2024

PatrickvL Jun 13, 2024

PatrickvL Jun 13, 2024

mborgerson Jun 13, 2024

mborgerson Jun 13, 2024

mborgerson Jun 13, 2024

mborgerson Jun 13, 2024

Implement XDVD security challenge responses #1659

Are you sure you want to change the base?

Implement XDVD security challenge responses #1659

Conversation

Ryzee119 commented Jun 10, 2024 • edited by mborgerson Loading

mborgerson left a comment

Choose a reason for hiding this comment

mborgerson Jun 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PatrickvL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ryzee119 commented Jun 10, 2024 •

edited by mborgerson

Loading

mborgerson Jun 10, 2024 •

edited

Loading