Skip to content

Commit

Permalink
fix validation for proposed vs committed log entries for intoto v0.0.1 (
Browse files Browse the repository at this point in the history 
sigstore#1309)

* fix validation for proposed vs committed log entries

Signed-off-by: Bob Callaway <bcallaway@google.com>

* be more accepting of existing clients

Signed-off-by: Bob Callaway <bcallaway@google.com>

* update code comment

Signed-off-by: Bob Callaway <bcallaway@google.com>

---------

Signed-off-by: Bob Callaway <bcallaway@google.com>
(cherry picked from commit db3428c)
  • Loading branch information
@bobcallaway
bobcallaway committed Feb 6, 2023
1 parent f03e33d commit 8612c1d
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 33 deletions.
4 changes: 2 additions & 2 deletions pkg/generated/models/intoto_v001_schema.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions pkg/generated/restapi/embedded_spec.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 25 additions & 2 deletions pkg/types/intoto/v0.0.1/entry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,16 @@ func (v *V001Entry) Unmarshal(pe models.ProposedEntry) error {

func (v *V001Entry) Canonicalize(ctx context.Context) ([]byte, error) {
if v.keyObj == nil {
return nil, errors.New("cannot canonicalze empty key")
return nil, errors.New("cannot canonicalize empty key")
}
if v.IntotoObj.Content == nil {
return nil, errors.New("missing content")
}
if v.IntotoObj.Content.Hash == nil {
return nil, errors.New("missing envelope hash")
}
if v.IntotoObj.Content.PayloadHash == nil {
return nil, errors.New("missing payload hash")
}
pk, err := v.keyObj.CanonicalValue()
if err != nil {
Expand Down Expand Up @@ -219,10 +228,24 @@ func (v *V001Entry) validate() error {
// TODO handle multiple
pk := v.keyObj.(*x509.PublicKey)

// This also gets called in the CLI, where we won't have this data
// one of two cases must be true:
// - ProposedEntry: client gives an envelope; (client provided hash/payloadhash are ignored as they are computed server-side) OR
// - CommittedEntry: NO envelope and hash/payloadHash must be present
if v.IntotoObj.Content.Envelope == "" {
if v.IntotoObj.Content.Hash == nil {
return fmt.Errorf("missing hash value for envelope")
} else if err := v.IntotoObj.Content.Hash.Validate(strfmt.Default); err != nil {
return fmt.Errorf("validation error on envelope hash: %w", err)
}
if v.IntotoObj.Content.PayloadHash == nil {
return fmt.Errorf("missing hash value for payload")
} else if err := v.IntotoObj.Content.PayloadHash.Validate(strfmt.Default); err != nil {
return fmt.Errorf("validation error on payload hash: %w", err)
}
// if there is no envelope, and hash/payloadHash are valid, then there's nothing else to do here
return nil
}

vfr, err := signature.LoadVerifier(pk.CryptoPubKey(), crypto.SHA256)
if err != nil {
return err
Expand Down
68 changes: 47 additions & 21 deletions pkg/types/intoto/v0.0.1/entry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,34 +148,77 @@ func TestV001Entry_Unmarshal(t *testing.T) {
wantErr: true,
},
{
name: "missing envelope",
name: "invalid key",
it: &models.IntotoV001Schema{
PublicKey: p([]byte("hello")),
},
wantErr: true,
},
{
name: "valid intoto",
it: &models.IntotoV001Schema{
PublicKey: p(pub),
Content: &models.IntotoV001SchemaContent{
Envelope: envelope(t, key, validPayload, "application/vnd.in-toto+json"),
},
},
wantErr: false,
wantVerifierErr: false,
},
{
name: "valid intoto but hash specified by client (should be ignored)",
it: &models.IntotoV001Schema{
PublicKey: p(pub),
Content: &models.IntotoV001SchemaContent{
Envelope: envelope(t, key, validPayload, "application/vnd.in-toto+json"),
Hash: &models.IntotoV001SchemaContentHash{
Algorithm: swag.String(models.IntotoV001SchemaContentHashAlgorithmSha256),
Value: swag.String("1a1707bb54e5fb4deddd19f07adcb4f1e022ca7879e3c8348da8d4fa496ae8e2"),
},
},
},
wantErr: false,
wantErr: false,
wantVerifierErr: false,
},
{
name: "valid dsse but invalid intoto",
name: "valid intoto but payloadhash specified by client (should be ignored)",
it: &models.IntotoV001Schema{
PublicKey: p(pub),
Content: &models.IntotoV001SchemaContent{
Envelope: envelope(t, key, validPayload, "text"),
Envelope: envelope(t, key, validPayload, "application/vnd.in-toto+json"),
PayloadHash: &models.IntotoV001SchemaContentPayloadHash{
Algorithm: swag.String(models.IntotoV001SchemaContentPayloadHashAlgorithmSha256),
Value: swag.String("1a1707bb54e5fb4deddd19f07adcb4f1e022ca7879e3c8348da8d4fa496ae8e2"),
},
},
},
wantErr: false,
wantVerifierErr: false,
},
{
name: "valid intoto but envelope and payloadhash specified by client (hash values should be ignored)",
it: &models.IntotoV001Schema{
PublicKey: p(pub),
Content: &models.IntotoV001SchemaContent{
Envelope: envelope(t, key, validPayload, "application/vnd.in-toto+json"),
Hash: &models.IntotoV001SchemaContentHash{
Algorithm: swag.String(models.IntotoV001SchemaContentHashAlgorithmSha256),
Value: swag.String("1a1707bb54e5fb4deddd19f07adcb4f1e022ca7879e3c8348da8d4fa496ae8e2"),
},
PayloadHash: &models.IntotoV001SchemaContentPayloadHash{
Algorithm: swag.String(models.IntotoV001SchemaContentPayloadHashAlgorithmSha256),
Value: swag.String("1a1707bb54e5fb4deddd19f07adcb4f1e022ca7879e3c8348da8d4fa496ae8e2"),
},
},
},
wantErr: false,
},
{
name: "valid dsse but invalid intoto",
it: &models.IntotoV001Schema{
PublicKey: p(pub),
Content: &models.IntotoV001SchemaContent{
Envelope: envelope(t, key, validPayload, "text"),
},
},
wantErr: false,
Expand All @@ -186,9 +229,6 @@ func TestV001Entry_Unmarshal(t *testing.T) {
PublicKey: p([]byte(pemBytes)),
Content: &models.IntotoV001SchemaContent{
Envelope: envelope(t, priv, validPayload, "text"),
Hash: &models.IntotoV001SchemaContentHash{
Algorithm: swag.String(models.IntotoV001SchemaContentHashAlgorithmSha256),
},
},
},
additionalIndexKeys: []string{"joe@schmoe.com"},
Expand All @@ -200,9 +240,6 @@ func TestV001Entry_Unmarshal(t *testing.T) {
PublicKey: p(pub),
Content: &models.IntotoV001SchemaContent{
Envelope: string(invalid),
Hash: &models.IntotoV001SchemaContentHash{
Algorithm: swag.String(models.IntotoV001SchemaContentHashAlgorithmSha256),
},
},
},
wantErr: true,
Expand All @@ -213,9 +250,6 @@ func TestV001Entry_Unmarshal(t *testing.T) {
PublicKey: p([]byte("notavalidkey")),
Content: &models.IntotoV001SchemaContent{
Envelope: envelope(t, key, validPayload, "text"),
Hash: &models.IntotoV001SchemaContentHash{
Algorithm: swag.String(models.IntotoV001SchemaContentHashAlgorithmSha256),
},
},
},
wantErr: true,
Expand All @@ -224,11 +258,6 @@ func TestV001Entry_Unmarshal(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
v := &V001Entry{}
if tt.it.Content != nil {
h := sha256.Sum256([]byte(tt.it.Content.Envelope))
tt.it.Content.Hash.Algorithm = swag.String(models.IntotoV001SchemaContentHashAlgorithmSha256)
tt.it.Content.Hash.Value = swag.String(hex.EncodeToString(h[:]))
}

it := &models.Intoto{
Spec: tt.it,
Expand All @@ -238,9 +267,6 @@ func TestV001Entry_Unmarshal(t *testing.T) {
if err := v.Unmarshal(it); err != nil {
return err
}
if err := v.validate(); err != nil {
return err
}
if v.IntotoObj.Content.Hash == nil || v.IntotoObj.Content.Hash.Algorithm != tt.it.Content.Hash.Algorithm || v.IntotoObj.Content.Hash.Value != tt.it.Content.Hash.Value {
return errors.New("missing envelope hash in validated object")
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/types/intoto/v0.0.1/intoto_v0_0_1_schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"writeOnly": true
},
"hash": {
"description": "Specifies the hash algorithm and value encompassing the entire signed envelope",
"description": "Specifies the hash algorithm and value encompassing the entire signed envelope; this is computed by the rekor server, client-provided values are ignored",
"type": "object",
"properties": {
"algorithm": {
Expand All @@ -36,7 +36,7 @@
"readOnly": true
},
"payloadHash": {
"description": "Specifies the hash algorithm and value covering the payload within the DSSE envelope",
"description": "Specifies the hash algorithm and value covering the payload within the DSSE envelope; this is computed by the rekor server, client-provided values are ignored",
"type": "object",
"properties": {
"algorithm": {
Expand Down

0 comments on commit 8612c1d

Please sign in to comment.