From 81291d616e4849e39f4fa1c1fd3ea7c928b30dd7 Mon Sep 17 00:00:00 2001 From: Susan Shi Date: Mon, 11 Dec 2023 16:31:12 -0800 Subject: [PATCH] feat: sbom verifier improvements (#1205) --- Makefile | 6 +-- charts/ratify/README.md | 2 +- .../config_v1beta1_verifier_sbom_deny.yaml | 14 ++++++ plugins/verifier/sbom/sbom.go | 23 +++++----- plugins/verifier/sbom/sbom_test.go | 33 +++++++++----- plugins/verifier/sbom/utils/spdxutils.go | 25 +++++++++++ plugins/verifier/sbom/utils/spdxutils_test.go | 43 +++++++++++++++++++ test/bats/plugin-test.bats | 8 +++- 8 files changed, 127 insertions(+), 27 deletions(-) create mode 100644 config/samples/config_v1beta1_verifier_sbom_deny.yaml diff --git a/Makefile b/Makefile index 119ba8967..3362a14e2 100644 --- a/Makefile +++ b/Makefile @@ -392,15 +392,15 @@ e2e-sbom-setup: ${GITHUB_WORKSPACE}/bin/oras attach \ --artifact-type application/spdx+json \ ${TEST_REGISTRY}/sbom:v0 \ - .staging/sbom/_manifest/spdx_2.2/manifest.spdx.json:application/spdx+json + .staging/sbom/_manifest/spdx_2.2/manifest.spdx.json ${GITHUB_WORKSPACE}/bin/oras attach \ --artifact-type application/spdx+json \ ${TEST_REGISTRY}/sbom:unsigned \ - .staging/sbom/_manifest/spdx_2.2/manifest.spdx.json:application/spdx+json + .staging/sbom/_manifest/spdx_2.2/manifest.spdx.json ${GITHUB_WORKSPACE}/bin/oras attach \ --artifact-type application/spdx+json \ ${TEST_REGISTRY}/all:v0 \ - .staging/sbom/_manifest/spdx_2.2/manifest.spdx.json:application/spdx+json + .staging/sbom/_manifest/spdx_2.2/manifest.spdx.json # Push Signature to sbom .staging/notation/notation sign -u ${TEST_REGISTRY_USERNAME} -p ${TEST_REGISTRY_PASSWORD} ${TEST_REGISTRY}/sbom@`oras discover -o json --artifact-type application/spdx+json ${TEST_REGISTRY}/sbom:v0 | jq -r ".manifests[0].digest"` diff --git a/charts/ratify/README.md b/charts/ratify/README.md index 407577496..1795a6def 100644 --- a/charts/ratify/README.md +++ b/charts/ratify/README.md @@ -58,7 +58,7 @@ $ helm upgrade -n gatekeeper-system [RELEASE_NAME] ratify/ratify | sbom.enabled | Enables/disables installation of sbom verification configuration | `false` | | sbom.notaryProjectSignatureRequired | requires validation of sbom notation signature | `false` | | sbom.disallowedLicenses | list of disallowed licenses | [] | -| sbom.disallowedPackages | list of disallowed packages defined by package name and version | [] | +| sbom.disallowedPackages | list of disallowed packages defined by package name and version. For example: --set sbom.disallowedPackages[0].name="busybox" --set sbom.disallowedPackages[0].version="1.36.1-r0" | [] | | resources.limits.cpu | CPU limits of Ratify Deployment | `1000m` | | resources.limits.memory | Memory limits of Ratify Deployment | `512Mi` | | resources.requests.cpu | CPU request of Ratify Deployment | `600m` | diff --git a/config/samples/config_v1beta1_verifier_sbom_deny.yaml b/config/samples/config_v1beta1_verifier_sbom_deny.yaml new file mode 100644 index 000000000..0106e40c8 --- /dev/null +++ b/config/samples/config_v1beta1_verifier_sbom_deny.yaml @@ -0,0 +1,14 @@ +apiVersion: config.ratify.deislabs.io/v1beta1 +kind: Verifier +metadata: + name: verifier-sbom +spec: + name: sbom + artifactTypes: application/spdx+json + parameters: + disallowedLicenses: + - Zlib + disallowedPackages: + - name: musl-utils + version: 1.2.3-r4 + nestedReferences: application/vnd.cncf.notary.signature \ No newline at end of file diff --git a/plugins/verifier/sbom/sbom.go b/plugins/verifier/sbom/sbom.go index a396d6ce8..5da724b41 100644 --- a/plugins/verifier/sbom/sbom.go +++ b/plugins/verifier/sbom/sbom.go @@ -86,8 +86,8 @@ func VerifyReference(args *skel.CmdArgs, subjectReference common.Reference, refe Name: input.Name, Type: verifierType, IsSuccess: false, - Message: fmt.Sprintf("Error fetching reference manifest for subject: %s reference descriptor: %v", subjectReference, referenceDescriptor.Descriptor), - }, err + Message: fmt.Sprintf("Error fetching reference manifest for subject: %s reference descriptor: %v, err: %v", subjectReference, referenceDescriptor.Descriptor, err), + }, nil } if len(referenceManifest.Blobs) == 0 { @@ -107,13 +107,13 @@ func VerifyReference(args *skel.CmdArgs, subjectReference common.Reference, refe Name: input.Name, Type: verifierType, IsSuccess: false, - Message: fmt.Sprintf("Error fetching blob for subject: %s digest: %s", subjectReference, blobDesc.Digest), - }, err + Message: fmt.Sprintf("Error fetching blob for subject: %s digest: %s, err: %v", subjectReference, blobDesc.Digest, err), + }, nil } switch artifactType { case SpdxJSONMediaType: - return processSpdxJSONMediaType(input.Name, verifierType, refBlob, input.DisallowedLicenses, input.DisallowedPackages) + return processSpdxJSONMediaType(input.Name, verifierType, refBlob, input.DisallowedLicenses, input.DisallowedPackages), nil default: return &verifier.VerifierResult{ Name: input.Name, @@ -159,7 +159,7 @@ func loadDisallowedPackagesMap(packages []utils.PackageInfo) (map[utils.PackageI } // parse through the spdx blob and returns the verifier result -func processSpdxJSONMediaType(name string, verifierType string, refBlob []byte, disallowedLicenses []string, disallowedPackages []utils.PackageInfo) (*verifier.VerifierResult, error) { +func processSpdxJSONMediaType(name string, verifierType string, refBlob []byte, disallowedLicenses []string, disallowedPackages []utils.PackageInfo) *verifier.VerifierResult { var err error var spdxDoc *v2_3.Document if spdxDoc, err = jsonLoader.Read(bytes.NewReader(refBlob)); spdxDoc != nil && err == nil { @@ -181,8 +181,8 @@ func processSpdxJSONMediaType(name string, verifierType string, refBlob []byte, Name: name, IsSuccess: false, Extensions: extensionData, - Message: "SBOM validation failed.", - }, err + Message: "SBOM validation failed. Please review extensions data for license and package violation found.", + } } } @@ -194,14 +194,14 @@ func processSpdxJSONMediaType(name string, verifierType string, refBlob []byte, CreationInfo: spdxDoc.CreationInfo, }, Message: "SBOM verification success. No license or package violation found.", - }, nil + } } return &verifier.VerifierResult{ Name: name, Type: verifierType, IsSuccess: false, Message: fmt.Sprintf("SBOM failed to parse: %v", err), - }, err + } } // iterate through all package info and check against the deny list @@ -213,8 +213,7 @@ func filterDisallowedPackages(packageLicenses []utils.PackageLicense, disallowed for _, packageInfo := range packageLicenses { // if license contains disallowed, add to violation for _, disallowed := range disallowedLicense { - license := packageInfo.License - if license != "" && strings.Contains(strings.ToLower(license), strings.ToLower(disallowed)) { + if utils.ContainsLicense(strings.ToLower(packageInfo.License), strings.ToLower(disallowed)) { violationLicense = append(violationLicense, packageInfo) } } diff --git a/plugins/verifier/sbom/sbom_test.go b/plugins/verifier/sbom/sbom_test.go index 0a5c45159..042c2c378 100644 --- a/plugins/verifier/sbom/sbom_test.go +++ b/plugins/verifier/sbom/sbom_test.go @@ -17,6 +17,7 @@ package main import ( "os" "path/filepath" + "strings" "testing" "github.com/deislabs/ratify/plugins/verifier/sbom/utils" @@ -27,10 +28,7 @@ func TestProcessSPDXJsonMediaType(t *testing.T) { if err != nil { t.Fatalf("error reading %s", filepath.Join("testdata", "bom.json")) } - vr, err := processSpdxJSONMediaType("test", "", b, nil, nil) - if err != nil { - t.Fatalf("expected to process spdx json file: %s", filepath.Join("testdata", "bom.json")) - } + vr := processSpdxJSONMediaType("test", "", b, nil, nil) if !vr.IsSuccess { t.Fatalf("expected to successfully verify schema") } @@ -41,8 +39,9 @@ func TestProcessInvalidSPDXJsonMediaType(t *testing.T) { if err != nil { t.Fatalf("error reading %s", filepath.Join("testdata", "invalid-bom.json")) } - _, err = processSpdxJSONMediaType("test", "", b, nil, nil) - if err == nil { + report := processSpdxJSONMediaType("test", "", b, nil, nil) + + if !strings.Contains(report.Message, "SBOM failed to parse") { t.Fatalf("expected to have an error processing spdx json file: %s", filepath.Join("testdata", "bom.json")) } } @@ -98,6 +97,12 @@ func TestGetViolations(t *testing.T) { expectedPackageViolations []utils.PackageLicense enabled bool }{ + { + description: "disallowed packages with no version", + disallowedLicenses: []string{"MPL"}, + expectedLicenseViolations: nil, + expectedPackageViolations: nil, + }, { description: "disallowed packages with no version", disallowedPackages: []utils.PackageInfo{disallowedPackageNoVersion}, @@ -113,7 +118,6 @@ func TestGetViolations(t *testing.T) { }, { description: "invalid disallow package", - disallowedLicenses: []string{"BSD-3-Clause", "Zlib"}, disallowedPackages: []utils.PackageInfo{disallowedPackageNoName}, expectedLicenseViolations: []utils.PackageLicense{}, expectedPackageViolations: []utils.PackageLicense{}, @@ -151,9 +155,18 @@ func TestGetViolations(t *testing.T) { for _, tc := range cases { t.Run("test scenario", func(t *testing.T) { - report, err := processSpdxJSONMediaType("test", "", b, tc.disallowedLicenses, tc.disallowedPackages) - if err != nil { - t.Fatalf("unexpected error processing spdx json file: %s", filepath.Join("testdata", "bom.json")) + report := processSpdxJSONMediaType("test", "", b, tc.disallowedLicenses, tc.disallowedPackages) + + if len(tc.expectedPackageViolations) != 0 || len(tc.expectedLicenseViolations) != 0 { + if report.IsSuccess { + t.Fatalf("Test %s failed. Expected IsSuccess: true, got: false", tc.description) + } + } + + if len(tc.expectedPackageViolations) == 0 && len(tc.expectedLicenseViolations) == 0 { + if !report.IsSuccess { + t.Fatalf("Test %s failed. Expected IsSuccess: false, got: true", tc.description) + } } if len(tc.expectedPackageViolations) != 0 { diff --git a/plugins/verifier/sbom/utils/spdxutils.go b/plugins/verifier/sbom/utils/spdxutils.go index a76b12da8..282f28c66 100644 --- a/plugins/verifier/sbom/utils/spdxutils.go +++ b/plugins/verifier/sbom/utils/spdxutils.go @@ -16,6 +16,8 @@ limitations under the License. package utils import ( + "strings" + "github.com/spdx/tools-golang/spdx" ) @@ -31,3 +33,26 @@ func GetPackageLicenses(doc spdx.Document) []PackageLicense { } return output } + +// returns true if the licenseExpression contains the disallowed license +// this implements a whole word match +func ContainsLicense(spdxLicenseExpression string, disallowed string) bool { + if len(spdxLicenseExpression) == 0 { + return false + } + + // if the licenseExpression is exactly the same as the disallowed license, return true + if spdxLicenseExpression == disallowed { + return true + } + + disallowed1 := disallowed + " " + disallowed2 := " " + disallowed + + // look for whole word match + if strings.Contains(spdxLicenseExpression, disallowed1) || strings.Contains(spdxLicenseExpression, disallowed2) { + return true + } + + return false +} diff --git a/plugins/verifier/sbom/utils/spdxutils_test.go b/plugins/verifier/sbom/utils/spdxutils_test.go index 897e70d3d..48d37ac3a 100644 --- a/plugins/verifier/sbom/utils/spdxutils_test.go +++ b/plugins/verifier/sbom/utils/spdxutils_test.go @@ -33,3 +33,46 @@ func TestGetPackageLicenses(t *testing.T) { t.Fatalf("unexpected packages count, expected 16") } } + +func TestContainsLicense(t *testing.T) { + tests := []struct { + name string + spdxLicenseExpression string + disallowed string + expected bool + }{ + { + name: "exact match", + spdxLicenseExpression: "MIT", + disallowed: "MIT", + expected: true, + }, + { + name: "exact match with space", + spdxLicenseExpression: "MPL-2.0 AND LicenseRef-AND AND MIT", + disallowed: "MPL", + expected: false, + }, + { + name: "exact match with space", + spdxLicenseExpression: "MPL-2.0 AND LicenseRef-AND AND MIT", + disallowed: "MPL-2.0", + expected: true, + }, + { + name: "exact match with space", + spdxLicenseExpression: "MIT AND LicenseRef-AND AND MPL-2.0", + disallowed: "MPL-2.0", + expected: true, + }, + } + + for _, tt := range tests { + t.Run("test scenario", func(t *testing.T) { + result := ContainsLicense(tt.spdxLicenseExpression, tt.disallowed) + if result != tt.expected { + t.Fatalf("expected %t, got %t", tt.expected, result) + } + }) + } +} diff --git a/test/bats/plugin-test.bats b/test/bats/plugin-test.bats index 3d07ab0c0..c2c744939 100644 --- a/test/bats/plugin-test.bats +++ b/test/bats/plugin-test.bats @@ -137,9 +137,15 @@ SLEEP_TIME=1 assert_success sleep 5 - run kubectl apply -f ./config/samples/config_v1beta1_verifier_sbom.yaml + run kubectl apply -f ./config/samples/config_v1beta1_verifier_sbom_deny.yaml sleep 5 run kubectl run sbom --namespace default --image=registry:5000/sbom:v0 + assert_failure + + run kubectl apply -f ./config/samples/config_v1beta1_verifier_sbom.yaml + # wait for the httpserver cache to be invalidated + sleep 15 + run kubectl run sbom --namespace default --image=registry:5000/sbom:v0 assert_success run kubectl delete verifiers.config.ratify.deislabs.io/verifier-sbom