Skip to content

Commit

Permalink
WIP Job Verification
Browse files Browse the repository at this point in the history
  • Loading branch information
moskyb committed Jul 27, 2023
1 parent 0d54916 commit 9ac378a
Show file tree
Hide file tree
Showing 8 changed files with 609 additions and 134 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
244 changes: 244 additions & 0 deletions agent/integration/job_verification_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
package integration

import (
"fmt"
"os"
"strings"
"testing"

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

const (
defaultJobID = "my-job-id"
signingKey = "llamasrock"
)

var (
jobWithInvalidSignature = &api.Job{
ID: defaultJobID,
ChunksMaxSizeBytes: 1024,
Step: pipeline.CommandStep{
Command: "echo hello world",
Signature: &pipeline.Signature{
Algorithm: "hmac-sha256",
SignedFields: []string{"command"},
Value: "bm90LXRoZS1yZWFsLXNpZ25hdHVyZQ==", // base64("not-the-real-signature"), an invalid signature
},
},
Env: map[string]string{
"BUILDKITE_COMMAND": "echo hello world",
},
}

jobWithValidSignature = &api.Job{
ID: defaultJobID,
ChunksMaxSizeBytes: 1024,
Step: pipeline.CommandStep{
Command: "echo hello world",
Signature: &pipeline.Signature{
Algorithm: "hmac-sha256",
SignedFields: []string{"command"},
// To calculate the below value:
// $ echo -n "llamasrock" > ~/secretkey.txt && \
// echo "steps: [{command: 'echo hello world'}]" \
// | buildkite-agent pipeline upload --dry-run --signing-key-path ~/secretkey.txt \
// | jq ".steps[0].signature.value"
Value: "lBpQXxY9mrqN4mnhhNXdXr7PfAjXSPG7nN0zoAPclG4=",
},
},
Env: map[string]string{
"BUILDKITE_COMMAND": "echo hello world",
},
}

jobWithNoSignature = &api.Job{
ChunksMaxSizeBytes: 1024,
ID: defaultJobID,
Step: pipeline.CommandStep{Command: "echo hello world"},
Env: map[string]string{"BUILDKITE_COMMAND": "echo hello world"},
}

jobWithMismatchedStepAndJob = &api.Job{
ID: defaultJobID,
ChunksMaxSizeBytes: 1024,
Step: pipeline.CommandStep{
Signature: &pipeline.Signature{
Algorithm: "hmac-sha256",
SignedFields: []string{"command"},
// To calculate the below value:
// $ echo -n "llamasrock" > ~/secretkey.txt && \
// echo "steps: [{command: 'echo hello world'}]" \
// | buildkite-agent pipeline upload --dry-run --signing-key-path ~/secretkey.txt \
// | jq ".steps[0].signature.value"
Value: "lBpQXxY9mrqN4mnhhNXdXr7PfAjXSPG7nN0zoAPclG4=",
},
Command: "echo hello world",
},
Env: map[string]string{
"BUILDKITE_COMMAND": "echo 'THIS ISN'T HELLO WORLD!!!! CRIMES'",
},
}
)

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

cases := []struct {
name string
agentConf agent.AgentConfiguration
job *api.Job
mockBootstrapExpectation func(*bintest.Mock)
expectedExitStatus string
expectedSignalReason string
expectLogsContain []string
}{
{
name: "when job signature is invalid, and InvalidSignatureBehavior is block, it refuses the job",
agentConf: agent.AgentConfiguration{JobVerificationInvalidSignatureBehavior: agent.VerificationBehaviourBlock},
job: jobWithInvalidSignature,
mockBootstrapExpectation: func(bt *bintest.Mock) { bt.Expect().NotCalled() },
expectedExitStatus: "-1",
expectedSignalReason: "job_verification_failed_invalid_signature",
expectLogsContain: []string{"⚠️ ERROR"},
},
{
name: "when job signature is invalid, and InvalidSignatureBehavior is warn, it warns and runs the job",
agentConf: agent.AgentConfiguration{JobVerificationInvalidSignatureBehavior: agent.VerificationBehaviourWarn},
job: jobWithInvalidSignature,
mockBootstrapExpectation: func(bt *bintest.Mock) { bt.Expect().Once().AndExitWith(0) },
expectedExitStatus: "0",
expectLogsContain: []string{"⚠️ WARNING"},
},
{
name: "when job signature is valid, it runs the job",
agentConf: agent.AgentConfiguration{JobVerificationInvalidSignatureBehavior: agent.VerificationBehaviourBlock},
job: jobWithValidSignature,
mockBootstrapExpectation: func(bt *bintest.Mock) { bt.Expect().Once().AndExitWith(0) },
expectedExitStatus: "0",
expectedSignalReason: "",
},
{
name: "when job signature is missing, and NoSignatureBehavior is block, it refuses the job",
agentConf: agent.AgentConfiguration{JobVerificationNoSignatureBehavior: agent.VerificationBehaviourBlock},
job: jobWithNoSignature,
mockBootstrapExpectation: func(bt *bintest.Mock) { bt.Expect().NotCalled() },
expectedExitStatus: "-1",
expectedSignalReason: "job_verification_failed_no_signature",
expectLogsContain: []string{"⚠️ ERROR"},
},
{
name: "when job signature is missing, and NoSignatureBehavior is warn, it warns and runs the job",
agentConf: agent.AgentConfiguration{JobVerificationNoSignatureBehavior: agent.VerificationBehaviourWarn},
job: jobWithNoSignature,
mockBootstrapExpectation: func(bt *bintest.Mock) { bt.Expect().Once().AndExitWith(0) },
expectedExitStatus: "0",
expectLogsContain: []string{"⚠️ WARNING"},
},
{
name: "when the step signature matches, but the job doesn't match the step, it fails signature verification",
agentConf: agent.AgentConfiguration{JobVerificationNoSignatureBehavior: agent.VerificationBehaviourBlock},
job: jobWithMismatchedStepAndJob,
mockBootstrapExpectation: func(bt *bintest.Mock) { bt.Expect().NotCalled() },
expectedExitStatus: "-1",
expectedSignalReason: "job_verification_failed_invalid_signature",
expectLogsContain: []string{
"⚠️ ERROR",
fmt.Sprintf(`the value of field "command" on the job (%q) does not match the value of the field on the step (%q)`,
jobWithMismatchedStepAndJob.Env["BUILDKITE_COMMAND"], jobWithMismatchedStepAndJob.Step.Command),
},
},
}

for _, tc := range cases {
tc := tc
t.Run(tc.name, func(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(signingKey))
if err != nil {
t.Fatalf("writing keyfile: %v", err)
}

tc.agentConf.JobVerificationKeyPath = keyFile.Name()

// create a mock agent API
e := createTestAgentEndpoint()
server := e.server(tc.job.ID)
defer server.Close()

mb := mockBootstrap(t)
tc.mockBootstrapExpectation(mb)
defer mb.CheckAndClose(t)

runJob(t, tc.job, server, tc.agentConf, mb)

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

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

logs := e.logsFor(t, tc.job.ID)

for _, want := range tc.expectLogsContain {
if !strings.Contains(logs, want) {
t.Errorf("logs = %q, want to contain %q", logs, want)
}
}

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

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

job := jobWithValidSignature

// create a mock agent API
e := createTestAgentEndpoint()
server := e.server(job.ID)
defer server.Close()

mb := mockBootstrap(t)
mb.Expect().NotCalled()
defer mb.CheckAndClose(t)

runJob(t, job, server, agent.AgentConfiguration{}, mb) // note no verification config

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

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

logs := e.logsFor(t, job.ID)

expectLogsContain := []string{
"⚠️ ERROR",
fmt.Sprintf("job %q was signed with signature %q, but no verification key was provided, so the job can't be verified", job.ID, job.Step.Signature.Value),
}

for _, want := range expectLogsContain {
if !strings.Contains(logs, want) {
t.Errorf("logs = %q, want to contain %q", logs, want)
}
}

if got, want := finish.SignalReason, "job_verification_failed_with_error"; got != want {
t.Errorf("job.SignalReason = %q, want %q", got, want)
}
}
36 changes: 21 additions & 15 deletions agent/integration/test_helpers.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package integration

import (
"bytes"
"compress/gzip"
"context"
"encoding/json"
"fmt"
Expand All @@ -10,6 +12,7 @@ import (
"os"
"path/filepath"
"runtime"
"strconv"
"strings"
"sync"
"testing"
Expand All @@ -26,6 +29,7 @@ func mockBootstrap(t *testing.T) *bintest.Mock {

// 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(), "/", "-")
name = strings.ReplaceAll(name, "'", "") // we also don"t like single quotes
bs, err := bintest.NewMock(fmt.Sprintf("buildkite-agent-bootstrap-%s", name))
if err != nil {
t.Fatalf("bintest.NewMock() error = %v", err)
Expand Down Expand Up @@ -64,13 +68,15 @@ func runJob(t *testing.T, j *api.Job, server *httptest.Server, cfg agent.AgentCo
}

type testAgentEndpoint struct {
calls map[string][][]byte
mtx sync.Mutex
calls map[string][][]byte
logChunks map[int]string
mtx sync.Mutex
}

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

Expand All @@ -94,22 +100,17 @@ func (tae *testAgentEndpoint) finishesFor(t *testing.T, jobID string) []api.Job
return finishes
}

func (tae *testAgentEndpoint) chunksFor(t *testing.T, jobID string) []api.Chunk {
func (tae *testAgentEndpoint) logsFor(t *testing.T, jobID string) string {
t.Helper()
tae.mtx.Lock()
defer tae.mtx.Unlock()

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

for _, b := range tae.calls[endpoint] {
var chunk api.Chunk
err := json.Unmarshal(b, &chunk)
if err != nil {
t.Fatalf("decoding accept request body: %v", err)
}
chunks = append(chunks, chunk)
logChunks := make([]string, len(tae.logChunks))
for seq, chunk := range tae.logChunks {
logChunks[seq-1] = chunk
}

return chunks
return strings.Join(logChunks, "")
}

func (t *testAgentEndpoint) server(jobID string) *httptest.Server {
Expand All @@ -127,6 +128,11 @@ func (t *testAgentEndpoint) server(jobID string) *httptest.Server {
case "/jobs/" + jobID + "/start":
rw.WriteHeader(http.StatusOK)
case "/jobs/" + jobID + "/chunks":
sequence := req.URL.Query().Get("sequence")
seqNo, _ := strconv.Atoi(sequence)
r, _ := gzip.NewReader(bytes.NewBuffer(b))
uz, _ := io.ReadAll(r)
t.logChunks[seqNo] = string(uz)
rw.WriteHeader(http.StatusCreated)
case "/jobs/" + jobID + "/finish":
rw.WriteHeader(http.StatusOK)
Expand Down
Loading

0 comments on commit 9ac378a

Please sign in to comment.