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

Checking policy signature against cert constraints #144

Merged

Conversation

ChaosInTheCRD
Copy link
Collaborator

This PR makes it possible to inspect the certificate attached to the signature (in the case that it was signed with x509 certificate) against user supplied cert constraints.

It should be noted that the constraint checks we perform is against any verifier that passes the initial policy verification logic. I am not sure whether this is an issue, but it is still something to be aware of. see here.

Another thing to note is that currently this logic has been broken out into a verifyPolicySignature function underneath the main Verify function. There may be a good place in the library to put this, but I couldn't think of one while working on it. Any suggestions on where we should put this would be most welcome.

mikhailswift
mikhailswift previously approved these changes Jan 31, 2024
Copy link
Contributor

@mikhailswift mikhailswift left a comment

Choose a reason for hiding this comment

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

These changes look reasonable. I like the refactoring of breaking Functionary.Validate into it's own function.

policy/step.go Show resolved Hide resolved
verify.go Show resolved Hide resolved
verify.go Show resolved Hide resolved
@ChaosInTheCRD
Copy link
Collaborator Author

@mikhailswift looks like we're getting the golangci-lint issue again 🤦

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
@jkjell jkjell force-pushed the checking-policy-against-cert-constraints branch from 47d27fe to 72a0910 Compare February 3, 2024 04:32
Signed-off-by: John Kjell <john@testifysec.com>
Copy link
Member

@jkjell jkjell left a comment

Choose a reason for hiding this comment

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

❤️

verify.go Show resolved Hide resolved
@jkjell jkjell merged commit 86f5096 into in-toto:main Feb 3, 2024
11 checks passed
jkjell added a commit that referenced this pull request Feb 3, 2024
* adding logic so policy signature can be checked against constraints
* threaded options into policy validation functionary
---------

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: John Kjell <john@testifysec.com>
Co-authored-by: John Kjell <john@testifysec.com>
jkjell added a commit that referenced this pull request Feb 5, 2024
* adding logic so policy signature can be checked against constraints
* threaded options into policy validation functionary
---------

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: John Kjell <john@testifysec.com>
Co-authored-by: John Kjell <john@testifysec.com>
Signed-off-by: John Kjell <john@testifysec.com>
ChaosInTheCRD added a commit that referenced this pull request May 9, 2024
* Initial link attestor

Signed-off-by: John Kjell <john@testifysec.com>

* refactor: move gitoid code to cyrptoutil, use digestvalue everywhere (#139)

When the functionality to calculate gitoids was added, there was a bit
of tech debt incurred since they didn't implement hash.Hash. This
remedies this with an admitedly hacky implementation of hash.Hash that
wraps the gitoid code. This also standardizes our cryptoutil fucntions
around the DigestValue struct that was added around this time to
differentiate between gitoids and regular hash functions.

Signed-off-by: Mikhail Swift <mikhail@testifysec.com>
Signed-off-by: John Kjell <john@testifysec.com>

* chore: bump actions/upload-artifact from 4.2.0 to 4.3.0 (#142)

Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4.2.0 to 4.3.0.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](actions/upload-artifact@694cdab...26f96df)

---
updated-dependencies:
- dependency-name: actions/upload-artifact
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: John Kjell <john@testifysec.com>

* chore: bump github/codeql-action from 3.23.1 to 3.23.2 (#143)

Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.23.1 to 3.23.2.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@0b21cf2...b7bf0a3)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tom Meadows <tom@tmlabs.co.uk>
Signed-off-by: John Kjell <john@testifysec.com>

* Adding job to auto cut releases (#141)

adding job to auto cut releases

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: John Kjell <john@testifysec.com>

* fixing error in github actions workflow (#147)

fixing error in workflow

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: John Kjell <john@testifysec.com>

* RunAttestors refactor (#131)

* improving run attestors

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

* finalising changes.

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

* improving run attestors

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

* finalising changes.

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

* addressing review, restoring run type order

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

* updating error handling logic

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

* updating to go 1.21 for errors.Join

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

---------

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: Tom Meadows <tom@tmlabs.co.uk>
Signed-off-by: John Kjell <john@testifysec.com>

* Adding workaround due to failing workflows (#145)

adding workaround due to failing workflows

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: John Kjell <john@testifysec.com>

* Checking policy signature against cert constraints (#144)

* adding logic so policy signature can be checked against constraints
* threaded options into policy validation functionary
---------

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: John Kjell <john@testifysec.com>
Co-authored-by: John Kjell <john@testifysec.com>
Signed-off-by: John Kjell <john@testifysec.com>

* [StepSecurity] ci: Harden GitHub Actions (#148)

Signed-off-by: StepSecurity Bot <bot@stepsecurity.io>
Signed-off-by: John Kjell <john@testifysec.com>

* Add import for init and export variables

Signed-off-by: John Kjell <john@testifysec.com>

* Add mulitple results to run to allow exporting attestors to indivudal files

Signed-off-by: John Kjell <john@testifysec.com>

* Add collection to result array

Signed-off-by: John Kjell <john@testifysec.com>

* Replace export parameters in run with attestor option

Signed-off-by: John Kjell <john@testifysec.com>

* Fix golang lint isues

Signed-off-by: John Kjell <john@testifysec.com>

* Update link attestor testing

Signed-off-by: John Kjell <john@testifysec.com>

* Add SLSA attestor

Signed-off-by: John Kjell <john@testifysec.com>

* Add interface for product attestor

Signed-off-by: John Kjell <john@testifysec.com>

* Add more attestor interfaces

Signed-off-by: John Kjell <john@testifysec.com>

* Address some review feedback, licenses, and golanglint

Signed-off-by: John Kjell <john@testifysec.com>

* More golangcilint errors

Signed-off-by: John Kjell <john@testifysec.com>

* WIP - Improve testing interfaces for exposing data fields

Signed-off-by: John Kjell <john@testifysec.com>

* added changes

* adding changes to merge into main PR

* Link attestor proposed changes  (#204)

* unmarshal the time in the attestation collection correctly (#203)
* add StepName to AttestorContext
* use CollectionAttestion to properly set start/end times
---------

Signed-off-by: John Kjell <john@testifysec.com>
Co-authored-by: Cole Kennedy <colek42@gmail.com>
Co-authored-by: Cole <cole@testifysec.com>
Co-authored-by: John Kjell <john@testifysec.com>

* Passing SLSA Attest tests for GitHub and GitLab

Signed-off-by: John Kjell <john@testifysec.com>

* Clean up

Signed-off-by: John Kjell <john@testifysec.com>

* Add attestation test for link attestor

Signed-off-by: John Kjell <john@testifysec.com>

* Add data function for git interface and remove unused code

Signed-off-by: John Kjell <john@testifysec.com>

* adding warning mesage for slsa attestor

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

* Try to gracefully handle gitlab jwt

Signed-off-by: John Kjell <john@testifysec.com>

* ran go mod tidy

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

* ensuring link and slsa attestation exporting is optional

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>

---------

Signed-off-by: John Kjell <john@testifysec.com>
Signed-off-by: Mikhail Swift <mikhail@testifysec.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Signed-off-by: Tom Meadows <tom@tmlabs.co.uk>
Signed-off-by: StepSecurity Bot <bot@stepsecurity.io>
Co-authored-by: Mikhail Swift <mikhail@testifysec.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tom Meadows <tom@tmlabs.co.uk>
Co-authored-by: StepSecurity Bot <bot@stepsecurity.io>
Co-authored-by: Cole Kennedy <colek42@gmail.com>
Co-authored-by: Cole <cole@testifysec.com>
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.

None yet

4 participants