refactor pgp types a bit to use the openpgp.Entity type

---
type: change
message: |-
  refactor pgp types a bit to use the openpgp.Entity type

  By using Entity the pgp code will have access to the public key's identity
  information, which will help in making more readable commit messages from
  anonymous users.
change_hash: AAJCn1/lybheXWv8Rfy2Y2r4UF9seGptC6uvFn894c8Y
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl6TSX4ACgkQlcRvpqQRSKzCpQ/+I/8BmEtsbwepiGZiIFg0HZK+z4CR5DWLpKQg3w5+NDSfzY9dkdwqbt0ohPyFmLXZz7s1cn/iU0OOb59fUPmH0TklMedY+l1q5XDNZ160l5494QXNXZDKMGC2QIjst+Fu9Qwys7U+1pTu8rN3K1x/ZQCCJvDVFH13jj7bIna2ltxhsqNY6OiVUc5EJsZHbNAgFXqYJNt4XbC+9QcOyDyyYytmJnjEX2oquj7skac9uyBjjJGkqA70sjD4yn44iFqEpFuJAqFuEJDj1D4bkt7Ib+syi/eMPZgDvHRtMGAxl+fBU2OAG8GIlxveYqeUejCp8SYoat+0Tdoi3lT/wj2lTQRtMBeOGUccpsOMRI1t+g5ggKx3YcQgbKLixuAgl649fnX7fmBLB3IuG5PdgnTqjQBPkTBYhkR80fDSg3nfuh+1TVDGiW6C8HtJRlFWWDRcyaYhcWZwKXVTMm7xtgNJYwpLeDmuTCJV6kvldxmBZEmfrQ/1u+wKG2/kCun9stSHdEuuZVM8sv4ECME4C+1fTNnEkzJhHvULV39b41ODZolxNj8ZRa1qt8emC1Hw6ZUjdkA0gXS70l8tfOtQunPTPLDGzNZNYNw8H2JqEM3O6NLIzwEUJVYOA6BmibQj5I7QMSvOS2PpD9KDKIR2vXNsk1yUqdpu9PmLhcclrMqEGOc=
  account: mediocregopher

cmd/dehub/cmd_commit.go

@@ -51,7 +51,7 @@ func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) {
 			}
 		} else {
 			var err error
-			if sigInt, err = sigcred.SignifierPGPFromKeyID(*pgpKeyID, true); err != nil {
+			if sigInt, err = sigcred.LoadSignifierPGP(*pgpKeyID, true); err != nil {
 				return fmt.Errorf("loading pgp key %q: %w", *pgpKeyID, err)
 			}
 		}

commit_change_test.go

@@ -110,7 +110,7 @@ func TestCombineCommitChanges(t *testing.T) {
 
 	// add a toot user and modify the access controls such that both accounts
 	// are required for the main branch
-	tootSig, tootPubKeyBody := sigcred.SignifierPGPTmp("toot", h.rand)
+	tootSig, tootPubKeyBody := sigcred.TestSignifierPGP("toot", h.rand)
 	h.cfg.Accounts = append(h.cfg.Accounts, Account{
 		ID: "toot",
 		Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{

commit_credential_test.go

@@ -13,7 +13,7 @@ func TestCredentialCommitVerify(t *testing.T) {
 
 	// create a new account and modify the config so that that account is only
 	// allowed to add verifications to a single branch
-	tootSig, tootPubKeyBody := sigcred.SignifierPGPTmp("toot", h.rand)
+	tootSig, tootPubKeyBody := sigcred.TestSignifierPGP("toot", h.rand)
 	h.cfg.Accounts = append(h.cfg.Accounts, Account{
 		ID: "toot",
 		Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{

commit_test.go

@@ -21,7 +21,7 @@ func TestConfigChange(t *testing.T) {
 
 	// create a new account and add it to the configuration. That commit should
 	// not be verifiable, though
-	newSig, newPubKeyBody := sigcred.SignifierPGPTmp("toot", h.rand)
+	newSig, newPubKeyBody := sigcred.TestSignifierPGP("toot", h.rand)
 	h.cfg.Accounts = append(h.cfg.Accounts, Account{
 		ID: "toot",
 		Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{
@@ -88,7 +88,7 @@ func TestMainAncestryRequirement(t *testing.T) {
 
 func TestAnonymousCommits(t *testing.T) {
 	h := newHarness(t)
-	anonSig, anonPubKeyBody := sigcred.SignifierPGPTmp("", h.rand)
+	anonSig, anonPubKeyBody := sigcred.TestSignifierPGP("", h.rand)
 
 	h.cfg.AccessControls = []accessctl.AccessControl{{
 		Action: accessctl.ActionAllow,

repo_test.go

@@ -26,7 +26,7 @@ type harness struct {
 
 func newHarness(t *testing.T) *harness {
 	rand := rand.New(rand.NewSource(0xb4eadb01))
-	sig, pubKeyBody := sigcred.SignifierPGPTmp("root", rand)
+	sig, pubKeyBody := sigcred.TestSignifierPGP("root", rand)
 	pubKeyPath := filepath.Join(DehubDir, "root.asc")
 
 	cfg := &Config{

sigcred/credential_test.go

@@ -20,7 +20,7 @@ func TestSelfVerifyingCredentials(t *testing.T) {
 		{
 			descr: "pgp sig no body",
 			mkCred: func(toSign []byte) (Credential, error) {
-				privKey, _ := SignifierPGPTmp("", rand)
+				privKey, _ := TestSignifierPGP("", rand)
 				return privKey.Sign(nil, toSign)
 			},
 			expErr: true,
@@ -28,7 +28,7 @@ func TestSelfVerifyingCredentials(t *testing.T) {
 		{
 			descr: "pgp sig with body",
 			mkCred: func(toSign []byte) (Credential, error) {
-				privKey, pubKeyBody := SignifierPGPTmp("", rand)
+				privKey, pubKeyBody := TestSignifierPGP("", rand)
 				cred, err := privKey.Sign(nil, toSign)
 				cred.PGPSignature.PubKeyBody = string(pubKeyBody)
 				return cred, err

sigcred/pgp.go

@@ -3,20 +3,19 @@ package sigcred
 import (
 	"bytes"
 	"crypto"
-	"crypto/ecdsa"
-	"crypto/elliptic"
 	"crypto/sha256"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"os/exec"
 	"path/filepath"
 	"strings"
-	"time"
 
 	"dehub.dev/src/dehub.git/fs"
 	"dehub.dev/src/dehub.git/yamlutil"
 
+	"golang.org/x/crypto/openpgp"
 	"golang.org/x/crypto/openpgp/armor"
 	"golang.org/x/crypto/openpgp/packet"
 )
@@ -42,39 +41,60 @@ func (c *CredentialPGPSignature) SelfVerify(data []byte) error {
 	return sig.Verify(nil, data, Credential{PGPSignature: c})
 }
 
-type pgpPubKey struct {
-	pubKey *packet.PublicKey
+type pgpKey struct {
+	entity *openpgp.Entity
 }
 
-func newPGPPubKey(r io.Reader) (pgpPubKey, error) {
+func newPGPPubKey(r io.Reader) (pgpKey, error) {
 	// TODO support non-armored keys as well
 	block, err := armor.Decode(r)
 	if err != nil {
-		return pgpPubKey{}, fmt.Errorf("could not decode armored PGP public key: %w", err)
+		return pgpKey{}, fmt.Errorf("could not decode armored PGP public key: %w", err)
 	}
 
-	pkt, err := packet.Read(block.Body)
+	entity, err := openpgp.ReadEntity(packet.NewReader(block.Body))
 	if err != nil {
-		return pgpPubKey{}, fmt.Errorf("could not read PGP public key: %w", err)
+		return pgpKey{}, fmt.Errorf("could not read PGP public key: %w", err)
 	}
+	return pgpKey{entity: entity}, nil
+}
 
-	pubKey, ok := pkt.(*packet.PublicKey)
-	if !ok {
-		return pgpPubKey{}, fmt.Errorf("packet is not a public key, it's a %T", pkt)
+func (s pgpKey) Sign(_ fs.FS, data []byte) (Credential, error) {
+	if s.entity.PrivateKey == nil {
+		return Credential{}, errors.New("private key not loaded")
+	}
+
+	h := sha256.New()
+	h.Write(data)
+	var sig packet.Signature
+	sig.Hash = crypto.SHA256
+	sig.PubKeyAlgo = s.entity.PrimaryKey.PubKeyAlgo
+	if err := sig.Sign(h, s.entity.PrivateKey, nil); err != nil {
+		return Credential{}, fmt.Errorf("signing data: %w", err)
+	}
+
+	body := new(bytes.Buffer)
+	if err := sig.Serialize(body); err != nil {
+		return Credential{}, fmt.Errorf("serializing signature: %w", err)
 	}
 
-	return pgpPubKey{pubKey: pubKey}, nil
+	return Credential{
+		PGPSignature: &CredentialPGPSignature{
+			PubKeyID: s.entity.PrimaryKey.KeyIdString(),
+			Body:     body.Bytes(),
+		},
+	}, nil
 }
 
-func (s pgpPubKey) Signed(_ fs.FS, cred Credential) (bool, error) {
+func (s pgpKey) Signed(_ fs.FS, cred Credential) (bool, error) {
 	if cred.PGPSignature == nil {
 		return false, nil
 	}
 
-	return cred.PGPSignature.PubKeyID == s.pubKey.KeyIdString(), nil
+	return cred.PGPSignature.PubKeyID == s.entity.PrimaryKey.KeyIdString(), nil
 }
 
-func (s pgpPubKey) Verify(_ fs.FS, data []byte, cred Credential) error {
+func (s pgpKey) Verify(_ fs.FS, data []byte, cred Credential) error {
 	credSig := cred.PGPSignature
 	if credSig == nil {
 		return fmt.Errorf("SignifierPGPFile cannot verify %+v", cred)
@@ -95,15 +115,15 @@ func (s pgpPubKey) Verify(_ fs.FS, data []byte, cred Credential) error {
 	// package expects you to do it yourself.
 	h := sigPkt.Hash.New()
 	h.Write(data)
-	return s.pubKey.VerifySignature(h, sigPkt)
+	return s.entity.PrimaryKey.VerifySignature(h, sigPkt)
 }
 
-func (s pgpPubKey) encode() ([]byte, error) {
+func (s pgpKey) MarshalBinary() ([]byte, error) {
 	body := new(bytes.Buffer)
 	armorEncoder, err := armor.Encode(body, "PGP PUBLIC KEY", nil)
 	if err != nil {
 		return nil, fmt.Errorf("error initializing armor encoder: %w", err)
-	} else if err := s.pubKey.Serialize(armorEncoder); err != nil {
+	} else if err := s.entity.Serialize(armorEncoder); err != nil {
 		return nil, fmt.Errorf("error encoding public key: %w", err)
 	} else if err := armorEncoder.Close(); err != nil {
 		return nil, fmt.Errorf("error closing armor encoder: %w", err)
@@ -111,68 +131,27 @@ func (s pgpPubKey) encode() ([]byte, error) {
 	return body.Bytes(), nil
 }
 
-func (s pgpPubKey) asSignfier() (SignifierPGP, error) {
-	body, err := s.encode()
-	if err != nil {
-		return SignifierPGP{}, err
-	}
-
-	return SignifierPGP{
-		Body: string(body),
-	}, nil
-}
-
-type pgpPrivKey struct {
-	pgpPubKey
-	privKey *packet.PrivateKey
-}
-
-// SignifierPGPTmp returns a direct implementation of the SignifierInterface
+// TestSignifierPGP returns a direct implementation of the SignifierInterface
 // which uses a random private key generated in memory, as well as an armored
 // version of its public key.
-func SignifierPGPTmp(accountID string, randReader io.Reader) (SignifierInterface, []byte) {
-	rawPrivKey, err := ecdsa.GenerateKey(elliptic.P521(), randReader)
+//
+// NOTE that the key returned is very weak, and should only be used for tests.
+func TestSignifierPGP(accountID string, randReader io.Reader) (SignifierInterface, []byte) {
+	entity, err := openpgp.NewEntity(accountID, "", accountID+"@example.com", &packet.Config{
+		Rand:    randReader,
+		RSABits: 512,
+	})
 	if err != nil {
 		panic(err)
 	}
 
-	privKeyRaw := packet.NewECDSAPrivateKey(time.Now(), rawPrivKey)
-	privKey := pgpPrivKey{
-		pgpPubKey: pgpPubKey{
-			pubKey: &privKeyRaw.PublicKey,
-		},
-		privKey: privKeyRaw,
-	}
-
-	pubKeyBody, err := privKey.pgpPubKey.encode()
+	pgpKey := pgpKey{entity: entity}
+	pubKeyBody, err := pgpKey.MarshalBinary()
 	if err != nil {
 		panic(err)
 	}
 
-	return accountSignifier(accountID, privKey), pubKeyBody
-}
-
-func (s pgpPrivKey) Sign(_ fs.FS, data []byte) (Credential, error) {
-	h := sha256.New()
-	h.Write(data)
-	var sig packet.Signature
-	sig.Hash = crypto.SHA256
-	sig.PubKeyAlgo = s.pubKey.PubKeyAlgo
-	if err := sig.Sign(h, s.privKey, nil); err != nil {
-		return Credential{}, fmt.Errorf("failed to sign data: %w", err)
-	}
-
-	body := new(bytes.Buffer)
-	if err := sig.Serialize(body); err != nil {
-		return Credential{}, fmt.Errorf("failed to serialize signature: %w", err)
-	}
-
-	return Credential{
-		PGPSignature: &CredentialPGPSignature{
-			PubKeyID: s.pubKey.KeyIdString(),
-			Body:     body.Bytes(),
-		},
-	}, nil
+	return accountSignifier(accountID, pgpKey), pubKeyBody
 }
 
 // SignifierPGP describes a pgp public key whose corresponding private key will
@@ -203,12 +182,12 @@ func cmdGPG(stdin []byte, args ...string) ([]byte, error) {
 	return out, nil
 }
 
-// SignifierPGPFromKeyID loads a pgp key using the given identifier. The key is
-// assumed to be stored. in the client's keyring already.
+// LoadSignifierPGP loads a pgp key using the given identifier. The key is
+// assumed to be stored in the client's keyring already.
 //
 // If setPubKeyBody is true, then CredentialPGPSignature instances produced by
 // the returned Signifier will have their PubKeyBody field set.
-func SignifierPGPFromKeyID(keyID string, setPubKeyBody bool) (SignifierInterface, error) {
+func LoadSignifierPGP(keyID string, setPubKeyBody bool) (SignifierInterface, error) {
 	pubKey, err := cmdGPG(nil, "-a", "--export", keyID)
 	if err != nil {
 		return nil, fmt.Errorf("loading public key: %w", err)
@@ -225,7 +204,7 @@ func SignifierPGPFromKeyID(keyID string, setPubKeyBody bool) (SignifierInterface
 	return sigInt, nil
 }
 
-func (s SignifierPGP) load(fs fs.FS) (pgpPubKey, error) {
+func (s SignifierPGP) load(fs fs.FS) (pgpKey, error) {
 	if s.Body != "" {
 		return newPGPPubKey(strings.NewReader(s.Body))
 	}
@@ -233,13 +212,13 @@ func (s SignifierPGP) load(fs fs.FS) (pgpPubKey, error) {
 	path := filepath.Clean(s.Path)
 	fr, err := fs.Open(path)
 	if err != nil {
-		return pgpPubKey{}, fmt.Errorf("opening PGP public key file at %q: %w", path, err)
+		return pgpKey{}, fmt.Errorf("opening PGP public key file at %q: %w", path, err)
 	}
 	defer fr.Close()
 
 	pubKeyB, err := ioutil.ReadAll(fr)
 	if err != nil {
-		return pgpPubKey{}, fmt.Errorf("reading PGP public key from file at %q: %w", s.Path, err)
+		return pgpKey{}, fmt.Errorf("reading PGP public key from file at %q: %w", s.Path, err)
 	}
 
 	return SignifierPGP{Body: string(pubKeyB)}.load(fs)
@@ -253,14 +232,15 @@ func (s SignifierPGP) Sign(fs fs.FS, data []byte) (Credential, error) {
 		return Credential{}, err
 	}
 
-	sig, err := cmdGPG(data, "--detach-sign", "--local-user", sigPGP.pubKey.KeyIdString())
+	keyID := sigPGP.entity.PrimaryKey.KeyIdString()
+	sig, err := cmdGPG(data, "--detach-sign", "--local-user", keyID)
 	if err != nil {
 		return Credential{}, fmt.Errorf("signing with pgp key: %w", err)
 	}
 
 	return Credential{
 		PGPSignature: &CredentialPGPSignature{
-			PubKeyID: sigPGP.pubKey.KeyIdString(),
+			PubKeyID: keyID,
 			Body:     sig,
 		},
 	}, nil

sigcred/pgp_test.go

@@ -38,7 +38,7 @@ func TestPGPVerification(t *testing.T) {
 			seed := time.Now().UnixNano()
 			t.Logf("seed: %d", seed)
 			rand := rand.New(rand.NewSource(seed))
-			privKey, pubKeyBody := SignifierPGPTmp("", rand)
+			privKey, pubKeyBody := TestSignifierPGP("", rand)
 
 			sig, fs := test.init(pubKeyBody)
 			data := make([]byte, rand.Intn(1024))