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.

Sign up for GitHub

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

Jump to bottom

User should be able to set what algos to use for digest calculation in config file #73

Closed
colek42 opened this issue Dec 17, 2021 · 11 comments
Closed

User should be able to set what algos to use for digest calculation in config file #73

colek42 opened this issue Dec 17, 2021 · 11 comments
Labels
good first issue Good for newcomers

Comments

@colek42
Copy link
Member

colek42 commented Dec 17, 2021

No description provided.

@mikhailswift
Copy link
Member

mikhailswift commented Dec 17, 2021 •
edited
Loading

Just need to hook the config file into this function https://github.com/testifysec/witness/blob/ebb7c3d23f2529a0f40d372b7b3ec5546e6a7847/pkg/attestation/context.go#L27

Any hash calculations done in attestors should ask the context which hashes it needs to use.

@colek42 colek42 added the good first issue Good for newcomers label Jan 8, 2022
@jkjell jkjell added the needs triage Issues to triage label Sep 13, 2023
@DataDavD
Copy link
Contributor

Is anyone working on this? I can take a look sometime next couple weeks, if no one else is

@jkjell
Copy link
Member

jkjell commented Oct 1, 2023

Appreciate the help @DataDavD. Let us know if you have any questions or need any help along the way.

@jkjell jkjell removed the needs triage Issues to triage label Oct 1, 2023
@DataDavD
Copy link
Contributor

DataDavD commented Oct 5, 2023 •
edited
Loading

hey no problem @jkjell. Actually, I just realized this probably needs to be in the go-witness repo based on me looking around the two repos. Should I create a corresponding issue there (and maybe closed this one)?

I'm guessing the pkg code was split into go-witness? (side question: any history about why that was done and not just kept with the witness repo)

@mikhailswift
Copy link
Member

mikhailswift commented Oct 5, 2023 •
edited
Loading

Hi @DataDavD! Thanks for taking a look into this.

You should be able to use the RunWithAttestationOptions function when calling go-witness.Run. In that function, you can supply attestation.WithHashes with a slice of the hash functions. This will pass through the hashes to the AttestationContext that Run creates.

As for why Witness and go-witness got split: We wanted to use go-witness in other applications, but some of Witness's go-modules were causing issues. Along that same thread, we tried to keep the go-witness dependencies lighter than what Witness as a whole would use -- for instance, we didn't want the library to be opinionated about what logging library was used.

Hopefully, this answers your question, and again thanks so much for looking into this.

@DataDavD
Copy link
Contributor

DataDavD commented Oct 13, 2023 •
edited
Loading

Hey, just a small update. Started really digging in. Should have something more concrete, or more concrete questions later this weekend. Thanks again!

@DataDavD
Copy link
Contributor

DataDavD commented Oct 18, 2023 •
edited
Loading

Hey @mikhailswift. Maybe I'm confused, but is the expectation to have the availability of adding algos cobra.Command run parts like what is done/handled here: https://github.com/testifysec/witness/blob/4a41144b6402e7d175a657a359a66b41022dccd4/cmd/run.go#L111-L118

Or, should it be handled solely in the witness lib itself?

For context/help: this is kind of what I was thinking....but maybe I'm on the wrong path?
https://github.com/testifysec/witness/compare/main...DataDavD:witness:feat/ISSUE-73/add-digest-algos-to-config?expand=1

Thanks,
dd

@DataDavD
Copy link
Contributor

Hey @mikhailswift just bumping this just in case you missed my previous comment. Thanks

@mikhailswift
Copy link
Member

Sorry, I did miss that. I think you're on the right path. My only suggestion would be to have []string{"sha256"} be the default of the cobra variable. This should allow for it to be defined through either the CLI or a config file and not change our default behavior of using sha256 out of the box.

@DataDavD
Copy link
Contributor

Oh no problem at all.

Got it, makes more sense. I'll make those changes.

Thank you

DataDavD added a commit to DataDavD/witness that referenced this issue Nov 1, 2023
@DataDavD
feat: add algo hash list for digest calc in config
5db8308 
Introduce cobra/config flag that allows for slice of string's indicating
hash algorithms to be used for digest calculation. Uses
go-witness.attestation.WithHashes to add the slice to attestation run
options. Will error if can't parse hash from its string value.

Add to run tests to test for acceptable/unacceptable hash algorithms.
accepted

Refs: in-toto#73
DataDavD added a commit to DataDavD/witness that referenced this issue Nov 1, 2023
@DataDavD
feat: add algo hash list for digest calc in config
9683e8d 
Introduce cobra/config flag that allows for slice of string's indicating
hash algorithms to be used for digest calculation. Uses
go-witness.attestation.WithHashes to add the slice to attestation run
options. Will error if can't parse hash from its string value.

Add to run tests to test for acceptable/unacceptable hash algorithms.
accepted

Refs: in-toto#73
DataDavD added a commit to DataDavD/witness that referenced this issue Nov 1, 2023
@DataDavD
feat: add algo hash list for digest calc in config
32d3c10 
Introduce cobra/config flag that allows for slice of string's indicating
hash algorithms to be used for digest calculation. Uses
go-witness.attestation.WithHashes to add the slice to attestation run
options. Will error if can't parse hash from its string value.

Add test for testing acceptable/unacceptable hash algorithms accepted.

Refs: in-toto#73
@DataDavD DataDavD mentioned this issue Nov 1, 2023
Merged
DataDavD added a commit to DataDavD/witness that referenced this issue Nov 2, 2023
@DataDavD
feat: add algo hash list for digest calc in config
58a49da 
Introduce cobra/config flag that allows for slice of string's indicating
hash algorithms to be used for digest calculation. Uses
go-witness.attestation.WithHashes to add the slice to attestation run
options. Will error if can't parse hash from its string value.

Add test for testing acceptable/unacceptable hash algorithms accepted.

Refs: in-toto#73
jkjell pushed a commit to DataDavD/witness that referenced this issue Nov 27, 2023
@DataDavD @jkjell
feat: add algo hash list for digest calc in config
01ff9ea 
Introduce cobra/config flag that allows for slice of string's indicating
hash algorithms to be used for digest calculation. Uses
go-witness.attestation.WithHashes to add the slice to attestation run
options. Will error if can't parse hash from its string value.

Add test for testing acceptable/unacceptable hash algorithms accepted.

Refs: in-toto#73
DataDavD added a commit to DataDavD/witness that referenced this issue Dec 2, 2023
@DataDavD
feat: add algo hash list for digest calc in config
6b97fa3 
Introduce cobra/config flag that allows for slice of string's indicating
hash algorithms to be used for digest calculation. Uses
go-witness.attestation.WithHashes to add the slice to attestation run
options. Will error if can't parse hash from its string value.

Add test for testing acceptable/unacceptable hash algorithms accepted.

Refs: in-toto#73
jkjell pushed a commit to DataDavD/witness that referenced this issue Dec 5, 2023
@DataDavD @jkjell
feat: add algo hash list for digest calc in config
3fac8af 
Introduce cobra/config flag that allows for slice of string's indicating
hash algorithms to be used for digest calculation. Uses
go-witness.attestation.WithHashes to add the slice to attestation run
options. Will error if can't parse hash from its string value.

Add test for testing acceptable/unacceptable hash algorithms accepted.

Refs: in-toto#73
jkjell pushed a commit that referenced this issue Dec 5, 2023
@DataDavD
feat: add algo hash list for digest calc in config (#292)
0bca967 
Introduce cobra/config flag that allows for slice of string's indicating
hash algorithms to be used for digest calculation. Uses
go-witness.attestation.WithHashes to add the slice to attestation run
options. Will error if can't parse hash from its string value.

Add test for testing acceptable/unacceptable hash algorithms accepted.

Refs: #73
@jkjell
Copy link
Member

jkjell commented Dec 5, 2023

Closed by #292

@jkjell jkjell closed this as completed Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jkjell @mikhailswift @colek42 @DataDavD