Base64 signature

pkg/kritis/attestation/attestation.go

@@ -68,9 +68,9 @@ func VerifyMessageAttestation(pubKey string, sig string, message string) error {
 func GetPlainMessage(pubKey string, sig string) ([]byte, error) {
 	keyring, err := openpgp.ReadArmoredKeyRing(strings.NewReader(pubKey))
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "read armored key ring")
 	}
-	buf := bytes.NewBuffer([]byte(sig))
+	buf := bytes.NewBufferString(sig)
 	armorBlock, err := armor.Decode(buf)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not decode armor signature")

pkg/kritis/metadata/containeranalysis/containeranalysis.go

@@ -17,6 +17,7 @@ limitations under the License.
 package containeranalysis
 
 import (
+	"encoding/base64"
 	"fmt"
 	"strings"
 
@@ -232,7 +233,7 @@ func (c Client) CreateAttestationOccurrence(note *grafeas.Note,
 		return nil, err
 	}
 	pgpSignedAttestation := &attestation.PgpSignedAttestation{
-		Signature: sig,
+		Signature: base64.StdEncoding.EncodeToString([]byte(sig)),
 		KeyId: &attestation.PgpSignedAttestation_PgpKeyId{
 			PgpKeyId: fingerprint,
 		},

pkg/kritis/review/review_test.go

@@ -46,13 +46,13 @@ func TestReviewGAP(t *testing.T) {
 	sMock := func(_, _ string) (*secrets.PGPSigningSecret, error) {
 		return sec, nil
 	}
-	validAtts := []metadata.PGPAttestation{{Signature: sig, KeyID: secFpr}}
+	validAtts := []metadata.PGPAttestation{{Signature: encodeB64(sig), KeyID: secFpr}}
 
 	invalidSig, err := util.CreateAttestationSignature(testutil.IntTestImage, sec)
 	if err != nil {
 		t.Fatalf("unexpected error %v", err)
 	}
-	invalidAtts := []metadata.PGPAttestation{{Signature: invalidSig, KeyID: secFpr}}
+	invalidAtts := []metadata.PGPAttestation{{Signature: encodeB64(invalidSig), KeyID: secFpr}}
 
 	// A policy with a single attestor 'test'.
 	oneGAP := []v1beta1.GenericAttestationPolicy{
@@ -231,7 +231,7 @@ func TestReviewISP(t *testing.T) {
 	sMock := func(_, _ string) (*secrets.PGPSigningSecret, error) {
 		return sec, nil
 	}
-	validAtts := []metadata.PGPAttestation{{Signature: sigVuln, KeyID: secFpr}}
+	validAtts := []metadata.PGPAttestation{{Signature: encodeB64(sigVuln), KeyID: secFpr}}
 	isps := []v1beta1.ImageSecurityPolicy{
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -339,7 +339,7 @@ func TestReviewISP(t *testing.T) {
 			name:              "no vulnz w attestation for cron should verify attestations",
 			image:             noVulnImage,
 			isWebhook:         false,
-			attestations:      []metadata.PGPAttestation{{Signature: sigNoVuln, KeyID: secFpr}},
+			attestations:      []metadata.PGPAttestation{{Signature: encodeB64(sigNoVuln), KeyID: secFpr}},
 			handledViolations: 0,
 			isAttested:        true,
 			shouldAttestImage: false,
@@ -375,6 +375,28 @@ func TestReviewISP(t *testing.T) {
 			shouldAttestImage: false,
 			shouldErr:         false,
 		},
+		{
+			name:      "regression: vulnz w old non-encoded attestation should handle violations",
+			image:     vulnImage,
+			isWebhook: true,
+			// Invalid because not base64-encoded.
+			attestations:      []metadata.PGPAttestation{{Signature: sigVuln, KeyID: secFpr}},
+			handledViolations: 1,
+			isAttested:        false,
+			shouldAttestImage: false,
+			shouldErr:         true,
+		},
+		{
+			name:      "regression: no vulnz w old non-encoded attestation should create new attestation",
+			image:     noVulnImage,
+			isWebhook: true,
+			// Invalid because not base64-encoded.
+			attestations:      []metadata.PGPAttestation{{Signature: sigNoVuln, KeyID: secFpr}},
+			handledViolations: 0,
+			isAttested:        false,
+			shouldAttestImage: true,
+			shouldErr:         false,
+		},
 	}
 	for _, tc := range tests {
 		th := violation.MemoryStrategy{

pkg/kritis/review/validating_transport.go

@@ -17,6 +17,8 @@ limitations under the License.
 package review
 
 import (
+	"encoding/base64"
+
 	"github.com/golang/glog"
 	"github.com/grafeas/kritis/pkg/kritis/apis/kritis/v1beta1"
 	"github.com/grafeas/kritis/pkg/kritis/attestation"
@@ -58,11 +60,16 @@ func (avt *AttestorValidatingTransport) GetValidatedAttestations(image string) (
 		return nil, err
 	}
 	for _, a := range attestations {
-		if err = host.VerifyAttestationSignature(keys[a.KeyID], a.Signature); err != nil {
-			glog.Errorf("Could not find or verify attestation for attestor %s: %s", a.KeyID, err.Error())
-		} else {
-			out = append(out, attestation.ValidatedAttestation{AttestorName: avt.Attestor.Name, Image: image})
+		decoded_sig, err := base64.StdEncoding.DecodeString(a.Signature)
+		if err != nil {
+			glog.Infof("Cannot base64 decode signature: %v", err)
+			continue
+		}
+		if err = host.VerifyAttestationSignature(keys[a.KeyID], string(decoded_sig)); err != nil {
+			glog.Infof("Could not find or verify attestation for attestor %s: %s", a.KeyID, err.Error())
+			continue
 		}
+		out = append(out, attestation.ValidatedAttestation{AttestorName: avt.Attestor.Name, Image: image})
 	}
 	return out, nil
 }

pkg/kritis/review/validating_transport_test.go

@@ -31,6 +31,10 @@ import (
 	"github.com/grafeas/kritis/pkg/kritis/util"
 )
 
+func encodeB64(in string) string {
+	return base64.StdEncoding.EncodeToString([]byte(in))
+}
+
 func TestValidatingTransport(t *testing.T) {
 	successSec, pub := testutil.CreateSecret(t, "test-success")
 	successFpr := successSec.PgpKey.Fingerprint()
@@ -71,31 +75,36 @@ func TestValidatingTransport(t *testing.T) {
 			},
 		}, attestations: []metadata.PGPAttestation{
 			{
-				Signature: sig,
+				Signature: encodeB64(sig),
 				KeyID:     successFpr,
 			}, {
-				Signature: "invalid-sig",
+				Signature: encodeB64("invalid-sig"),
 				KeyID:     successFpr,
 			}}, errorExpected: false, attError: nil},
 		{name: "no valid sig", auth: validAuth, expected: []attestation.ValidatedAttestation{}, attestations: []metadata.PGPAttestation{
 			{
-				Signature: "invalid-sig",
+				Signature: encodeB64("invalid-sig"),
+				KeyID:     successFpr,
+			}}, errorExpected: false, attError: nil},
+		{name: "sig not base64 encoded", auth: validAuth, expected: []attestation.ValidatedAttestation{}, attestations: []metadata.PGPAttestation{
+			{
+				Signature: sig,
 				KeyID:     successFpr,
 			}}, errorExpected: false, attError: nil},
 		{name: "invalid secret", auth: validAuth, expected: []attestation.ValidatedAttestation{}, attestations: []metadata.PGPAttestation{
 			{
-				Signature: "invalid-sig",
+				Signature: encodeB64("invalid-sig"),
 				KeyID:     "invalid-fpr",
 			}}, errorExpected: false, attError: nil},
 		{name: "valid sig over another host", auth: validAuth, expected: []attestation.ValidatedAttestation{}, attestations: []metadata.PGPAttestation{
 			{
-				Signature: anotherSig,
+				Signature: encodeB64(anotherSig),
 				KeyID:     successFpr,
 			}}, errorExpected: false, attError: nil},
 		{name: "attestation fetch error", auth: validAuth, expected: nil, attestations: nil, errorExpected: true, attError: errors.New("can't fetch attestations")},
 		{name: "invalid attestation authority error", auth: invalidAuth, expected: nil, attestations: []metadata.PGPAttestation{
 			{
-				Signature: sig,
+				Signature: encodeB64(sig),
 				KeyID:     successFpr,
 			}}, errorExpected: true, attError: nil},
 	}