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

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

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

mikhailswift
Copy link
Member

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.

@mikhailswift
Copy link
Member Author

This is the first part of breaking up the VSA PR into a few smaller, easier to digest PRs.

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>
Copy link
Collaborator

@ChaosInTheCRD ChaosInTheCRD left a comment

Choose a reason for hiding this comment

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

sorry for some reason it's not letting me comment inline - at attestation/context.go:107, sorry for my ignorance, but why do we create 3 separate digest values?

@ChaosInTheCRD
Copy link
Collaborator

This seems okay enough, tried my best to proof read and while I agree the solution is somewhat hacky but seems neater than how it was implemented before 👍

@ChaosInTheCRD
Copy link
Collaborator

I've approved, but won't merge as I have one small question. Don't think it's a blocker to merging though

@mikhailswift
Copy link
Member Author

sorry for some reason it's not letting me comment inline - at attestation/context.go:107, sorry for my ignorance, but why do we create 3 separate digest values?

So these are the default hashes, and currently we were only using sha256 as our default. However, the material/product attestors were hard coded to also record sha256-gitoid and sha1-gitoid hashes, so they were added here to maintain consistent functionality

@mikhailswift mikhailswift merged commit 027b47d into main Jan 29, 2024
13 checks passed
@mikhailswift mikhailswift deleted the refactor/gitoid-in-cryptoutil branch January 29, 2024 15:33
ChaosInTheCRD added a commit to ChaosInTheCRD/witness that referenced this pull request Jan 29, 2024
jkjell pushed a commit to in-toto/witness that referenced this pull request Feb 3, 2024
…mented in go-witness (#371)

* updated to put crypto hashes in digestvalue slice after updates to in-toto/go-witness#139
* adding gitoid false to use
* bumping go-witness version
---------

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
jkjell pushed a commit that referenced this pull request Feb 3, 2024
…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>
jkjell pushed a commit that referenced this pull request Feb 5, 2024
…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>
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.

2 participants