Skip to content

Commit

Permalink
WIP Signature verification
Browse files Browse the repository at this point in the history
  • Loading branch information
moskyb committed Jul 25, 2023
1 parent e36f353 commit ece0f77
Show file tree
Hide file tree
Showing 10 changed files with 706 additions and 370 deletions.
47 changes: 26 additions & 21 deletions agent/agent_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,32 @@ package agent
// AgentConfiguration is the run-time configuration for an agent that
// has been loaded from the config file and command-line params
type AgentConfiguration struct {
ConfigPath string
BootstrapScript string
BuildPath string
HooksPath string
SocketsPath string
GitMirrorsPath string
GitMirrorsLockTimeout int
GitMirrorsSkipUpdate bool
PluginsPath string
GitCheckoutFlags string
GitCloneFlags string
GitCloneMirrorFlags string
GitCleanFlags string
GitFetchFlags string
GitSubmodules bool
SSHKeyscan bool
CommandEval bool
PluginsEnabled bool
PluginValidation bool
LocalHooksEnabled bool
RunInPty bool
ConfigPath string
BootstrapScript string
BuildPath string
HooksPath string
SocketsPath string
GitMirrorsPath string
GitMirrorsLockTimeout int
GitMirrorsSkipUpdate bool
PluginsPath string
GitCheckoutFlags string
GitCloneFlags string
GitCloneMirrorFlags string
GitCleanFlags string
GitFetchFlags string
GitSubmodules bool
SSHKeyscan bool
CommandEval bool
PluginsEnabled bool
PluginValidation bool
LocalHooksEnabled bool
RunInPty bool

JobVerificationKeyPath string
JobVerificationNoSignatureBehavior string
JobVerificationInvalidSignatureBehavior string

ANSITimestamps bool
TimestampLines bool
HealthCheckAddr string
Expand Down
132 changes: 0 additions & 132 deletions agent/integration/job_runner_integration_test.go
Original file line number Diff line number Diff line change
@@ -1,23 +1,13 @@
package integration

import (
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"runtime"
"strconv"
"strings"
"testing"

"github.com/buildkite/agent/v3/agent"
"github.com/buildkite/agent/v3/api"
"github.com/buildkite/agent/v3/logger"
"github.com/buildkite/agent/v3/metrics"
"github.com/buildkite/bintest/v3"
)

Expand Down Expand Up @@ -196,125 +186,3 @@ func TestJobRunnerIgnoresPipelineChangesToProtectedVars(t *testing.T) {

runJob(t, j, server, agent.AgentConfiguration{CommandEval: true}, mb)
}

func mockBootstrap(t *testing.T) *bintest.Mock {
t.Helper()

// tests run using t.Run() will have a slash in their name, which will mess with paths to bintest binaries
name := strings.ReplaceAll(t.Name(), "/", "-")
bs, err := bintest.NewMock(fmt.Sprintf("buildkite-agent-bootstrap-%s", name))
if err != nil {
t.Fatalf("bintest.NewMock() error = %v", err)
}

return bs
}

func runJob(t *testing.T, j *api.Job, server *httptest.Server, cfg agent.AgentConfiguration, bs *bintest.Mock) {
l := logger.Discard

// minimal metrics, this could be cleaner
m := metrics.NewCollector(l, metrics.CollectorConfig{})
scope := m.Scope(metrics.Tags{})

// set the bootstrap into the config
cfg.BootstrapScript = bs.Path

client := api.NewClient(l, api.Config{
Endpoint: server.URL,
Token: "llamasrock",
})

jr, err := agent.NewJobRunner(l, client, agent.JobRunnerConfig{
Job: j,
AgentConfiguration: cfg,
MetricsScope: scope,
})
if err != nil {
t.Fatalf("agent.NewJobRunner() error = %v", err)
}

if err := jr.Run(context.Background()); err != nil {
t.Errorf("jr.Run() = %v", err)
}
}

type testAgentEndpoint struct {
calls map[string][][]byte
}

func createTestAgentEndpoint() *testAgentEndpoint {
return &testAgentEndpoint{
calls: make(map[string][][]byte, 4),
}
}

func (tae *testAgentEndpoint) finishesFor(t *testing.T, jobID string) []api.Job {
t.Helper()

endpoint := fmt.Sprintf("/jobs/%s/finish", jobID)
finishes := make([]api.Job, 0, len(tae.calls))

for _, b := range tae.calls[endpoint] {
var job api.Job
err := json.Unmarshal(b, &job)
if err != nil {
t.Fatalf("decoding accept request body: %v", err)
}
finishes = append(finishes, job)
}

return finishes
}

func (t *testAgentEndpoint) server(jobID string) *httptest.Server {
return httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
b, _ := io.ReadAll(req.Body)
t.calls[req.URL.Path] = append(t.calls[req.URL.Path], b)

switch req.URL.Path {
case "/jobs/" + jobID:
rw.WriteHeader(http.StatusOK)
fmt.Fprintf(rw, `{"state":"running"}`)
case "/jobs/" + jobID + "/start":
rw.WriteHeader(http.StatusOK)
case "/jobs/" + jobID + "/chunks":
rw.WriteHeader(http.StatusCreated)
case "/jobs/" + jobID + "/finish":
rw.WriteHeader(http.StatusOK)
default:
http.Error(rw, fmt.Sprintf("not found; method = %q, path = %q", req.Method, req.URL.Path), http.StatusNotFound)
}
}))
}

func mockPreBootstrap(t *testing.T, hooksDir string) *bintest.Mock {
t.Helper()

mock, err := bintest.NewMock(fmt.Sprintf("buildkite-agent-pre-bootstrap-hook-%s", t.Name()))
if err != nil {
t.Fatalf("bintest.NewMock() error = %v", err)
}

hookScript := filepath.Join(hooksDir, "pre-bootstrap")
body := ""

if runtime.GOOS == "windows" {
// You may be tempted to change this to `@%q`, but please do not. bintest doesn't like it when things change.
// (%q escapes backslashes, which are windows path separators and leads to this test failing on windows)
body = fmt.Sprintf(`@"%s"`, mock.Path)
hookScript += ".bat"
} else {
body = "#!/bin/sh\n" + mock.Path
}

if err := os.MkdirAll(hooksDir, 0o700); err != nil {
t.Fatalf("creating pre-bootstrap hook mock: os.MkdirAll() error = %v", err)
}

if err := os.WriteFile(hookScript, []byte(body), 0o777); err != nil {
t.Fatalf("creating pre-bootstrap hook mock: s.WriteFile() error = %v", err)
}

return mock
}
66 changes: 66 additions & 0 deletions agent/integration/job_verification_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package integration

import (
"os"
"testing"

"github.com/buildkite/agent/v3/agent"
"github.com/buildkite/agent/v3/api"
"github.com/buildkite/agent/v3/internal/pipeline"
)

func Test_WhenJobSignatureIsInvalid_AndInvalidSignatureBehaviorIsFail_ItRefusesTheJob(t *testing.T) {
t.Parallel()

keyFile, err := os.CreateTemp("", "keyfile")
if err != nil {
t.Fatalf("making keyfile: %v", err)
}
defer os.Remove(keyFile.Name())

_, err = keyFile.Write([]byte("llamasrock"))
if err != nil {
t.Fatalf("writing keyfile: %v", err)
}

jobID := "my-job-id"
j := &api.Job{
ID: jobID,
ChunksMaxSizeBytes: 1024,
Step: pipeline.CommandStep{
Command: "echo hello world",
Signature: &pipeline.Signature{
Algorithm: "hmac-sha256",
SignedFields: []string{"command"},
Value: "not-the-real-signature",
},
},
Env: map[string]string{
"BUILDKITE_COMMAND": "echo hello world",
},
}

// create a mock agent API
e := createTestAgentEndpoint()
server := e.server("my-job-id")
defer server.Close()

mb := mockBootstrap(t)
mb.Expect().NotCalled() // The bootstrap won't be called, as the pre-bootstrap hook failed
defer mb.CheckAndClose(t)

runJob(t, j, server, agent.AgentConfiguration{
JobVerificationKeyPath: keyFile.Name(),
JobVerificationInvalidSignatureBehavior: "block",
}, mb)

job := e.finishesFor(t, jobID)[0]

if got, want := job.ExitStatus, "-1"; got != want {
t.Errorf("job.ExitStatus = %q, want %q", got, want)
}

if got, want := job.SignalReason, "agent_refused"; got != want {
t.Errorf("job.SignalReason = %q, want %q", got, want)
}
}
Loading

0 comments on commit ece0f77

Please sign in to comment.