diff --git a/agent/integration/job_verification_integration_test.go b/agent/integration/job_verification_integration_test.go index ed2a896b54..24ce845fb5 100644 --- a/agent/integration/job_verification_integration_test.go +++ b/agent/integration/job_verification_integration_test.go @@ -22,15 +22,52 @@ var ( job = api.Job{ ChunksMaxSizeBytes: 1024, ID: defaultJobID, - Step: pipeline.CommandStep{Command: "echo hello world"}, - Env: map[string]string{"BUILDKITE_COMMAND": "echo hello world"}, + Step: pipeline.CommandStep{ + Command: "echo hello world", + Plugins: pipeline.Plugins{ + { + Name: "some-plugin#v1.0.0", + Config: map[string]string{ + "key": "value", + }, + }, + }, + }, + Env: map[string]string{ + "BUILDKITE_COMMAND": "echo hello world", + "BUILDKITE_PLUGINS": `[{"some-plugin#v1.0.0":{"key":"value"}}]`, + }, } jobWithMismatchedStepAndJob = api.Job{ ID: defaultJobID, ChunksMaxSizeBytes: 1024, - Step: pipeline.CommandStep{Command: "echo hello world"}, - Env: map[string]string{"BUILDKITE_COMMAND": "echo 'THIS ISN'T HELLO WORLD!!!! CRIMES'"}, + Step: pipeline.CommandStep{ + Command: "echo hello world", + }, + Env: map[string]string{ + "BUILDKITE_COMMAND": "echo 'THIS ISN'T HELLO WORLD!!!! CRIMES'", + }, + } + + jobWithMismatchedPlugins = api.Job{ + ChunksMaxSizeBytes: 1024, + ID: defaultJobID, + Step: pipeline.CommandStep{ + Command: "echo hello world", + Plugins: pipeline.Plugins{ + { + Name: "some-plugin#v1.0.0", + Config: map[string]string{ + "key": "value", + }, + }, + }, + }, + Env: map[string]string{ + "BUILDKITE_COMMAND": "echo hello world", + "BUILDKITE_PLUGINS": `[{"crimes-plugin#v1.0.0":{"steal":"everything"}}]`, + }, } ) @@ -115,6 +152,21 @@ func TestJobVerification(t *testing.T) { jobWithMismatchedStepAndJob.Env["BUILDKITE_COMMAND"], jobWithMismatchedStepAndJob.Step.Command), }, }, + { + name: "when the step signature matches, but the plugins doesn't match the step, it fails signature verification", + agentConf: agent.AgentConfiguration{JobVerificationNoSignatureBehavior: agent.VerificationBehaviourBlock}, + job: jobWithMismatchedPlugins, + signingKey: symmetricJWKFor(t, signingKeyLlamas), + verificationJWKS: jwksFromKeys(t, symmetricJWKFor(t, signingKeyLlamas)), + mockBootstrapExpectation: func(bt *bintest.Mock) { bt.Expect().NotCalled() }, + expectedExitStatus: "-1", + expectedSignalReason: agent.SignalReasonSignatureRejected, + expectLogsContain: []string{ + "⚠️ ERROR", + fmt.Sprintf(`the value of field "plugins" on the job (%q) does not match the value of the field on the step (%q)`, + jobWithMismatchedPlugins.Env["BUILDKITE_PLUGINS"], job.Env["BUILDKITE_PLUGINS"]), + }, + }, { name: "when the step has a signature, but the JobRunner doesn't have a verification key, it fails signature verification", agentConf: agent.AgentConfiguration{}, diff --git a/api/jobs.go b/api/jobs.go index 46cbb0be82..dee7d6e420 100644 --- a/api/jobs.go +++ b/api/jobs.go @@ -32,6 +32,10 @@ func (j *Job) ValuesForFields(fields []string) (map[string]string, error) { switch f { case "command": o[f] = j.Env["BUILDKITE_COMMAND"] + + case "plugins": + o[f] = j.Env["BUILDKITE_PLUGINS"] + default: return nil, fmt.Errorf("unknown or unsupported field on Job struct for signing/verification: %q", f) } diff --git a/internal/pipeline/sign.go b/internal/pipeline/sign.go index 4d89eaf218..1acafd41c0 100644 --- a/internal/pipeline/sign.go +++ b/internal/pipeline/sign.go @@ -24,7 +24,10 @@ type Signature struct { func Sign(sf SignedFielder, key jwk.Key) (*Signature, error) { payload := &bytes.Buffer{} - values := sf.SignedFields() + values, err := sf.SignedFields() + if err != nil { + return nil, err + } if len(values) == 0 { return nil, errors.New("sign: no fields to sign") } @@ -89,7 +92,7 @@ func (s *Signature) Verify(sf SignedFielder, keySet jwk.Set) error { type SignedFielder interface { // SignedFields returns the default set of fields to sign, and their values. // This is called by Sign. - SignedFields() map[string]string + SignedFields() (map[string]string, error) // ValuesForFields looks up each field and produces a map of values. This is // called by Verify. The set of fields might differ from the default, e.g. diff --git a/internal/pipeline/sign_test.go b/internal/pipeline/sign_test.go index ba691dd50c..8802076d3c 100644 --- a/internal/pipeline/sign_test.go +++ b/internal/pipeline/sign_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/buildkite/agent/v3/internal/ordered" "github.com/lestrrat-go/jwx/v2/jwa" "github.com/lestrrat-go/jwx/v2/jwk" ) @@ -14,6 +15,21 @@ import ( func TestSignVerify(t *testing.T) { step := &CommandStep{ Command: "llamas", + Plugins: Plugins{ + { + Name: "some-plugin#v1.0.0", + Config: nil, + }, + { + Name: "another-plugin#v3.4.5", + Config: ordered.MapFromItems( + ordered.TupleSA{ + Key: "llama", + Value: "Kuzco", + }, + ), + }, + }, } cases := []struct { @@ -26,19 +42,19 @@ func TestSignVerify(t *testing.T) { name: "HMAC-SHA256", generateSigner: func(alg jwa.SignatureAlgorithm) (jwk.Key, jwk.Set) { return newSymmetricKeyPair(t, "alpacas", alg) }, alg: jwa.HS256, - expectedDeterministicSignature: "eyJhbGciOiJIUzI1NiIsImtpZCI6IlRlc3RTaWduVmVyaWZ5In0..f5NQYQtR0Eg-0pzzCon2ykzGy5oDPYtQw0C0fTKGI38", + expectedDeterministicSignature: "eyJhbGciOiJIUzI1NiIsImtpZCI6IlRlc3RTaWduVmVyaWZ5In0..NDiUjV0myH279-OQi6eOKjgyhAPUnc5ZmZoynhUUvIo", }, { name: "HMAC-SHA384", generateSigner: func(alg jwa.SignatureAlgorithm) (jwk.Key, jwk.Set) { return newSymmetricKeyPair(t, "alpacas", alg) }, alg: jwa.HS384, - expectedDeterministicSignature: "eyJhbGciOiJIUzM4NCIsImtpZCI6IlRlc3RTaWduVmVyaWZ5In0..HgHltOlatth2TCc4swArP1UL_Zm2Rh2ccEC26s1sFBO6FOW5qfW37uQ9CHAz6dhh", + expectedDeterministicSignature: "eyJhbGciOiJIUzM4NCIsImtpZCI6IlRlc3RTaWduVmVyaWZ5In0..XGdZ7TG0lBSg7rXc091A3OaXAjODyI7aFkAjFJblD0YUnC5WW6WHgmJqlrG94x7z", }, { name: "HMAC-SHA512", generateSigner: func(alg jwa.SignatureAlgorithm) (jwk.Key, jwk.Set) { return newSymmetricKeyPair(t, "alpacas", alg) }, alg: jwa.HS512, - expectedDeterministicSignature: "eyJhbGciOiJIUzUxMiIsImtpZCI6IlRlc3RTaWduVmVyaWZ5In0..mcph5zwioGkmx-aPrxExzc9QRzO4afn_kK_89aEuo4xYD0tcUD8OJom09x2xcvK6eRkOpvVlkrKLBzvh-7uu6w", + expectedDeterministicSignature: "eyJhbGciOiJIUzUxMiIsImtpZCI6IlRlc3RTaWduVmVyaWZ5In0..GvKR_cGqNcF8EgffnkSoymJORoH60W36O80tYnGwnKXTUTh0XVmnEp0gT03YYRdf39JnwqbMGCticQJFFA_jWg", }, { name: "RSA-PSS 256", @@ -110,7 +126,7 @@ func TestSignVerify(t *testing.T) { type testFields map[string]string -func (m testFields) SignedFields() map[string]string { return m } +func (m testFields) SignedFields() (map[string]string, error) { return m, nil } func (m testFields) ValuesForFields(fields []string) (map[string]string, error) { out := make(map[string]string, len(fields)) diff --git a/internal/pipeline/step_command.go b/internal/pipeline/step_command.go index 34dd4f0709..2bb4a32989 100644 --- a/internal/pipeline/step_command.go +++ b/internal/pipeline/step_command.go @@ -1,7 +1,7 @@ package pipeline import ( - "errors" + "encoding/json" "fmt" "strings" @@ -58,25 +58,62 @@ func (c *CommandStep) UnmarshalOrdered(src any) error { } // SignedFields returns the default fields for signing. -func (c *CommandStep) SignedFields() map[string]string { +func (c *CommandStep) SignedFields() (map[string]string, error) { + plugins := "" + if len(c.Plugins) > 0 { + // TODO: Reconsider using JSON here - is it stable enough? + pj, err := json.Marshal(c.Plugins) + if err != nil { + return nil, err + } + plugins = string(pj) + } return map[string]string{ "command": c.Command, - } + "plugins": plugins, + }, nil } // ValuesForFields returns the contents of fields to sign. func (c *CommandStep) ValuesForFields(fields []string) (map[string]string, error) { + // Make a set of required fields. As fields is processed, mark them off by + // deleting them. + required := map[string]struct{}{ + "command": {}, + "plugins": {}, + } + out := make(map[string]string, len(fields)) for _, f := range fields { + delete(required, f) + switch f { case "command": out["command"] = c.Command + + case "plugins": + if len(c.Plugins) == 0 { + out["plugins"] = "" + break + } + // TODO: Reconsider using JSON here - is it stable enough? + val, err := json.Marshal(c.Plugins) + if err != nil { + return nil, err + } + out["plugins"] = string(val) + default: return nil, fmt.Errorf("unknown or unsupported field for signing %q", f) } } - if _, ok := out["command"]; !ok { - return nil, errors.New("command is required for signature verification") + + if len(required) > 0 { + missing := make([]string, 0, len(required)) + for k := range required { + missing = append(missing, k) + } + return nil, fmt.Errorf("one or more required fields are not present: %v", missing) } return out, nil }