From 6ae68f109fcc4f0c1dcc65a55ad34904f93adf19 Mon Sep 17 00:00:00 2001
From: Bob Callaway <bobcallaway@users.noreply.github.com>
Date: Mon, 7 Dec 2020 04:24:01 -0500
Subject: [PATCH] Types unit test (#57)
* add types unit tests
* explicitly ignore error from ValidateLeaf
* improve PGP unit test coverage (#58)
* update & improve pgp unit tests
* add types unit tests
* explicitly ignore error from ValidateLeaf
* use files instead of embedded strings
---
go.mod | 1 +
go.sum | 8 +
pkg/types/types.go | 31 ++-
pkg/types/types_test.go | 418 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 452 insertions(+), 6 deletions(-)
create mode 100644 pkg/types/types_test.go
diff --git a/go.mod b/go.mod
index 6e1245f..58eb0d7 100644
--- a/go.mod
+++ b/go.mod
@@ -13,6 +13,7 @@ require (
github.com/prometheus/common v0.10.0
github.com/spf13/cobra v1.0.0
github.com/spf13/viper v1.7.1
+ github.com/tidwall/sjson v1.1.2
go.etcd.io/etcd v3.3.25+incompatible // indirect
go.uber.org/goleak v1.1.10
go.uber.org/zap v1.16.0
diff --git a/go.sum b/go.sum
index 791cea8..8bf2d70 100644
--- a/go.sum
+++ b/go.sum
@@ -514,6 +514,14 @@ github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJy
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s=
github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
+github.com/tidwall/gjson v1.6.1 h1:LRbvNuNuvAiISWg6gxLEFuCe72UKy5hDqhxW/8183ws=
+github.com/tidwall/gjson v1.6.1/go.mod h1:BaHyNc5bjzYkPqgLq7mdVzeiRtULKULXLgZFKsxEHI0=
+github.com/tidwall/match v1.0.1 h1:PnKP62LPNxHKTwvHHZZzdOAOCtsJTjo6dZLCwpKm5xc=
+github.com/tidwall/match v1.0.1/go.mod h1:LujAq0jyVjBy028G1WhWfIzbpQfMO8bBZ6Tyb0+pL9E=
+github.com/tidwall/pretty v1.0.2 h1:Z7S3cePv9Jwm1KwS0513MRaoUe3S01WPbLNV40pwWZU=
+github.com/tidwall/pretty v1.0.2/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
+github.com/tidwall/sjson v1.1.2 h1:NC5okI+tQ8OG/oyzchvwXXxRxCV/FVdhODbPKkQ25jQ=
+github.com/tidwall/sjson v1.1.2/go.mod h1:SEzaDwxiPzKzNfUEO4HbYF/m4UCSJDsGgNqsS1LvdoY=
github.com/timakin/bodyclose v0.0.0-20190721030226-87058b9bfcec/go.mod h1:Qimiffbc6q9tBWlVV6x0P9sat/ao1xEkREYPPj9hphk=
github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 h1:LnC5Kc/wtumK+WB441p7ynQJzVuNRJiqddSIE3IlSEQ=
diff --git a/pkg/types/types.go b/pkg/types/types.go
index fd13f89..d58e056 100644
--- a/pkg/types/types.go
+++ b/pkg/types/types.go
@@ -28,6 +28,8 @@ import (
"fmt"
"io"
"net/http"
+ "net/url"
+ "reflect"
"github.com/projectrekor/rekor/pkg/pki"
"golang.org/x/sync/errgroup"
@@ -59,10 +61,17 @@ func (r *RekorLeaf) MarshalJSON() ([]byte, error) {
cLeaf.SHA = r.SHA
var err error
+ if reflect.ValueOf(r.sigObject).IsNil() {
+ return nil, errors.New("signature has not been initialized")
+ }
cLeaf.Signature, err = r.sigObject.CanonicalValue()
if err != nil {
return nil, err
}
+
+ if reflect.ValueOf(r.keyObject).IsNil() {
+ return nil, errors.New("public key has not been initialized")
+ }
cLeaf.PublicKey, err = r.keyObject.CanonicalValue()
if err != nil {
return nil, err
@@ -87,7 +96,7 @@ func (l *RekorLeaf) ValidateLeaf() error {
// validate fields
if l.SHA != "" {
if _, err := hex.DecodeString(l.SHA); err != nil || len(l.SHA) != 64 {
- return fmt.Errorf("Invalid SHA hash provided")
+ return fmt.Errorf("invalid SHA hash provided")
}
}
@@ -109,6 +118,10 @@ func (l *RekorLeaf) ValidateLeaf() error {
}
func ParseRekorEntry(r io.Reader, leaf RekorLeaf) (*RekorEntry, error) {
+ if err := leaf.ValidateLeaf(); err != nil {
+ return nil, err
+ }
+
var e RekorEntry
dec := json.NewDecoder(r)
if err := dec.Decode(&e); err != nil && err != io.EOF {
@@ -119,11 +132,17 @@ func ParseRekorEntry(r io.Reader, leaf RekorLeaf) (*RekorEntry, error) {
e.RekorLeaf = leaf
if e.Data == nil && e.URL == "" {
- return nil, errors.New("one of Contents or ContentsRef must be set")
+ return nil, errors.New("one of Data or URL must be set")
}
- if e.URL != "" && e.SHA == "" {
- return nil, errors.New("SHA hash must be specified if URL is set")
+ if e.URL != "" {
+ if _, err := url.ParseRequestURI(e.URL); err != nil {
+ return nil, fmt.Errorf("url parsing failed: %w", err)
+ }
+
+ if e.SHA == "" {
+ return nil, errors.New("SHA hash must be specified if URL is set")
+ }
}
return &e, nil
@@ -143,7 +162,7 @@ func (r *RekorEntry) Load(ctx context.Context) error {
return err
}
if resp.StatusCode < 200 || resp.StatusCode > 299 {
- return fmt.Errorf("Error received while fetching artifact: %v", resp.Status)
+ return fmt.Errorf("error received while fetching artifact: %v", resp.Status)
}
defer resp.Body.Close()
@@ -154,7 +173,7 @@ func (r *RekorEntry) Load(ctx context.Context) error {
return err
}
- if "application/x+gzip" == http.DetectContentType(ctBuf) {
+ if http.DetectContentType(ctBuf) == "application/x-gzip" {
dataReader, _ = gzip.NewReader(io.MultiReader(bufReader, resp.Body))
} else {
dataReader = io.MultiReader(bufReader, resp.Body)
diff --git a/pkg/types/types_test.go b/pkg/types/types_test.go
new file mode 100644
index 0000000..62722ea
--- /dev/null
+++ b/pkg/types/types_test.go
@@ -0,0 +1,418 @@
+/*
+Copyright © 2020 Bob Callaway <bcallawa@redhat.com>
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package types
+
+import (
+ "bytes"
+ "compress/gzip"
+ "context"
+ "encoding/base64"
+ "encoding/json"
+ "errors"
+ "io/ioutil"
+ "net/http"
+ "net/http/httptest"
+ "reflect"
+ "strings"
+ "testing"
+
+ "github.com/tidwall/sjson"
+ "go.uber.org/goleak"
+)
+
+func TestMain(m *testing.M) {
+ goleak.VerifyTestMain(m)
+}
+
+type BadReader struct {
+}
+
+func (br BadReader) Read(p []byte) (n int, err error) {
+ return 0, errors.New("test error")
+}
+
+func jsonSetNoError(input, path string, value interface{}) string {
+ ret, _ := sjson.Set(input, path, value)
+ return ret
+}
+
+func jsonDeleteNoError(input, path string) string {
+ ret, _ := sjson.Delete(input, path)
+ return ret
+}
+
+func TestRekorLeaf(t *testing.T) {
+
+ type Test struct {
+ caseDesc string
+ json string
+ expectSuccess bool
+ }
+
+ kernelJSONBytes, _ := ioutil.ReadFile("../../tests/kernel.json")
+ originalEntry := string(kernelJSONBytes)
+ originalLeaf := jsonDeleteNoError(originalEntry, "URL")
+
+ tests := []Test{
+ {
+ caseDesc: "Valid full entry",
+ json: originalEntry,
+ expectSuccess: true,
+ },
+ {
+ caseDesc: "Valid full leaf",
+ json: originalLeaf,
+ expectSuccess: true,
+ },
+ {
+ caseDesc: "Valid leaf without SHA",
+ json: jsonDeleteNoError(originalLeaf, "SHA"),
+ expectSuccess: true,
+ },
+ {
+ caseDesc: "Invalid SHA",
+ json: jsonSetNoError(originalLeaf, "SHA", "not_a_valid_sha_hash!!!"),
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Invalid signature",
+ json: jsonSetNoError(originalLeaf, "Signature", "not_a_signature!!!"),
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Invalid public key",
+ json: jsonSetNoError(originalLeaf, "PublicKey", "c29tZV9kYXRhCg=="),
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Missing signature",
+ json: jsonDeleteNoError(originalLeaf, "Signature"),
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Missing public key",
+ json: jsonDeleteNoError(originalLeaf, "PublicKey"),
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Invalid JSON",
+ json: `This is not valid JSON!`,
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Empty JSON",
+ json: `{}`,
+ expectSuccess: false,
+ },
+ }
+
+ for _, tc := range tests {
+ leaf, err := ParseRekorLeaf(strings.NewReader(tc.json))
+ if (err == nil) != tc.expectSuccess {
+ t.Errorf("Error in test case '%v': %v", tc.caseDesc, err)
+ }
+
+ if tc.expectSuccess == true {
+ jsonMap := make(map[string]string)
+ err := json.Unmarshal([]byte(tc.json), &jsonMap)
+ if err != nil {
+ t.Errorf("Error in test case '%v' comparing leaf to input: %v", tc.caseDesc, err)
+ }
+ for key, val := range jsonMap {
+ if key == "SHA" {
+ if val != reflect.Indirect(reflect.ValueOf(leaf)).FieldByName(key).String() {
+ t.Errorf("Error in test case '%v': leaf does not reflect input for '%v'", tc.caseDesc, key)
+ }
+ } else if key == "Signature" || key == "PublicKey" {
+ leafVal := base64.StdEncoding.EncodeToString(reflect.Indirect(reflect.ValueOf(leaf)).FieldByName(key).Bytes())
+ if val != leafVal {
+ t.Errorf("Error in test case '%v': leaf does not reflect input for '%v' %v != %v", tc.caseDesc, key, val, leafVal)
+ }
+ }
+ }
+ }
+ }
+
+ if _, err := ParseRekorLeaf(&BadReader{}); err == nil {
+ t.Errorf("No error thrown if io.Reader failed")
+ }
+}
+
+func TestRekorEntry(t *testing.T) {
+ type Test struct {
+ caseDesc string
+ json string
+ leaf *RekorLeaf
+ expectSuccess bool
+ }
+
+ kernelJSONBytes, _ := ioutil.ReadFile("../../tests/kernel.json")
+ originalEntry := string(kernelJSONBytes)
+
+ tests := []Test{
+ {
+ caseDesc: "Valid full entry with URL & SHA",
+ json: originalEntry,
+ leaf: nil,
+ expectSuccess: true,
+ },
+ {
+ caseDesc: "Valid full entry with Data",
+ json: jsonSetNoError(jsonDeleteNoError(jsonDeleteNoError(originalEntry, "URL"), "SHA"), "Data", "c29tZV9kYXRhCg=="),
+ leaf: nil,
+ expectSuccess: true,
+ },
+ {
+ caseDesc: "Invalid entry without URL or Data",
+ json: jsonDeleteNoError(originalEntry, "URL"),
+ leaf: nil,
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Invalid entry with URL but no SHA",
+ json: jsonDeleteNoError(originalEntry, "SHA"),
+ leaf: nil,
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Invalid entry with bad URL",
+ json: jsonSetNoError(originalEntry, "URL", "not_a_url"),
+ leaf: nil,
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Valid entry but empty leaf",
+ json: originalEntry,
+ leaf: &RekorLeaf{},
+ expectSuccess: false,
+ },
+ }
+
+ for _, tc := range tests {
+ if tc.leaf == nil {
+ leaf, err := ParseRekorLeaf(strings.NewReader(tc.json))
+ if err != nil {
+ t.Errorf("unexpected error in '%v': %v", tc.caseDesc, err)
+ }
+ tc.leaf = &leaf
+ }
+
+ if _, err := ParseRekorEntry(strings.NewReader(tc.json), *(tc.leaf)); (err == nil) != tc.expectSuccess {
+ t.Errorf("Error in test case '%v': %v", tc.caseDesc, err)
+ }
+ }
+
+ if _, err := ParseRekorEntry(&BadReader{}, RekorLeaf{}); err == nil {
+ t.Errorf("No error thrown if io.Reader failed")
+ }
+
+ // valid leaf, invalid JSON
+ leaf, err := ParseRekorLeaf(strings.NewReader(tests[0].json))
+ if err != nil {
+ t.Errorf("unexpected error in '%v': %v", tests[0].caseDesc, err)
+ }
+
+ if _, err := ParseRekorEntry(strings.NewReader("not json"), leaf); err == nil {
+ t.Errorf("No error thrown for invalid JSON but valid leaf")
+ }
+}
+
+func TestRekorLoad(t *testing.T) {
+ type Test struct {
+ caseDesc string
+ entry *RekorEntry
+ expectSuccess bool
+ }
+
+ validLeafBytes, _ := ioutil.ReadFile("../../tests/kernel.json")
+ validLeaf, _ := ParseRekorLeaf(bytes.NewReader(validLeafBytes))
+
+ smallValidFileBytes, _ := ioutil.ReadFile("../../tests/test_file.txt")
+ smallValidLeafBytes, _ := ioutil.ReadFile("../../tests/rekor.json")
+ smallValidLeaf, _ := ParseRekorLeaf(bytes.NewReader(smallValidLeafBytes))
+
+ gzipValidFile := new(bytes.Buffer)
+ gzipW := gzip.NewWriter(gzipValidFile)
+ _, _ = gzipW.Write(smallValidFileBytes)
+ gzipW.Close()
+
+ testServer := httptest.NewServer(http.HandlerFunc(
+ func(w http.ResponseWriter, r *http.Request) {
+ if r.URL.Path[1:] == "premature_close" {
+ return
+ }
+ if r.URL.Path[1:] == "not_found" {
+ w.WriteHeader(http.StatusNotFound)
+ return
+ }
+ if r.URL.Path[1:] == "invalidFile" {
+ w.WriteHeader(http.StatusOK)
+ _, _ = w.Write([]byte("doesn't match"))
+ }
+ if r.URL.Path[1:] == "smallValidFile" {
+ w.WriteHeader(http.StatusOK)
+ _, _ = w.Write(smallValidFileBytes)
+ }
+ if r.URL.Path[1:] == "gzipValidFile" {
+ w.WriteHeader(http.StatusOK)
+ _, _ = w.Write(gzipValidFile.Bytes())
+ }
+ }))
+ defer testServer.Close()
+
+ tests := []Test{
+ {
+ caseDesc: "Invalid entry",
+ entry: &RekorEntry{},
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Invalid URL",
+ entry: &RekorEntry{
+ nil,
+ "not_a_url",
+ RekorLeaf{validLeaf.SHA, validLeaf.Signature, validLeaf.PublicKey, nil, nil},
+ },
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "HTTP Server Closes Prematurely",
+ entry: &RekorEntry{
+ nil,
+ testServer.URL + "/premature_close",
+ RekorLeaf{validLeaf.SHA, validLeaf.Signature, validLeaf.PublicKey, nil, nil},
+ },
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "HTTP Server 404 not found",
+ entry: &RekorEntry{
+ nil,
+ testServer.URL + "/not_found",
+ RekorLeaf{validLeaf.SHA, validLeaf.Signature, validLeaf.PublicKey, nil, nil},
+ },
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Valid small file (to test content_type detection lower limit",
+ entry: &RekorEntry{
+ nil,
+ testServer.URL + "/smallValidFile",
+ RekorLeaf{smallValidLeaf.SHA, smallValidLeaf.Signature, smallValidLeaf.PublicKey, nil, nil},
+ },
+ expectSuccess: true,
+ },
+ {
+ caseDesc: "Valid file with mismatched SHA value",
+ entry: &RekorEntry{
+ nil,
+ testServer.URL + "/smallValidFile",
+ RekorLeaf{"1" + smallValidLeaf.SHA[1:], smallValidLeaf.Signature, smallValidLeaf.PublicKey, nil, nil},
+ },
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Valid file with failed signature validation",
+ entry: &RekorEntry{
+ nil,
+ testServer.URL + "/invalidFile",
+ RekorLeaf{validLeaf.SHA, validLeaf.Signature, validLeaf.PublicKey, nil, nil},
+ },
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Valid gzipped file",
+ entry: &RekorEntry{
+ nil,
+ testServer.URL + "/gzipValidFile",
+ RekorLeaf{smallValidLeaf.SHA, smallValidLeaf.Signature, smallValidLeaf.PublicKey, nil, nil},
+ },
+ expectSuccess: true,
+ },
+ {
+ caseDesc: "Valid file passed in through Data field",
+ entry: &RekorEntry{
+ smallValidFileBytes,
+ "",
+ RekorLeaf{"", smallValidLeaf.Signature, smallValidLeaf.PublicKey, nil, nil},
+ },
+ expectSuccess: true,
+ },
+ }
+
+ for _, tc := range tests {
+ ctx := context.TODO()
+ beforeSHA := tc.entry.SHA
+ if err := tc.entry.Load(ctx); (err == nil) != tc.expectSuccess {
+ t.Errorf("Error in test case '%v': %v", tc.caseDesc, err)
+ }
+ if tc.expectSuccess && beforeSHA == "" && tc.entry.SHA == "" {
+ t.Errorf("Error in test case '%v': SHA not set after call to Load()", tc.caseDesc)
+ }
+ }
+}
+
+func TestRekorLeafMarshalJSON(t *testing.T) {
+ type Test struct {
+ caseDesc string
+ leaf *RekorLeaf
+ expectSuccess bool
+ }
+
+ validLeafBytes, _ := ioutil.ReadFile("../../tests/kernel.json")
+ validLeaf, _ := ParseRekorLeaf(bytes.NewReader(validLeafBytes))
+
+ tests := []Test{
+ {
+ caseDesc: "Valid Leaf",
+ leaf: &RekorLeaf{validLeaf.SHA, validLeaf.Signature, validLeaf.PublicKey, nil, nil},
+ expectSuccess: true,
+ },
+ {
+ caseDesc: "Invalid Leaf, bad sig",
+ leaf: &RekorLeaf{validLeaf.SHA, []byte("not a sig"), validLeaf.PublicKey, nil, nil},
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Invalid Leaf, bad key",
+ leaf: &RekorLeaf{validLeaf.SHA, validLeaf.Signature, []byte("not a key"), nil, nil},
+ expectSuccess: false,
+ },
+ {
+ caseDesc: "Invalid Leaf, empty",
+ leaf: &RekorLeaf{},
+ expectSuccess: false,
+ },
+ }
+
+ for _, tc := range tests {
+ _ = tc.leaf.ValidateLeaf()
+ jsonOutput, err := json.Marshal(tc.leaf)
+ if (err == nil) != tc.expectSuccess {
+ t.Errorf("Error in test case '%v': %v", tc.caseDesc, err)
+ }
+ if tc.expectSuccess {
+ var marshalledLeaf RekorLeaf
+ if err := json.Unmarshal(jsonOutput, &marshalledLeaf); err != nil {
+ t.Errorf("Error in test case '%v': unable to unmarshal JSON returned", tc.caseDesc)
+ }
+ if tc.leaf.SHA != marshalledLeaf.SHA {
+ t.Errorf("Error in test case '%v': output SHA in JSON did not match input", tc.caseDesc)
+ }
+ }
+ }
+}
--
GitLab