Notarization through the Notary API #593

roblabla · 2022-06-12T12:01:32Z

This MR adds notarization through the Notary API.

Fixes #591

Currently, this is only implemented for code bundle signing. I need to implement pkg and dmg signing next, and do a lot of code cleanup.

This API is quite a bit faster than the old method. My code gets notarized in around 20 seconds with this API, whereas it took around a minute with the old Transporter API.

Note that I'm also having a very messy dependency hell issue. PyOxidizer depends on pgp, that fell in the pitfall of using non-standard dependency version requirements on zeroize... This means that it is currently impossible to use pgp and aws-sdk-s3 in the same dependency graph. I have reported the problem to pgp, and have a patch to fix the issue.

Hopefully pgp will release a new version to fix this issue.

indygreg · 2022-06-12T21:42:02Z

Thank you for submitting this!

Regarding the dependency hell issue, yes, this is something I've run into as well. pgp is using some old crate versions and is holding back upgrading a lot of dependencies in apple-codesign as well as other crates in this repo. It is super annoying. rpgp/rpgp#165 should hopefully fix most issues. Although I wasn't aware of that non-standard pinning of the zeroize crate. Maybe your PR will be sufficient to unlock progress, as the zeroize crate is the one that breaks most frequently when I try to modernize crypto dependencies in this repo.

indygreg

Thank you so much for this PR! Using the new notarization API and deleting the old transporter code should definitely be the marquee feature for the next release of this create and I can't wait to ship pure Rust notarization!

The approach so far in this PR seems very reasonable to me. I don't see any roadblocks other than the pgp crate patching. Regarding the pgp crate, ideally we'd wait for them to publish a new version. But I'm not opposed to shipping a version with a patched pgp crate as a temporary workaround. At the very least I'd like to see that crate accept a PR with a fix, as I would prefer to pin to an unreleased commit in the crate's canonical repo. I suppose we'll wait and see what happens with your PR.

Since this is marked as a work in progress, I guess ping me when you think this is ready for a real review. If you don't want to push this PR over the finish line, that's fine by me too: I can finish it the next time I sit down to hack on this project.

Thank you again for your work on this feature!

apple-codesign/src/notarization.rs

roblabla · 2022-06-13T22:49:20Z

Alright so I should have gotten PKG and DMG in. I still need to actually test it works (only bundle was tested for now), and assuming it all works, make the commit history a bit nicer :^).

Another thing I'd like to improve is getting richer error information by extracting it from the submission logs. There's a lot of really useful information in there, but right now it's printed as an ugly json blob. I want to add some method to pretty print it.

indygreg · 2022-07-01T01:43:06Z

Looks like your zeroize PR merged in the rpgp repo! Hopefully that means a release is near. If not, I'm not opposed to throwing up a fork on crates.io as a workaround: I'm tired of working around dependency hell resulting from that crate.

roblabla · 2022-07-01T09:15:05Z

rpgp 0.8 just got released 🎉

indygreg · 2022-07-07T02:32:31Z

I was having my own go at upgrading dependencies and it looks like yubikey depending on rsa 0.5 is going to be a hurdle. I filed iqlusioninc/yubikey.rs#393 to request a release.

roblabla · 2022-07-07T09:06:08Z

Yeah, I ran into yubikey-rs problems, and also rpm-rs problems. See https://github.com/roblabla/rpm-rs/tree/update-packages

This replaces the notarization codepath to use the new REST API made available by Apple in WWDC22. This API is entirely documented, and meant for direct user consumption[0]. As a result, this commit also removes the codepath using the Transporter API for notarization, and thus the requirement for the iTMSTransporter tool to be present on the machine for notarization. Note that the API has some undocumented bits that were discovered through trial and error. One such bit is that the S3 token that apple returns to upload files are valid for the us-west-2 region. [0]: https://developer.apple.com/documentation/notaryapi/submitting_software_for_notarization_over_the_web

indygreg · 2022-08-06T00:46:20Z

I haven't forgotten about this PR. I wanted to get the dependency upgrades merged first because they are a pain to update. As I was doing that, I discovered a dependency hell due to the yubikey-rs crate and wanted to get that resolved. It is kind of in limbo at the moment. I'll try to find time to take a look at this over the weekend.

indygreg

I incrementally applied this PR locally, crafting a few separate commits from your work. I retained your authorship of those commits.

I need to spend some time testing this and updating docs to reflect the new state. But at this point the code is pretty much locked in and will be pushed, hopefully some time this weekend. The PR should automatically close when I do so.

I sincerely thank you for this contribution. Pure Rust notarization was the last major missing feature for this crate and I'm excited it is finally implemented! I'll try to push out a new release with this major feature soon as well. (But there are some other known bugs with the crate that I may want to try to fix first.)

indygreg · 2022-08-06T04:25:59Z

apple-codesign/src/app_store_connect.rs

+#[derive(Clone, Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct Issue {
+    pub architecture: String,
+    pub code: Option<u64>,
+    pub doc_url: Option<String>,
+    pub message: String,
+    pub path: String,
+    pub severity: String,
+}
+
+#[derive(Clone, Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct TicketContent {
+    pub arch: String,
+    pub cdhash: String,
+    pub digest_algorithm: String,
+    pub path: String,
+}
+
+#[derive(Clone, Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct NotarizationLogs {
+    pub archive_filename: String,
+    #[serde(default)]
+    pub issues: Vec<Issue>,
+    pub job_id: String,
+    pub log_format_version: u64,
+    pub sha256: String,
+    pub status: SubmissionResponseStatus,
+    pub status_code: u64,
+    pub status_summary: String,
+    #[serde(default)]
+    pub ticket_contents: Vec<TicketContent>,
+    pub upload_date: String,
+}


Out of curiosity, how did you derive these types? AFAICT they are undocumented.

I just looked at what data the endpoint was returning.

Also another thing: I found out by trial and error that the correct S3 region to upload the data to is us-west-2, but this is not documented anywhere... I'm not sure how apple expects us to figure this out.

This defines the types and HTTP APIs for interacting with Apple's Notary API. Commit derived from #593.

indygreg · 2022-08-06T23:56:35Z

As you can see, I pushed this. I also did a bit of refactoring after the push, mostly to clean up the UI around notarization.

The one part of your PR I did not preserve is the structs for parsing the log JSON. We don't need that yet, as we just print the JSON instead.

I did add your GitHub username and reported full name from your GitHub profile to the changelog. And I plan to say something similar in an upcoming blog post about the pending 0.17 release. If you would like me to withhold your username or full name or identify you some other way, just let me know and I'll respect your wishes. I'll likely publish 0.17 on Sunday and the blog post on Monday morning California time.

Thanks again for this amazing contribution!

indygreg reviewed Jun 12, 2022

View reviewed changes

apple-codesign/src/notarization.rs Outdated Show resolved Hide resolved

apple-codesign/src/notarization.rs Show resolved Hide resolved

roblabla force-pushed the notarization-api branch 2 times, most recently from 3ade94c to 30252ca Compare July 5, 2022 12:02

Update pgp to 0.8

0a3e218

roblabla force-pushed the notarization-api branch from 30252ca to ae0f75a Compare August 5, 2022 13:22

roblabla force-pushed the notarization-api branch from ae0f75a to 51d557b Compare August 5, 2022 13:32

indygreg approved these changes Aug 6, 2022

View reviewed changes

This was referenced Aug 6, 2022

apple-codesign: notarize: Option to pass -u / -p to iTMSTransporter instead of key parameters #535

Closed

avoid requiring transporter for notarization #548

Closed

indygreg pushed a commit that referenced this pull request Aug 6, 2022

apple-codesign: implement Notary API client

adeac27

This defines the types and HTTP APIs for interacting with Apple's Notary API. Commit derived from #593.

indygreg closed this in 3e4944f Aug 6, 2022

indygreg mentioned this pull request Aug 6, 2022

apple-codesign: add support for component notarization #576

Closed

roblabla deleted the notarization-api branch August 7, 2022 10:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Notarization through the Notary API #593

Notarization through the Notary API #593

roblabla commented Jun 12, 2022

indygreg commented Jun 12, 2022

indygreg left a comment

roblabla commented Jun 13, 2022

indygreg commented Jul 1, 2022

roblabla commented Jul 1, 2022

indygreg commented Jul 7, 2022

roblabla commented Jul 7, 2022

indygreg commented Aug 6, 2022

indygreg left a comment

indygreg Aug 6, 2022

roblabla Aug 6, 2022

roblabla Aug 6, 2022

indygreg commented Aug 6, 2022

Notarization through the Notary API #593

Notarization through the Notary API #593

Conversation

roblabla commented Jun 12, 2022

indygreg commented Jun 12, 2022

indygreg left a comment

Choose a reason for hiding this comment

roblabla commented Jun 13, 2022

indygreg commented Jul 1, 2022

roblabla commented Jul 1, 2022

indygreg commented Jul 7, 2022

roblabla commented Jul 7, 2022

indygreg commented Aug 6, 2022

indygreg left a comment

Choose a reason for hiding this comment

indygreg Aug 6, 2022

Choose a reason for hiding this comment

roblabla Aug 6, 2022

Choose a reason for hiding this comment

roblabla Aug 6, 2022

Choose a reason for hiding this comment

indygreg commented Aug 6, 2022