diff --git a/agent/agent_configuration.go b/agent/agent_configuration.go index c0043825fc..4066c1f6e7 100644 --- a/agent/agent_configuration.go +++ b/agent/agent_configuration.go @@ -5,28 +5,34 @@ import "time" // 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 - StrictSingleHooks 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 + StrictSingleHooks bool + RunInPty bool + + JobSigningKeyPath string + JobVerificationKeyPath string + JobVerificationNoSignatureBehavior string + JobVerificationInvalidSignatureBehavior string + ANSITimestamps bool TimestampLines bool HealthCheckAddr string diff --git a/agent/integration/job_runner_integration_test.go b/agent/integration/job_runner_integration_test.go index a9926ca6f7..3cc62254bd 100644 --- a/agent/integration/job_runner_integration_test.go +++ b/agent/integration/job_runner_integration_test.go @@ -1,24 +1,13 @@ package integration import ( - "context" - "encoding/json" "fmt" - "io" - "net/http" - "net/http/httptest" "os" - "path/filepath" - "runtime" "strconv" - "strings" - "sync" "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" ) @@ -197,131 +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 { - mu sync.RWMutex - 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() - - tae.mu.RLock() - defer tae.mu.RUnlock() - - 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.mu.Lock() - t.calls[req.URL.Path] = append(t.calls[req.URL.Path], b) - t.mu.Unlock() - - 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 -} diff --git a/agent/integration/job_verification_integration_test.go b/agent/integration/job_verification_integration_test.go new file mode 100644 index 0000000000..7707808ad9 --- /dev/null +++ b/agent/integration/job_verification_integration_test.go @@ -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: agent.SignalReasonSignatureRejected, + 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: agent.SignalReasonSignatureRejected, + 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: agent.SignalReasonSignatureRejected, + 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, agent.SignalReasonSignatureRejected; got != want { + t.Errorf("job.SignalReason = %q, want %q", got, want) + } +} diff --git a/agent/integration/test_helpers.go b/agent/integration/test_helpers.go new file mode 100644 index 0000000000..d7dc8a9e48 --- /dev/null +++ b/agent/integration/test_helpers.go @@ -0,0 +1,176 @@ +package integration + +import ( + "bytes" + "compress/gzip" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strconv" + "strings" + "sync" + "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" +) + +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(), "/", "-") + 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) + } + + return bs +} + +func runJob(t *testing.T, j *api.Job, server *httptest.Server, cfg agent.AgentConfiguration, bs *bintest.Mock) { + t.Helper() + + 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 + logChunks map[int]string + mtx sync.Mutex +} + +func createTestAgentEndpoint() *testAgentEndpoint { + return &testAgentEndpoint{ + calls: make(map[string][][]byte, 4), + logChunks: make(map[int]string), + } +} + +func (tae *testAgentEndpoint) finishesFor(t *testing.T, jobID string) []api.Job { + t.Helper() + tae.mtx.Lock() + defer tae.mtx.Unlock() + + 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 (tae *testAgentEndpoint) logsFor(t *testing.T, jobID string) string { + t.Helper() + tae.mtx.Lock() + defer tae.mtx.Unlock() + + logChunks := make([]string, len(tae.logChunks)) + for seq, chunk := range tae.logChunks { + logChunks[seq-1] = chunk + } + + return strings.Join(logChunks, "") +} + +func (t *testAgentEndpoint) server(jobID string) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + t.mtx.Lock() + defer t.mtx.Unlock() + + 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": + 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) + 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 +} diff --git a/agent/job_runner.go b/agent/job_runner.go index 05c08caf86..01c4f8f1b0 100644 --- a/agent/job_runner.go +++ b/agent/job_runner.go @@ -41,6 +41,9 @@ const ( // BuildkiteMessageName is the env var name of the build/commit message. BuildkiteMessageName = "BUILDKITE_MESSAGE" + + VerificationBehaviourWarn = "warn" + VerificationBehaviourBlock = "block" ) // Certain env can only be set by agent configuration. @@ -105,6 +108,10 @@ type JobRunner struct { // The configuration for the job runner conf JobRunnerConfig + // How the JobRunner should respond in various signature failure modes + InvalidSignatureBehavior string + NoSignatureBehavior string + // The logger to use logger logger.Logger @@ -158,6 +165,17 @@ func NewJobRunner(l logger.Logger, apiClient APIClient, conf JobRunnerConfig) (j apiClient: apiClient, } + var err error + r.InvalidSignatureBehavior, err = r.normalizeVerificationBehavior(conf.AgentConfiguration.JobVerificationInvalidSignatureBehavior) + if err != nil { + return nil, fmt.Errorf("setting invalid signature behavior: %w", err) + } + + r.NoSignatureBehavior, err = r.normalizeVerificationBehavior(conf.AgentConfiguration.JobVerificationNoSignatureBehavior) + if err != nil { + return nil, fmt.Errorf("setting no signature behavior: %w", err) + } + if conf.JobStatusInterval == 0 { conf.JobStatusInterval = 1 * time.Second } @@ -358,6 +376,22 @@ func NewJobRunner(l logger.Logger, apiClient APIClient, conf JobRunnerConfig) (j return r, nil } +func (r *JobRunner) normalizeVerificationBehavior(behavior string) (string, error) { + if r.conf.AgentConfiguration.JobVerificationKeyPath == "" { + // We won't be verifying jobs, so it doesn't matter + return "if you're seeing this string, there's a problem with the job verification code in the agent. contact support@buildkite.com", nil + } + + switch behavior { + case VerificationBehaviourBlock, VerificationBehaviourWarn: + return behavior, nil + case "": + return VerificationBehaviourBlock, nil + default: + return "", fmt.Errorf("invalid job verification behavior: %q", behavior) + } +} + // Creates the environment variables that will be used in the process and writes a flat environment file func (r *JobRunner) createEnvironment() ([]string, error) { // Create a clone of our jobs environment. We'll then set the @@ -463,6 +497,11 @@ func (r *JobRunner) createEnvironment() ([]string, error) { env["BUILDKITE_PTY"] = "false" } + // Pass signing details through to the executor - any pipelines uploaded by this agent will be signed + if r.conf.AgentConfiguration.JobSigningKeyPath != "" { + env["BUILDKITE_PIPELINE_UPLOAD_SIGNING_KEY_PATH"] = r.conf.AgentConfiguration.JobSigningKeyPath + } + enablePluginValidation := r.conf.AgentConfiguration.PluginValidation // Allow BUILDKITE_PLUGIN_VALIDATION to be enabled from env for easier // per-pipeline testing @@ -577,97 +616,6 @@ func (r *JobRunner) startJob(ctx context.Context, startedAt time.Time) error { }) } -// finishJob finishes the job in the Buildkite Agent API. If the FinishJob call -// cannot return successfully, this will retry for a long time. -func (r *JobRunner) finishJob(ctx context.Context, finishedAt time.Time, exit processExit, failedChunkCount int) error { - r.conf.Job.FinishedAt = finishedAt.UTC().Format(time.RFC3339Nano) - r.conf.Job.ExitStatus = strconv.Itoa(exit.Status) - r.conf.Job.Signal = exit.Signal - r.conf.Job.SignalReason = exit.SignalReason - r.conf.Job.ChunksFailedCount = failedChunkCount - - r.logger.Debug("[JobRunner] Finishing job with exit_status=%s, signal=%s and signal_reason=%s", - r.conf.Job.ExitStatus, r.conf.Job.Signal, r.conf.Job.SignalReason) - - ctx, cancel := context.WithTimeout(ctx, 48*time.Hour) - defer cancel() - - return roko.NewRetrier( - roko.TryForever(), - roko.WithJitter(), - roko.WithStrategy(roko.Constant(1*time.Second)), - ).DoWithContext(ctx, func(retrier *roko.Retrier) error { - response, err := r.apiClient.FinishJob(ctx, r.conf.Job) - if err != nil { - // If the API returns with a 422, that means that we - // succesfully tried to finish the job, but Buildkite - // rejected the finish for some reason. This can - // sometimes mean that Buildkite has cancelled the job - // before we get a chance to send the final API call - // (maybe this agent took too long to kill the - // process). In that case, we don't want to keep trying - // to finish the job forever so we'll just bail out and - // go find some more work to do. - if response != nil && response.StatusCode == 422 { - r.logger.Warn("Buildkite rejected the call to finish the job (%s)", err) - retrier.Break() - } else { - r.logger.Warn("%s (%s)", err, retrier) - } - } - - return err - }) -} - -// jobLogStreamer waits for the process to start, then grabs the job output -// every few seconds and sends it back to Buildkite. -func (r *JobRunner) jobLogStreamer(ctx context.Context, wg *sync.WaitGroup) { - ctx, setStat, done := status.AddSimpleItem(ctx, "Job Log Streamer") - defer done() - setStat("🏃 Starting...") - - defer func() { - wg.Done() - r.logger.Debug("[JobRunner] Routine that processes the log has finished") - }() - - select { - case <-r.process.Started(): - case <-ctx.Done(): - return - } - - for { - setStat("📨 Sending process output to log streamer") - - // Send the output of the process to the log streamer - // for processing - chunk := r.output.ReadAndTruncate() - if err := r.logStreamer.Process(chunk); err != nil { - r.logger.Error("Could not stream the log output: %v", err) - // So far, the only error from logStreamer.Process is if the log has - // reached the limit. - // Since we're not writing any more, close the buffer, which will - // cause future Writes to return an error. - r.output.Close() - } - - setStat("😴 Sleeping for a bit") - - // Sleep for a bit, or until the job is finished - select { - case <-time.After(1 * time.Second): - case <-ctx.Done(): - return - case <-r.process.Done(): - return - } - } - - // The final output after the process has finished is processed in Run(). -} - // jobCancellationChecker waits for the processs to start, then continuously // polls GetJobState to see if the job has been cancelled server-side. If so, // it calls r.Cancel. diff --git a/agent/run_job.go b/agent/run_job.go index 4d708b57f7..11f8ad92e5 100644 --- a/agent/run_job.go +++ b/agent/run_job.go @@ -2,16 +2,28 @@ package agent import ( "context" + "errors" + "fmt" "os" "strconv" "sync" "time" "github.com/buildkite/agent/v3/hook" + "github.com/buildkite/agent/v3/internal/pipeline" "github.com/buildkite/agent/v3/kubernetes" "github.com/buildkite/agent/v3/metrics" "github.com/buildkite/agent/v3/process" "github.com/buildkite/agent/v3/status" + "github.com/buildkite/roko" +) + +const ( + SignalReasonAgentRefused = "agent_refused" + SignalReasonAgentStop = "agent_stop" + SignalReasonCancel = "cancel" + SignalReasonSignatureRejected = "signature_rejected" + SignalReasonProcessRunError = "process_run_error" ) // Runs the job @@ -23,6 +35,19 @@ func (r *JobRunner) Run(ctx context.Context) error { r.startedAt = time.Now() + var verifier pipeline.Verifier + if r.conf.AgentConfiguration.JobVerificationKeyPath != "" { + verificationKey, err := os.ReadFile(r.conf.AgentConfiguration.JobVerificationKeyPath) + if err != nil { + return fmt.Errorf("failed to read job verification key: %w", err) + } + + verifier, err = pipeline.NewVerifier("hmac-sha256", verificationKey) + if err != nil { + return fmt.Errorf("failed to create job verifier: %w", err) + } + } + // Start the build in the Buildkite Agent API. This is the first thing // we do so if it fails, we don't have to worry about cleaning things // up like started log streamer workers, and so on. @@ -62,6 +87,49 @@ func (r *JobRunner) Run(ctx context.Context) error { r.cleanup(ctx, wg, exit) }(ctx, &wg) // Note the non-cancellable context (ctx rather than cctx) here - we don't want to be interrupted during cleanup + job := r.conf.Job + + if verifier == nil && job.Step.Signature != nil { + r.verificationFailureLogs( + fmt.Errorf("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), + VerificationBehaviourBlock, + ) + exit.Status = -1 + exit.SignalReason = SignalReasonSignatureRejected + return nil + } + + if verifier != nil { + ise := &invalidSignatureError{} + switch err := r.verifyJob(verifier); { + case errors.Is(err, ErrNoSignature): + r.verificationFailureLogs(err, r.NoSignatureBehavior) + if r.NoSignatureBehavior == VerificationBehaviourBlock { + exit.Status = -1 + exit.SignalReason = SignalReasonSignatureRejected + return nil + } + + case errors.As(err, &ise): + r.verificationFailureLogs(err, r.InvalidSignatureBehavior) + if r.InvalidSignatureBehavior == VerificationBehaviourBlock { + exit.Status = -1 + exit.SignalReason = SignalReasonSignatureRejected + return nil + } + + case err != nil: // some other error + r.verificationFailureLogs(err, VerificationBehaviourBlock) // errors in verification are always fatal + exit.Status = -1 + exit.SignalReason = SignalReasonSignatureRejected + return nil + + default: // no error, all good, keep going + r.logger.Info("Successfully verified job %s with signature %s", job.ID, job.Step.Signature.Value) + r.logStreamer.Process([]byte(fmt.Sprintf("✅ Verified job with signature %s\n", job.Step.Signature.Value))) + } + } + // Before executing the bootstrap process with the received Job env, execute the pre-bootstrap hook (if present) for // it to tell us whether it is happy to proceed. if hook, _ := hook.Find(r.conf.AgentConfiguration.HooksPath, "pre-bootstrap"); hook != "" { @@ -74,7 +142,7 @@ func (r *JobRunner) Run(ctx context.Context) error { r.logger.Error("pre-bootstrap hook rejected this job: %s", err) exit.Status = -1 - exit.SignalReason = "agent_refused" + exit.SignalReason = SignalReasonAgentRefused return nil } @@ -85,7 +153,7 @@ func (r *JobRunner) Run(ctx context.Context) error { go r.jobLogStreamer(cctx, &wg) go r.jobCancellationChecker(cctx, &wg) - exit = r.runJob(cctx) // Ignore gostaticcheck here, the value of exit is captured by the deferred cleanup function above + exit = r.runJob(cctx) return nil } @@ -106,7 +174,7 @@ func (r *JobRunner) runJob(ctx context.Context) processExit { // The process did not run at all, so make sure it fails return processExit{ Status: -1, - SignalReason: "process_run_error", + SignalReason: SignalReasonProcessRunError, } } // Intended to capture situations where the job-exec (aka bootstrap) container did not @@ -133,11 +201,11 @@ func (r *JobRunner) runJob(ctx context.Context) processExit { case r.stopped: // The agent is being gracefully stopped, and we signaled the job to end. Often due // to pending host shutdown or EC2 spot instance termination - exit.SignalReason = "agent_stop" + exit.SignalReason = SignalReasonAgentStop case r.cancelled: // The job was signaled because it was cancelled via the buildkite web UI - exit.SignalReason = "cancel" + exit.SignalReason = SignalReasonCancel } return exit @@ -187,6 +255,96 @@ func (r *JobRunner) cleanup(ctx context.Context, wg *sync.WaitGroup, exit proces r.logger.Info("Finished job %s", r.conf.Job.ID) } +// finishJob finishes the job in the Buildkite Agent API. If the FinishJob call +// cannot return successfully, this will retry for a long time. +func (r *JobRunner) finishJob(ctx context.Context, finishedAt time.Time, exit processExit, failedChunkCount int) error { + r.conf.Job.FinishedAt = finishedAt.UTC().Format(time.RFC3339Nano) + r.conf.Job.ExitStatus = strconv.Itoa(exit.Status) + r.conf.Job.Signal = exit.Signal + r.conf.Job.SignalReason = exit.SignalReason + r.conf.Job.ChunksFailedCount = failedChunkCount + + r.logger.Debug("[JobRunner] Finishing job with exit_status=%s, signal=%s and signal_reason=%s", + r.conf.Job.ExitStatus, r.conf.Job.Signal, r.conf.Job.SignalReason) + + ctx, cancel := context.WithTimeout(ctx, 48*time.Hour) + defer cancel() + + return roko.NewRetrier( + roko.TryForever(), + roko.WithJitter(), + roko.WithStrategy(roko.Constant(1*time.Second)), + ).DoWithContext(ctx, func(retrier *roko.Retrier) error { + response, err := r.apiClient.FinishJob(ctx, r.conf.Job) + if err != nil { + // If the API returns with a 422, that means that we + // succesfully tried to finish the job, but Buildkite + // rejected the finish for some reason. This can + // sometimes mean that Buildkite has cancelled the job + // before we get a chance to send the final API call + // (maybe this agent took too long to kill the + // process). In that case, we don't want to keep trying + // to finish the job forever so we'll just bail out and + // go find some more work to do. + if response != nil && response.StatusCode == 422 { + r.logger.Warn("Buildkite rejected the call to finish the job (%s)", err) + retrier.Break() + } else { + r.logger.Warn("%s (%s)", err, retrier) + } + } + + return err + }) +} + +// jobLogStreamer waits for the process to start, then grabs the job output +// every few seconds and sends it back to Buildkite. +func (r *JobRunner) jobLogStreamer(ctx context.Context, wg *sync.WaitGroup) { + ctx, setStat, done := status.AddSimpleItem(ctx, "Job Log Streamer") + defer done() + setStat("🏃 Starting...") + + defer func() { + wg.Done() + r.logger.Debug("[JobRunner] Routine that processes the log has finished") + }() + + select { + case <-r.process.Started(): + case <-ctx.Done(): + return + } + + for { + setStat("📨 Sending process output to log streamer") + + // Send the output of the process to the log streamer + // for processing + chunk := r.output.ReadAndTruncate() + if err := r.logStreamer.Process(chunk); err != nil { + r.logger.Error("Could not stream the log output: %v", err) + // So far, the only error from logStreamer.Process is if the log has + // reached the limit. + // Since we're not writing any more, close the buffer, which will + // cause future Writes to return an error. + r.output.Close() + } + + setStat("😴 Sleeping for a bit") + + // Sleep for a bit, or until the job is finished + select { + case <-time.After(1 * time.Second): + case <-ctx.Done(): + return + case <-r.process.Done(): + return + } + } + + // The final output after the process has finished is processed in Run(). +} func (r *JobRunner) CancelAndStop() error { r.cancelLock.Lock() r.stopped = true diff --git a/agent/verify_job.go b/agent/verify_job.go new file mode 100644 index 0000000000..f3f2277e0d --- /dev/null +++ b/agent/verify_job.go @@ -0,0 +1,83 @@ +package agent + +import ( + "errors" + "fmt" + + "github.com/buildkite/agent/v3/internal/pipeline" +) + +var ErrNoSignature = errors.New("job had no signature to verify") + +type invalidSignatureError struct { + underlying error +} + +func newInvalidSignatureError(err error) *invalidSignatureError { + return &invalidSignatureError{underlying: err} +} + +func (e *invalidSignatureError) Error() string { + return fmt.Sprintf("invalid signature: %v", e.underlying) +} + +func (e *invalidSignatureError) Unwrap() error { + return e.underlying +} + +func (r *JobRunner) verificationFailureLogs(err error, behavior string) { + label := "WARNING" + if behavior == VerificationBehaviourBlock { + label = "ERROR" + } + + r.logger.Warn("Job verification failed: %s", err.Error()) + r.logStreamer.Process([]byte(fmt.Sprintf("⚠️ %s: Job verification failed: %s\n", label, err.Error()))) + + if behavior == VerificationBehaviourWarn { + r.logger.Warn("Job will be run without verification - this is not recommended. You can change this behavior with the `job-verification-no-signature-behavior` agent configuration option.") + r.logStreamer.Process([]byte(fmt.Sprintf("⚠️ %s: Job will be run without verification\n", label))) + } +} + +func (r *JobRunner) verifyJob(verifier pipeline.Verifier) error { + step := r.conf.Job.Step + + if step.Matrix != nil { + r.logger.Warn("Signing/Verification of matrix jobs is not currently supported") + r.logger.Warn("Watch this space 👀") + + return nil + } + + if step.Signature == nil { + return ErrNoSignature + } + + // Verify the signature + if err := step.Signature.Verify(&step, verifier); err != nil { + return newInvalidSignatureError(err) + } + + // Now that the signature of the job's step is verified, we need to check if the fields on the job match those on the + // step. If they don't, we need to fail the job - more or less the only reason that the job and the step would have + // different fields would be if someone had modified the job on the backend after it was signed (aka crimes) + signedFields := step.Signature.SignedFields + jobFields, err := r.conf.Job.ValuesForFields(signedFields) + if err != nil { + return fmt.Errorf("failed to get values for fields %v on job %s: %w", signedFields, r.conf.Job.ID, err) + } + + stepFields, err := step.ValuesForFields(signedFields) + if err != nil { + return fmt.Errorf("failed to get values for fields %v on step: %w", signedFields, err) + } + + for _, field := range signedFields { + if jobFields[field] != stepFields[field] { + return newInvalidSignatureError(fmt.Errorf("job %q was signed with signature %q, but the value of field %q on the job (%q) does not match the value of the field on the step (%q)", r.conf.Job.ID, step.Signature.Value, field, jobFields[field], stepFields[field])) + } + } + + return nil +} diff --git a/api/jobs.go b/api/jobs.go index 8ffba6dad4..46cbb0be82 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -9,21 +9,35 @@ import ( // Job represents a Buildkite Agent API Job type Job struct { - ID string `json:"id,omitempty"` - Endpoint string `json:"endpoint"` - State string `json:"state,omitempty"` - Env map[string]string `json:"env,omitempty"` - Step *pipeline.CommandStep `json:"step,omitempty"` - ChunksMaxSizeBytes uint64 `json:"chunks_max_size_bytes,omitempty"` - LogMaxSizeBytes uint64 `json:"log_max_size_bytes,omitempty"` - Token string `json:"token,omitempty"` - ExitStatus string `json:"exit_status,omitempty"` - Signal string `json:"signal,omitempty"` - SignalReason string `json:"signal_reason,omitempty"` - StartedAt string `json:"started_at,omitempty"` - FinishedAt string `json:"finished_at,omitempty"` - RunnableAt string `json:"runnable_at,omitempty"` - ChunksFailedCount int `json:"chunks_failed_count,omitempty"` + ID string `json:"id,omitempty"` + Endpoint string `json:"endpoint"` + State string `json:"state,omitempty"` + Env map[string]string `json:"env,omitempty"` + Step pipeline.CommandStep `json:"step,omitempty"` + ChunksMaxSizeBytes uint64 `json:"chunks_max_size_bytes,omitempty"` + LogMaxSizeBytes uint64 `json:"log_max_size_bytes,omitempty"` + Token string `json:"token,omitempty"` + ExitStatus string `json:"exit_status,omitempty"` + Signal string `json:"signal,omitempty"` + SignalReason string `json:"signal_reason,omitempty"` + StartedAt string `json:"started_at,omitempty"` + FinishedAt string `json:"finished_at,omitempty"` + RunnableAt string `json:"runnable_at,omitempty"` + ChunksFailedCount int `json:"chunks_failed_count,omitempty"` +} + +func (j *Job) ValuesForFields(fields []string) (map[string]string, error) { + o := make(map[string]string, len(fields)) + for _, f := range fields { + switch f { + case "command": + o[f] = j.Env["BUILDKITE_COMMAND"] + default: + return nil, fmt.Errorf("unknown or unsupported field on Job struct for signing/verification: %q", f) + } + } + + return o, nil } type JobState struct { diff --git a/clicommand/agent_start.go b/clicommand/agent_start.go index 75ff60bf00..9458447c1a 100644 --- a/clicommand/agent_start.go +++ b/clicommand/agent_start.go @@ -34,6 +34,7 @@ import ( "github.com/mitchellh/go-homedir" "github.com/urfave/cli" "golang.org/x/exp/maps" + "golang.org/x/exp/slices" ) const startDescription = `Usage: @@ -53,6 +54,8 @@ Example: $ buildkite-agent start --token xxx` +var verificationFailureBehaviors = []string{agent.VerificationBehaviourBlock, agent.VerificationBehaviourWarn} + // Adding config requires changes in a few different spots // - The AgentStartConfig struct with a cli parameter // - As a flag in the AgentStartCommand (with matching env) @@ -60,65 +63,82 @@ Example: // - Into clicommand/bootstrap.go to read it from the env into the bootstrap config type AgentStartConfig struct { - Config string `cli:"config"` - Name string `cli:"name"` - Priority string `cli:"priority"` - AcquireJob string `cli:"acquire-job"` - DisconnectAfterJob bool `cli:"disconnect-after-job"` - DisconnectAfterIdleTimeout int `cli:"disconnect-after-idle-timeout"` - BootstrapScript string `cli:"bootstrap-script" normalize:"commandpath"` - CancelGracePeriod int `cli:"cancel-grace-period"` - EnableJobLogTmpfile bool `cli:"enable-job-log-tmpfile"` - JobLogPath string `cli:"job-log-path" normalize:"filepath"` - WriteJobLogsToStdout bool `cli:"write-job-logs-to-stdout"` - BuildPath string `cli:"build-path" normalize:"filepath" validate:"required"` - HooksPath string `cli:"hooks-path" normalize:"filepath"` - SocketsPath string `cli:"sockets-path" normalize:"filepath"` - PluginsPath string `cli:"plugins-path" normalize:"filepath"` - Shell string `cli:"shell"` - Tags []string `cli:"tags" normalize:"list"` - TagsFromEC2MetaData bool `cli:"tags-from-ec2-meta-data"` - TagsFromEC2MetaDataPaths []string `cli:"tags-from-ec2-meta-data-paths" normalize:"list"` - TagsFromEC2Tags bool `cli:"tags-from-ec2-tags"` - TagsFromECSMetaData bool `cli:"tags-from-ecs-meta-data"` - TagsFromGCPMetaData bool `cli:"tags-from-gcp-meta-data"` - TagsFromGCPMetaDataPaths []string `cli:"tags-from-gcp-meta-data-paths" normalize:"list"` - TagsFromGCPLabels bool `cli:"tags-from-gcp-labels"` - TagsFromHost bool `cli:"tags-from-host"` - WaitForEC2TagsTimeout string `cli:"wait-for-ec2-tags-timeout"` - WaitForEC2MetaDataTimeout string `cli:"wait-for-ec2-meta-data-timeout"` - WaitForECSMetaDataTimeout string `cli:"wait-for-ecs-meta-data-timeout"` - WaitForGCPLabelsTimeout string `cli:"wait-for-gcp-labels-timeout"` - GitCheckoutFlags string `cli:"git-checkout-flags"` - GitCloneFlags string `cli:"git-clone-flags"` - GitCloneMirrorFlags string `cli:"git-clone-mirror-flags"` - GitCleanFlags string `cli:"git-clean-flags"` - GitFetchFlags string `cli:"git-fetch-flags"` - GitMirrorsPath string `cli:"git-mirrors-path" normalize:"filepath"` - GitMirrorsLockTimeout int `cli:"git-mirrors-lock-timeout"` - GitMirrorsSkipUpdate bool `cli:"git-mirrors-skip-update"` - NoGitSubmodules bool `cli:"no-git-submodules"` - NoSSHKeyscan bool `cli:"no-ssh-keyscan"` - NoCommandEval bool `cli:"no-command-eval"` - NoLocalHooks bool `cli:"no-local-hooks"` - NoPlugins bool `cli:"no-plugins"` - NoPluginValidation bool `cli:"no-plugin-validation"` - NoPTY bool `cli:"no-pty"` - NoFeatureReporting bool `cli:"no-feature-reporting"` - NoANSITimestamps bool `cli:"no-ansi-timestamps"` - TimestampLines bool `cli:"timestamp-lines"` - HealthCheckAddr string `cli:"health-check-addr"` - MetricsDatadog bool `cli:"metrics-datadog"` - MetricsDatadogHost string `cli:"metrics-datadog-host"` - MetricsDatadogDistributions bool `cli:"metrics-datadog-distributions"` - TracingBackend string `cli:"tracing-backend"` - TracingServiceName string `cli:"tracing-service-name"` - Spawn int `cli:"spawn"` - SpawnWithPriority bool `cli:"spawn-with-priority"` - LogFormat string `cli:"log-format"` - CancelSignal string `cli:"cancel-signal"` - SignalGracePeriodSeconds int `cli:"signal-grace-period-seconds"` - RedactedVars []string `cli:"redacted-vars" normalize:"list"` + Config string `cli:"config"` + + Name string `cli:"name"` + Priority string `cli:"priority"` + Spawn int `cli:"spawn"` + SpawnWithPriority bool `cli:"spawn-with-priority"` + RedactedVars []string `cli:"redacted-vars" normalize:"list"` + CancelSignal string `cli:"cancel-signal"` + + JobSigningKeyPath string `cli:"job-signing-key-path" normalize:"filepath"` + JobVerificationKeyPath string `cli:"job-verification-key-path" normalize:"filepath"` + JobVerificationNoSignatureBehavior string `cli:"job-verification-no-signature-behavior"` + JobVerificationInvalidSignatureBehavior string `cli:"job-verification-invalid-signature-behavior"` + + AcquireJob string `cli:"acquire-job"` + DisconnectAfterJob bool `cli:"disconnect-after-job"` + DisconnectAfterIdleTimeout int `cli:"disconnect-after-idle-timeout"` + CancelGracePeriod int `cli:"cancel-grace-period"` + SignalGracePeriodSeconds int `cli:"signal-grace-period-seconds"` + + EnableJobLogTmpfile bool `cli:"enable-job-log-tmpfile"` + JobLogPath string `cli:"job-log-path" normalize:"filepath"` + + LogFormat string `cli:"log-format"` + WriteJobLogsToStdout bool `cli:"write-job-logs-to-stdout"` + + BuildPath string `cli:"build-path" normalize:"filepath" validate:"required"` + HooksPath string `cli:"hooks-path" normalize:"filepath"` + SocketsPath string `cli:"sockets-path" normalize:"filepath"` + PluginsPath string `cli:"plugins-path" normalize:"filepath"` + + Shell string `cli:"shell"` + BootstrapScript string `cli:"bootstrap-script" normalize:"commandpath"` + NoPTY bool `cli:"no-pty"` + + NoANSITimestamps bool `cli:"no-ansi-timestamps"` + TimestampLines bool `cli:"timestamp-lines"` + + Tags []string `cli:"tags" normalize:"list"` + TagsFromEC2MetaData bool `cli:"tags-from-ec2-meta-data"` + TagsFromEC2MetaDataPaths []string `cli:"tags-from-ec2-meta-data-paths" normalize:"list"` + TagsFromEC2Tags bool `cli:"tags-from-ec2-tags"` + TagsFromECSMetaData bool `cli:"tags-from-ecs-meta-data"` + TagsFromGCPMetaData bool `cli:"tags-from-gcp-meta-data"` + TagsFromGCPMetaDataPaths []string `cli:"tags-from-gcp-meta-data-paths" normalize:"list"` + TagsFromGCPLabels bool `cli:"tags-from-gcp-labels"` + TagsFromHost bool `cli:"tags-from-host"` + WaitForEC2TagsTimeout string `cli:"wait-for-ec2-tags-timeout"` + WaitForEC2MetaDataTimeout string `cli:"wait-for-ec2-meta-data-timeout"` + WaitForECSMetaDataTimeout string `cli:"wait-for-ecs-meta-data-timeout"` + WaitForGCPLabelsTimeout string `cli:"wait-for-gcp-labels-timeout"` + + GitCheckoutFlags string `cli:"git-checkout-flags"` + GitCloneFlags string `cli:"git-clone-flags"` + GitCloneMirrorFlags string `cli:"git-clone-mirror-flags"` + GitCleanFlags string `cli:"git-clean-flags"` + GitFetchFlags string `cli:"git-fetch-flags"` + GitMirrorsPath string `cli:"git-mirrors-path" normalize:"filepath"` + GitMirrorsLockTimeout int `cli:"git-mirrors-lock-timeout"` + GitMirrorsSkipUpdate bool `cli:"git-mirrors-skip-update"` + NoGitSubmodules bool `cli:"no-git-submodules"` + + NoSSHKeyscan bool `cli:"no-ssh-keyscan"` + NoCommandEval bool `cli:"no-command-eval"` + NoLocalHooks bool `cli:"no-local-hooks"` + NoPlugins bool `cli:"no-plugins"` + NoPluginValidation bool `cli:"no-plugin-validation"` + NoFeatureReporting bool `cli:"no-feature-reporting"` + + HealthCheckAddr string `cli:"health-check-addr"` + + MetricsDatadog bool `cli:"metrics-datadog"` + MetricsDatadogHost string `cli:"metrics-datadog-host"` + MetricsDatadogDistributions bool `cli:"metrics-datadog-distributions"` + TracingBackend string `cli:"tracing-backend"` + TracingServiceName string `cli:"tracing-service-name"` // Global flags Debug bool `cli:"debug"` @@ -559,6 +579,26 @@ var AgentStartCommand = cli.Command{ EnvVar: "BUILDKITE_TRACING_SERVICE_NAME", Value: "buildkite-agent", }, + cli.StringFlag{ + Name: "job-verification-key-path", + Usage: "Path to a file containing a verification key. Passing this flag enables job verification. For hmac-sha256, the raw file content is used as the shared key", + EnvVar: "BUILDKITE_AGENT_JOB_VERIFICATION_KEY_PATH", + }, + cli.StringFlag{ + Name: "job-signing-key-path", + Usage: "Path to a file containing a signing key. Passing this flag enables pipeline signing for all pipelines uploaded by this agent. For hmac-sha256, the raw file content is used as the shared key", + EnvVar: "BUILDKITE_PIPELINE_JOB_SIGNING_KEY_PATH", + }, + cli.StringFlag{ + Name: "job-verification-no-signature-behavior", + Usage: fmt.Sprintf("The behavior when a job is received without a signature. One of: %v", verificationFailureBehaviors), + EnvVar: "BUILDKITE_AGENT_JOB_VERIFICATION_NO_SIGNATURE_BEHAVIOR", + }, + cli.StringFlag{ + Name: "job-verification-invalid-signature-behavior", + Usage: fmt.Sprintf("The behavior when a job is received, and the signature calculated is different from the one specified. One of: %v", verificationFailureBehaviors), + EnvVar: "BUILDKITE_AGENT_JOB_VERIFICATION_INVALID_SIGNATURE_BEHAVIOR", + }, // API Flags AgentRegisterTokenFlag, @@ -662,6 +702,16 @@ var AgentStartCommand = cli.Command{ os.Exit(1) } + if cfg.JobVerificationKeyPath != "" { + if !slices.Contains(verificationFailureBehaviors, cfg.JobVerificationNoSignatureBehavior) { + l.Fatal("Invalid job verification no signature behavior %q. Must be one of: %v", cfg.JobVerificationNoSignatureBehavior, verificationFailureBehaviors) + } + + if !slices.Contains(verificationFailureBehaviors, cfg.JobVerificationInvalidSignatureBehavior) { + l.Fatal("Invalid job verification invalid signature behavior %q. Must be one of: %v", cfg.JobVerificationInvalidSignatureBehavior, verificationFailureBehaviors) + } + } + // Force some settings if on Windows (these aren't supported yet) if runtime.GOOS == "windows" { cfg.NoPTY = true @@ -774,42 +824,46 @@ var AgentStartCommand = cli.Command{ // AgentConfiguration is the runtime configuration for an agent agentConf := agent.AgentConfiguration{ - BootstrapScript: cfg.BootstrapScript, - BuildPath: cfg.BuildPath, - SocketsPath: cfg.SocketsPath, - GitMirrorsPath: cfg.GitMirrorsPath, - GitMirrorsLockTimeout: cfg.GitMirrorsLockTimeout, - GitMirrorsSkipUpdate: cfg.GitMirrorsSkipUpdate, - HooksPath: cfg.HooksPath, - PluginsPath: cfg.PluginsPath, - GitCheckoutFlags: cfg.GitCheckoutFlags, - GitCloneFlags: cfg.GitCloneFlags, - GitCloneMirrorFlags: cfg.GitCloneMirrorFlags, - GitCleanFlags: cfg.GitCleanFlags, - GitFetchFlags: cfg.GitFetchFlags, - GitSubmodules: !cfg.NoGitSubmodules, - SSHKeyscan: !cfg.NoSSHKeyscan, - CommandEval: !cfg.NoCommandEval, - PluginsEnabled: !cfg.NoPlugins, - PluginValidation: !cfg.NoPluginValidation, - LocalHooksEnabled: !cfg.NoLocalHooks, - StrictSingleHooks: cfg.StrictSingleHooks, - RunInPty: !cfg.NoPTY, - ANSITimestamps: !cfg.NoANSITimestamps, - TimestampLines: cfg.TimestampLines, - DisconnectAfterJob: cfg.DisconnectAfterJob, - DisconnectAfterIdleTimeout: cfg.DisconnectAfterIdleTimeout, - CancelGracePeriod: cfg.CancelGracePeriod, - SignalGracePeriod: signalGracePeriod, - EnableJobLogTmpfile: cfg.EnableJobLogTmpfile, - JobLogPath: cfg.JobLogPath, - WriteJobLogsToStdout: cfg.WriteJobLogsToStdout, - LogFormat: cfg.LogFormat, - Shell: cfg.Shell, - RedactedVars: cfg.RedactedVars, - AcquireJob: cfg.AcquireJob, - TracingBackend: cfg.TracingBackend, - TracingServiceName: cfg.TracingServiceName, + BootstrapScript: cfg.BootstrapScript, + BuildPath: cfg.BuildPath, + SocketsPath: cfg.SocketsPath, + GitMirrorsPath: cfg.GitMirrorsPath, + GitMirrorsLockTimeout: cfg.GitMirrorsLockTimeout, + GitMirrorsSkipUpdate: cfg.GitMirrorsSkipUpdate, + HooksPath: cfg.HooksPath, + PluginsPath: cfg.PluginsPath, + GitCheckoutFlags: cfg.GitCheckoutFlags, + GitCloneFlags: cfg.GitCloneFlags, + GitCloneMirrorFlags: cfg.GitCloneMirrorFlags, + GitCleanFlags: cfg.GitCleanFlags, + GitFetchFlags: cfg.GitFetchFlags, + GitSubmodules: !cfg.NoGitSubmodules, + SSHKeyscan: !cfg.NoSSHKeyscan, + CommandEval: !cfg.NoCommandEval, + PluginsEnabled: !cfg.NoPlugins, + PluginValidation: !cfg.NoPluginValidation, + LocalHooksEnabled: !cfg.NoLocalHooks, + StrictSingleHooks: cfg.StrictSingleHooks, + RunInPty: !cfg.NoPTY, + ANSITimestamps: !cfg.NoANSITimestamps, + TimestampLines: cfg.TimestampLines, + DisconnectAfterJob: cfg.DisconnectAfterJob, + DisconnectAfterIdleTimeout: cfg.DisconnectAfterIdleTimeout, + CancelGracePeriod: cfg.CancelGracePeriod, + SignalGracePeriod: signalGracePeriod, + EnableJobLogTmpfile: cfg.EnableJobLogTmpfile, + JobLogPath: cfg.JobLogPath, + WriteJobLogsToStdout: cfg.WriteJobLogsToStdout, + LogFormat: cfg.LogFormat, + Shell: cfg.Shell, + RedactedVars: cfg.RedactedVars, + AcquireJob: cfg.AcquireJob, + TracingBackend: cfg.TracingBackend, + TracingServiceName: cfg.TracingServiceName, + JobSigningKeyPath: cfg.JobSigningKeyPath, + JobVerificationKeyPath: cfg.JobVerificationKeyPath, + JobVerificationNoSignatureBehavior: cfg.JobVerificationNoSignatureBehavior, + JobVerificationInvalidSignatureBehavior: cfg.JobVerificationInvalidSignatureBehavior, } if loader.File != nil { diff --git a/internal/pipeline/json.go b/internal/pipeline/json.go index ae5c7da4e0..a28a02e3ea 100644 --- a/internal/pipeline/json.go +++ b/internal/pipeline/json.go @@ -74,6 +74,10 @@ func inlineFriendlyMarshalJSON(q any) ([]byte, error) { // stolen from encoding/json func isEmptyValue(q any) bool { + if q == nil { // not stolen from encoding/json, but oddly missing from it? + return true + } + v := reflect.ValueOf(q) switch v.Kind() { diff --git a/internal/pipeline/json_test.go b/internal/pipeline/json_test.go index 4f7b0a3667..bd1bf40634 100644 --- a/internal/pipeline/json_test.go +++ b/internal/pipeline/json_test.go @@ -4,7 +4,7 @@ import "testing" type strukt struct { Foo string `yaml:"foo"` - Bar string `yaml:"bar,omitempty"` + Bar any `yaml:"bar,omitempty"` Baz string `yaml:"-"` Qux map[string]any `yaml:",inline"` } @@ -32,7 +32,7 @@ func TestInlineFriendlyMarshalJSON(t *testing.T) { want: `{"foo":""}`, strukt: strukt{ Foo: "", // doesn't have omitempty, should show up in the result object - Bar: "", + Bar: nil, }, }, { diff --git a/internal/pipeline/sign.go b/internal/pipeline/sign.go index 4e2ad6c04a..a2205da8f2 100644 --- a/internal/pipeline/sign.go +++ b/internal/pipeline/sign.go @@ -229,7 +229,7 @@ func (h hmacSHA256) Sign() ([]byte, error) { func (h hmacSHA256) Verify(sig []byte) error { c := h.Hash.Sum(nil) if !bytes.Equal(c, sig) { - return fmt.Errorf("message digest mismatch (%x != %x)", c, sig) + return errors.New("signature mismatch") } return nil } diff --git a/internal/pipeline/sign_test_helpers.go b/internal/pipeline/sign_test_helpers.go new file mode 100644 index 0000000000..7577d1ff8c --- /dev/null +++ b/internal/pipeline/sign_test_helpers.go @@ -0,0 +1,33 @@ +package pipeline + +// import ( +// "testing" + +// "github.com/lestrrat-go/jwx/v2/jwk" +// ) + +// func newSymmetricKeyPair(t *testing.T, key string) (jwk.Key, jwk.Set) { +// t.Helper() +// k, err := jwk.FromRaw([]byte(key)) +// if err != nil { +// t.Fatalf("failed to create symmetric key: %s", err) +// } + +// err = k.Set(jwk.AlgorithmKey, "HS256") +// if err != nil { +// t.Fatalf("failed to set algorithm: %s", err) +// } + +// k.Set(jwk.KeyIDKey, "test-key") +// if err != nil { +// t.Fatalf("failed to set key id: %s", err) +// } + +// set := jwk.NewSet() +// err = set.AddKey(k) +// if err != nil { +// t.Fatalf("failed to add key to set: %s", err) +// } + +// return k, set +// } diff --git a/internal/pipeline/step_command.go b/internal/pipeline/step_command.go index f8bd79ecad..34cd4e504f 100644 --- a/internal/pipeline/step_command.go +++ b/internal/pipeline/step_command.go @@ -18,6 +18,7 @@ type CommandStep struct { Command string `yaml:"command"` Plugins Plugins `yaml:"plugins,omitempty"` Signature *Signature `yaml:"signature,omitempty"` + Matrix any `yaml:"matrix,omitempty"` // RemainingFields stores any other top-level mapping items so they at least // survive an unmarshal-marshal round-trip. @@ -72,6 +73,9 @@ func (c *CommandStep) unmarshalMap(m *ordered.MapSA) error { } c.Signature = sig + case "matrix": + c.Matrix = v + default: // Preserve any other key. if c.RemainingFields == nil { @@ -113,13 +117,20 @@ func (c *CommandStep) interpolate(env interpolate.Env) error { if err != nil { return err } + if err := interpolateSlice(env, c.Plugins); err != nil { return err } + + if c.Matrix, err = interpolateAny(env, c.Matrix); err != nil { + return err + } + // NB: Do not interpolate Signature. if err := interpolateMap(env, c.RemainingFields); err != nil { return err } + c.Command = cmd return nil } diff --git a/internal/pipeline/steps.go b/internal/pipeline/steps.go index a8836f0dd6..0e3fd32d67 100644 --- a/internal/pipeline/steps.go +++ b/internal/pipeline/steps.go @@ -141,6 +141,11 @@ func (s Steps) sign(signer Signer) error { for _, step := range s { switch step := step.(type) { case *CommandStep: + if step.Matrix != nil { + // Don't sign matrix steps... yet + continue + } + sig, err := Sign(step, signer) if err != nil { return fmt.Errorf("signing step with command %q: %w", step.Command, err)