Add notarization support #42

MichalMMac · 2019-06-06T12:37:11Z

This PR adds the ability to notarize signed installer packages. Implementation is inspired by Apple document Customizing the Notarization Workflow.

Code style is currently the same as in #39 after applying formatters. If you don't like the #39 changes I can format the code in this PR to match the original code style.

Possible improvements:

Better English description in README.md since I am not a native speaker.
Imporoved handling of the time delay between xcrun altool and xcrun stapler. Current approach just runs the stapler on the package again and again (for limited amount of tries) until it succedes.

gregneagle · 2019-06-06T16:23:42Z

This is really interesting.

Since, if I merge this, people will expect me to support it and fix it when it breaks, I'll need to take a long hard look at it and make sure I really understand it. I have not yet had any need to investigate notarization (and frankly, so far have and almost no internal need for signing) so this is not something I've paid a lot of attention to.

erikng · 2019-06-06T18:57:40Z

Given that every package built after June 1st requires signing/notarization, I think it would be worth the effort to merge this.

gregneagle · 2019-06-06T19:06:25Z

"Given that every package built after June 1st requires signing/notarization" I have not found that to be the case...

README.md

poundbangbash · 2019-06-06T19:10:28Z

Notarization is required for Apps, installers, and KEXTs, documented here for Catalina: https://developer.apple.com/news/?id=06032019i <https://developer.apple.com/news/?id=06032019i> Notarization Requirement for Mac Software June 3, 2019 Mac apps, installer packages, and kernel extensions that are signed with Developer ID must also be notarized by Apple in order to run on macOS Catalina. This will help give users more confidence that the software they download and run, no matter where they get it from, is not malware by showing a more streamlined Gatekeeper interface. Notarization for 10.14.5 is for new DevIDs and any updated KEXT: https://developer.apple.com/news/?id=04102019a <https://developer.apple.com/news/?id=04102019a> New Notarization Requirements April 10, 2019 We're working with developers to create a safer Mac user experience through a process where all software, whether distributed on the App Store or outside of it, is signed or notarized by Apple. With the public release of macOS 10.14.5, we require that all developers creating a Developer ID certificate for the first time notarize their apps, and that all new and updated kernel extensions be notarized as well. This will help give users more confidence that the software they download and run, no matter where they get it from, is not malware by showing a more streamlined Gatekeeper interface.

…

On Jun 6, 2019, at 1:57 PM, Erik Gomez ***@***.***> wrote: Given that every package built after June 1st requires signing, I think it would be worth the effort to merge this. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#42?email_source=notifications&email_token=AAKZYYD35N7KVO34MEIC5RDPZFM2LA5CNFSM4HVBRHIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXD2N2Y#issuecomment-499623659>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKZYYDZOFFM3TH242VPCZDPZFM2LANCNFSM4HVBRHIA>.

gregneagle · 2019-06-06T19:11:43Z

And yet Munki's Managed Software Center, which is neither signed nor notarized, runs just fine in Catalina.

README.md

homebysix · 2019-06-06T19:17:59Z

Mac apps, installer packages, and kernel extensions that are signed with Developer ID must also be notarized by Apple in order to run on macOS Catalina.

I think what that means is unsigned/non-notarized is still OK, but if you're distributing signed pkgs they also need to be notarized to work with Catalina. Will probably affect a small but not insignificant portion of MunkiPkg users.

gregneagle · 2019-06-06T20:07:21Z

"I think what that means" is most of the problem here.

gregneagle · 2019-06-06T20:21:56Z

I created a signed (but not notarized) pkg containing a signed (but not notarized) MSC.app and installed on a machine running the Catalina beta. I was not prompted or blocked or warned or challenged in any way when installing the pkg, either via /usr/sbin/installer or manually double-clicking it, and I also could launch MSC.app without any harrassment.

homebysix · 2019-06-06T20:45:59Z

We should ask somebody at Apple what the word "must" means in the above context. Maybe enforcement hasn't been turned on yet (similar to the path UAMDM and kext whitelisting followed in early versions of High Sierra).

erikng · 2019-06-06T22:36:23Z

I believe starting in beta 2 we may see the enforcement. Thanks, Erik Gomez

________________________________ From: Greg Neagle <notifications@github.com> Sent: Thursday, June 6, 2019 4:57:17 PM To: munki/munki-pkg Cc: Erik Gomez; Comment Subject: Re: [munki/munki-pkg] Add notarization support (#42) @gregneagle commented on this pull request.

________________________________ In README.md<#42 (comment)>:

+

+The only required keys/values in the notarization dictionary are `username` and `password`: + +- `username` is the login email adress of your developer Apple ID. +- `password` is the 2FA app specific password. For information about the password and saving +it to the login keychain see the web page [Customizing the Notarization Workflow](https://developer.apple.com/documentation/security/notarizing_your_app_before_distribution/customizing_the_notarization_workflow). + + +Notarization process is described on Apple web page [Customizing the Notarization Workflow](https://developer.apple.com/documentation/security/notarizing_your_app_before_distribution/customizing_the_notarization_workflow). + +`munki-pkg` basically runs two commands: + +```shell +xcrun altool --notarize-app --primary-bundle-id "com.github.munki.pkg.munki-kickstart" --username "john.appleseed@apple.com" --password "@keychain:AC_PASSWORD" --file munki_kickstart.pkg +xcrun stapler staple munki_kickstart.pkg +``` Error message is incorrect, the flag needed is --asc-provider. The value you need to provide is not immediately obvious and I'll post about that in a bit. — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#42?email_source=notifications&email_token=ABLL6GESO6BRNIBBXB3QYQ3PZGB33A5CNFSM4HVBRHIKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB23GX7Y#discussion_r291387835>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABLL6GA7JN7NM2NMNNKR7QLPZGB33ANCNFSM4HVBRHIA>.

gregneagle · 2019-06-06T23:23:38Z

munkipkg

+        # notarize the pkg
+        if "notarization" in build_info and not options.skip_notarization:
+            try:
+                notarize(build_info, options)


In my (brief) testing I'm seeing it take far longer than 120 seconds for stapling to become available, and of course if there are errors in the notarization process it will never become available. Perhaps we should be running xcrun altool --notarization-info {output["notarization-upload"]["RequestUUID"]} periodically waiting for a status .

On status: success proceed to staple, on status: in progress, sleep and try again, on status: invalid (or other error status, print the error (better still, retrieve the log from the LogFileURL) and exit...

I'm not suggesting we get all this in before I'll merge this -- but I think it's needed for a good implementation.

I can improve this before the merge. I admit current approach is a bit naive. Even mentioned this problem is PR comment.

Check using altool --notarization-info now implemented in 9a6e0d0. Default timeout increased to 300 seconds.

Not sure about the retrieving the log from LogFileURL though. Currently we display the Status, Status Code, Status Message and LogFileURL after a failed notarization attempt. Log can be a very long json. We could download it and put someplace in the munki-pkg project directory (Perhaps create log directory?).

munkipkg

MichalMMac · 2019-06-13T15:10:17Z

Added new key primary_bundle_id for situations where altool --primary-bundle-id must be different from package identifier. See 2eb8273

MichalMMac · 2019-06-14T14:27:03Z

Added big commit 9a6e0d0.

After successful upload of the signed package to the Apple notary service munki-pkg now uses to altool --notarization-info to check for notarization result. No more blind stapling attempts.

Other changes:

Increase STAPLE_TIMEOUT to 300 seconds
Start using incremental time delay between notary process check
Change wording of some docstrings

There are a lot of fixup commits. When all threads with references to them are resolved I'll squash them using rebase.

gregneagle · 2019-06-16T15:41:18Z

README.md

+| username          | String  | Yes      | Login email adress of your developer Apple ID |
+| password          | String  | Yes      | 2FA app specific password. For information about the password and saving it to the login keychain see the web page [Customizing the Notarization Workflow](https://developer.apple.com/documentation/security/notarizing_your_app_before_distribution/customizing_the_notarization_workflow) |
+| asc_provider      | String  | No       | Only needed when a user account is associated with multiple providers |
+| primary_bundle_id | String  | No       | Defaults to `identifier`. `primary_bundle_id` is usefull when `identifier` contains characters such as '_' Apple notary service does not like |


I think providing this option is fine, but since we know that Apple's notary service does not like underscores in the primary bundle id, if this option is not provided and you/we "auto-generate" the primary bundle id from the pkg id, why don't we automatically substitute hyphens for underscores?

Also: s/usefull/useful/

Is basic string.replace() like this 81fec0e ok?

See my comment on that commit.

README.md

gregneagle · 2019-06-16T15:53:16Z

munkipkg

+
+        if notarization_done(state, sleep_time, options):
+            break
+


Looks like it will exit "normally" after the staple_timeout -- in other words, when wait_for_notarization returns notarization might have been successful, or we might have just timed out. So in lines 880 and 881 we might proceed to staple even if the notarization is not complete.

I think it would be better to recognize the timeout and 1) Not try to staple, and 2) Print out information that would allow the admin to manually staple later if/when they get notification of a successful notarization.

I agree. Definitely bug I created.

See the fixup 5af2ae3.

btw I do nothing with exit codes. munkipkg exits with 0 when notaratization fails or times out. I was thinking primary job of munkipkg is to build packages. If package is built correctly process should exit with code 0 even when notarization failed. Do you agree?

Not sure I do. Primary job might be to build pkgs, but if you give it extra jobs it should tell if if/when it fails.

See 1b13928.

In case of problem or failed notarization we return with -1 from build() and it gets passed to sys.exit().
When we give up on waiting for notarization we still exit with 0.

sts · 2020-08-18T08:33:00Z

Error handling might still need some improvement, at least when the account is not enabled for notarization the following exception is raised:

Traceback (most recent call last):
  File "./munkipkg", line 1311, in <module>
    main()
  File "./munkipkg", line 1307, in main
    result = build(arguments[0], options)
  File "./munkipkg", line 929, in build
    request_uuid = upload_to_notary(build_info, options)
  File "./munkipkg", line 729, in upload_to_notary
    if proc_stdout.startswith('Generated JWT'):
TypeError: startswith first arg must be bytes or a tuple of bytes, not str

In this case proc_stdout is set to the following xml string:

b'<?xml version="1.0" encoding="UTF-8"?>\n<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">\n<plist version="1.0">\n<dict>\n\t<key>os-version</key>\n\t<string>10.15.5</string>\n\t<key>product-errors</key>\n\t<array>\n\t\t<dict>\n\t\t\t<key>code</key>\n\t\t\t<integer>1048</integer>\n\t\t\t<key>message</key>\n\t\t\t<string>You must first sign the relevant contracts online. (1048)</string>\n\t\t\t<key>userInfo</key>\n\t\t\t<dict>\n\t\t\t\t<key>NSLocalizedDescription</key>\n\t\t\t\t<string>You must first sign the relevant contracts online. (1048)</string>\n\t\t\t\t<key>NSLocalizedFailureReason</key>\n\t\t\t\t<string>You must first sign the relevant contracts online. (1048)</string>\n\t\t\t\t<key>NSLocalizedRecoverySuggestion</key>\n\t\t\t\t<string>You must first sign the relevant contracts online. (1048)</string>\n\t\t\t</dict>\n\t\t</dict>\n\t</array>\n\t<key>tool-path</key>\n\t<string>/Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/Frameworks/AppStoreService.framework</string>\n\t<key>tool-version</key>\n\t<string>4.01.1182</string>\n</dict>\n</plist>\n\n'

MichalMMac · 2020-08-18T13:48:09Z

@sts I suspect you run munkipkg with Python 3. Although munkipkg should be mostly compatible with Python 3 shebang still point to macOS default 2.7 Python.

Described problem is not caused by (lack of) error handling. Problem is only with Python 3 compatibility I did not test.

I've added new commit 72d30e2 to deal with this. Please test and report issues if you find any

- Check result of the notary process with `altool --notarization-info` - Increase STAPLE_TIMEOUT to 300 seconds - Start using incremental time delay between notary process check - Update README.md to reflect the changes - Change wording of some docstrings

MichalMMac · 2021-05-28T11:54:14Z

Rebased against latest code. Updated version changing commit.

gregneagle · 2021-05-28T15:28:09Z

You've done a lot of work on this and you've kept it rebased so it can be merged and it's clear that at least some people want this functionality. I'm going to merge it. My one hesitation is that because I personally won't be using the functionality and I didn't write the functionality, if there are issues reported against the notarization functionality, I cannot promise to fix those issues in a timely manner, and I hope you will be able to "stick around" and maintain it.

MichalMMac · 2021-05-28T19:15:49Z

Sure. I use notarization quite often so I guess I will keep using this as long as I do Mac Admin stuff :-)

rodchristiansen · 2021-05-29T00:05:07Z

Nice to see this merged into main.
Been using this notarization pull request since inception.
Works great!

gregneagle · 2021-06-08T03:15:32Z

…and now altool is deprecated in macOS Monterey…

erikng · 2021-06-08T03:46:24Z

I must admit I chuckled when I saw that earlier. I immediately thought about this PR.

flammable · 2021-06-08T04:10:55Z

Ah nuts, I was excited to use this. I hope there's an alternative way to do this on Monterey.

rodchristiansen · 2021-06-08T04:12:50Z

New notarytool replaces it: https://developer.apple.com/documentation/security/notarizing_macos_software_before_distribution/customizing_the_notarization_workflow

MichalMMac · 2021-06-08T06:06:30Z

I wake up in the morning and go check the Twitter. I Learn altool is deprecated and immediately go to this thread. 4 people beat me to it 😁

@gregneagle Don't worry. I'll look into it

I haven't downloaded the beta yet. Can anyone confirm altool is still present and working in Xcode 13? (I presume it is. It even got new features in Xcode 12.5)

I'll take a look at notarytool this week. I hope it has feature parity with altool. If it does it should not be that hard to replace it within munki-pkg. There might be even possible to do some changes like using new --wait instead of current polling implementation.

MichalMMac · 2021-06-08T16:34:30Z

I've created a new issue #53 where I shall discuss the altool deprecation.

gregneagle reviewed Jun 6, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

gregneagle reviewed Jun 6, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

gregneagle reviewed Jun 6, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

gregneagle reviewed Jun 6, 2019

View reviewed changes

MichalMMac mentioned this pull request Jun 7, 2019

Apply black and isort formatteres #39

Closed

MichalMMac force-pushed the notarization branch from 5436801 to 63111cd Compare June 10, 2019 08:04

gregneagle reviewed Jun 10, 2019

View reviewed changes

munkipkg Outdated Show resolved Hide resolved

gregneagle reviewed Jun 10, 2019

View reviewed changes

munkipkg Outdated Show resolved Hide resolved

gregneagle reviewed Jun 10, 2019

View reviewed changes

munkipkg Outdated Show resolved Hide resolved

gregneagle reviewed Jun 10, 2019

View reviewed changes

munkipkg Outdated Show resolved Hide resolved

gregneagle reviewed Jun 10, 2019

View reviewed changes

munkipkg Outdated Show resolved Hide resolved

MichalMMac force-pushed the notarization branch 2 times, most recently from cbf1c3f to ee6520c Compare June 13, 2019 14:43

MichalMMac force-pushed the notarization branch from c5917ff to 9eaeab4 Compare June 14, 2019 14:58

gregneagle reviewed Jun 16, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

gregneagle reviewed Jun 16, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

gregneagle reviewed Jun 16, 2019

View reviewed changes

MichalMMac added 16 commits May 28, 2021 13:52

Add readPlistFromString() function

0873196

Add run_subprocess() function

a3f1e3b

Allow passing toolname to display()

03c5fe6

Add notarization support

937a148

Add --skip-notarization and --skip-stapling options

bbd0508

Bump version to 0.8

ab1f06f

Add support for asc_provider parameter

5ef6245

Enhance error handling of xcrun altool

dc15590

Improve dictionary key retrieval

127c0f0

Add support for custom notarization primary-bundle-id

87aec47

Enable automatic underscore replace for primary-bundle-id

9f9ed72

Add non zero exit for failed notarization

41f2158

Handle unexpected response in get_notarization_state()

826c9e8

Add App Store Connect API key support

83381db

Make notarization code Python3 compatible

c016cc1

MichalMMac force-pushed the notarization branch from 72d30e2 to c016cc1 Compare May 28, 2021 11:53

gregneagle merged commit 71d2708 into munki:main May 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add notarization support #42

Add notarization support #42

MichalMMac commented Jun 6, 2019 •

edited

Loading

gregneagle commented Jun 6, 2019

erikng commented Jun 6, 2019 •

edited

Loading

gregneagle commented Jun 6, 2019

poundbangbash commented Jun 6, 2019 via email

gregneagle commented Jun 6, 2019

homebysix commented Jun 6, 2019

gregneagle commented Jun 6, 2019

gregneagle commented Jun 6, 2019 •

edited

Loading

homebysix commented Jun 6, 2019

erikng commented Jun 6, 2019 via email

gregneagle Jun 6, 2019

MichalMMac Jun 7, 2019

MichalMMac Jun 14, 2019

MichalMMac commented Jun 13, 2019 •

edited

Loading

MichalMMac commented Jun 14, 2019 •

edited

Loading

gregneagle Jun 16, 2019 •

edited

Loading

MichalMMac Jun 17, 2019 •

edited

Loading

gregneagle Jun 17, 2019

gregneagle Jun 16, 2019

MichalMMac Jun 17, 2019

gregneagle Jun 17, 2019

MichalMMac Jun 18, 2019

sts commented Aug 18, 2020

MichalMMac commented Aug 18, 2020 •

edited

Loading

MichalMMac commented May 28, 2021

gregneagle commented May 28, 2021

MichalMMac commented May 28, 2021

rodchristiansen commented May 29, 2021

gregneagle commented Jun 8, 2021

erikng commented Jun 8, 2021

flammable commented Jun 8, 2021

rodchristiansen commented Jun 8, 2021

MichalMMac commented Jun 8, 2021 •

edited

Loading

MichalMMac commented Jun 8, 2021

Add notarization support #42

Add notarization support #42

Conversation

MichalMMac commented Jun 6, 2019 • edited Loading

gregneagle commented Jun 6, 2019

erikng commented Jun 6, 2019 • edited Loading

gregneagle commented Jun 6, 2019

poundbangbash commented Jun 6, 2019 via email

gregneagle commented Jun 6, 2019

homebysix commented Jun 6, 2019

gregneagle commented Jun 6, 2019

gregneagle commented Jun 6, 2019 • edited Loading

homebysix commented Jun 6, 2019

erikng commented Jun 6, 2019 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichalMMac commented Jun 13, 2019 • edited Loading

MichalMMac commented Jun 14, 2019 • edited Loading

gregneagle Jun 16, 2019 • edited Loading

Choose a reason for hiding this comment

MichalMMac Jun 17, 2019 • 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

sts commented Aug 18, 2020

MichalMMac commented Aug 18, 2020 • edited Loading

MichalMMac commented May 28, 2021

gregneagle commented May 28, 2021

MichalMMac commented May 28, 2021

rodchristiansen commented May 29, 2021

gregneagle commented Jun 8, 2021

erikng commented Jun 8, 2021

flammable commented Jun 8, 2021

rodchristiansen commented Jun 8, 2021

MichalMMac commented Jun 8, 2021 • edited Loading

MichalMMac commented Jun 8, 2021

MichalMMac commented Jun 6, 2019 •

edited

Loading

erikng commented Jun 6, 2019 •

edited

Loading

gregneagle commented Jun 6, 2019 •

edited

Loading

MichalMMac commented Jun 13, 2019 •

edited

Loading

MichalMMac commented Jun 14, 2019 •

edited

Loading

gregneagle Jun 16, 2019 •

edited

Loading

MichalMMac Jun 17, 2019 •

edited

Loading

MichalMMac commented Aug 18, 2020 •

edited

Loading

MichalMMac commented Jun 8, 2021 •

edited

Loading