Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add package signing configuration for GitHub Actions #14708

Closed
wants to merge 1 commit into from

Conversation

TETRA2000
Copy link
Contributor

@TETRA2000 TETRA2000 commented Feb 19, 2023

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

fixes Homebrew/install#739

This PR add a signature for generated the package.

Prerequirement

You need to create a certificate for signing. And need to put it and its password as repository secrets.
You can look detailed instruction on GitHub's doc.

You also need to create an app-specific password for notarization.

1. Create an app identifier

(I'm pretty sure you already registered the identifier for homebrew but I write for others who want to try this PR.)

Register an App ID - Manage identifiers - Account - Help - Apple Developer

It must be identical as $IDENTIFIER env variable.

I used dev.tetra2000.brew for testing on my private repo.

2. Create a Developer ID Installer certificate

You need to create a Developer ID Installer certificate using an account with Account Holder role.

Create Developer ID certificates - Create certificates - Account - Help - Apple Developer
Apple Developer Program Roles - Support - Apple Developer

3. Generate an app-specific password

You need to generate an "app-specific password" using an account with Account Holder role.

Sign in to apps with your Apple ID using app-specific passwords - Apple Support

4. Add repository secrets

Export the Developer ID Installer certificate as p12 file and convert it to Base64.

$ base64 -i Certificates.p12 | pbcopy

Then you need to add it and its password as repository secrets.

You also need to add you email address of an account with Account Holder role, an app-specific password which generated in previous step.

Repository secret list

  • PKG_BUILD_CERTIFICATE_BASE64: The Base64 representation of the exported Developer ID Installer certificate.
  • PKG_BUILD_P12_PASSWORD: The password of the exported Developer ID Installer certificate.
  • APPLE_ID_USERNAME: The email address of an account with Account Holder role
  • APPLE_ID_PASSWORD: The app-specific password which generated in previous step

The settings page would looks like this.

References

ToDos

*Resolved

Fix verification problem

Even though it seems to have valid signature, macOS won't let me to open the package from Finder.
A package which build on my local machine with same build command has succeeded to be opened from Finder.

I'm still finding the root cause of this problem.

$ pkgutil --check-signature Homebrew-898cb9c5f.pkg
Package "Homebrew-898cb9c5f.pkg":
   Status: signed by a developer certificate issued by Apple for distribution
   Signed with a trusted timestamp on: 2023-02-19 11:59:21 AM +0000
   Certificate Chain:
    1. Developer ID Installer: Takahiko Inayama (788YH48Y3Q)
       Expires: 2027-02-01 10:12:15 PM +0000
       SHA256 Fingerprint:
           5B 6A 24 E5 1A 63 CC 74 BB CA F5 91 71 4A F4 AE AE 24 68 43 41 8F
           6F 11 5E 22 32 79 F1 BB E0 C0
       ------------------------------------------------------------------------
    2. Developer ID Certification Authority
       Expires: 2027-02-01 10:12:15 PM +0000
       SHA256 Fingerprint:
           7A FC 9D 01 A6 2F 03 A2 DE 96 37 93 6D 4A FE 68 09 0D 2D E1 8D 03
           F2 9C 88 CF B0 B1 BA 63 58 7F
       ------------------------------------------------------------------------
    3. Apple Root CA
       Expires: 2035-02-09 9:40:36 PM +0000
       SHA256 Fingerprint:
           B0 B1 73 0E CB C7 FF 45 05 14 2C 49 F1 29 5E 6E DA 6B CA ED 7E 2C
           68 C5 BE 91 B5 A1 10 01 F0 24
Screenshot 2023-02-19 at 9 00 26 PM Screenshot 2023-02-19 at 9 04 37 PM

Updated:

It's likely to be related to notarization.

spctl --assess --verbose --type install ./Homebrew-898cb9c5f.pkg
./Homebrew-898cb9c5f.pkg: rejected
source=Unnotarized Developer ID

Add notarization (optional)

I think it's not mandatory but it may be good to add a configuration for notarization.
Apple scans malicious components during notarization process.

Notarizing macOS software before distribution | Apple Developer Documentation

It can be done from CLI but it requires you to store app-specific password for Apple ID as a repository secret.

Customizing the notarization workflow | Apple Developer Documentation

@TETRA2000
Copy link
Contributor Author

I'm currently testing a notarize configuration like this.

      - name: Notarize package
        run: |
          xcrun notarytool submit Homebrew-${{ steps.print-version.outputs.version }}.pkg \
               --apple-id "$APPLE_ID_USERNAME" \
               --team-id "$SIGNING_IDENTITY" \
               --password "$APPLE_ID_PASSWORD" \
               --wait

Ref: Automatic Code-signing and Notarization for macOS apps using GitHub Actions

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work so far here @TETRA2000!

@SMillerDev
Copy link
Member

I'm currently testing a notarize configuration like this.

reading the article, that doesn't seem to apply for simple packages like Homebrews

@TETRA2000
Copy link
Contributor Author

TETRA2000 commented Feb 20, 2023

@MikeMcQuaid @SMillerDev
Thank you for taking a look at my PR!! I appreciate it.


reading the article, that doesn't seem to apply for simple packages like Homebrews

I thought same at first too. But this Apple document says you can notarize flat installer packages. And this line drawn my eyes.

Beginning in macOS 10.15, all software built after June 1, 2019, and distributed with Developer ID must be notarized.

And this error message I previously posted also suggests notarization may be still a problem.

spctl --assess --verbose --type install ./Homebrew-898cb9c5f.pkg
./Homebrew-898cb9c5f.pkg: rejected
source=Unnotarized Developer ID

Progress update: I succeeded to make a package which doesn't trigger warnings!

Build commands

# I removed fixtures (Reasons are described in 'Notarization errors')
rm -rf Library/Homebrew/test/support/fixtures/

# I built with this command. (But I need to tweak `--filter` a bit.)
pkgbuild --root brew \
    --scripts brew/package/scripts \
    --install-location "/tmp/brew" \
    --identifier "dev.tetra2000.brew" \
    --sign "788YH48Y3Q" \
    --min-os-version "11.0" \
    --filter .DS_Store \
    --version tmp \
    Homebrew-tmp.pkg

# Then I notarized with this command
xcrun notarytool submit Homebrew-tmp.pkg \
               --apple-id "***" \
               --team-id "788YH48Y3Q" \
               --password "***"  \
               --wait
Outputs from `xcrun notarytool submit`.
Conducting pre-submission checks for Homebrew-tmp.pkg and initiating connection to the Apple notary service...
Submission ID received
  id: 88c02a2c-f3c2-4290-80aa-e9c852e1da45
Upload progress: 100.00% (71.3 MB of 71.3 MB)
Successfully uploaded file
  id: 88c02a2c-f3c2-4290-80aa-e9c852e1da45
  path: /Users/takahiko/repo/Homebrew-tmp.pkg
Waiting for processing to complete.
Current status: Accepted............
Processing complete
  id: 88c02a2c-f3c2-4290-80aa-e9c852e1da45
  status: Accepted

After this, spctl command returned differently. It says "accepted".

spctl --assess --verbose --type install Homebrew-tmp.pkg
Homebrew-tmp.pkg: accepted
source=Notarized Developer ID

And macOS no longer shows warning dialogs.

Screen Recording 2023-02-20 at 11 34 40 PM

Notarization errors

This repository contains unsigned binaries in Library/Homebrew/test/support/fixtures/.
They cause errors during notarization process.

Error messages
{
  "logFormatVersion": 1,
  "jobId": "7963d438-abbc-4dd6-a50a-c4a5380b7a31",
  "status": "Invalid",
  "statusSummary": "Archive contains critical validation errors",
  "statusCode": 4000,
  "archiveFilename": "Homebrew-tmp.pkg",
  "uploadDate": "2023-02-20T13:35:44.060Z",
  "sha256": "47cebcc355ffd5363e8e3396024f959bea82cd03130c39663cff2f8263f484c1",
  "ticketContents": null,
  "issues": [
    {
      "severity": "error",
      "code": null,
      "path": "Homebrew-tmp.pkg/Homebrew-tmp.pkg Contents/Payload/tmp/brew/Library/Homebrew/test/support/fixtures/bottles/testball_bottle-0.1.yosemite.bottle.tar.gz/testball_bottle-0.1.yosemite.bottle.tar/testball_bottle/0.1/bin/helloworld",
      "message": "The binary is not signed.",
      "docUrl": "https://developer.apple.com/documentation/security/notarizing_macos_software_before_distribution/resolving_common_notarization_issues#3087721",
      "architecture": "i386"
    },
    {
      "severity": "error",
      "code": null,
      "path": "Homebrew-tmp.pkg/Homebrew-tmp.pkg Contents/Payload/tmp/brew/Library/Homebrew/test/support/fixtures/bottles/testball_bottle-0.1.yosemite.bottle.tar.gz/testball_bottle-0.1.yosemite.bottle.tar/testball_bottle/0.1/bin/helloworld",
      "message": "The signature does not include a secure timestamp.",
      "docUrl": "https://developer.apple.com/documentation/security/notarizing_macos_software_before_distribution/resolving_common_notarization_issues#3087733",
      "architecture": "i386"
    },

    ...

    {
      "severity": "warning",
      "code": null,
      "path": "Homebrew-tmp.pkg/Homebrew-tmp.pkg Contents/Payload/tmp/brew/Library/Homebrew/test/support/fixtures/cask/AppWithEmbeddedBinary.zip/App.app",
      "message": "Unable to notarize Homebrew-tmp.pkg/Homebrew-tmp.pkg Contents/Payload/tmp/brew/Library/Homebrew/test/support/fixtures/cask/AppWithEmbeddedBinary.zip/App.app",
      "docUrl": null,
      "architecture": null
    }
  ]
}

(You can obtain these logs via xcrun notarytool log command after xcrun notarytool submit.)

So we need to exclude files like Library/Homebrew/test/support/fixtures/bottles/testball_bottle-0.1.yosemite.bottle.tar.gz from install packages.
I know this can be done via --filter option. But options like --filter "brew/Library/Homebrew/test/.*" didn't work for me. I'll take a look later.

I simply removed entire directory for now.

Finally, I post the actual package. (Of course you don't have to install for reviewing. 😄)
https://drive.google.com/file/d/1SMBZT2F80NVKfVei9uBh6nU2eoviNt7G/view?usp=sharing

I'll update this PR to include changes above.

Comment on lines 32 to 35
BUILD_CERTIFICATE_BASE64: ${{ secrets.BUILD_CERTIFICATE_BASE64 }}
P12_PASSWORD: ${{ secrets.P12_PASSWORD }}
KEYCHAIN_PASSWORD: ${{ secrets.KEYCHAIN_PASSWORD }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you prefix these with PKG_BUILD? Just p12 password isn't very specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the comment.
I've changed to PKG_BUILD_P12_PASSWORD and PKG_BUILD_CERTIFICATE_BASE64

@TETRA2000 TETRA2000 changed the title WIP: Add package signing configuration for GitHub Actions Add package signing configuration for GitHub Actions Feb 25, 2023
@TETRA2000 TETRA2000 marked this pull request as ready for review February 25, 2023 01:57
@TETRA2000
Copy link
Contributor Author

TETRA2000 commented Feb 25, 2023

@SMillerDev @MikeMcQuaid
I've resolved all unfinished problems. Would you please take a look again?

I updated the PR description according to changes.

I also created a demo PR with my App ID and credentials.
You can see actual output from here.
TETRA2000#1

@TETRA2000 TETRA2000 requested review from MikeMcQuaid and SMillerDev and removed request for MikeMcQuaid February 25, 2023 02:01
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me so far!

@SMillerDev
Copy link
Member

Just to ensure this doesn't go stale: we're still getting the certificate, Homebrew never signed anything centrally before and not being it's own legal entity is making this a little slow.

env:
PKG_BUILD_CERTIFICATE_BASE64: ${{ secrets.PKG_BUILD_CERTIFICATE_BASE64 }}
PKG_BUILD_P12_PASSWORD: ${{ secrets.PKG_BUILD_P12_PASSWORD }}
KEYCHAIN_PASSWORD: ${{ secrets.KEYCHAIN_PASSWORD }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can just do uuidgen in the run block below to generate a UUID as a password for this?

--min-os-version "$MIN_OS" \
--filter .DS_Store \
--filter "(.*)/Library/Homebrew/test/support/fixtures/" \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MikeMcQuaid any suggestions how to deal with these better?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SMillerDev This seems fine with me but the package itself will need to do a git checkout master and/or git reset --hard origin/master after installation on disk or brew doctor will complain immediately.

Relatedly, that'd be good CI to add here at some point (doesn't need to block this PR): actually install the package and run brew doctor on the result to ensure it's setup correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with that is finding a CI for macOS without brew pre-installed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Mar 27, 2023
@github-actions github-actions bot closed this Apr 4, 2023
@SMillerDev SMillerDev reopened this Apr 4, 2023
@github-actions github-actions bot removed the stale No recent activity label Apr 5, 2023
Copy link

@madrh madrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label May 14, 2023
@SMillerDev
Copy link
Member

Nope, still waiting for a certificate

@SMillerDev SMillerDev added the in progress Maintainers are working on this label May 14, 2023
@SMillerDev
Copy link
Member

@MikeMcQuaid I uploaded the certificate to 1Password and I'll add the app password in a minute.

@MikeMcQuaid
Copy link
Member

@TETRA2000 thanks for the PR and great work so far! I'm going to work on getting this finished up and it'll be much easier to do so in a PR that actually runs this workflow (which requires it to be from a non-fork). I've cherry-picked your commit into #15743 and I'll be continuing your work over there. Thanks again for contributing to Homebrew!

@TETRA2000
Copy link
Contributor Author

@MikeMcQuaid @SMillerDev

I'm glad that I could submit something meaningful to the OSS project that I use everyday.
Actually, this is my first contribution to Homebrew.

Thank you for your reviews so far!
I really like this welcoming community!!

@TETRA2000 TETRA2000 deleted the macos_signing branch July 25, 2023 15:22
@github-actions github-actions bot added the outdated PR was locked due to age label Aug 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
in progress Maintainers are working on this outdated PR was locked due to age stale No recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MacOS doesn't trust the .pkg from Github Actions
4 participants