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

Fixing CA Path Flag to be used and adding policy timestamp server flag #353

Merged
merged 30 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
6d85e84
fixing ca path flag to be used and allowing timestamp server to be
ChaosInTheCRD Jan 11, 2024
995e01a
adding tests
ChaosInTheCRD Jan 11, 2024
7c04733
Merge branch 'main' into fixing-ca-path
ChaosInTheCRD Jan 22, 2024
d3ecb0e
fixing function definitions
ChaosInTheCRD Jan 23, 2024
8aaf8b7
Merge branch 'fixing-ca-path' of github.com:ChaosInTheCRD/witness int…
ChaosInTheCRD Jan 23, 2024
2f10938
Merge branch 'main' into fixing-ca-path
ChaosInTheCRD Jan 29, 2024
3071f44
Merge branch 'main' of github.com:in-toto/witness into fixing-ca-path
ChaosInTheCRD Jan 29, 2024
a9189f0
added intermediates and made final changes
ChaosInTheCRD Jan 29, 2024
2e00dc1
Merge branch 'fixing-ca-path' of github.com:ChaosInTheCRD/witness int…
ChaosInTheCRD Jan 29, 2024
461a90c
passing cert constraint options into verify func
ChaosInTheCRD Jan 29, 2024
c289ad2
Merge branch 'main' into fixing-ca-path
ChaosInTheCRD Feb 19, 2024
086a263
fixing if statement
ChaosInTheCRD Feb 19, 2024
595a2f3
tidying up
ChaosInTheCRD Feb 19, 2024
a1a1955
forgot docgen
ChaosInTheCRD Feb 19, 2024
220dc78
fixing gitignore issue
ChaosInTheCRD Feb 21, 2024
541a8ed
Merge branch 'main' into fixing-ca-path
jkjell May 2, 2024
002ba00
Merge branch 'main' into fixing-ca-path
ChaosInTheCRD May 3, 2024
a5697da
Merge branch 'main' of github.com:in-toto/witness into fixing-ca-path
ChaosInTheCRD May 10, 2024
b907b4c
Merge branch 'fixing-ca-path' of github.com:ChaosInTheCRD/witness int…
ChaosInTheCRD May 10, 2024
11d8eaa
saving the flags for now, need to finish
ChaosInTheCRD May 10, 2024
e1e0afb
added the flags for fulcio extensions
ChaosInTheCRD May 13, 2024
ec2e837
Merge branch 'main' into fixing-ca-path
ChaosInTheCRD May 13, 2024
a0b9ff6
fixing the test
ChaosInTheCRD May 13, 2024
2ba07f7
Merge branch 'fixing-ca-path' of github.com:ChaosInTheCRD/witness int…
ChaosInTheCRD May 13, 2024
d648399
running docgen
ChaosInTheCRD May 13, 2024
fd6d4de
Merge branch 'main' into fixing-ca-path
jkjell May 13, 2024
d1354de
Update to latest commit of go-witness after related PR merged
jkjell May 13, 2024
f320be5
policy json gone for some reason? adding it back
ChaosInTheCRD May 14, 2024
8b85256
think i've been silly
ChaosInTheCRD May 16, 2024
6e5265e
getting go-witness main
ChaosInTheCRD May 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
66 changes: 55 additions & 11 deletions cmd/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import (
"context"
"crypto"
"crypto/x509"
"encoding/json"
"errors"
"fmt"
Expand All @@ -28,6 +29,7 @@
"github.com/in-toto/go-witness/dsse"
"github.com/in-toto/go-witness/log"
"github.com/in-toto/go-witness/source"
"github.com/in-toto/go-witness/timestamp"
"github.com/in-toto/witness/options"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -56,23 +58,58 @@
// todo: this logic should be broken out and moved to pkg/
Copy link
Member

Choose a reason for hiding this comment

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

We should address this todo as a part of this as well. The verification for a policy and an attestation should be the same I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah for sure. The only difference with policy is that there isn't going to be a policy document for verifying the policy... if that makes sense. So we ought to keep the number of ways that one can use to verify the policy signature small...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you think it makes sense to make all these changes in this PR?

Copy link
Member

Choose a reason for hiding this comment

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

only difference with policy is that there isn't going to be a policy document for verifying the policy

I think it would be the same as the step verification: https://github.com/in-toto/go-witness/blob/main/policy/policy.go#L230

Copy link
Member

Choose a reason for hiding this comment

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

We're essentially defining the functionary from the CLI

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah the problem though is that defining all the available parameters for the functionaries from the CLI could quickly become unwieldly imo. Consider this functionary:

        {
          "type": "root",
          "certConstraint": {
            "commonname": "*",
            "dnsnames": [
              "*"
            ],
            "emails": [
              "tpmeadows1@gmail.com"
            ],
            "organizations": [
              "*"
            ],
            "uris": [
              "*"
            ],
            "roots": [
              "wippywoo"
            ]
          }
        }

In this case, we would need to define:

  1. cert-constraints (commonName, DNSName, emails, organizations, uris)
  2. roots (this includes root CA cert, intermediates (which doesn't quite seem clear in the tools current form), and we might need to supply timestamp servers alongside that.

How we would present that in a flag form I am not quite sure on. To me I think that limiting support for the number of policy signing mechanisms might be the way forward? It also seems to be the case to me that TUF may be a solution to this. @kairoaraujo any thoughts?

Copy link
Member

@colek42 colek42 Jan 14, 2024

Choose a reason for hiding this comment

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

I agree that it is unwieldy, however I would normally expect the user to wrap this in a bash script, or maybe a "witness verify" github action. I also think we could provide some syntactic sugar for sigstore that would let us provide better UX for the common use case while being secure by default.

ex:

for enterprise use:
witness verify -p signedpolicy.json -k ca.pem -i int.pem --commonname="*" --dnsname="*" --email="tpmeadows1@gmail.com" --organization="*" --uri="*" --root="wippywoo" artifact.tar

for sigstore:
witness verify -p signedpolicy.json --use-sigstore --use-archivista --email="tpmeadows1@gmail.com" artifact.tar

We would also need to provide a TSA pem or add support for Rekor

// we need to abstract where keys are coming from, etc
func runVerify(ctx context.Context, vo options.VerifyOptions) error {
if vo.KeyPath == "" && len(vo.CAPaths) == 0 {
return fmt.Errorf("must suply public key or ca paths")
if vo.KeyPath == "" && len(vo.PolicyCAPaths) == 0 {
return fmt.Errorf("must supply public key or ca paths")
}

var verifier cryptoutil.Verifier
var verifiers []cryptoutil.Verifier
if vo.KeyPath != "" {
keyFile, err := os.Open(vo.KeyPath)
if err != nil {
return fmt.Errorf("failed to open key file: %w", err)
}
defer keyFile.Close()

verifier, err = cryptoutil.NewVerifierFromReader(keyFile)
v, err := cryptoutil.NewVerifierFromReader(keyFile)
if err != nil {
return fmt.Errorf("failed to create verifier: %w", err)
}

verifiers = append(verifiers, v)
}

var policyRoots []*x509.Certificate
if len(vo.PolicyCAPaths) > 0 {
for _, caPath := range vo.PolicyCAPaths {
caFile, err := os.ReadFile(caPath)
if err != nil {
return fmt.Errorf("failed to read CA certificate file: %w", err)
}

cert, err := cryptoutil.TryParseCertificate(caFile)
if err != nil {
return fmt.Errorf("failed to parse Timestamp Server CA certificate: %w", err)
}

policyRoots = append(policyRoots, cert)
}
}

ptsv := make([]dsse.TimestampVerifier, 0)
if len(vo.PolicyTimestampServers) > 0 {
for _, server := range vo.PolicyTimestampServers {
f, err := os.ReadFile(server)
if err != nil {
return fmt.Errorf("failed to open Timestamp Server CA certificate file: %w", err)
}

cert, err := cryptoutil.TryParseCertificate(f)
if err != nil {
return fmt.Errorf("failed to parse Timestamp Server CA certificate: %w", err)
}

ptsv = append(ptsv, timestamp.NewVerifier(timestamp.VerifyWithCerts([]*x509.Certificate{cert})))
}
}

inFile, err := os.Open(vo.PolicyFilePath)
Expand Down Expand Up @@ -121,26 +158,33 @@
verifiedEvidence, err := witness.Verify(
ctx,
policyEnvelope,
[]cryptoutil.Verifier{verifier},
verifiers,
witness.VerifyWithSubjectDigests(subjects),
witness.VerifyWithCollectionSource(collectionSource),
witness.VerifyWithPolicyTimestampServers(ptsv),

Check failure on line 164 in cmd/verify.go

View workflow job for this annotation

GitHub Actions / Verify Docgen

undefined: witness.VerifyWithPolicyTimestampServers

Check failure on line 164 in cmd/verify.go

View workflow job for this annotation

GitHub Actions / unit-test / witness

undefined: witness.VerifyWithPolicyTimestampServers

Check failure on line 164 in cmd/verify.go

View workflow job for this annotation

GitHub Actions / sast / witness

undefined: witness.VerifyWithPolicyTimestampServers
witness.VerifyWithPolicyCACerts(policyRoots),

Check failure on line 165 in cmd/verify.go

View workflow job for this annotation

GitHub Actions / Verify Docgen

undefined: witness.VerifyWithPolicyCACerts

Check failure on line 165 in cmd/verify.go

View workflow job for this annotation

GitHub Actions / unit-test / witness

undefined: witness.VerifyWithPolicyCACerts

Check failure on line 165 in cmd/verify.go

View workflow job for this annotation

GitHub Actions / sast / witness

undefined: witness.VerifyWithPolicyCACerts
)

if err != nil {
return fmt.Errorf("failed to verify policy: %w", err)

}

log.Info("Verification succeeded")
log.Info("Evidence:")
log.Infof("Evidence:")
num := 0
for _, stepEvidence := range verifiedEvidence {
for step, stepEvidence := range verifiedEvidence {
for _, e := range stepEvidence {
log.Info(fmt.Sprintf("%d: %s", num, e.Reference))
log.Info(fmt.Sprintf("step %d: %s", num, step))
log.Info(fmt.Sprintf("reference: %s", e.Reference))
for _, v := range e.Verifiers {
k, err := v.KeyID()
if err != nil {
return fmt.Errorf("failed to get verifier key id: %w", err)
}
log.Info(fmt.Sprintf("Verified Key IDs: %s", k))
}
num++
}
}

return nil

}
19 changes: 10 additions & 9 deletions options/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ package options
import "github.com/spf13/cobra"

type VerifyOptions struct {
ArchivistaOptions ArchivistaOptions
KeyPath string
AttestationFilePaths []string
PolicyFilePath string
ArtifactFilePath string
AdditionalSubjects []string
CAPaths []string
ArchivistaOptions ArchivistaOptions
KeyPath string
AttestationFilePaths []string
PolicyFilePath string
ArtifactFilePath string
AdditionalSubjects []string
PolicyCAPaths []string
PolicyTimestampServers []string
}

func (vo *VerifyOptions) AddFlags(cmd *cobra.Command) {
Expand All @@ -33,6 +34,6 @@ func (vo *VerifyOptions) AddFlags(cmd *cobra.Command) {
cmd.Flags().StringVarP(&vo.PolicyFilePath, "policy", "p", "", "Path to the policy to verify")
cmd.Flags().StringVarP(&vo.ArtifactFilePath, "artifactfile", "f", "", "Path to the artifact to verify")
cmd.Flags().StringSliceVarP(&vo.AdditionalSubjects, "subjects", "s", []string{}, "Additional subjects to lookup attestations")
cmd.Flags().StringSliceVarP(&vo.CAPaths, "policy-ca", "", []string{}, "Paths to CA certificates to use for verifying the policy")

cmd.Flags().StringSliceVarP(&vo.PolicyCAPaths, "policy-ca", "", []string{}, "Paths to CA certificates to use for verifying the policy")
cmd.Flags().StringSliceVarP(&vo.PolicyTimestampServers, "policy-timestamp-servers", "", []string{}, "Paths to the CA certificates for Timestamp Authority Servers to use when verifying policy")
}