diff --git a/ROADMAP.md b/ROADMAP.md index 007cc4b..2a5c8c5 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -11,9 +11,6 @@ to accept help from people asking to help. * Fast-forward perms on branches (so they can be deleted) * Figure out commit range syntax, use that everywhere. -* Ability to specify a pgp key manually, even if it's not in the project. -* Ability to require _any_ signature on a commit, even if it's not in the - config. * Create a branch which is just a public "welcome thread", which can be part of the tutorials. * Tutorials diff --git a/accessctl/access_control.go b/accessctl/access_control.go index a47afb4..e59cd05 100644 --- a/accessctl/access_control.go +++ b/accessctl/access_control.go @@ -3,10 +3,11 @@ package accessctl import ( - "dehub.dev/src/dehub.git/sigcred" "errors" "fmt" + "dehub.dev/src/dehub.git/sigcred" + yaml "gopkg.in/yaml.v2" ) diff --git a/accessctl/filter_sig.go b/accessctl/filter_sig.go index 93ed62f..cd5f8d4 100644 --- a/accessctl/filter_sig.go +++ b/accessctl/filter_sig.go @@ -11,17 +11,21 @@ import ( // FilterSignature represents the configuration of a Filter which requires one // or more signature credentials to be present on a commit. // -// Either AccountIDs or AnyAccount must be filled in. +// Either AccountIDs, AnyAccount, or Any must be filled in; all are mutually +// exclusive. type FilterSignature struct { AccountIDs []string `yaml:"account_ids,omitempty"` + Any bool `yaml:"any,omitempty"` AnyAccount bool `yaml:"any_account,omitempty"` - Count string `yaml:"count"` + Count string `yaml:"count,omitempty"` } var _ FilterInterface = FilterSignature{} func (f FilterSignature) targetNum() (int, error) { - if !strings.HasSuffix(f.Count, "%") { + if f.Count == "" { + return 1, nil + } else if !strings.HasSuffix(f.Count, "%") { return strconv.Atoi(f.Count) } else if f.AnyAccount { return 0, errors.New("cannot use AnyAccount and a percent Count together") @@ -56,17 +60,29 @@ func (f FilterSignature) MatchCommit(req CommitRequest) error { return fmt.Errorf("computing target number of accounts: %w", err) } + var numSigs int credAccountIDs := map[string]struct{}{} for _, cred := range req.Credentials { // TODO support other kinds of signatures if cred.PGPSignature == nil { continue } - credAccountIDs[cred.AccountID] = struct{}{} + numSigs++ + if cred.AccountID != "" { + credAccountIDs[cred.AccountID] = struct{}{} + } + } + + if numSigs == 0 { + return ErrFilterNoMatch{ + Err: ErrFilterSignatureUnsatisfied{TargetNumAccounts: targetN}, + } } var n int - if f.AnyAccount { + if f.Any { + return nil + } else if f.AnyAccount { // TODO this doesn't actually check that the accounts are defined in the // Config. It works for now as long as the Credentials are valid, since // only an Account defined in the Config could create a valid diff --git a/accessctl/filter_sig_test.go b/accessctl/filter_sig_test.go index 2cabf1d..00ae25b 100644 --- a/accessctl/filter_sig_test.go +++ b/accessctl/filter_sig_test.go @@ -1,8 +1,9 @@ package accessctl import ( - "dehub.dev/src/dehub.git/sigcred" "testing" + + "dehub.dev/src/dehub.git/sigcred" ) func TestFilterSignature(t *testing.T) { @@ -99,5 +100,25 @@ func TestFilterSignature(t *testing.T) { NumAccounts: 1, }, }, + { + descr: "any sig at all", + filter: FilterSignature{ + Any: true, + }, + req: CommitRequest{ + Credentials: []sigcred.Credential{ + {PGPSignature: new(sigcred.CredentialPGPSignature)}, + }, + }, + match: true, + }, + { + descr: "not any sig at all", + filter: FilterSignature{Any: true}, + req: CommitRequest{}, + matchErr: ErrFilterSignatureUnsatisfied{ + TargetNumAccounts: 1, + }, + }, }) } diff --git a/cmd/dehub/cmd_commit.go b/cmd/dehub/cmd_commit.go index 0e8f65b..920d71c 100644 --- a/cmd/dehub/cmd_commit.go +++ b/cmd/dehub/cmd_commit.go @@ -7,44 +7,57 @@ import ( "dehub.dev/src/dehub.git" "dehub.dev/src/dehub.git/cmd/dehub/dcmd" + "dehub.dev/src/dehub.git/sigcred" "gopkg.in/src-d/go-git.v4/plumbing" ) func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) { flag := cmd.FlagSet() - accountID := flag.String("account-id", "", "Account to accredit commit with") + accountID := flag.String("as", "", "Account to accredit commit with") + pgpKeyID := flag.String("anon-pgp-key", "", "ID of pgp key to sign with instead of using an account") var repo repo repo.initFlags(flag) accreditAndCommit := func(commit dehub.Commit) error { - cfg, err := repo.LoadConfig() - if err != nil { - return err - } + var sigInt sigcred.SignifierInterface + if *accountID != "" { + cfg, err := repo.LoadConfig() + if err != nil { + return err + } - var account dehub.Account - var ok bool - for _, account = range cfg.Accounts { - if account.ID == *accountID { - ok = true - break + var account dehub.Account + var ok bool + for _, account = range cfg.Accounts { + if account.ID == *accountID { + ok = true + break + } + } + if !ok { + return fmt.Errorf("account ID %q not found in config", *accountID) + } else if l := len(account.Signifiers); l == 0 || l > 1 { + return fmt.Errorf("account %q has %d signifiers, only one is supported right now", *accountID, l) + } + + sig := account.Signifiers[0] + sigInt, err = sig.Interface(*accountID) + if err != nil { + return fmt.Errorf("casting %#v to SignifierInterface: %w", sig, err) + + } + } else { + var err error + if sigInt, err = sigcred.SignifierPGPFromKeyID(*pgpKeyID, true); err != nil { + return fmt.Errorf("loading pgp key %q: %w", *pgpKeyID, err) } - } - if !ok { - return fmt.Errorf("account ID %q not found in config", *accountID) - } else if l := len(account.Signifiers); l == 0 || l > 1 { - return fmt.Errorf("account %q has %d signifiers, only one is supported right now", *accountID, l) } - sig := account.Signifiers[0] - sigInt, err := sig.Interface(*accountID) + commit, err := repo.AccreditCommit(commit, sigInt) if err != nil { - return fmt.Errorf("casting %#v to SignifierInterface: %w", sig, err) - - } else if commit, err = repo.AccreditCommit(commit, sigInt); err != nil { return fmt.Errorf("accrediting commit: %w", err) } @@ -59,8 +72,8 @@ func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) { var hasStaged bool body := func() (context.Context, error) { - if *accountID == "" { - return nil, errors.New("-account-id is required") + if *accountID == "" && *pgpKeyID == "" { + return nil, errors.New("-as or -anon-pgp-key is required") } if err := repo.openRepo(); err != nil { diff --git a/commit.go b/commit.go index 9c43a0a..dc5dde2 100644 --- a/commit.go +++ b/commit.go @@ -427,11 +427,17 @@ func (r *Repo) verifyCommit(branch plumbing.ReferenceName, gitCommit GitCommit, // verify all credentials for _, cred := range gitCommit.Commit.Common.Credentials { - sig, err := r.signifierForCredential(sigFS, cred) - if err != nil { - return fmt.Errorf("finding signifier for credential %+v: %w", cred, err) - } else if err := sig.Verify(sigFS, expectedCommitHash, cred); err != nil { - return fmt.Errorf("verifying credential %+v: %w", cred, err) + if cred.AccountID == "" { + if err := cred.SelfVerify(expectedCommitHash); err != nil { + return fmt.Errorf("verifying credential %+v: %w", cred, err) + } + } else { + sig, err := r.signifierForCredential(sigFS, cred) + if err != nil { + return fmt.Errorf("finding signifier for credential %+v: %w", cred, err) + } else if err := sig.Verify(sigFS, expectedCommitHash, cred); err != nil { + return fmt.Errorf("verifying credential %+v: %w", cred, err) + } } } diff --git a/commit_change_test.go b/commit_change_test.go index 0b5851c..98e716e 100644 --- a/commit_change_test.go +++ b/commit_change_test.go @@ -1,11 +1,12 @@ package dehub import ( - "dehub.dev/src/dehub.git/sigcred" "reflect" "strings" "testing" + "dehub.dev/src/dehub.git/sigcred" + "github.com/davecgh/go-spew/spew" "gopkg.in/src-d/go-git.v4/plumbing" yaml "gopkg.in/yaml.v2" diff --git a/commit_test.go b/commit_test.go index 9e3b5b5..9b86a26 100644 --- a/commit_test.go +++ b/commit_test.go @@ -3,6 +3,7 @@ package dehub import ( "testing" + "dehub.dev/src/dehub.git/accessctl" "dehub.dev/src/dehub.git/sigcred" "gopkg.in/src-d/go-git.v4/plumbing" @@ -84,3 +85,27 @@ func TestMainAncestryRequirement(t *testing.T) { h.tryCommit(false, badCommit, h.cfg.Accounts[0].ID, h.sig) }) } + +func TestAnonymousCommits(t *testing.T) { + h := newHarness(t) + anonSig, anonPubKeyBody := sigcred.SignifierPGPTmp("", h.rand) + + h.cfg.AccessControls = []accessctl.AccessControl{{ + Action: accessctl.ActionAllow, + Filters: []accessctl.Filter{ + {Signature: &accessctl.FilterSignature{Any: true}}, + }, + }} + h.stageCfg() + + // manually accredit the commit this time + goodCommit, err := h.repo.NewCommitChange("this will work") + if err != nil { + t.Fatalf("creating CommitChange: %v", err) + } else if goodCommit, err = h.repo.AccreditCommit(goodCommit, anonSig); err != nil { + t.Fatalf("accreditting CommitChange: %v", err) + } + // There is, unfortunately, not a prettier way to do this + goodCommit.Common.Credentials[0].PGPSignature.PubKeyBody = string(anonPubKeyBody) + h.tryCommit(true, goodCommit, "", nil) +} diff --git a/config.go b/config.go index fd0484c..9a618d1 100644 --- a/config.go +++ b/config.go @@ -1,11 +1,12 @@ package dehub import ( + "errors" + "fmt" + "dehub.dev/src/dehub.git/accessctl" "dehub.dev/src/dehub.git/fs" "dehub.dev/src/dehub.git/sigcred" - "errors" - "fmt" yaml "gopkg.in/yaml.v2" ) diff --git a/repo.go b/repo.go index 7f5b4ab..7ae6ccb 100644 --- a/repo.go +++ b/repo.go @@ -428,7 +428,6 @@ func (r *Repo) GetGitCommitRange(start, end plumbing.Hash) ([]GitCommit, error) } var ( - hashLen = len(plumbing.ZeroHash) hashStrLen = len(plumbing.ZeroHash.String()) errNotHex = errors.New("not a valid hex string") ) diff --git a/sigcred/credential.go b/sigcred/credential.go index 8935aef..6bde560 100644 --- a/sigcred/credential.go +++ b/sigcred/credential.go @@ -1,6 +1,10 @@ package sigcred -import "dehub.dev/src/dehub.git/typeobj" +import ( + "fmt" + + "dehub.dev/src/dehub.git/typeobj" +) // Credential represents a credential which has been attached to a commit which // hopefully will allow it to be included in the main. Exactly one field tagged @@ -8,10 +12,12 @@ import "dehub.dev/src/dehub.git/typeobj" type Credential struct { PGPSignature *CredentialPGPSignature `type:"pgp_signature"` - // AccountID specifies the account which generated this Credential. The - // Credentials produced by the Signifier.Sign method do not fill this field - // in. - AccountID string `yaml:"account"` + // AccountID specifies the account which generated this Credential. + // + // NOTE that the Credentials produced by the Signifier.Sign method do not + // fill this field in, and it may be empty in cases where a non-account user + // has added a credential to a commit. + AccountID string `yaml:"account,omitempty"` } // MarshalYAML implements the yaml.Marshaler interface. @@ -23,3 +29,44 @@ func (c Credential) MarshalYAML() (interface{}, error) { func (c *Credential) UnmarshalYAML(unmarshal func(interface{}) error) error { return typeobj.UnmarshalYAML(c, unmarshal) } + +// ErrNotSelfVerifying is returned from the SelfVerify method of Credential when +// the Credential does not implement the SelfVerifyingCredential interface. It +// may also be returned from the SelfVerify method of the +// SelfVerifyingCredential itself, if the Credential can only self-verify under +// certain circumstances. +type ErrNotSelfVerifying struct { + // Subject is a descriptor of the value which could not be verified. It may + // be a type name or some other identifying piece of information. + Subject string +} + +func (e ErrNotSelfVerifying) Error() string { + return fmt.Sprintf("%s cannot verify itself", e.Subject) +} + +// SelfVerify will attempt to cast the Credential as a SelfVerifyingCredential, +// and returns the result of the SelfVerify method being called on it. +func (c Credential) SelfVerify(data []byte) error { + el, _, err := typeobj.Element(c) + if err != nil { + return err + } else if selfVerifyingCred, ok := el.(SelfVerifyingCredential); !ok { + return ErrNotSelfVerifying{Subject: fmt.Sprintf("Credential of type %T", el)} + } else if err := selfVerifyingCred.SelfVerify(data); err != nil { + return fmt.Errorf("self-verifying Credential of type %T: %w", el, err) + } + return nil +} + +// SelfVerifyingCredential is one which is able to prove its own authenticity by +// some means or another. It is not required for a Credential to implement this +// interface. +type SelfVerifyingCredential interface { + // SelfVerify should return nil if the Credential has successfully verified + // that it has accredited the given data, or an error describing why it + // could not do so. It may return ErrNotSelfVerifying if the Credential can + // only self-verify under certain circumstances, and those circumstances are + // not met. + SelfVerify(data []byte) error +} diff --git a/sigcred/credential_test.go b/sigcred/credential_test.go new file mode 100644 index 0000000..4456853 --- /dev/null +++ b/sigcred/credential_test.go @@ -0,0 +1,60 @@ +package sigcred + +import ( + "errors" + "math/rand" + "testing" + "time" +) + +func TestSelfVerifyingCredentials(t *testing.T) { + seed := time.Now().UnixNano() + t.Logf("seed: %d", seed) + rand := rand.New(rand.NewSource(seed)) + + tests := []struct { + descr string + mkCred func(toSign []byte) (Credential, error) + expErr bool + }{ + { + descr: "pgp sig no body", + mkCred: func(toSign []byte) (Credential, error) { + privKey, _ := SignifierPGPTmp("", rand) + return privKey.Sign(nil, toSign) + }, + expErr: true, + }, + { + descr: "pgp sig with body", + mkCred: func(toSign []byte) (Credential, error) { + privKey, pubKeyBody := SignifierPGPTmp("", rand) + cred, err := privKey.Sign(nil, toSign) + cred.PGPSignature.PubKeyBody = string(pubKeyBody) + return cred, err + }, + }, + } + + for _, test := range tests { + t.Run(test.descr, func(t *testing.T) { + data := make([]byte, rand.Intn(1024)) + if _, err := rand.Read(data); err != nil { + t.Fatal(err) + } + + cred, err := test.mkCred(data) + if err != nil { + t.Fatal(err) + } + + err = cred.SelfVerify(data) + isNotSelfVerifying := errors.As(err, new(ErrNotSelfVerifying)) + if test.expErr && !isNotSelfVerifying { + t.Fatalf("expected ErrNotSelfVerifying but got: %v", err) + } else if !test.expErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/sigcred/pgp.go b/sigcred/pgp.go index 50c13c9..cb8c4b0 100644 --- a/sigcred/pgp.go +++ b/sigcred/pgp.go @@ -6,8 +6,6 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/sha256" - "dehub.dev/src/dehub.git/fs" - "dehub.dev/src/dehub.git/yamlutil" "fmt" "io" "io/ioutil" @@ -16,6 +14,9 @@ import ( "strings" "time" + "dehub.dev/src/dehub.git/fs" + "dehub.dev/src/dehub.git/yamlutil" + "golang.org/x/crypto/openpgp/armor" "golang.org/x/crypto/openpgp/packet" ) @@ -23,8 +24,22 @@ import ( // CredentialPGPSignature describes a PGP signature which has been used to sign // a commit. type CredentialPGPSignature struct { - PubKeyID string `yaml:"pub_key_id"` - Body yamlutil.Blob `yaml:"body"` + PubKeyID string `yaml:"pub_key_id"` + PubKeyBody string `yaml:"pub_key_body,omitempty"` + Body yamlutil.Blob `yaml:"body"` +} + +// SelfVerify will only work if PubKeyBody is filled in. If so, Body will +// attempt to be verified by that public key. +func (c *CredentialPGPSignature) SelfVerify(data []byte) error { + if c.PubKeyBody == "" { + return ErrNotSelfVerifying{ + Subject: "PGP signature Credential with no pub_key_body field", + } + } + + sig := SignifierPGP{Body: c.PubKeyBody} + return sig.Verify(nil, data, Credential{PGPSignature: c}) } type pgpPubKey struct { @@ -134,10 +149,7 @@ func SignifierPGPTmp(accountID string, randReader io.Reader) (SignifierInterface panic(err) } - return accountSignifier{ - accountID: accountID, - SignifierInterface: privKey, - }, pubKeyBody + return accountSignifier(accountID, privKey), pubKeyBody } func (s pgpPrivKey) Sign(_ fs.FS, data []byte) (Credential, error) { @@ -164,120 +176,119 @@ func (s pgpPrivKey) Sign(_ fs.FS, data []byte) (Credential, error) { } // SignifierPGP describes a pgp public key whose corresponding private key will -// be used as a signing key. +// be used as a signing key. The public key can be described by one of multiple +// fields, each being a different method of loading the public key. Only one +// field should be set. type SignifierPGP struct { - Body string `yaml:"body"` -} - -var _ SignifierInterface = SignifierPGP{} + // An armored string encoding of the public key, as exported via + // `gpg -a --export ` + Body string `yaml:"body,omitempty"` -func (s SignifierPGP) load() (pgpPubKey, error) { - return newPGPPubKey(strings.NewReader(s.Body)) + // Path, relative to the root of the repo, of the armored public key file. + Path string `yaml:"path,omitempty"` } -// Sign will sign the given arbitrary bytes using the private key corresponding -// to the pgp public key embedded in this Signifier. -func (s SignifierPGP) Sign(fs fs.FS, data []byte) (Credential, error) { - sigPGP, err := s.load() - if err != nil { - return Credential{}, err - } +var _ SignifierInterface = SignifierPGP{} +func cmdGPG(stdin []byte, args ...string) ([]byte, error) { + args = append([]string{"--openpgp"}, args...) stderr := new(bytes.Buffer) - cmd := exec.Command("gpg", - "--openpgp", - "--detach-sign", - "--local-user", sigPGP.pubKey.KeyIdString()) - cmd.Stdin = bytes.NewBuffer(data) + cmd := exec.Command("gpg", args...) + cmd.Stdin = bytes.NewBuffer(stdin) cmd.Stderr = stderr - sig, err := cmd.Output() + out, err := cmd.Output() if err != nil { - return Credential{}, fmt.Errorf("error signing with gpg (%v): %s", err, stderr.String()) + return nil, fmt.Errorf("calling gpg command (%v): %s", err, stderr.String()) } - - return Credential{ - PGPSignature: &CredentialPGPSignature{ - PubKeyID: sigPGP.pubKey.KeyIdString(), - Body: sig, - }, - }, nil + return out, nil } -// Signed returns true if the private key corresponding to the pgp public key -// embedded in this Signifier was used to produce the given Credential. -func (s SignifierPGP) Signed(fs fs.FS, cred Credential) (bool, error) { - sigPGP, err := s.load() +// SignifierPGPFromKeyID 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) { + pubKey, err := cmdGPG(nil, "-a", "--export", keyID) if err != nil { - return false, err + return nil, fmt.Errorf("loading public key: %w", err) } - - return sigPGP.Signed(fs, cred) -} - -// Verify asserts that the given signature was produced by this key signing the -// given piece of data. -func (s SignifierPGP) Verify(fs fs.FS, data []byte, cred Credential) error { - sigPGP, err := s.load() - if err != nil { - return err + var sigInt SignifierInterface = &SignifierPGP{Body: string(pubKey)} + if setPubKeyBody { + sigInt = signifierMiddleware{ + SignifierInterface: sigInt, + signCallback: func(cred *Credential) { + cred.PGPSignature.PubKeyBody = string(pubKey) + }, + } } - return sigPGP.Verify(fs, data, cred) -} - -// SignifierPGPFile is the same as SignifierPGP, except that the public key is -// found in the repo rather than encoded into the object. -type SignifierPGPFile struct { - Path string `yaml:"path"` + return sigInt, nil } -var _ SignifierInterface = SignifierPGPFile{} +func (s SignifierPGP) load(fs fs.FS) (pgpPubKey, error) { + if s.Body != "" { + return newPGPPubKey(strings.NewReader(s.Body)) + } -func (s SignifierPGPFile) load(fs fs.FS) (SignifierPGP, error) { path := filepath.Clean(s.Path) fr, err := fs.Open(path) if err != nil { - return SignifierPGP{}, fmt.Errorf("could not open PGP public key file at %q: %w", path, err) + return pgpPubKey{}, fmt.Errorf("opening PGP public key file at %q: %w", path, err) } defer fr.Close() pubKeyB, err := ioutil.ReadAll(fr) if err != nil { - return SignifierPGP{}, fmt.Errorf("could not read PGP public key from file blob at %q: %w", s.Path, err) + return pgpPubKey{}, fmt.Errorf("reading PGP public key from file at %q: %w", s.Path, err) } - return SignifierPGP{Body: string(pubKeyB)}, nil + return SignifierPGP{Body: string(pubKeyB)}.load(fs) } // Sign will sign the given arbitrary bytes using the private key corresponding -// to the pgp public key located by this Signifier. -func (s SignifierPGPFile) Sign(fs fs.FS, data []byte) (Credential, error) { +// to the pgp public key embedded in this Signifier. +func (s SignifierPGP) Sign(fs fs.FS, data []byte) (Credential, error) { sigPGP, err := s.load(fs) if err != nil { return Credential{}, err } - return sigPGP.Sign(fs, data) -} -// Signed returns true if the private key corresponding to the pgp public key -// located by this Signifier was used to produce the given Credential. -func (s SignifierPGPFile) Signed(fs fs.FS, cred Credential) (bool, error) { - if cred.PGPSignature == nil { - return false, nil + sig, err := cmdGPG(data, "--detach-sign", "--local-user", sigPGP.pubKey.KeyIdString()) + if err != nil { + return Credential{}, fmt.Errorf("signing with pgp key: %w", err) } + return Credential{ + PGPSignature: &CredentialPGPSignature{ + PubKeyID: sigPGP.pubKey.KeyIdString(), + Body: sig, + }, + }, nil +} + +// Signed returns true if the private key corresponding to the pgp public key +// embedded in this Signifier was used to produce the given Credential. +func (s SignifierPGP) Signed(fs fs.FS, cred Credential) (bool, error) { sigPGP, err := s.load(fs) if err != nil { return false, err } + return sigPGP.Signed(fs, cred) } // Verify asserts that the given signature was produced by this key signing the // given piece of data. -func (s SignifierPGPFile) Verify(fs fs.FS, data []byte, cred Credential) error { +func (s SignifierPGP) Verify(fs fs.FS, data []byte, cred Credential) error { sigPGP, err := s.load(fs) if err != nil { return err } return sigPGP.Verify(fs, data, cred) } + +// SignifierPGPFile is deprecated and should not be used, use the Path field of +// SignifierPGP instead. +type SignifierPGPFile struct { + Path string `yaml:"path"` +} diff --git a/sigcred/pgp_test.go b/sigcred/pgp_test.go index 1465947..0905cd1 100644 --- a/sigcred/pgp_test.go +++ b/sigcred/pgp_test.go @@ -1,10 +1,11 @@ package sigcred import ( - "dehub.dev/src/dehub.git/fs" "math/rand" "testing" "time" + + "dehub.dev/src/dehub.git/fs" ) // There are not currently tests for testing pgp signature creation, as they @@ -17,18 +18,17 @@ func TestPGPVerification(t *testing.T) { init func(pubKeyBody []byte) (SignifierInterface, fs.FS) }{ { - descr: "SignifierPGP", + descr: "SignifierPGP Body", init: func(pubKeyBody []byte) (SignifierInterface, fs.FS) { return SignifierPGP{Body: string(pubKeyBody)}, nil }, }, { - descr: "SignifierPGPFile", + descr: "SignifierPGP Path", init: func(pubKeyBody []byte) (SignifierInterface, fs.FS) { pubKeyPath := "some/dir/pubkey.asc" fs := fs.Stub{pubKeyPath: pubKeyBody} - sigPGPFile := SignifierPGPFile{Path: pubKeyPath} - return sigPGPFile, fs + return SignifierPGP{Path: pubKeyPath}, fs }, }, } @@ -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("foo", rand) + privKey, pubKeyBody := SignifierPGPTmp("", rand) sig, fs := test.init(pubKeyBody) data := make([]byte, rand.Intn(1024)) diff --git a/sigcred/signifier.go b/sigcred/signifier.go index 8e69cc0..66922d6 100644 --- a/sigcred/signifier.go +++ b/sigcred/signifier.go @@ -8,7 +8,9 @@ import ( // Signifier reprsents a single signing method being defined in the Config. Only // one field should be set on each Signifier. type Signifier struct { - PGPPublicKey *SignifierPGP `type:"pgp_public_key"` + PGPPublicKey *SignifierPGP `type:"pgp_public_key"` + + // PGPPublicKeyFile is deprecated, only PGPPublicKey should be used PGPPublicKeyFile *SignifierPGPFile `type:"pgp_public_key_file"` } @@ -19,7 +21,16 @@ func (s Signifier) MarshalYAML() (interface{}, error) { // UnmarshalYAML implements the yaml.Unmarshaler interface. func (s *Signifier) UnmarshalYAML(unmarshal func(interface{}) error) error { - return typeobj.UnmarshalYAML(s, unmarshal) + if err := typeobj.UnmarshalYAML(s, unmarshal); err != nil { + return err + } + + // TODO deprecate PGPPublicKeyFile + if s.PGPPublicKeyFile != nil { + s.PGPPublicKey = &SignifierPGP{Path: s.PGPPublicKeyFile.Path} + s.PGPPublicKeyFile = nil + } + return nil } // Interface returns the SignifierInterface instance encapsulated by this @@ -33,7 +44,7 @@ func (s Signifier) Interface(accountID string) (SignifierInterface, error) { if err != nil { return nil, err } - return accountSignifier{accountID, el.(SignifierInterface)}, nil + return accountSignifier(accountID, el.(SignifierInterface)), nil } // SignifierInterface describes the methods that all Signifiers must implement. @@ -53,19 +64,31 @@ type SignifierInterface interface { Verify(fs fs.FS, data []byte, cred Credential) error } +type signifierMiddleware struct { + SignifierInterface + signCallback func(*Credential) +} + +func (sm signifierMiddleware) Sign(fs fs.FS, data []byte) (Credential, error) { + cred, err := sm.SignifierInterface.Sign(fs, data) + if err != nil || sm.signCallback == nil { + return cred, err + } + sm.signCallback(&cred) + return cred, nil +} + // 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 +func accountSignifier(accountID string, sigInt SignifierInterface) SignifierInterface { + return signifierMiddleware{ + SignifierInterface: sigInt, + signCallback: func(cred *Credential) { + cred.AccountID = accountID + }, + } }