From 6e774128237f21d42cb2d87703ffd82589d84d02 Mon Sep 17 00:00:00 2001
From: Mustafa Gezen <mustafa@ctrliq.com>
Date: Sat, 5 Nov 2022 18:32:58 +0100
Subject: [PATCH] Import RPM key to verify signature and stop blocking on
 failure

Previously Keykeeper had a faulty verify check, where `rpm --checksig` didn't actually work because the RPM key was never imported. This would normally be caught but the TaskSignature creation was done after every signature without a transaction. That led to the activity succeeding next launch with either a faulty signed RPM or a correctly signed RPM.

We caught all instances of this by verifying signature of all artifacts during compose, but it was an annoying problem that we would run into occasionally. This should fix that.
---
 peridot/keykeeper/v1/keywarming.go | 25 +++++++++++++++++++++++++
 peridot/keykeeper/v1/sign.go       | 21 ++++++++++++++++-----
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/peridot/keykeeper/v1/keywarming.go b/peridot/keykeeper/v1/keywarming.go
index 640a860..768710d 100644
--- a/peridot/keykeeper/v1/keywarming.go
+++ b/peridot/keykeeper/v1/keywarming.go
@@ -38,6 +38,8 @@ import (
 	"fmt"
 	"github.com/ProtonMail/gopenpgp/v2/crypto"
 	"github.com/google/uuid"
+	"io/ioutil"
+	"os"
 	"os/exec"
 	"peridot.resf.org/peridot/db/models"
 	"peridot.resf.org/utils"
@@ -75,6 +77,24 @@ func (s *Server) importGpgKey(armoredKey string) error {
 	return err
 }
 
+func (s *Server) importRpmKey(publicKey string) error {
+	tmpFile, err := ioutil.TempFile("/tmp", "peridot-key-")
+	if err != nil {
+		return err
+	}
+	defer os.Remove(tmpFile.Name())
+	_, err = tmpFile.Write([]byte(publicKey))
+	if err != nil {
+		return err
+	}
+	cmd := gpgCmdEnv(exec.Command("rpm", "--import", tmpFile.Name()))
+	out, err := logCmdRun(cmd)
+	if err != nil {
+		s.log.Errorf("failed to import rpm key: %s", out.String())
+	}
+	return err
+}
+
 // WarmGPGKey warms up a specific GPG key
 // This involves shelling out to GPG to import the key
 func (s *Server) WarmGPGKey(key string, armoredKey string, gpgKey *crypto.Key, db *models.Key) (*LoadedKey, error) {
@@ -89,6 +109,11 @@ func (s *Server) WarmGPGKey(key string, armoredKey string, gpgKey *crypto.Key, d
 		return nil, err
 	}
 
+	err = s.importRpmKey(db.PublicKey)
+	if err != nil {
+		return nil, err
+	}
+
 	cachedKey := &LoadedKey{
 		keyUuid: db.ID,
 		gpgId:   gpgKey.GetHexKeyID(),
diff --git a/peridot/keykeeper/v1/sign.go b/peridot/keykeeper/v1/sign.go
index 234d1e5..eca5607 100644
--- a/peridot/keykeeper/v1/sign.go
+++ b/peridot/keykeeper/v1/sign.go
@@ -188,6 +188,13 @@ func (s *Server) SignArtifactActivity(ctx context.Context, artifactId string, ke
 
 	switch ext {
 	case ".rpm":
+		beginTx, err := s.db.Begin()
+		if err != nil {
+			s.log.Errorf("failed to begin transaction: %v", err)
+			return nil, status.Error(codes.Internal, "failed to begin transaction")
+		}
+		tx := s.db.UseTransaction(beginTx)
+
 		rpmSign := func() (*keykeeperpb.SignedArtifact, error) {
 			var outBuf bytes.Buffer
 			opts := []string{
@@ -233,7 +240,7 @@ func (s *Server) SignArtifactActivity(ctx context.Context, artifactId string, ke
 			}
 			hash := hex.EncodeToString(hasher.Sum(nil))
 
-			err = s.db.CreateTaskArtifactSignature(artifact.ID.String(), key.keyUuid.String(), hash)
+			err = tx.CreateTaskArtifactSignature(artifact.ID.String(), key.keyUuid.String(), hash)
 			if err != nil {
 				s.log.Errorf("failed to create task artifact signature: %v", err)
 				return nil, fmt.Errorf("failed to create task artifact signature: %v", err)
@@ -257,22 +264,26 @@ func (s *Server) SignArtifactActivity(ctx context.Context, artifactId string, ke
 			err := cmd.Run()
 			if err != nil {
 				s.log.Errorf("failed to verify artifact %s: %v", artifact.Name, err)
+				s.log.Errorf("buf: %s", outBuf.String())
 				return fmt.Errorf("failed to verify artifact %s: %v", artifact.Name, err)
 			}
-			if !strings.Contains(outBuf.String(), "digest signatures OK") {
-				s.log.Errorf("artifact %s not signed(?), retrying", artifact.Name)
-				return fmt.Errorf("artifact %s not signed(?), retrying", artifact.Name)
-			}
 			return nil
 		}
 		res, err := rpmSign()
 		if err != nil {
+			_ = beginTx.Rollback()
 			return nil, err
 		}
 		err = verifySig()
 		if err != nil {
+			_ = beginTx.Rollback()
 			return nil, err
 		}
+		err = beginTx.Commit()
+		if err != nil {
+			s.log.Errorf("failed to commit transaction: %v", err)
+			return nil, status.Error(codes.Internal, "failed to commit transaction")
+		}
 
 		return res, nil
 	default: