Skip to content

Commit

Permalink
fix: calculate subjects on demand always
Browse files Browse the repository at this point in the history 
We encountered errors with `witness verify` failing unexpectedly despite
having the appropriate attestations available. The issue laid with some
backrefs not being able to be calculated appropriately, which the policy
engine then was unable to build the graph of attestations to satisfy
the policy.

The root cause of the issue wound up being that in some attestors
subjects were calculated in the `Attest` function and stored for later
use in the `Subjects` function. However, this causes issues when we are
reading attestations from a source and in a context where `Attest` would
have never been run on the attestor being inspected.

The solution I implemented is to calculate subjects (and by extenstion
backrefs) on demand from the unexported fields on the attestor structs.
This way when we unmarshal them from json we will always be able to
re-calculate the subjects.

However, since in many cases we were using the AttestationContext's
Hashes functions to decide which hashing functions to use while
calculating the subjects, I am currently hard-coding in the hashes
instead for the time being.
  • Loading branch information
@mikhailswift @colek42
mikhailswift authored and colek42 committed Jun 6, 2023
1 parent 77252cf commit 31e6790
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 138 deletions.
52 changes: 29 additions & 23 deletions attestation/aws-iid/aws-iid.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ func init() {

type Attestor struct {
ec2metadata.EC2InstanceIdentityDocument
subjects map[string]cryptoutil.DigestSet
hashes []crypto.Hash
session session.Session
conf *aws.Config
Expand All @@ -96,11 +95,9 @@ func New() *Attestor {
}

conf := &aws.Config{}

return &Attestor{
session: *sess,
conf: conf,
subjects: make(map[string]cryptoutil.DigestSet),
session: *sess,
conf: conf,
}

}
Expand Down Expand Up @@ -130,20 +127,6 @@ func (a *Attestor) Attest(ctx *attestation.AttestationContext) error {
return err
}

subjects := make(map[string]string)
subjects[fmt.Sprintf("instanceid:%s", a.EC2InstanceIdentityDocument.InstanceID)] = a.EC2InstanceIdentityDocument.InstanceID
subjects[fmt.Sprintf("accountid:%s", a.EC2InstanceIdentityDocument.AccountID)] = a.EC2InstanceIdentityDocument.AccountID
subjects[fmt.Sprintf("imageid:%s", a.EC2InstanceIdentityDocument.ImageID)] = a.EC2InstanceIdentityDocument.ImageID
subjects[fmt.Sprintf("privateip:%s", a.EC2InstanceIdentityDocument.PrivateIP)] = a.EC2InstanceIdentityDocument.PrivateIP

for k, v := range subjects {
subj, err := cryptoutil.CalculateDigestSetFromBytes([]byte(v), ctx.Hashes())
if err != nil {
continue
}
a.subjects[k] = subj
}

return nil
}

Expand Down Expand Up @@ -171,7 +154,6 @@ func (a *Attestor) getIID() error {
}

func (a *Attestor) Verify() error {

if len(a.RawIID) == 0 || len(a.RawSig) == 0 {
return nil
}
Expand Down Expand Up @@ -213,11 +195,36 @@ func (a *Attestor) Verify() error {
}

func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
return a.subjects
hashes := []crypto.Hash{crypto.SHA256}
subjects := make(map[string]cryptoutil.DigestSet)
if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.InstanceID), hashes); err == nil {
subjects[fmt.Sprintf("instanceid:%s", a.EC2InstanceIdentityDocument.InstanceID)] = ds
} else {
log.Debugf("(attestation/aws) failed to record aws instanceid subject: %v", err)
}

if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.AccountID), hashes); err == nil {
subjects[fmt.Sprintf("accountid:%s", a.EC2InstanceIdentityDocument.AccountID)] = ds
} else {
log.Debugf("(attestation/aws) failed to record aws accountid subject: %v", err)
}

if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.ImageID), hashes); err == nil {
subjects[fmt.Sprintf("imageid:%s", a.EC2InstanceIdentityDocument.ImageID)] = ds
} else {
log.Debugf("(attestation/aws) failed to record aws imageid subject: %v", err)
}

if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.PrivateIP), hashes); err == nil {
subjects[fmt.Sprintf("privateip:%s", a.EC2InstanceIdentityDocument.PrivateIP)] = ds
} else {
log.Debugf("(attestation/aws) failed to record aws privateip subject: %v", err)
}

return subjects
}

func getAWSCAPublicKey() (*rsa.PublicKey, error) {

block, rest := pem.Decode([]byte(awsCACertPEM))
if len(rest) > 0 {
return nil, fmt.Errorf("failed to decode PEM block containing the public key")
Expand All @@ -229,5 +236,4 @@ func getAWSCAPublicKey() (*rsa.PublicKey, error) {
}

return cert.PublicKey.(*rsa.PublicKey), nil

}
10 changes: 4 additions & 6 deletions attestation/aws-iid/aws-iid_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,8 @@ func TestAttestor_Attest(t *testing.T) {
}

a := &Attestor{
session: *sess,
conf: conf,
subjects: make(map[string]cryptoutil.DigestSet),
session: *sess,
conf: conf,
}

ctx, err := attestation.NewContext([]attestation.Attestor{a})
Expand Down Expand Up @@ -151,9 +150,8 @@ func TestAttestor_Subjects(t *testing.T) {
require.NoError(t, err)

a := &Attestor{
session: *sess,
conf: conf,
subjects: map[string]cryptoutil.DigestSet{},
session: *sess,
conf: conf,
}

ctx, err := attestation.NewContext([]attestation.Attestor{a})
Expand Down
75 changes: 36 additions & 39 deletions attestation/gcp-iit/gcp-iit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package gcpiit

import (
"crypto"
"fmt"
"io"
"net/http"
Expand All @@ -34,8 +35,7 @@ const (
Type = "https://witness.dev/attestations/gcp-iit/v0.1"
RunType = attestation.PreMaterialRunType

jwksUrl = "https://www.googleapis.com/oauth2/v3/certs"

jwksUrl = "https://www.googleapis.com/oauth2/v3/certs"
defaultIdentityTokenHost = "metadata.google.internal"
identityTokenURLPathTemplate = "/computeMetadata/v1/instance/service-accounts/%s/identity"
identityTokenAudience = "witness-node-attestor" //nolint: gosec // false positive
Expand Down Expand Up @@ -80,13 +80,10 @@ type Attestor struct {
ClusterLocation string `json:"cluster_location"`

isWorkloadIdentity bool
subjects map[string]cryptoutil.DigestSet
}

func New() *Attestor {
return &Attestor{
subjects: make(map[string]cryptoutil.DigestSet),
}
return &Attestor{}
}

func (a *Attestor) Name() string {
Expand All @@ -102,9 +99,7 @@ func (a *Attestor) RunType() attestation.RunType {
}

func (a *Attestor) Attest(ctx *attestation.AttestationContext) error {

tokenURL := identityTokenURL(defaultIdentityTokenHost, defaultServiceAccount)

identityToken, err := getMetadata(tokenURL)
if err != nil {
return status.Errorf(codes.Internal, "unable to retrieve valid identity token: %v", err)
Expand Down Expand Up @@ -135,36 +130,6 @@ func (a *Attestor) Attest(ctx *attestation.AttestationContext) error {
a.getInstanceData()
}

instanceIDSubject, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.InstanceID), ctx.Hashes())
if err != nil {
return err
}
a.subjects[fmt.Sprintf("instanceid:%v", a.InstanceID)] = instanceIDSubject

instanceHostnameSubject, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.InstanceHostname), ctx.Hashes())
if err != nil {
return err
}
a.subjects[fmt.Sprintf("instancename:%v", a.InstanceHostname)] = instanceHostnameSubject

projectIDSubject, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectID), ctx.Hashes())
if err != nil {
return err
}
a.subjects[fmt.Sprintf("projectid:%v", a.ProjectID)] = projectIDSubject

projectNumberSubject, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectNumber), ctx.Hashes())
if err != nil {
return err
}
a.subjects[fmt.Sprintf("projectnumber:%v", a.ProjectNumber)] = projectNumberSubject

clusterUIDSubject, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ClusterUID), ctx.Hashes())
if err != nil {
return err
}
a.subjects[fmt.Sprintf("clusteruid:%v", a.ClusterUID)] = clusterUIDSubject

return nil
}

Expand Down Expand Up @@ -209,7 +174,39 @@ func (a *Attestor) getInstanceData() {
}

func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
return a.subjects
subjects := make(map[string]cryptoutil.DigestSet)
hashes := []crypto.Hash{crypto.SHA256}
if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.InstanceID), hashes); err == nil {
subjects[fmt.Sprintf("instanceid:%v", a.InstanceID)] = ds
} else {
log.Debugf("(attestation/gcp) failed to record gcp instanceid subject: %v", err)
}

if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.InstanceHostname), hashes); err == nil {
subjects[fmt.Sprintf("instancename:%v", a.InstanceHostname)] = ds
} else {
log.Debugf("(attestation/gcp) failed to record gcp instancename subject: %v", err)
}

if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectID), hashes); err == nil {
subjects[fmt.Sprintf("projectid:%v", a.ProjectID)] = ds
} else {
log.Debugf("(attestation/gcp) failed to record gcp projectid subject: %v", err)
}

if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectNumber), hashes); err == nil {
subjects[fmt.Sprintf("projectnumber:%v", a.ProjectNumber)] = ds
} else {
log.Debugf("(attestation/gcp) failed to record gcp projectnumber subject: %v", err)
}

if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ClusterUID), hashes); err == nil {
subjects[fmt.Sprintf("clusteruid:%v", a.ClusterUID)] = ds
} else {
log.Debugf("(attestation/gcp) failed to record gcp clusteruid subject: %v", err)
}

return subjects
}

func getMetadata(url string) ([]byte, error) {
Expand Down
10 changes: 4 additions & 6 deletions attestation/git/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ type Attestor struct {
TreeHash string `json:"treehash,omitempty"`
Refs []string `json:"refs,omitempty"`
Tags []Tag `json:"tags,omitempty"`
hashes []crypto.Hash
}

func New() *Attestor {
Expand All @@ -97,8 +96,6 @@ func (a *Attestor) RunType() attestation.RunType {
}

func (a *Attestor) Attest(ctx *attestation.AttestationContext) error {
a.hashes = ctx.Hashes()

repo, err := git.PlainOpenWithOptions(ctx.WorkingDir(), &git.PlainOpenOptions{
DetectDotGit: true,
})
Expand Down Expand Up @@ -219,6 +216,7 @@ func (a *Attestor) Attest(ctx *attestation.AttestationContext) error {

func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
subjects := make(map[string]cryptoutil.DigestSet)
hashes := []crypto.Hash{crypto.SHA256}

subjectName := fmt.Sprintf("commithash:%v", a.CommitHash)
subjects[subjectName] = cryptoutil.DigestSet{
Expand All @@ -230,7 +228,7 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {

//add author email
subjectName = fmt.Sprintf("authoremail:%v", a.AuthorEmail)
ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.AuthorEmail), a.hashes)
ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.AuthorEmail), hashes)
if err != nil {
return nil
}
Expand All @@ -239,7 +237,7 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {

//add committer email
subjectName = fmt.Sprintf("committeremail:%v", a.CommitterEmail)
ds, err = cryptoutil.CalculateDigestSetFromBytes([]byte(a.CommitterEmail), a.hashes)
ds, err = cryptoutil.CalculateDigestSetFromBytes([]byte(a.CommitterEmail), hashes)
if err != nil {
return nil
}
Expand All @@ -249,7 +247,7 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
//add parent hashes
for _, parentHash := range a.ParentHashes {
subjectName = fmt.Sprintf("parenthash:%v", parentHash)
ds, err = cryptoutil.CalculateDigestSetFromBytes([]byte(parentHash), a.hashes)
ds, err = cryptoutil.CalculateDigestSetFromBytes([]byte(parentHash), hashes)
if err != nil {
return nil
}
Expand Down
47 changes: 28 additions & 19 deletions attestation/github/github.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,20 @@ package github

import (
"bytes"
"crypto"
"encoding/json"
"fmt"
"io"
"net/http"
"net/url"
"os"
"strings"

"github.com/davecgh/go-spew/spew"
"github.com/testifysec/go-witness/attestation"
"github.com/testifysec/go-witness/attestation/jwt"
"github.com/testifysec/go-witness/cryptoutil"
"github.com/testifysec/go-witness/log"
)

const (
Expand Down Expand Up @@ -72,15 +75,14 @@ type Attestor struct {
CIServerUrl string `json:"ciserverurl"`
RunnerArch string `json:"runnerarch"`
RunnerOS string `json:"runneros"`
jwksURL string
tokenURL string
aud string
subjects map[string]cryptoutil.DigestSet

jwksURL string
tokenURL string
aud string
}

func New() *Attestor {
return &Attestor{
subjects: make(map[string]cryptoutil.DigestSet),
aud: tokenAudience,
jwksURL: jwksURL,
tokenURL: os.Getenv("ACTIONS_ID_TOKEN_REQUEST_URL"),
Expand Down Expand Up @@ -129,29 +131,36 @@ func (a *Attestor) Attest(ctx *attestation.AttestationContext) error {
a.RunnerArch = os.Getenv("RUNNER_ARCH")
a.RunnerOS = os.Getenv("RUNNER_OS")
a.PipelineUrl = fmt.Sprintf("%s/actions/runs/%s", a.ProjectUrl, a.PipelineID)
return nil
}

pipelineSubj, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.PipelineUrl), ctx.Hashes())
if err != nil {
return err
func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
subjects := make(map[string]cryptoutil.DigestSet)
hashes := []crypto.Hash{crypto.SHA256}
if pipelineSubj, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.PipelineUrl), hashes); err == nil {
subjects[fmt.Sprintf("pipelineurl:%v", a.PipelineUrl)] = pipelineSubj
} else {
log.Debugf("(attestation/github) failed to record github pipelineurl subject: %v", err)
}
a.subjects[fmt.Sprintf("pipelineurl:%v", a.PipelineUrl)] = pipelineSubj

projectSubj, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectUrl), ctx.Hashes())
if err != nil {
return err
if projectSubj, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectUrl), hashes); err == nil {
subjects[fmt.Sprintf("projecturl:%v", a.ProjectUrl)] = projectSubj
} else {
log.Debugf("(attestation/github) failed to record github projecturl subject: %v", err)
}
a.subjects[fmt.Sprintf("projecturl:%v", a.ProjectUrl)] = projectSubj
return nil
}

func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet {
return a.subjects
return subjects
}

func (a *Attestor) BackRefs() map[string]cryptoutil.DigestSet {
backRefs := make(map[string]cryptoutil.DigestSet)
pipelineUrl := fmt.Sprintf("pipelineurl:%v", a.PipelineUrl)
backRefs[pipelineUrl] = a.subjects[pipelineUrl]
for subj, ds := range a.Subjects() {
if strings.HasPrefix(subj, "pipelineurl:") {
backRefs[subj] = ds
break
}
}

return backRefs
}

Expand Down
Loading

0 comments on commit 31e6790

Please sign in to comment.