Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Verify git commit signatures #1791

Merged
merged 5 commits into from
May 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ jobs:
- checkout
- setup_remote_docker

- run:
# Ensure latest version of git
command: |
echo "deb http://deb.debian.org/debian stretch-backports main" | sudo tee -a /etc/apt/sources.list.d/stretch-backports.list
sudo apt-get update
sudo apt-get install -t stretch-backports -y --only-upgrade git
git version
- run: curl https://raw.githubusercontent.com/golang/dep/master/install.sh | sh
- run: dep ensure -vendor-only
- run: make check-generated
Expand Down
1 change: 1 addition & 0 deletions cmd/fluxctl/await.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func awaitJob(ctx context.Context, client api.Server, jobID job.ID) (job.Result,
}
switch j.StatusString {
case job.StatusFailed:
result = j.Result
return false, j
case job.StatusSucceeded:
if j.Err != "" {
Expand Down
13 changes: 11 additions & 2 deletions cmd/fluxctl/sync_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"fmt"
"strings"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -59,13 +60,15 @@ func (opts *syncOpts) RunE(cmd *cobra.Command, args []string) error {
return err
}
result, err := awaitJob(ctx, opts.API, jobID)
if err != nil {
if isUnverifiedHead(err) {
fmt.Fprintf(cmd.OutOrStderr(), "Warning: %s\n", err)
} else if err != nil {
fmt.Fprintf(cmd.OutOrStderr(), "Failed to complete sync job (ID %q)\n", jobID)
return err
}

rev := result.Revision[:7]
fmt.Fprintf(cmd.OutOrStderr(), "HEAD of %s is %s\n", gitConfig.Remote.Branch, rev)
fmt.Fprintf(cmd.OutOrStderr(), "Revision of %s to apply is %s\n", gitConfig.Remote.Branch, rev)
fmt.Fprintf(cmd.OutOrStderr(), "Waiting for %s to be applied ...\n", rev)
err = awaitSync(ctx, opts.API, rev)
if err != nil {
Expand All @@ -74,3 +77,9 @@ func (opts *syncOpts) RunE(cmd *cobra.Command, args []string) error {
fmt.Fprintln(cmd.OutOrStderr(), "Done.")
return nil
}

func isUnverifiedHead(err error) bool {
return err != nil &&
(strings.Contains(err.Error(), "branch HEAD in the git repo is not verified") &&
strings.Contains(err.Error(), "last verified commit was"))
}
36 changes: 20 additions & 16 deletions cmd/fluxd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,9 @@ func main() {
gitTimeout = fs.Duration("git-timeout", 20*time.Second, "duration after which git operations time out")

// GPG commit signing
gitImportGPG = fs.String("git-gpg-key-import", "", "keys at the path given (either a file or a directory) will be imported for use in signing commits")
gitSigningKey = fs.String("git-signing-key", "", "if set, commits will be signed with this GPG key")
gitImportGPG = fs.StringSlice("git-gpg-key-import", []string{}, "keys at the paths given will be imported for use of signing and verifying commits")
gitSigningKey = fs.String("git-signing-key", "", "if set, commits Flux makes will be signed with this GPG key")
gitVerifySignatures = fs.Bool("git-verify-signatures", false, "if set, the signature of commits will be verified before Flux applies them")

// syncing
syncInterval = fs.Duration("sync-interval", 5*time.Minute, "apply config in git to cluster at least this often, even if there are no new commits")
Expand Down Expand Up @@ -152,6 +153,7 @@ func main() {
k8sSecretDataKey = fs.String("k8s-secret-data-key", "identity", "data key holding the private SSH key within the k8s secret")
k8sNamespaceWhitelist = fs.StringSlice("k8s-namespace-whitelist", []string{}, "experimental, optional: restrict the view of the cluster to the namespaces listed. All namespaces are included if this is not set")
k8sAllowNamespace = fs.StringSlice("k8s-allow-namespace", []string{}, "experimental: restrict all operations to the provided namespaces")

// SSH key generation
sshKeyBits = optionalVar(fs, &ssh.KeyBitsValue{}, "ssh-keygen-bits", "-b argument to ssh-keygen (default unspecified)")
sshKeyType = optionalVar(fs, &ssh.KeyTypeValue{}, "ssh-keygen-type", "-t argument to ssh-keygen (default unspecified)")
Expand Down Expand Up @@ -242,13 +244,13 @@ func main() {
}

// Import GPG keys, if we've been told where to look for them
if *gitImportGPG != "" {
keyfiles, err := gpg.ImportKeys(*gitImportGPG)
for _, p := range *gitImportGPG {
hiddeco marked this conversation as resolved.
Show resolved Hide resolved
keyfiles, err := gpg.ImportKeys(p, *gitVerifySignatures)
if err != nil {
logger.Log("error", "failed to import GPG keys", "err", err.Error())
logger.Log("error", fmt.Sprintf("failed to import GPG key(s) from %s", p), "err", err.Error())
}
if keyfiles != nil {
logger.Log("info", "imported GPG keys", "files", fmt.Sprintf("%v", keyfiles))
logger.Log("info", fmt.Sprintf("imported GPG key(s) from %s", p), "files", fmt.Sprintf("%v", keyfiles))
}
}

Expand Down Expand Up @@ -483,15 +485,15 @@ func main() {

gitRemote := git.Remote{URL: *gitURL}
gitConfig := git.Config{
Paths: *gitPath,
Branch: *gitBranch,
SyncTag: *gitSyncTag,
NotesRef: *gitNotesRef,
UserName: *gitUser,
UserEmail: *gitEmail,
SigningKey: *gitSigningKey,
SetAuthor: *gitSetAuthor,
SkipMessage: *gitSkipMessage,
Paths: *gitPath,
Branch: *gitBranch,
SyncTag: *gitSyncTag,
NotesRef: *gitNotesRef,
UserName: *gitUser,
UserEmail: *gitEmail,
SigningKey: *gitSigningKey,
SetAuthor: *gitSetAuthor,
SkipMessage: *gitSkipMessage,
}

repo := git.NewRepo(gitRemote, git.PollInterval(*gitPollInterval), git.Timeout(*gitTimeout), git.Branch(*gitBranch))
Expand All @@ -510,6 +512,7 @@ func main() {
"user", *gitUser,
"email", *gitEmail,
"signing-key", *gitSigningKey,
"verify-signatures", *gitVerifySignatures,
"sync-tag", *gitSyncTag,
"notes-ref", *gitNotesRef,
"set-author", *gitSetAuthor,
Expand All @@ -534,7 +537,8 @@ func main() {
LoopVars: &daemon.LoopVars{
SyncInterval: *syncInterval,
RegistryPollInterval: *registryPollInterval,
GitOpTimeout: *gitTimeout,
GitTimeout: *gitTimeout,
GitVerifySignatures: *gitVerifySignatures,
},
}

Expand Down
95 changes: 92 additions & 3 deletions daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,9 @@ func (d *Daemon) makeJobFromUpdate(update updateFunc) jobFunc {
var result job.Result
err := d.WithClone(ctx, func(working *git.Checkout) error {
var err error
if err = verifyWorkingRepo(ctx, d.Repo, working, d.GitConfig); d.GitVerifySignatures && err != nil {
return err
}
result, err = update(ctx, jobID, working, logger)
if err != nil {
return err
Expand All @@ -257,7 +260,7 @@ func (d *Daemon) executeJob(id job.ID, do jobFunc, logger log.Logger) (job.Resul
d.JobStatusCache.SetStatus(id, job.Status{StatusString: job.StatusRunning})
result, err := do(ctx, id, logger)
if err != nil {
d.JobStatusCache.SetStatus(id, job.Status{StatusString: job.StatusFailed, Err: err.Error()})
d.JobStatusCache.SetStatus(id, job.Status{StatusString: job.StatusFailed, Err: err.Error(), Result: result})
return result, err
}
d.JobStatusCache.SetStatus(id, job.Status{StatusString: job.StatusSucceeded, Result: result})
Expand Down Expand Up @@ -353,12 +356,25 @@ func (d *Daemon) sync() jobFunc {
if err != nil {
return result, err
}
head, err := d.Repo.Revision(ctx, d.GitConfig.Branch)
head, err := d.Repo.BranchHead(ctx)
if err != nil {
return result, err
}
if d.GitVerifySignatures {
var latestValidRev string
if latestValidRev, _, err = latestValidRevision(ctx, d.Repo, d.GitConfig); err != nil {
return result, err
} else if head != latestValidRev {
result.Revision = latestValidRev
return result, fmt.Errorf(
"The branch HEAD in the git repo is not verified, and fluxd is unable to sync to it. The last verified commit was %.8s. HEAD is %.8s.",
latestValidRev,
head,
)
}
}
result.Revision = head
return result, nil
return result, err
}
}

Expand Down Expand Up @@ -751,3 +767,76 @@ func policyEventTypes(u policy.Update) []string {
sort.Strings(result)
return result
}

// latestValidRevision returns the HEAD of the configured branch if it
// has a valid signature, or the SHA of the latest valid commit it
// could find plus the invalid commit thereafter.
//
// Signature validation happens for commits between the revision of the
// sync tag and the HEAD, after the signature of the sync tag itself
// has been validated, as the branch can not be trusted when the tag
// originates from an unknown source.
//
// In case the signature of the tag can not be verified, or it points
// towards a revision we can not get a commit range for, it returns an
// error.
func latestValidRevision(ctx context.Context, repo *git.Repo, gitConfig git.Config) (string, git.Commit, error) {
squaremo marked this conversation as resolved.
Show resolved Hide resolved
var invalidCommit = git.Commit{}
newRevision, err := repo.BranchHead(ctx)
if err != nil {
return "", invalidCommit, err
}

// Validate tag and retrieve the revision it points to
tagRevision, err := repo.VerifyTag(ctx, gitConfig.SyncTag)
if err != nil && !strings.Contains(err.Error(), "not found.") {
return "", invalidCommit, errors.Wrap(err, "failed to verify signature of sync tag")
}

var commits []git.Commit
if tagRevision == "" {
commits, err = repo.CommitsBefore(ctx, newRevision)
} else {
// Assure the revision from the tag is a signed and valid commit
if err = repo.VerifyCommit(ctx, tagRevision); err != nil {
return "", invalidCommit, errors.Wrap(err, "failed to verify signature of sync tag revision")
}
commits, err = repo.CommitsBetween(ctx, tagRevision, newRevision)
}

if err != nil {
return tagRevision, invalidCommit, err
}

// Loop through commits in ascending order, validating the
// signature of each commit. In case we hit an invalid commit, we
// return the revision of the commit before that, as that one is
// valid.
for i := len(commits) - 1; i >= 0; i-- {
if !commits[i].Signature.Valid() {
if i+1 < len(commits) {
return commits[i+1].Revision, commits[i], nil
}
return tagRevision, commits[i], nil
}
}

return newRevision, invalidCommit, nil
}

func verifyWorkingRepo(ctx context.Context, repo *git.Repo, working *git.Checkout, gitConfig git.Config) error {
if latestVerifiedRev, _, err := latestValidRevision(ctx, repo, gitConfig); err != nil {
return err
} else if headRev, err := working.HeadRevision(ctx); err != nil {
return err
} else if headRev != latestVerifiedRev {
return unsignedHeadRevisionError(latestVerifiedRev, headRev)
}
return nil
}

func isUnknownRevision(err error) bool {
return err != nil &&
(strings.Contains(err.Error(), "unknown revision or path not in the working tree.") ||
strings.Contains(err.Error(), "bad revision"))
}
2 changes: 1 addition & 1 deletion daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ func mockDaemon(t *testing.T) (*Daemon, func(), func(), *cluster.Mock, *mockEven
JobStatusCache: &job.StatusCache{Size: 100},
EventWriter: events,
Logger: logger,
LoopVars: &LoopVars{GitOpTimeout: timeout},
LoopVars: &LoopVars{GitTimeout: timeout},
}

start := func() {
Expand Down
18 changes: 18 additions & 0 deletions daemon/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,21 @@ daemon if possible:
`,
}
}

func unsignedHeadRevisionError(latestValidRevision, headRevision string) error {
hiddeco marked this conversation as resolved.
Show resolved Hide resolved
return &fluxerr.Error{
Type: fluxerr.User,
Err: fmt.Errorf("HEAD revision is unsigned"),
Help: `HEAD is not a verified commit.

The branch HEAD in the git repo is not verified, and fluxd is unable to
make a change on top of it. The last verified commit was

` + latestValidRevision + `

HEAD is

` + headRevision + `.
`,
}
}
Loading