From aff3daab192d4d65f0dd1d98f07c1763fecfb9ee Mon Sep 17 00:00:00 2001 From: mediocregopher <> Date: Fri, 13 Mar 2020 15:24:46 -0600 Subject: [PATCH] Modify how SignifierInterface is produced so it always sets AccountID on Credentials --- type: change message: |- Modify how SignifierInterface is produced so it always sets AccountID on Credentials Previously it was the responsibility of the caller of the Sign method to set the AccountID on the produced Credential, but this didn't really make sense. This commit makes it so that all SignifierInterface's produced by Signifier implicitly set the AccountID field. The solution here is still a bit hacky, and ultimately the real solution will probably be to refactor the structore of Credential, so that it doesn't have AccountID. change_hash: ADPuz04GuyxWwjo/0/jc7DcsPMl5rK0osSpaqmUxv818 credentials: - type: pgp_signature pub_key_id: 95C46FA6A41148AC body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl5r+hgACgkQlcRvpqQRSKzwYBAAsY4tj+E5xtJSZ1TvrS0mwJ/lSHYWE4rS3eDMY3JUJLE1tr5k3OTRtUhh2UHCsArXSVF4sU8cBSCtf2noaThQm8KQghPMgoZ1LnPd4BnxxlE2gPik4FMcv+mCv9OgUh0AUO+rSXeYJA3oWunaW9kYollUdX/mVTQTmmbLBqBpeXF/TQO/bJTEEzA853j5QDT8//onfSIlzUw0UB57IZZZImp5/XrggHBbKdfhUTJ75LGMgDEDvDNIdV8lBys+RnMzK0Yj6EvLQhsw426+0Sf9vX3jtzj6WKhmi8QyYvcxIbcrWUScEfA/RAgf0A8KhqKq91bicSHjvyK1TZRSSWcS43ewamgvVWx0KSYYoIn7PPwOTmpHP8u6RzGEQFjOhP1EaGytQJKMXidU6CPTh+pYVtPZc8oLAwk+DyMquqfUSbzN/63t90HpTm7uycuzOnQxilYe2HKlbMJCId0a0DyAFrA+0pNRz0tyd3DvF4svCdEy82rzlUGEhq7aIJKoXIut+fKGEBd6Znz6oX15CyQq0oPthZcCqgFR0oTqufvV2iWo+26cd9dVTPVbJA9kSbaFchgdAqCkPA5wDVuNJJtMftf7STW8Lm6dnU6q9YFjZVdR55WtvUCINxBUtOirRzG1jcS0VNhhtb+SMNATEvDGJmt6neHM6Z17MAdwGS+s/hA= account: mediocregopher --- cmd/dehub/main.go | 4 ++-- commit.go | 5 ++--- commit_test.go | 2 +- config.go | 2 +- repo_test.go | 4 ++-- sigcred/pgp.go | 8 ++++++-- sigcred/pgp_test.go | 2 +- sigcred/signifier.go | 25 +++++++++++++++++++++++-- 8 files changed, 38 insertions(+), 14 deletions(-) diff --git a/cmd/dehub/main.go b/cmd/dehub/main.go index 547733e..08cf4e0 100644 --- a/cmd/dehub/main.go +++ b/cmd/dehub/main.go @@ -150,7 +150,7 @@ var subCmds = []subCmd{ } sig := account.Signifiers[0] - sigInt, err := sig.Interface() + sigInt, err := sig.Interface(*accountID) if err != nil { return fmt.Errorf("could not cast %+v to SignifierInterface: %w", sig, err) } @@ -160,7 +160,7 @@ var subCmds = []subCmd{ return fmt.Errorf("could not construct change commit: %w", err) } - commit, err = sctx.repo().AccreditCommit(commit, *accountID, sigInt) + commit, err = sctx.repo().AccreditCommit(commit, sigInt) if err != nil { return fmt.Errorf("could not accredit commit: %w", err) } diff --git a/commit.go b/commit.go index 775166e..3774c6d 100644 --- a/commit.go +++ b/commit.go @@ -108,8 +108,8 @@ func (c *Commit) UnmarshalText(msg []byte) error { } // AccreditCommit returns the given Commit with an appended Credential provided -// by the given account and its Signifier. -func (r *Repo) AccreditCommit(commit Commit, accountID string, sigInt sigcred.SignifierInterface) (Commit, error) { +// by the given SignifierInterface. +func (r *Repo) AccreditCommit(commit Commit, sigInt sigcred.SignifierInterface) (Commit, error) { commitInt, err := commit.Interface() if err != nil { return commit, fmt.Errorf("could not cast commit %+v to interface: %w", commit, err) @@ -124,7 +124,6 @@ func (r *Repo) AccreditCommit(commit Commit, accountID string, sigInt sigcred.Si if err != nil { return commit, fmt.Errorf("could not accreddit change commit: %w", err) } - cred.AccountID = accountID commit.Credentials = append(commit.Credentials, cred) return commit, nil } diff --git a/commit_test.go b/commit_test.go index f50f92e..32457b8 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(h.rand) + newSig, newPubKeyBody := sigcred.SignifierPGPTmp("toot", h.rand) h.cfg.Accounts = append(h.cfg.Accounts, Account{ ID: "toot", Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{ diff --git a/config.go b/config.go index 637fa80..f65e9e3 100644 --- a/config.go +++ b/config.go @@ -70,7 +70,7 @@ func (r *Repo) signifierForCredential(fs fs.FS, cred sigcred.Credential) (sigcre } for i, sig := range account.Signifiers { - if sigInt, err := sig.Interface(); err != nil { + if sigInt, err := sig.Interface(cred.AccountID); err != nil { return nil, fmt.Errorf("error converting signifier index:%d to inteface: %w", i, err) } else if ok, err := sigInt.Signed(fs, cred); err != nil { return nil, fmt.Errorf("error checking if signfier index:%d signed credential: %w", i, err) diff --git a/repo_test.go b/repo_test.go index 605d13e..855cae2 100644 --- a/repo_test.go +++ b/repo_test.go @@ -25,7 +25,7 @@ type harness struct { func newHarness(t *testing.T) *harness { rand := rand.New(rand.NewSource(0xb4eadb01)) - sig, pubKeyBody := sigcred.SignifierPGPTmp(rand) + sig, pubKeyBody := sigcred.SignifierPGPTmp("root", rand) pubKeyPath := filepath.Join(DehubDir, "root.asc") cfg := &Config{ @@ -114,7 +114,7 @@ func (h *harness) changeCommit(msg, accountID string, sig sigcred.SignifierInter } if sig != nil { - if commit, err = h.repo.AccreditCommit(commit, accountID, sig); err != nil { + if commit, err = h.repo.AccreditCommit(commit, sig); err != nil { h.t.Fatalf("failed to accredit commit: %v", err) } } diff --git a/sigcred/pgp.go b/sigcred/pgp.go index d419178..c5ddea1 100644 --- a/sigcred/pgp.go +++ b/sigcred/pgp.go @@ -115,7 +115,7 @@ type pgpPrivKey struct { // SignifierPGPTmp 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(randReader io.Reader) (SignifierInterface, []byte) { +func SignifierPGPTmp(accountID string, randReader io.Reader) (SignifierInterface, []byte) { rawPrivKey, err := ecdsa.GenerateKey(elliptic.P521(), randReader) if err != nil { panic(err) @@ -133,7 +133,11 @@ func SignifierPGPTmp(randReader io.Reader) (SignifierInterface, []byte) { if err != nil { panic(err) } - return privKey, pubKeyBody + + return accountSignifier{ + accountID: accountID, + SignifierInterface: privKey, + }, pubKeyBody } func (s pgpPrivKey) Sign(_ fs.FS, data []byte) (Credential, error) { diff --git a/sigcred/pgp_test.go b/sigcred/pgp_test.go index d24edfc..ecc5598 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 := SignifierPGPTmp("foo", rand) sig, fs := test.init(pubKeyBody) data := make([]byte, rand.Intn(1024)) diff --git a/sigcred/signifier.go b/sigcred/signifier.go index ef99c48..f97e76c 100644 --- a/sigcred/signifier.go +++ b/sigcred/signifier.go @@ -24,12 +24,16 @@ func (s *Signifier) UnmarshalYAML(unmarshal func(interface{}) error) error { // Interface returns the SignifierInterface instance encapsulated by this // Signifier object. -func (s Signifier) Interface() (SignifierInterface, error) { +// +// accountID is given so as to automatically fill the AccountID field of +// Credentials returned from Sign, since the underlying implementation doesn't +// know what account it's signing for. +func (s Signifier) Interface(accountID string) (SignifierInterface, error) { el, _, err := typeobj.Element(s) if err != nil { return nil, err } - return el.(SignifierInterface), nil + return accountSignifier{accountID, el.(SignifierInterface)}, nil } // SignifierInterface describes the methods that all Signifiers must implement. @@ -48,3 +52,20 @@ type SignifierInterface interface { // tree can be used to find the Signifier at a particular snapshot. Verify(fs fs.FS, data []byte, cred Credential) error } + +// accountSignifier wraps a SignifierInterface to always set the accountID field +// on Credentials it produces via the Sign method. +// +// TODO accountSignifier shouldn't be necessary, it's very ugly. Which indicates +// that Credential probably shouldn't have AccountID on it, which makes sense. +// Some refactoring is required here. +type accountSignifier struct { + accountID string + SignifierInterface +} + +func (as accountSignifier) Sign(fs fs.FS, data []byte) (Credential, error) { + cred, err := as.SignifierInterface.Sign(fs, data) + cred.AccountID = as.accountID + return cred, err +}