From 0414cb5b326c4954664a768c8a4105921e64a647 Mon Sep 17 00:00:00 2001 From: Hayden Blauzvern Date: Wed, 6 Sep 2023 18:29:37 +0000 Subject: [PATCH 1/5] Fail timestamp verification if no root is provided The bug was that we would conditionally check a timestamp if a root was provided. If no root was provided even if a timestamp was provided, then signature verification would succeed. The good news is this will not show a successful signature if the transparency log does not contain the entry too, for timestamp verification. Signed-off-by: Hayden Blauzvern --- pkg/cosign/verify.go | 19 ++++++++++--------- pkg/cosign/verify_test.go | 9 +++++++++ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/pkg/cosign/verify.go b/pkg/cosign/verify.go index dffd8691e1f..2d6cf7383df 100644 --- a/pkg/cosign/verify.go +++ b/pkg/cosign/verify.go @@ -648,14 +648,12 @@ func verifyInternal(ctx context.Context, sig oci.Signature, h v1.Hash, bundleVerified bool, err error) { var acceptableRFC3161Time, acceptableRekorBundleTime *time.Time // Timestamps for the signature we accept, or nil if not applicable. - if co.TSARootCertificates != nil { - acceptableRFC3161Timestamp, err := VerifyRFC3161Timestamp(sig, co) - if err != nil { - return false, fmt.Errorf("unable to verify RFC3161 timestamp bundle: %w", err) - } - if acceptableRFC3161Timestamp != nil { - acceptableRFC3161Time = &acceptableRFC3161Timestamp.Time - } + acceptableRFC3161Timestamp, err := VerifyRFC3161Timestamp(sig, co) + if err != nil { + return false, fmt.Errorf("unable to verify RFC3161 timestamp bundle: %w", err) + } + if acceptableRFC3161Timestamp != nil { + acceptableRFC3161Time = &acceptableRFC3161Timestamp.Time } if !co.IgnoreTlog { @@ -1099,13 +1097,16 @@ func VerifyBundle(sig oci.Signature, co *CheckOpts) (bool, error) { // VerifyRFC3161Timestamp verifies that the timestamp in sig is correctly signed, and if so, // returns the timestamp value. -// It returns (nil, nil) if there is no timestamp, or (nil, err) if there is an invalid timestamp. +// It returns (nil, nil) if there is no timestamp, or (nil, err) if there is an invalid timestamp or if +// no root is provided with a timestamp. func VerifyRFC3161Timestamp(sig oci.Signature, co *CheckOpts) (*timestamp.Timestamp, error) { ts, err := sig.RFC3161Timestamp() if err != nil { return nil, err } else if ts == nil { return nil, nil + } else if co.TSARootCertificates == nil { + return nil, errors.New("no TSA root certificate(s) provided to verify timestamp") } b64Sig, err := sig.Base64Signature() diff --git a/pkg/cosign/verify_test.go b/pkg/cosign/verify_test.go index bf08413b831..d5dcb4cf6fb 100644 --- a/pkg/cosign/verify_test.go +++ b/pkg/cosign/verify_test.go @@ -1420,4 +1420,13 @@ func TestVerifyRFC3161Timestamp(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "hashed messages don't match") { t.Fatalf("expected error verifying mismatched signatures, got: %v", err) } + + // failure without root certificate + _, err = VerifyRFC3161Timestamp(ociSig, &CheckOpts{ + TSACertificate: leaves[0], + TSAIntermediateCertificates: intermediates, + }) + if err == nil || !strings.Contains(err.Error(), "no TSA root certificate(s) provided to verify timestamp") { + t.Fatalf("expected error verifying without a root certificate, got: %v", err) + } } From d2d395b2c7cc3ef92ca7fcc750c67eef3dd97e72 Mon Sep 17 00:00:00 2001 From: Hayden Blauzvern Date: Wed, 6 Sep 2023 18:37:25 +0000 Subject: [PATCH 2/5] Switch to switch Signed-off-by: Hayden Blauzvern --- pkg/cosign/verify.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/cosign/verify.go b/pkg/cosign/verify.go index 2d6cf7383df..747f356b2a5 100644 --- a/pkg/cosign/verify.go +++ b/pkg/cosign/verify.go @@ -1101,11 +1101,12 @@ func VerifyBundle(sig oci.Signature, co *CheckOpts) (bool, error) { // no root is provided with a timestamp. func VerifyRFC3161Timestamp(sig oci.Signature, co *CheckOpts) (*timestamp.Timestamp, error) { ts, err := sig.RFC3161Timestamp() - if err != nil { + switch { + case err != nil: return nil, err - } else if ts == nil { + case ts == nil: return nil, nil - } else if co.TSARootCertificates == nil { + case co.TSARootCertificates == nil: return nil, errors.New("no TSA root certificate(s) provided to verify timestamp") } From 85f1d4bbb67192584e802e7cfb56fb7fd5af1e13 Mon Sep 17 00:00:00 2001 From: Hayden Blauzvern Date: Wed, 6 Sep 2023 19:56:57 +0000 Subject: [PATCH 3/5] Fix e2e test Signed-off-by: Hayden Blauzvern --- test/e2e_tsa_mtls.sh | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/e2e_tsa_mtls.sh b/test/e2e_tsa_mtls.sh index 8ee32b09098..084bd306718 100755 --- a/test/e2e_tsa_mtls.sh +++ b/test/e2e_tsa_mtls.sh @@ -31,6 +31,7 @@ TIMESTAMP_SERVER_CERT=$CERT_BASE/tsa-mtls-server.crt TIMESTAMP_SERVER_KEY=$CERT_BASE/tsa-mtls-server.key TIMESTAMP_SERVER_NAME="server.example.com" TIMESTAMP_SERVER_URL=https://localhost:3000/api/v1/timestamp +TIMESTAMP_CHAIN_FILE="timestamp-chain.pem" set +e COSIGN_CLI=./cosign @@ -50,6 +51,12 @@ timestamp-server serve --disable-ntp-monitoring --tls-host 0.0.0.0 --tls-port 30 --scheme https --tls-ca $TIMESTAMP_CACERT --tls-key $TIMESTAMP_SERVER_KEY \ --tls-certificate $TIMESTAMP_SERVER_CERT & +sleep 1 +curl -k -s --key test/testdata/tsa-mtls-client.key \ + --cert test/testdata/tsa-mtls-client.crt \ + --cacert test/testdata/tsa-mtls-ca.crt https://localhost:3000/api/v1/timestamp/certchain \ + > $TIMESTAMP_CHAIN_FILE +echo "DONE: $(ls -l $TIMESTAMP_CHAIN_FILE)" IMG=${IMAGE_URI_DIGEST:-} if [[ "$#" -ge 1 ]]; then @@ -85,8 +92,8 @@ rm -f key.pem import-cosign.* echo "cosign verify:" $COSIGN_CLI verify --insecure-ignore-tlog --insecure-ignore-sct --check-claims=true \ --certificate-identity-regexp 'xyz@nosuchprovider.com' --certificate-oidc-issuer-regexp '.*' \ - --certificate-chain cacert.pem $IMG + --certificate-chain cacert.pem --timestamp-certificate-chain $TIMESTAMP_CHAIN_FILE $IMG # cleanup -rm -fr ca-key.pem cacert.pem cert.pem /tmp/timestamp-authority +rm -fr ca-key.pem cacert.pem cert.pem timestamp-chain.pem /tmp/timestamp-authority pkill -f 'timestamp-server' From 142c3e9413cae221ae378f90d09ec7f9a4c9a019 Mon Sep 17 00:00:00 2001 From: Hayden Blauzvern Date: Wed, 6 Sep 2023 20:12:42 +0000 Subject: [PATCH 4/5] Fix e2e test Signed-off-by: Hayden Blauzvern --- test/e2e_tsa_mtls.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e_tsa_mtls.sh b/test/e2e_tsa_mtls.sh index 084bd306718..71b2294c53b 100755 --- a/test/e2e_tsa_mtls.sh +++ b/test/e2e_tsa_mtls.sh @@ -31,7 +31,7 @@ TIMESTAMP_SERVER_CERT=$CERT_BASE/tsa-mtls-server.crt TIMESTAMP_SERVER_KEY=$CERT_BASE/tsa-mtls-server.key TIMESTAMP_SERVER_NAME="server.example.com" TIMESTAMP_SERVER_URL=https://localhost:3000/api/v1/timestamp -TIMESTAMP_CHAIN_FILE="timestamp-chain.pem" +TIMESTAMP_CHAIN_FILE="timestamp-chain" set +e COSIGN_CLI=./cosign @@ -95,5 +95,5 @@ $COSIGN_CLI verify --insecure-ignore-tlog --insecure-ignore-sct --check-claims=t --certificate-chain cacert.pem --timestamp-certificate-chain $TIMESTAMP_CHAIN_FILE $IMG # cleanup -rm -fr ca-key.pem cacert.pem cert.pem timestamp-chain.pem /tmp/timestamp-authority +rm -fr ca-key.pem cacert.pem cert.pem timestamp-chain /tmp/timestamp-authority pkill -f 'timestamp-server' From 65843b4a70e992f1495df7754dd3bff6e47956b5 Mon Sep 17 00:00:00 2001 From: Hayden B Date: Thu, 7 Sep 2023 17:01:50 -0700 Subject: [PATCH 5/5] Update e2e_tsa_mtls.sh --- test/e2e_tsa_mtls.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/test/e2e_tsa_mtls.sh b/test/e2e_tsa_mtls.sh index 71b2294c53b..adb0d062b77 100755 --- a/test/e2e_tsa_mtls.sh +++ b/test/e2e_tsa_mtls.sh @@ -74,7 +74,6 @@ echo "IMG (IMAGE_URI_DIGEST): $IMG, TIMESTAMP_SERVER_URL: $TIMESTAMP_SERVER_URL" rm -f *.pem import-cosign.* key.pem - # use gencert to generate CA, keys and certificates echo "generate keys and certificates with gencert"