Python signing and minor updates

macmule commented 3 years ago

As mentioned in the #munki-rebrand slack, this PR has 3 commits atm.. and they address:

https://github.com/ox-it/munki-rebrand/commit/f1e9c73ab4b70d57871469b01d4c9d7a6d836641 - Corrected --sign-binaries description
https://github.com/ox-it/munki-rebrand/commit/e9b39d11db637e1862b666c107a179727ca56183 - Added error when icon file is declared but cannot be found, without this we hit an issue later on
https://github.com/ox-it/munki-rebrand/commit/733980cd96593153045e2e5cd0a22dc02e860f29 - Added support sign munki's python. Right now, this is enabled by the --sign-binaries flag, can move if wanted.

Next I'll likely refactor things some more and pylint.

macmule commented 3 years ago

$ /usr/local/munki/munki-python 
Python 3.9.5 (v3.9.5:0a7dcbdb13, May  3 2021, 13:05:53) 
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.

$ sudo codesign --display -r- --deep -v /usr/local/munki/Python.framework 
Executable=/usr/local/munki/Python.framework/Versions/Current/Python
Identifier=org.python.python
Format=bundle with Mach-O universal (x86_64 arm64)
CodeDirectory v=20200 size=29713 flags=0x0(none) hashes=923+3 location=embedded
Signature size=9004
Timestamp=7 Jul 2021 at 14:58:59
Info.plist entries=11
TeamIdentifier=82K2XFN8L6
Sealed Resources version=2 rules=13 files=6448
designated => identifier "org.python.python" and anchor apple generic and certificate 1[field.1.2.840.113635.100.6.2.6] /* exists */ and certificate leaf[field.1.2.840.113635.100.6.1.13] /* exists */ and certificate leaf[subject.OU] = "82K2XFN8L6"

$ sudo codesign --display -r- --deep -v /Applications/Managed\ Software\ Center.app 
Executable=/Applications/Managed Software Center.app/Contents/MacOS/Managed Software Center
Identifier=com.googlecode.munki.ManagedSoftwareCenter
Format=app bundle with Mach-O universal (x86_64 arm64)
CodeDirectory v=20500 size=8726 flags=0x10000(runtime) hashes=265+3 location=embedded
Signature size=9004
Timestamp=7 Jul 2021 at 14:56:32
Info.plist entries=32
TeamIdentifier=82K2XFN8L6
Runtime Version=11.1.0
Sealed Resources version=2 rules=13 files=212
Nested=Frameworks/libswiftCloudKit.dylib
Nested=PlugIns/MSCDockTilePlugin.docktileplugin
Nested=Frameworks/libswiftCore.dylib
Nested=Frameworks/libswiftCoreGraphics.dylib
Nested=Frameworks/libswiftCoreLocation.dylib
Nested=Frameworks/libswiftFoundation.dylib
Nested=Frameworks/libswiftCoreFoundation.dylib
Nested=Frameworks/libswiftQuartzCore.dylib
Nested=Frameworks/libswiftCoreData.dylib
Nested=Frameworks/libswiftDispatch.dylib
Nested=Frameworks/libswiftos.dylib
Nested=Frameworks/libswiftCoreImage.dylib
Nested=Frameworks/libswiftDarwin.dylib
Nested=Frameworks/libswiftXPC.dylib
Nested=Frameworks/libswiftAppKit.dylib
Nested=Frameworks/libswiftIOKit.dylib
Nested=Frameworks/libswiftContacts.dylib
Nested=Frameworks/libswiftMetal.dylib
Nested=Frameworks/libswiftObjectiveC.dylib
designated => identifier "com.googlecode.munki.ManagedSoftwareCenter" and anchor apple generic and certificate 1[field.1.2.840.113635.100.6.2.6] /* exists */ and certificate leaf[field.1.2.840.113635.100.6.1.13] /* exists */ and certificate leaf[subject.OU] = "82K2XFN8L6"

Looking good.

I'll work on the PR to pylint and tidy some munki-rebrand now

fuzzylogiq commented 3 years ago

I have a few issues with what you've done so far which hopefully your refactoring will address:

Having multiple calls to codesign in a separate python-specific function is not great, the sign_binaries function should be extended to do what you want with passed in values.
I'd rather use built in python functionality where possible, so shelling out to find goes against that. I'd rather we were using glob.glob, os.stat etc to find the binaries that need signing and plistlib instead of PlistBuddy
Ideally there'd be a function that found and made a list of binaries in the order they were to be signed, and whether they needed entitlements, deep, force etc, and then passed that to sign_binaries
The repetition of all the code to expand and flatten the python pkg isn't great

I'm glad i've seen the code but it's a bit confusing you doing the development all in the PR, cause I don't know when you are expecting it to be merged :)

fuzzylogiq commented 3 years ago

This is what I'm getting at for sign_binary (done from memory so please don't assume works!)

def sign_binary(signing_id, binary, verbose=False, deep=False, options=[],
                entitlements="", force=False):
    """ Signs a binary with a signing id, with optional arguments for command line args"""
    cmd = [
        CODESIGN,
        "--sign",
        signing_id
        ]
    if force:
        cmd.append("--force")
    if deep:
        cmd.append("--deep")
    if verbose:
        cmd.append("--verbose")
    if entitlements:
        cmd.append("--entitlements")
        cmd.append(entitlements)
    if options:
        cmd.append("--options")
        cmd.append(','.join([option for option in options]))
    cmd.append(binary)
    run_cmd(cmd)

fuzzylogiq commented 3 years ago

And this sort of thing to replace shelling out to find:

>>> os.getcwd()
'/private/tmp/munkitools-5.3.0.4335/munkitools_python.pkg/Payload/usr/local/munki/Python.framework/Versions/3.9/bin'
>>> files = os.listdir()
>>> [f for f in files if os.path.isfile(f) and os.access(f, os.X_OK)]
['pip3.9', '2to3', 'python3', 'pip3', 'easy_install-3.9', 'idle3', 'python3.9-config', 'python3.9', 'python3-config', 'pip', 'pydoc3.9', 'idle3.9', '2to3-3.9', 'pydoc3', 'xattr']

macmule commented 3 years ago

@fuzzylogiq, cool.. tbh wasn't sure where to put this, but consider it so far a successful proof of concept.

Happy for me to pylint this and refactor too?

I could sack off this PR and do another with all the changes if wanted too.

fuzzylogiq commented 3 years ago

check out https://github.com/ox-it/munki-rebrand/releases/tag/5.0b1 for my version of this

fuzzylogiq commented 3 years ago

thanks for your work on this. closing now due to release of v5.0

ox-it / munki-rebrand

Python signing and minor updates #50