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 entitlements support #314

Merged
merged 8 commits into from
Apr 23, 2024
Merged

Add entitlements support #314

merged 8 commits into from
Apr 23, 2024

Conversation

cfergeau
Copy link
Contributor

@cfergeau cfergeau commented Mar 1, 2024

My project (https://github.com/crc-org/vfkit) needs entitlements to be able to use the virtualization framework.

This PR reworks special slot handling in the signing code and then adds entitlements support.

@cfergeau cfergeau force-pushed the entitlements branch 2 times, most recently from 6c52957 to d9fd649 Compare March 1, 2024 15:32
@dustin-decker
Copy link

Thanks for adding this, it's a timely addition for me! Looking forward to it in the next quill release.

@dustin-decker
Copy link

This worked perfectly for my needs! Thanks again, Christophe.

@spiffcs
Copy link
Contributor

spiffcs commented Apr 16, 2024

Thanks @cfergeau this also worked for me! We'll get this in and release Quill =)

@spiffcs
Copy link
Contributor

spiffcs commented Apr 16, 2024

I'm trying to replicate a failure I'm seeing locally in CI. Apologies, I want to isolate and figure out what's wrong before merging:

On my local when checking out this branch"

--- FAIL: TestSign (3.10s)
    --- FAIL: TestSign/sign_the_syft_binary_(with_a_password) (0.00s)
        sign_test.go:219:
            	Error Trace:	/Users/hal/GolandProjects/quill/quill/sign_test.go:219
            	Error:      	Received unexpected error:
            	            	unable to parse certificate 1 of 1: x509: certificate contains duplicate extensions
            	Test:       	TestSign/sign_the_syft_binary_(with_a_password)

@spiffcs
Copy link
Contributor

spiffcs commented Apr 16, 2024

👋 @cfergeau I tried to push to this branch to fix the Static Analysis error, but got an authentication problem against your branch.

Here is the diff, but you can also just run make lint-fix

diff --git a/quill/sign/entitlements.go b/quill/sign/entitlements.go
index 31e0a02..4eb8d57 100644
--- a/quill/sign/entitlements.go
+++ b/quill/sign/entitlements.go
@@ -4,8 +4,9 @@ import (
        "fmt"
        "hash"

-       "github.com/anchore/quill/quill/macho"
        "github.com/go-restruct/restruct"
+
+       "github.com/anchore/quill/quill/macho"
 )

@cfergeau cfergeau force-pushed the entitlements branch 2 times, most recently from d05c1be to 511497e Compare April 18, 2024 10:33
@cfergeau
Copy link
Contributor Author

I'm trying to replicate a failure I'm seeing locally in CI. Apologies, I want to isolate and figure out what's wrong before merging:

On my local when checking out this branch"

--- FAIL: TestSign (3.10s)
    --- FAIL: TestSign/sign_the_syft_binary_(with_a_password) (0.00s)
        sign_test.go:219:
            	Error Trace:	/Users/hal/GolandProjects/quill/quill/sign_test.go:219
            	Error:      	Received unexpected error:
            	            	unable to parse certificate 1 of 1: x509: certificate contains duplicate extensions
            	Test:       	TestSign/sign_the_syft_binary_(with_a_password)

I'm seeing this issue with origin/master (+ a7d454d ), this is not new in this PR.

@cfergeau
Copy link
Contributor Author

👋 @cfergeau I tried to push to this branch to fix the Static Analysis error, but got an authentication problem against your branch.

"allow edit by maintainers" is checked in this PR, I don't know why you could not push.
I've fixed this now, though this comes with some additional lint changes as make lint was initially not working for me.

dependabot bot and others added 7 commits April 19, 2024 04:14
Bumps [github.com/blacktop/go-macho](https://github.com/blacktop/go-macho) from 1.1.198 to 1.1.199.
- [Release notes](https://github.com/blacktop/go-macho/releases)
- [Commits](blacktop/go-macho@v1.1.198...v1.1.199)

---
updated-dependencies:
- dependency-name: github.com/blacktop/go-macho
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
This requires additional configuration golangci/golangci-lint#3877 (comment)
With the default config, a lot of errors are reported.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
This can be tested with https://github.com/crc-org/vfkit/releases/download/v0.5.1/vfkit
The "entitlements" and "entitlementsDER" fields are theoritically 2
distinct slots/.., but handling them together in `describe` should not
make a big differenc.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Special slots (requirements, entitlements, ...) are handled in
2 places: in GenerateSigningSuperBlob and in newCodeDirectory.

This handling mostly hardcodes that there's a macho.CsSlotRequirements
slot, and nothing else. For examples, to add handling for a new slot
type, newCodeDirectory needs changes in at least 3 non-obvious places
(`hashOff` computation, writing of the hashes, and NSpecialSlots).

This code abstracts special slots handling by:
- adding a new SpecialSlot struct to describe a special slot - the rest
  of the code no longer needs to know it's dealing with
  CsSlotRequirements or a CsSlotEntitlements (which I want to add
  support for)
- it adds a SpecialSlotHashWriter type for use in newCodeDirectory to
  count the number of special slots, compute the `hashOff` value
  accordingly, write the slots in the correct order, ...

This will be useful in the commits which add support for entitlements.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
With the abstraction work done in the previous commit, adding support
for entitlements is now fairly straightforward, just need to build the
entitlements blob and hashes using user-provided XML data.

This fixes anchore#4

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
@spiffcs spiffcs enabled auto-merge (squash) April 23, 2024 14:08
@spiffcs spiffcs merged commit 072cfb5 into anchore:main Apr 23, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants