New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ncch_container: support encrypted games #4020

Merged
merged 1 commit into from Aug 8, 2018

Conversation

Projects
None yet
6 participants
@wwylele
Member

wwylele commented Jul 28, 2018

We originally wanted to have VFS first before implementing this. I now have a different idea about it, and currently agree more on the "abstract only when needed" philosophy: if we build up VFS class hierarchy before implementing real features, we might end up with a bunch of over-abstracted useless structure that we don't need for the finite problem we want to resolve, which might have been grown to simpler code if we only abstract on the need. So I want to start from implementing real thing, and start abstraction from where it needs change.

RomFSReader is introduced to represent a RomFS file that can be either encrypted or decrypted, which replaces the original IOFile + offset + size parameter passing.


The keys should be provided by user via sysdata/aes_keys.txt, as it is in the existing facilities. However, HW::AES::Init is changed to initial keys only once in during citra execution, and can be called when ever the key is needed. This change is made because the game list, which displays icon and titles of the game, also needs to decrypt the game outside the 3DS system. HW::AES doesn't have any dependency on other 3DS system component, so we can just make it a standalone module and init from the first time being used.


All known encryption methods are implemented, except for the seed crypto. Seed crypto is used in newer eshop games, which essentially uses an additional title-unique key that is stored outside the ROM, so we need to discuss where to load the key before implementing it.


Right now, decryption failure due to any reason (missing keys, unsupported seed crypto) would result in the same "Encrypted games" message in the frontend, as we had for encrypted games. I understand that this is not ideal for UX, but I don't want to interfere the ongoing frontend text improvements (#4012). Anyone has good idea about the error message?


This change is Reviewable

@cluezbot

This comment has been minimized.

cluezbot commented Jul 28, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-07-28T16:31:36Z: 383cde8...wwylele:c7935d14fa659c3c9970db9e1ed56e6ec296b7b9

// System archives and DLC don't have an extended header but have RomFS
if (ncch_header.extended_header_size) {
if (file.ReadBytes(&exheader_header, sizeof(ExHeader_Header)) !=
sizeof(ExHeader_Header))
return Loader::ResultStatus::Error;
if (is_encrypted) {
if ((exheader_header.system_info.jump_id & 0xFFFFFFFF) ==
(ncch_header.program_id & 0xFFFFFFFF)) {

This comment has been minimized.

@wwylele

wwylele Jul 28, 2018

Member

This ID check is masked to low 32-bit as a toleration to ill-formed ROM created by merging games and its updates.

This comment has been minimized.

@B3n30

B3n30 Jul 29, 2018

Contributor

maybe comment that in the code

@B3n30

I sadly ran out of power on my laptop while reviewing, so I will do a second round as soon as possible

exefs_ctr = romfs_ctr = exheader_ctr;
exheader_ctr[8] = 1;
exefs_ctr[8] = 2;
romfs_ctr[8] = 3;

This comment has been minimized.

@B3n30

B3n30 Jul 29, 2018

Contributor

Can those numbers here get a bit of more documentation? I have no clue whats happening here without digging into 3dbrew

This comment has been minimized.

@wwylele

wwylele Jul 29, 2018

Member

3dbrew doesn't really have documentation on this other than

NOTE: For a full understanding of the steps involved in decryption, consult the ctrtool and Decrypt9 source code instead..

You can also see godmode9 code that does the same work
https://github.com/d0k3/GodMode9/blob/99af6a73be48fa7872649aaa7456136da0df7938/arm9/source/game/ncch.c#L48

Honestly these are really just some magic numbers that no one can explain other than ninty, so the code itself is already the best documentation. Do you want me to cross link the godmode9 code in the comment?

// System archives and DLC don't have an extended header but have RomFS
if (ncch_header.extended_header_size) {
if (file.ReadBytes(&exheader_header, sizeof(ExHeader_Header)) !=
sizeof(ExHeader_Header))
return Loader::ResultStatus::Error;
if (is_encrypted) {
if ((exheader_header.system_info.jump_id & 0xFFFFFFFF) ==
(ncch_header.program_id & 0xFFFFFFFF)) {

This comment has been minimized.

@B3n30

B3n30 Jul 29, 2018

Contributor

maybe comment that in the code

@@ -13,6 +13,10 @@ namespace AES {
enum KeySlotID : size_t {
// AES Keyslot used to generate the UDS data frame CCMP key.

This comment has been minimized.

@B3n30

B3n30 Jul 29, 2018

Contributor

That comment should probably stay with the UDSDataKey = 0x2D, line

}
std::array<u8, 16> key_y;
std::copy(ncch_header.signature, ncch_header.signature + 16, key_y.begin());

This comment has been minimized.

@lioncash

lioncash Jul 29, 2018

Member

+ key_y.size()

is_encrypted = true;
if (ncch_header.fixed_key) {
LOG_INFO(Service_FS, "Fixed-key crypto");
std::fill(primary_key.begin(), primary_key.end(), 0);

This comment has been minimized.

@lioncash

lioncash Jul 29, 2018

Member

primary_key.fill(0); ditto for secondary_key.

exheader_ctr.begin());
exefs_ctr = romfs_ctr = exheader_ctr;
auto u32ToBEArray = [](u32 value) -> std::array<u8, 4> {
return std::array<u8, 4>{(u8)(value >> 24), (u8)((value >> 16) & 0xFF),

This comment has been minimized.

@lioncash

lioncash Jul 29, 2018

Member

static_cast

key = secondary_key;
}
CryptoPP::CTR_Mode<CryptoPP::AES>::Decryption dec(key.data(), 16, exefs_ctr.data());

This comment has been minimized.

@lioncash

lioncash Jul 29, 2018

Member

key.size() instead of a hardcoded 16

@cluezbot

This comment has been minimized.

cluezbot commented Jul 29, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-07-29T11:42:58Z: 383cde8...wwylele:12034e58af20da68e26e0ffdc870192f13ba6117

@B3n30

B3n30 approved these changes Jul 29, 2018

@windwakr

This comment has been minimized.

windwakr commented Jul 29, 2018

@Demos25
You need these keys

generator=FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
slot0x2CKeyX=FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
slot0x25KeyX=FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
slot0x18KeyX=FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
slot0x1BKeyX=FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF

Obviously, replace FF..FF with the actual keys.

@zhaowenlan1779

This comment has been minimized.

Member

zhaowenlan1779 commented Jul 31, 2018

I tested this out on latest Canary, and it can indeed load encrypted ROMs! That's a great feature!
However it seems that encrypted CIAs still cannot be installed. Are there any plans for it?
Also, the Service.FS is creating some kind of logging spams by "No crypto". Changing it to Debug might be better, and only erroring out when can't be decrypted.

InitKeys();
if (ncch_header.seed_crypto) {
// Seed crypto is unimplemented.
LOG_ERROR(Service_FS, "Unsupoorted seed crypto");

This comment has been minimized.

@zhaowenlan1779

zhaowenlan1779 Jul 31, 2018

Member

*supported

@wwylele

This comment has been minimized.

Member

wwylele commented Jul 31, 2018

@zhaowenlan1779 CIA encryption is a different layer on top of encrypted NCCH, so its decryption would go in another PR.

I will lower the log level to Debug.

@cluezbot

This comment has been minimized.

cluezbot commented Jul 31, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-07-31T10:33:37Z: 383cde8...wwylele:4f1fc0349e3ffa30a5a3ecb4f45d9676067f8696

@cluezbot

This comment has been minimized.

cluezbot commented Jul 31, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-07-31T21:15:09Z: 383cde8...wwylele:dd34cab42c6b35f730e36c94eadb640434ac80af

@wwylele

This comment has been minimized.

Member

wwylele commented Jul 31, 2018

Did some small changes. See commit message for detail

@wwylele wwylele force-pushed the wwylele:loader-crypto-new branch from dd34cab to c5a32b5 Aug 4, 2018

@cluezbot

This comment has been minimized.

cluezbot commented Aug 4, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-08-04T12:45:55Z: b926604...wwylele:c5a32b5ee8b64ff861a577fd32d7bbb40fd6f47d

@B3n30

B3n30 approved these changes Aug 7, 2018

Just that tiny clang-format change and I think this PR is a LGTM

Reviewed 11 of 17 files at r1, 2 of 4 files at r2, 4 of 4 files at r3.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @wwylele, @B3n30, @lioncash, and @zhaowenlan1779)


src/core/file_sys/ncch_container.h, line 288 at r3 (raw file):

    bool is_encrypted = false;
    std::array<u8, 16>
        primary_key{}; // for decrypting exheader, exefs header and icon/banner section

comment above in it's separate line for better clang-format

@wwylele wwylele force-pushed the wwylele:loader-crypto-new branch from c5a32b5 to d4a808c Aug 7, 2018

@cluezbot

This comment has been minimized.

cluezbot commented Aug 7, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-08-07T15:53:19Z: 1e724b0...wwylele:d4a808c885333eaacb0f7c125a9542cc81810ab6

@wwylele wwylele merged commit 84fc8ea into citra-emu:master Aug 8, 2018

2 of 3 checks passed

code-review/reviewable 1 file, 9 discussions left (B3n30, lioncash, wwylele, zhaowenlan1779)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wwylele wwylele deleted the wwylele:loader-crypto-new branch Aug 8, 2018

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