munki / munki-pkg

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

Fix notarization #65

Closed strlng closed 1 year ago

strlng commented 1 year ago

This updates the notarization code to use notarytool instead of altool, which is gone in Xcode 13 and later.

gregneagle commented 1 year ago

xcrun altool still works for me with Xcode 14. I don't actually use the signing or notarization features in munki-pkg, so I think others must test and provide feedback before this is merged.

gregneagle commented 1 year ago

One thing that occurs to me immediately: these changes (as-is) look like they will break everyone currently notarizing with munki-pkg: IOW, admins will have to update all of their package projects. That seems pretty unfriendly. It would be much nicer if there was a transition or conversion or something to ease the change.

erikng commented 1 year ago

I mean Apple does it lol. People don't have to just arbitrarily upgrade to the new Munki-pkg version. They can update when needed and update their tooling at the same time.

grahampugh commented 1 year ago

Not sure how helpful I'm being here, but I just attempted to use this fork and got an error.

pkgbuild: Inferring bundle components from contents of munkipkg/payload
pkgbuild: Writing new component property list to /var/folders/tf/_qj73tzx16388jr7v1t6pl200000gr/T/tmpj_2ypqla/component.plist
munkipkg: Adding package signing info to command
pkgbuild: Reading components from /var/folders/tf/_qj73tzx16388jr7v1t6pl200000gr/T/tmpj_2ypqla/component.plist
pkgbuild: Adding top-level postinstall script
pkgbuild: Using timestamp authority for signature
pkgbuild: Signing package with identity "Developer ID Installer: My Org (ASDFGHJKL)" from keychain /Users/company/Library/Keychains/login.keychain-db
pkgbuild: Adding certificate "Developer ID Certification Authority"
pkgbuild: Adding certificate "Apple Root CA"
pkgbuild: Wrote package to munkipkg/build/CiscoSecureClient-5.0.00556.pkg
munkipkg: Uploading package to Apple notary service
Traceback (most recent call last):
  File "/usr/local/bin/munkipkg", line 1303, in <module>
    main()
  File "/usr/local/bin/munkipkg", line 1299, in main
    result = build(arguments[0], options)
  File "/usr/local/bin/munkipkg", line 921, in build
    request_id = upload_to_notary(build_info, options)
  File "/usr/local/bin/munkipkg", line 728, in upload_to_notary
    output = readPlistFromString(proc_stdout.encode("UTF-8"))
  File "/usr/local/bin/munkipkg", line 90, in readPlistFromString
    return plistlib.loads(data)
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/plistlib.py", line 883, in loads
    return load(fp, fmt=fmt, dict_type=dict_type)
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/plistlib.py", line 869, in load
    raise InvalidFileException()
plistlib.InvalidFileException: Invalid file

Probably because the generated component.plist is empty?

$ cat /var/folders/tf/_qj73tzx16388jr7v1t6pl200000gr/T/tmpj_2ypqla/component.plist
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<array/>
</plist>

The build-info.plist is as follows:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
    <dict>
        <key>distribution_style</key>
        <false/>
        <key>identifier</key>
        <string>my.org.pkg.CiscoSecureClient</string>
        <key>install_location</key>
        <string>/</string>
        <key>name</key>
        <string>CiscoSecureClient-${version}.pkg</string>
        <key>ownership</key>
        <string>recommended</string>
        <key>postinstall_action</key>
        <string>none</string>
        <key>preserve_xattr</key>
        <false/>
        <key>version</key>
        <string>5.0.00556</string>
        <key>signing_info</key>
        <dict>
            <key>identity</key>
            <string>Developer ID Installer: My Org (ASDFGHJKL)</string>
        </dict>
        <key>notarization_info</key>
        <dict>
            <key>keychain_profile</key>
            <string>CiscoSecureClient_AppSpecificPassword</string>
            <key>staple_timeout</key>
            <integer>600</integer>
        </dict>
    </dict>
</plist>

Perhaps I'm missing a newly required key?

strlng commented 1 year ago

It is pretty unfriendly, agreed. I can go back and look at keeping the old notarization code in place for those not running Xcode 14. Not sure about the issue @grahampugh found. I have not seen that.

grahampugh commented 1 year ago

I found the issue. The keychain item is different with notarytool as with altool. I needed to generate a new keychain item.

erikng commented 1 year ago

I think it's a bit of wasted time to make both tools work.

From Apple

The use of altool for notarization is deprecated in Xcode 13 and later. Consider moving to notarytool as soon as possible. For information about the deprecation timeline, see the WWDC22 session What's new in notarization for Mac apps.

rodchristiansen commented 1 year ago

Notarizing software with altool was deprecated in Xcode 13, and the Apple notary service will no longer accept uploads from altool as of November 1, 2023.

gregneagle commented 1 year ago

Again: I don't use the notarization stuff -- I don't build any signed or notarized packages with munki-pkg, and so I don't test any of this code. I'm not sure of the status of this -- does it currently work? Is it mergeable? Can people use it successfully? (I don't think it needs to support both methods but it would be really really nice if it detected that a package project needed to be updated and told the user that rather than just crashing or spitting out vague errors. I don't want to be the target of a lot of support questions)

gregneagle commented 1 year ago

I've merged this into a new "notarytool" branch to make it easier for others to test and iterate before we (possibly) merge it to main.

strlng commented 1 year ago

That sounds good to me. I'll try to do some more testing in the next month, and I bet the code could be cleaned up a bit.

rodchristiansen commented 9 months ago

I've been using this branch successfully for the past few months, no issues what so ever. I can add a guide how to save credentials to keychain if that's helpful for folks.