Skip to content
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

IOS/ES: Add support for V1Ticket #11240

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

noahpistilli
Copy link
Member

Adds support for the V1Ticket format within ES.

ES::TicketReader has been edited to support this format, as well as a couple ES IOCTLV's.

I have only encountered this file format within the Japanese channel Wii no Ma, where it is used in the theatre to track purchased movies. This has been tested with that channel and works perfectly.

@AdmiralCurtiss
Copy link
Contributor

@leoetlino You're probably the right person to review this?

@leoetlino leoetlino self-requested a review November 1, 2022 01:15
@leoetlino
Copy link
Member

Huh, so v1 tickets are used after all... interesting. I'm currently sick but I'll try to review this by the end of this week

@@ -135,16 +135,56 @@ ReturnCode ESDevice::GetV0TicketFromView(const u8* ticket_view, u8* ticket) cons
return IPC_SUCCESS;
}

ReturnCode ESDevice::GetV1TicketFromView(const u8* ticket_view, u8* ticket,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optional size seems unnecessary. Couldn't you just use u32*. Then just pass nullptr in the case below where ticket != nullptr.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On top of what @iwubcode said (regarding optional and nullptr), is there a reason not to move the V1 specific code to GetV0TicketFromView and maybe rename the function since V0/V1 almost look like the same?

ERROR_LOG_FMT(IOS_ES, "GetV1TicketFromView: Unimplemented -- returning -1028");
return ES_NO_TICKET;
if (ticket == nullptr)
return GetV1TicketFromView(ticket_view, ticket, ticket_size);
Copy link
Contributor

@iwubcode iwubcode Nov 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a suggestion but I wonder if the code would be clearer as two functions:

ReturnCode GetV1TicketFromViewAndExistingTicket(const u8* ticket_view, u8* ticket) const
{
  // same as current function except remove the code for ticket_size
}


ReturnCode GetV1TicketFromViewAndSize(const u8* ticket_view, u32* ticket_size) const
{
  const u64 title_id = Common::swap64(&ticket_view[offsetof(ES::TicketView, title_id)]);

  const auto installed_ticket = FindSignedTicket(title_id);
  if (!installed_ticket.IsValid())
    return ES_NO_TICKET;

   *ticket_size = installed_ticket.GetTicketSize();
    return IPC_SUCCESS;
}

then

ReturnCode ESDevice::GetTicketFromView(const u8* ticket_view, u8* ticket, u32* ticket_size) const
{
  const u8 version = ticket_view[offsetof(ES::TicketView, version)];
  if (version == 1)
  {
    if (ticket == nullptr)
      return GetV1TicketFromViewAndSize(ticket_view, ticket_size);
    return GetV1TicketFromViewAndExistingTicket(ticket_view, ticket);
  }
  ...
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way would be to make GetTicketFromView, GetV0TicketFromView and GetV1TicketFromView a single function.

AFAICT, GetTicketFromView set *ticket_size just like GetV1TicketFromView and GetV1TicketFromView behaves like GetV0TicketFromView but adds a size check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does sound like a better way to do it, will change.

Comment on lines 380 to 383
const u32 ticket_size =
Common::swap32(m_bytes.data() + sizeof(Ticket) + offsetof(V1TicketHeader, v1_ticket_size)) +
sizeof(Ticket);
return m_bytes.size() == ticket_size;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't it be simplified as return m_bytes.size() == GetTicketSize()?

@@ -135,16 +135,56 @@ ReturnCode ESDevice::GetV0TicketFromView(const u8* ticket_view, u8* ticket) cons
return IPC_SUCCESS;
}

ReturnCode ESDevice::GetV1TicketFromView(const u8* ticket_view, u8* ticket,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On top of what @iwubcode said (regarding optional and nullptr), is there a reason not to move the V1 specific code to GetV0TicketFromView and maybe rename the function since V0/V1 almost look like the same?

ERROR_LOG_FMT(IOS_ES, "GetV1TicketFromView: Unimplemented -- returning -1028");
return ES_NO_TICKET;
if (ticket == nullptr)
return GetV1TicketFromView(ticket_view, ticket, ticket_size);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way would be to make GetTicketFromView, GetV0TicketFromView and GetV1TicketFromView a single function.

AFAICT, GetTicketFromView set *ticket_size just like GetV1TicketFromView and GetV1TicketFromView behaves like GetV0TicketFromView but adds a size check.

@leoetlino
Copy link
Member

Other ES commands that we might(?) want to update to add v1 ticket support:

@noahpistilli
Copy link
Member Author

I have tested ticket imports which works correctly, however I have not tested deletion yet.

I also condensed the GetTicketFromView, GetV0TicketFromView and GetV1TicketFromView functions into one function as @sepalani and @iwubcode have suggested.

Source/Core/Core/IOS/ES/TitleManagement.cpp Outdated Show resolved Hide resolved
Source/Core/Core/IOS/ES/TitleManagement.cpp Outdated Show resolved Hide resolved
Source/Core/Core/IOS/ES/Views.cpp Outdated Show resolved Hide resolved
Source/Core/Core/IOS/ES/Views.cpp Show resolved Hide resolved
Source/Core/Core/IOS/ES/NandUtils.cpp Outdated Show resolved Hide resolved
Source/Core/Core/IOS/ES/Views.cpp Outdated Show resolved Hide resolved
Source/Core/Core/IOS/ES/Views.cpp Outdated Show resolved Hide resolved
@noahpistilli
Copy link
Member Author

If that is all would you like me to squash the commits?

@leoetlino
Copy link
Member

LGTM other than the lint issues. (And yes, please squash the commits)

@leoetlino
Copy link
Member

LGTM. Feel free to ignore the FifoCI diff (which is unrelated) and squash the commits.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants