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

Increase certificate flexibility #14

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

adventureisyou
Copy link
Contributor

@adventureisyou adventureisyou commented Jun 11, 2024

Hi! Thanks for the neat package. Everything works well for me, but when digging into the code I have discovered two assumptions that I think aren't valid (and would cause things to stop working if Apple changed some stuff under the hood).

  1. It is assumed that the list of certificates in the JWS header x5c field will always be of length 3.
    • I don't think there's any reason to assume this is the case; the chain could be as short or as long as they want.
    • This PR attempts to solve this issue by continuing to use the first certificate in the list as the leaf cert, but considering all others that follow as intermediate certs when calling leafCert.Verify().
  2. It is assumed that the certificate chain included in the JWS will always be signed by Apple CA-G3.

I also snuck in a few performance improvements:

  • Build the default cert pool once at creation, rather than on every verification.
  • Deduplicate the work done on parsing certs from the header.

I have run the tests locally using my own keys etc and everything still passes.

Copy link
Owner

@richzw richzw left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you for your PR

@richzw richzw merged commit f782b71 into richzw:master Jun 12, 2024
1 check passed
@richzw
Copy link
Owner

richzw commented Jun 12, 2024

@adventureisyou Thank you very much for your PR and solved the hard code cert of Apple CA-G3, this issue I've always wanted to solve but haven't solved.

My suggested usage of this is to pass in the cert pool created by https://github.com/devsisters/go-applereceipt/tree/master/applepki - Potentially we should fork that and use it?

Thank you for the good package to retrieve the certs. It is a good idea to fork and use it.

@adventureisyou adventureisyou deleted the flexible-certs branch June 13, 2024 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants