From efe47dd71d4874d47301446e80a6613bc779f7e6 Mon Sep 17 00:00:00 2001 From: Josh Deprez Date: Mon, 26 Aug 2024 13:56:46 +1000 Subject: [PATCH] Canonicalise empty env/plugins/matrix to nil for sign/verify --- signature/pipeline_invariants.go | 12 +-- signature/sign.go | 47 ++++++++++++ signature/sign_test.go | 124 +++++++++++++++++++++++++++++++ step_command_matrix.go | 6 ++ 4 files changed, 183 insertions(+), 6 deletions(-) diff --git a/signature/pipeline_invariants.go b/signature/pipeline_invariants.go index d643a7d..8040115 100644 --- a/signature/pipeline_invariants.go +++ b/signature/pipeline_invariants.go @@ -19,9 +19,9 @@ type CommandStepWithInvariants struct { func (c *CommandStepWithInvariants) SignedFields() (map[string]any, error) { return map[string]any{ "command": c.Command, - "env": c.Env, - "plugins": c.Plugins, - "matrix": c.Matrix, + "env": EmptyToNilMap(c.Env), + "plugins": EmptyToNilSlice(c.Plugins), + "matrix": EmptyToNilPtr(c.Matrix), "repository_url": c.RepositoryURL, }, nil } @@ -47,13 +47,13 @@ func (c *CommandStepWithInvariants) ValuesForFields(fields []string) (map[string out["command"] = c.Command case "env": - out["env"] = c.Env + out["env"] = EmptyToNilMap(c.Env) case "plugins": - out["plugins"] = c.Plugins + out["plugins"] = EmptyToNilSlice(c.Plugins) case "matrix": - out["matrix"] = c.Matrix + out["matrix"] = EmptyToNilPtr(c.Matrix) case "repository_url": out["repository_url"] = c.RepositoryURL diff --git a/signature/sign.go b/signature/sign.go index 677658f..9c34938 100644 --- a/signature/sign.go +++ b/signature/sign.go @@ -207,6 +207,53 @@ func Verify(ctx context.Context, s *pipeline.Signature, keySet jwk.Set, sf Signe return err } +// EmptyToNilMap returns a nil map if m is empty, otherwise it returns m. +// This can be used to canonicalise empty/nil values if there is no semantic +// distinction between nil and empty. +// Sign and Verify do not apply this automatically. +// nil was chosen as the canonical value, since it is the zero value for the +// type. (A user would have to write e.g. "env: {}" to get a zero-length +// non-nil env map.) +func EmptyToNilMap[K comparable, V any, M ~map[K]V](m M) M { + if len(m) == 0 { + return nil + } + return m +} + +// EmptyToNilSlice returns a nil slice if s is empty, otherwise it returns s. +// This can be used to canonicalise empty/nil values if there is no semantic +// distinction between nil and empty. +// Sign and Verify do not apply this automatically. +// nil was chosen as the canonical value, since it is the zero value for the +// type. (A user would have to write e.g. "plugins: []" to get a zero-length +// non-nil plugins slice.) +func EmptyToNilSlice[E any, S ~[]E](s S) S { + if len(s) == 0 { + return nil + } + return s +} + +type pointerEmptyable[V any] interface { + ~*V + IsEmpty() bool +} + +// EmptyToNilPtr returns a nil pointer if p points to a variable containing +// an empty value for V, otherwise it returns p. Emptiness is determined by +// calling IsEmpty on p. +// Sign and Verify do not apply this automatically. +// nil was chosen as the canonical value since it is the zero value for pointer +// types. (A user would have to write e.g. "matrix: {}" to get an empty non-nil +// matrix specification.) +func EmptyToNilPtr[V any, P pointerEmptyable[V]](p P) P { + if p.IsEmpty() { + return nil + } + return p +} + // canonicalPayload returns a unique sequence of bytes representing the given // algorithm and values using JCS (RFC 8785). func canonicalPayload(alg string, values map[string]any) ([]byte, error) { diff --git a/signature/sign_test.go b/signature/sign_test.go index 338ccf9..3b56ceb 100644 --- a/signature/sign_test.go +++ b/signature/sign_test.go @@ -384,6 +384,130 @@ func TestSignVerifyEnv(t *testing.T) { } } +func TestSignVerify_NilVsEmpty(t *testing.T) { + t.Parallel() + ctx := context.Background() + + cases := []struct { + name string + stepSign *pipeline.CommandStep + stepVerify *pipeline.CommandStep + }{ + { + name: "env both non-empty", + stepSign: &pipeline.CommandStep{ + Command: "llamas", + Env: map[string]string{ + "CONTEXT": "cats", + "DEPLOY": "0", + }, + }, + stepVerify: &pipeline.CommandStep{ + Command: "llamas", + Env: map[string]string{ + "CONTEXT": "cats", + "DEPLOY": "0", + }, + }, + }, + { + name: "env sign nil verify nil", + stepSign: &pipeline.CommandStep{Command: "llamas", Env: nil}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Env: nil}, + }, + { + name: "env sign empty verify nil", + stepSign: &pipeline.CommandStep{Command: "llamas", Env: map[string]string{}}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Env: nil}, + }, + { + name: "env sign nil verify empty", + stepSign: &pipeline.CommandStep{Command: "llamas", Env: nil}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Env: map[string]string{}}, + }, + { + name: "env sign empty verify empty", + stepSign: &pipeline.CommandStep{Command: "llamas", Env: map[string]string{}}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Env: map[string]string{}}, + }, + { + name: "plugins sign nil verify nil", + stepSign: &pipeline.CommandStep{Command: "llamas", Plugins: nil}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Plugins: nil}, + }, + { + name: "plugins sign nil verify empty", + stepSign: &pipeline.CommandStep{Command: "llamas", Plugins: nil}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Plugins: pipeline.Plugins{}}, + }, + { + name: "plugins sign empty verify nil", + stepSign: &pipeline.CommandStep{Command: "llamas", Plugins: pipeline.Plugins{}}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Plugins: nil}, + }, + { + name: "plugins sign empty verify empty", + stepSign: &pipeline.CommandStep{Command: "llamas", Plugins: pipeline.Plugins{}}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Plugins: pipeline.Plugins{}}, + }, + { + name: "matrix sign nil verify nil", + stepSign: &pipeline.CommandStep{Command: "llamas", Matrix: nil}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Matrix: nil}, + }, + { + name: "matrix sign nil verify empty", + stepSign: &pipeline.CommandStep{Command: "llamas", Matrix: nil}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Matrix: &pipeline.Matrix{}}, + }, + { + name: "matrix sign empty verify nil", + stepSign: &pipeline.CommandStep{Command: "llamas", Matrix: &pipeline.Matrix{}}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Matrix: nil}, + }, + { + name: "matrix sign empty verify empty", + stepSign: &pipeline.CommandStep{Command: "llamas", Matrix: &pipeline.Matrix{}}, + stepVerify: &pipeline.CommandStep{Command: "llamas", Matrix: &pipeline.Matrix{}}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + keyStr, keyAlg := "alpacas", jwa.HS256 + signer, verifier, err := jwkutil.NewSymmetricKeyPairFromString(keyID, keyStr, keyAlg) + if err != nil { + t.Fatalf("jwkutil.NewSymmetricKeyPairFromString(%q, %q, %q) error = %v", keyID, keyStr, keyAlg, err) + } + + key, ok := signer.Key(0) + if !ok { + t.Fatalf("signer.Key(0) = _, false, want true") + } + + toSign := &CommandStepWithInvariants{ + CommandStep: *tc.stepSign, + RepositoryURL: fakeRepositoryURL, + } + toVerify := &CommandStepWithInvariants{ + CommandStep: *tc.stepVerify, + RepositoryURL: fakeRepositoryURL, + } + + sig, err := Sign(ctx, key, toSign) + if err != nil { + t.Fatalf("Sign(ctx, key, %v) error = %v", toSign, err) + } + + if err := Verify(ctx, sig, verifier, toVerify); err != nil { + t.Errorf("Verify(ctx, %v, verifier, %v) = %v", sig, toVerify, err) + } + }) + } +} + func TestSignatureStability(t *testing.T) { t.Parallel() ctx := context.Background() diff --git a/step_command_matrix.go b/step_command_matrix.go index a89c242..e5e6b62 100644 --- a/step_command_matrix.go +++ b/step_command_matrix.go @@ -50,6 +50,12 @@ type Matrix struct { RemainingFields map[string]any `yaml:",inline"` } +// IsEmpty reports whether the matrix is empty (is nil, or has no setup, +// no adjustments, and no other data within it). +func (m *Matrix) IsEmpty() bool { + return m == nil || (len(m.Setup) == 0 && len(m.Adjustments) == 0 && len(m.RemainingFields) == 0) +} + // UnmarshalOrdererd unmarshals from either []any or *ordered.MapSA. func (m *Matrix) UnmarshalOrdered(o any) error { switch src := o.(type) {