munki / munki-pkg

Repo for the munkipkg tool and example projects
Other
343 stars 75 forks source link

Add notarization support #42

Closed MichalMMac closed 3 years ago

MichalMMac commented 5 years ago

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:

gregneagle commented 5 years ago

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 commented 5 years ago

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

gregneagle commented 5 years ago

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

poundbangbash commented 5 years ago

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 notifications@github.com 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 https://github.com/munki/munki-pkg/pull/42?email_source=notifications&email_token=AAKZYYD35N7KVO34MEIC5RDPZFM2LA5CNFSM4HVBRHIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXD2N2Y#issuecomment-499623659, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKZYYDZOFFM3TH242VPCZDPZFM2LANCNFSM4HVBRHIA.

gregneagle commented 5 years ago

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

homebysix commented 5 years ago

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 commented 5 years ago

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

gregneagle commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.mdhttps://github.com/munki/munki-pkg/pull/42#discussion_r291387835:

+ +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. + + +Notarization process is described on Apple web page 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 GitHubhttps://github.com/munki/munki-pkg/pull/42?email_source=notifications&email_token=ABLL6GESO6BRNIBBXB3QYQ3PZGB33A5CNFSM4HVBRHIKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB23GX7Y#discussion_r291387835, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABLL6GA7JN7NM2NMNNKR7QLPZGB33ANCNFSM4HVBRHIA.

MichalMMac commented 5 years ago

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

MichalMMac commented 5 years ago

Added big commit 9a6e0d04fa7d518f294cdd649b79f946a99b279d.

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:

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

MichalMMac commented 4 years ago

New commit d34ccd0 fixes handling of unexpected response in get_notarization_state().

MichalMMac commented 4 years ago

Should we implement --apiKey and --apiIssuer authentication options? I am not sure which for which role I should generate appstoreconnect.apple.com API key.

Have enyone used apiKey with altool for notarization?

gregneagle commented 4 years ago

Are the Apple tools here a moving target?

MichalMMac commented 4 years ago

I don’t think so. I believe ´apiKey´ is alternative way to authenticate next to AppleID+password we already implemented. I don’t have an immediate need to implement it but it could be nice in sense of feature completeness 😅

erikng commented 4 years ago

you may want to handle this error:

        You must first sign the relevant contracts online. (1048)
2019-12-03 13:19:30.411 altool[2776:18620320] *** Error: You must first sign the relevant contracts online. (1048)
MichalMMac commented 4 years ago

@erikng is there a traceback?

MichalMMac commented 4 years ago

Rebased against current master. There are still multiple fixups which need squashing before merge.

rodchristiansen commented 4 years ago

I have been using the fork that includes notarization for munki-pkg for a few months now and it has been working great, no hiccups. I added the app-specific password to all .json files and it just does it. Thanks for this MichaelMMac.

erikng commented 4 years ago

:(

gregneagle commented 4 years ago

This was probably auto-closed by GitHub when I renamed the primary branch to “main”.

erikng commented 4 years ago

I see. That would make sense why so many things just closed across multiple repos.

gregneagle commented 4 years ago

Perhaps if I’d waited for GitHub to do the rename the auto close wouldn’t have happened, but there’s no assurance that GitHub will actually do that.

flammable commented 4 years ago

Would it be possible to reopen this? It'll be important when Apple finally gets around to requiring notarization for pkgs.

flammable commented 4 years ago

Thanks, Greg! 🙂

MichalMMac commented 4 years ago

TLDR I've added support for authentication using App Store Connect API keys.

After I created my new company personal managed Apple ID I discovered this type of Apple ID is unable to set App-Specific password which is needed for altool to authenticate with --password option. I finally get into implementing App Store Connect API keys support which turned out to be quite easy.

sts commented 3 years ago

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 commented 3 years ago

@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

MichalMMac commented 3 years ago

Rebased against latest code. Updated version changing commit.

gregneagle commented 3 years ago

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 commented 3 years ago

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

rodchristiansen commented 3 years ago

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

gregneagle commented 3 years ago

…and now altool is deprecated in macOS Monterey…

erikng commented 3 years ago

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

flammable commented 3 years ago

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

rodchristiansen commented 3 years ago

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

MichalMMac commented 3 years ago

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 commented 3 years ago

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