From 43b564e71179ff7df3b9dda4a86db18dcdd6b69f Mon Sep 17 00:00:00 2001 From: mediocregopher <> Date: Sun, 12 Apr 2020 11:02:05 -0600 Subject: [PATCH] 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 | 2 +- commit_change_test.go | 2 +- commit_credential_test.go | 2 +- commit_test.go | 4 +- repo_test.go | 2 +- sigcred/credential_test.go | 4 +- sigcred/pgp.go | 138 ++++++++++++++++--------------------- sigcred/pgp_test.go | 2 +- 8 files changed, 68 insertions(+), 88 deletions(-) diff --git a/cmd/dehub/cmd_commit.go b/cmd/dehub/cmd_commit.go index 920d71c..d51b73b 100644 --- a/cmd/dehub/cmd_commit.go +++ b/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) } } diff --git a/commit_change_test.go b/commit_change_test.go index 98e716e..d5a63f0 100644 --- a/commit_change_test.go +++ b/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{ diff --git a/commit_credential_test.go b/commit_credential_test.go index c99fb0e..a5bef0d 100644 --- a/commit_credential_test.go +++ b/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{ diff --git a/commit_test.go b/commit_test.go index 9b86a26..6c4541e 100644 --- a/commit_test.go +++ b/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, diff --git a/repo_test.go b/repo_test.go index 6044795..141dc72 100644 --- a/repo_test.go +++ b/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{ diff --git a/sigcred/credential_test.go b/sigcred/credential_test.go index 4456853..5989551 100644 --- a/sigcred/credential_test.go +++ b/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 diff --git a/sigcred/pgp.go b/sigcred/pgp.go index cb8c4b0..dcce4e4 100644 --- a/sigcred/pgp.go +++ b/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 diff --git a/sigcred/pgp_test.go b/sigcred/pgp_test.go index 0905cd1..caa3bed 100644 --- a/sigcred/pgp_test.go +++ b/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))