search

erikberglund / SwiftPrivilegedHelper

Example application using a privileged helper tool with authentication in an unsandboxed application written in Swift
MIT License
182 stars 34 forks source link

Update project to Swift 5, and replaced CodeSignUpdate shell script with a Swift script to do the same thing but allow errors to be seen #28

Open chipjarred opened 4 years ago

chipjarred commented 4 years ago

I'm sorry for such a large pull request. I should have broken it up into two parts. One for the migration to Swift 5, and the other for the new CodeSignUpdate script written in Swift. I had an earlier pull request but managed to fix some of the needless diffs in it, so I closed it and submitting this new one.

The changes needed for Swift 5 were really minor. Swift 5 warns about there various temporaries pointers being passed to parameters to initializers needing to out-live the initializer call. This was a problem for the calls to the Authorization API. The problem comes down to Swift having different variable life-time standards than, say, C++. Basically a variable is only guaranteed to live until its last use, at which point it might be immediately deinitialized, rather than for the life of the block that it's in. For a fully Swift API that's not a problem, because reference counts keep things alive, or they're actually copied, but the Authorization API is C and uses pointers. So passing in a String or AuthorizationItem by reference to those functions is translated to a pointer to those things, which gets stored away, but then the String or AuthorizationItem it points to might be immediately deinitialized. The solution was just to wrap the calls and the whole block that depends on them with String's withCString method, or with Swift.withUnsafePointer to ensure the pointers remain valid. One could perhaps accomplish the same thing with withExtendedLifetime, but since the API wants pointers anyway that's what I used.

Your CodeSignUpdate shell script is great, much better than manually running SMJobBlessUtil.py -setreq manually. However it had some limitations owing to the fact that it used I/O redirection to build strings. None of the error messages it produced could ever get to the log, but instead were consumed in variable assignment. It would just give a build error stating the script terminated with exit code 1, and no other information. I re-wrote it as a Swift script so that string building doesn't interfere with I/O. Its error messages show up in the build log now. It also allowed me to do a bit more fine-grained error reporting. The biggest improvement regarding the script, which could have been done with the shell script as well, was removing the hard coding of bundle identifiers. They are now provided by exporting them just prior to calling the script in the Run Script phase. That way users of the script don't need to modify the script itself at all. I added a README for the Scripts folder about it. Of course since it's intended to run as part of the build process, CodeSignUpdate is run as a script rather than compiling it normally.

toonetown commented 2 years ago

BTW - I have to say, this is a VERY helpful pull request. Thank you for providing it. The swift-based signing script is very nice.

chipjarred commented 2 years ago

Thank you for the kind words, and I'm glad to know it was helpful to you in fixing your build script issues.

I'll need to do some more thinking about the case when action == install, and in general there are lots of improvements that could be made. My Swift version of the script is more or less a straight-forward translation of Erik's shell script so that error messages wouldn't be gobbled up in string construction. As such it retains a lot of that original structure and logic flow. Of course, Swift isn't bash/zsh, and I haven't spent a ton of time making it more Swifty or expanding the checks it does, though I did add one or two additional ones, and moved the hard coded bundle IDs out of the script into the environment.

chipjarred commented 2 years ago

@toonetown I like your suggestion about keying off of Developer ID. It sounds like you have a clear idea and are more recently in the depths of code signing details than I have been. If you have the time to do it, would you mind writing it up as a pull request on my fork? Otherwise I'll put it on my to do list.

toonetown commented 2 years ago

I absolutely can. I actually had it done that way earlier but decided against the additional changes in case you or others didn’t want that. I will get something put up in the next couple days.

chipjarred commented 2 years ago

Great! I look forward to seeing it.