diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f6d9740..fa51f5f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,11 +14,15 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: '1.24.0' + go-version-file: 'go.mod' - name: Set up Buf uses: bufbuild/buf-setup-action@v1 with: version: 1.50.0 + # Without a token the action resolves its download through the + # anonymous GitHub API and is rate limited by shared runner IP, + # which fails the job before any protobuf check runs. + github_token: ${{ github.token }} - name: Verify generated protobuf files are up to date run: | make proto diff --git a/go.mod b/go.mod index 461016b..697acae 100644 --- a/go.mod +++ b/go.mod @@ -1,14 +1,15 @@ module github.com/capiscio/capiscio-core/v2 -go 1.25.10 +go 1.25.13 require ( github.com/go-jose/go-jose/v4 v4.1.4 github.com/google/uuid v1.6.0 + github.com/gowebpki/jcs v1.0.1 github.com/open-policy-agent/opa v1.14.1 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 - google.golang.org/grpc v1.79.3 + google.golang.org/grpc v1.82.1 google.golang.org/protobuf v1.36.11 gopkg.in/yaml.v3 v3.0.1 ) @@ -40,14 +41,13 @@ require ( github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/yashtewari/glob-intersection v0.2.0 // indirect - go.opentelemetry.io/otel/sdk/metric v1.40.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/crypto v0.48.0 // indirect - golang.org/x/net v0.51.0 // indirect + golang.org/x/crypto v0.51.0 // indirect + golang.org/x/net v0.55.0 // indirect golang.org/x/sync v0.20.0 // indirect - golang.org/x/sys v0.41.0 // indirect - golang.org/x/text v0.34.0 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409 // indirect + golang.org/x/sys v0.45.0 // indirect + golang.org/x/text v0.37.0 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 // indirect sigs.k8s.io/yaml v1.6.0 // indirect ) diff --git a/go.sum b/go.sum index 4f3e716..6f3559d 100644 --- a/go.sum +++ b/go.sum @@ -46,6 +46,8 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/gowebpki/jcs v1.0.1 h1:Qjzg8EOkrOTuWP7DqQ1FbYtcpEbeTzUoTN9bptp8FOU= +github.com/gowebpki/jcs v1.0.1/go.mod h1:CID1cNZ+sHp1CCpAR8mPf6QRtagFBgPJE0FCUQ6+BrI= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/klauspost/compress v1.18.2 h1:iiPHWW0YrcFgpBYhsA6D1+fqHssJscY/Tm/y2Uqnapk= @@ -104,6 +106,7 @@ github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3A github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= @@ -121,40 +124,40 @@ github.com/yashtewari/glob-intersection v0.2.0 h1:8iuHdN88yYuCzCdjt0gDe+6bAhUwBe github.com/yashtewari/glob-intersection v0.2.0/go.mod h1:LK7pIC3piUjovexikBbJ26Yml7g8xa5bsjfx2v1fwok= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= -go.opentelemetry.io/otel v1.40.0 h1:oA5YeOcpRTXq6NN7frwmwFR0Cn3RhTVZvXsP4duvCms= -go.opentelemetry.io/otel v1.40.0/go.mod h1:IMb+uXZUKkMXdPddhwAHm6UfOwJyh4ct1ybIlV14J0g= -go.opentelemetry.io/otel/metric v1.40.0 h1:rcZe317KPftE2rstWIBitCdVp89A2HqjkxR3c11+p9g= -go.opentelemetry.io/otel/metric v1.40.0/go.mod h1:ib/crwQH7N3r5kfiBZQbwrTge743UDc7DTFVZrrXnqc= -go.opentelemetry.io/otel/sdk v1.40.0 h1:KHW/jUzgo6wsPh9At46+h4upjtccTmuZCFAc9OJ71f8= -go.opentelemetry.io/otel/sdk v1.40.0/go.mod h1:Ph7EFdYvxq72Y8Li9q8KebuYUr2KoeyHx0DRMKrYBUE= -go.opentelemetry.io/otel/sdk/metric v1.40.0 h1:mtmdVqgQkeRxHgRv4qhyJduP3fYJRMX4AtAlbuWdCYw= -go.opentelemetry.io/otel/sdk/metric v1.40.0/go.mod h1:4Z2bGMf0KSK3uRjlczMOeMhKU2rhUqdWNoKcYrtcBPg= -go.opentelemetry.io/otel/trace v1.40.0 h1:WA4etStDttCSYuhwvEa8OP8I5EWu24lkOzp+ZYblVjw= -go.opentelemetry.io/otel/trace v1.40.0/go.mod h1:zeAhriXecNGP/s2SEG3+Y8X9ujcJOTqQ5RgdEJcawiA= +go.opentelemetry.io/otel v1.43.0 h1:mYIM03dnh5zfN7HautFE4ieIig9amkNANT+xcVxAj9I= +go.opentelemetry.io/otel v1.43.0/go.mod h1:JuG+u74mvjvcm8vj8pI5XiHy1zDeoCS2LB1spIq7Ay0= +go.opentelemetry.io/otel/metric v1.43.0 h1:d7638QeInOnuwOONPp4JAOGfbCEpYb+K6DVWvdxGzgM= +go.opentelemetry.io/otel/metric v1.43.0/go.mod h1:RDnPtIxvqlgO8GRW18W6Z/4P462ldprJtfxHxyKd2PY= +go.opentelemetry.io/otel/sdk v1.43.0 h1:pi5mE86i5rTeLXqoF/hhiBtUNcrAGHLKQdhg4h4V9Dg= +go.opentelemetry.io/otel/sdk v1.43.0/go.mod h1:P+IkVU3iWukmiit/Yf9AWvpyRDlUeBaRg6Y+C58QHzg= +go.opentelemetry.io/otel/sdk/metric v1.43.0 h1:S88dyqXjJkuBNLeMcVPRFXpRw2fuwdvfCGLEo89fDkw= +go.opentelemetry.io/otel/sdk/metric v1.43.0/go.mod h1:C/RJtwSEJ5hzTiUz5pXF1kILHStzb9zFlIEe85bhj6A= +go.opentelemetry.io/otel/trace v1.43.0 h1:BkNrHpup+4k4w+ZZ86CZoHHEkohws8AY+WTX09nk+3A= +go.opentelemetry.io/otel/trace v1.43.0/go.mod h1:/QJhyVBUUswCphDVxq+8mld+AvhXZLhe+8WVFxiFff0= go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI= go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/crypto v0.48.0 h1:/VRzVqiRSggnhY7gNRxPauEQ5Drw9haKdM0jqfcCFts= -golang.org/x/crypto v0.48.0/go.mod h1:r0kV5h3qnFPlQnBSrULhlsRfryS2pmewsg+XfMgkVos= -golang.org/x/mod v0.32.0 h1:9F4d3PHLljb6x//jOyokMv3eX+YDeepZSEo3mFJy93c= -golang.org/x/mod v0.32.0/go.mod h1:SgipZ/3h2Ci89DlEtEXWUk/HteuRin+HHhN+WbNhguU= -golang.org/x/net v0.51.0 h1:94R/GTO7mt3/4wIKpcR5gkGmRLOuE/2hNGeWq/GBIFo= -golang.org/x/net v0.51.0/go.mod h1:aamm+2QF5ogm02fjy5Bb7CQ0WMt1/WVM7FtyaTLlA9Y= +golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= +golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= +golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= +golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= +golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= +golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= -golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k= -golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/text v0.34.0 h1:oL/Qq0Kdaqxa1KbNeMKwQq0reLCCaFtqu2eNuSeNHbk= -golang.org/x/text v0.34.0/go.mod h1:homfLqTYRFyVYemLBFl5GgL/DWEiH5wcsQ5gSh1yziA= -golang.org/x/tools v0.41.0 h1:a9b8iMweWG+S0OBnlU36rzLp20z1Rp10w+IY2czHTQc= -golang.org/x/tools v0.41.0/go.mod h1:XSY6eDqxVNiYgezAVqqCeihT4j1U2CCsqvH3WhQpnlg= -gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk= -gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409 h1:H86B94AW+VfJWDqFeEbBPhEtHzJwJfTbgE2lZa54ZAQ= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409/go.mod h1:j9x/tPzZkyxcgEFkiKEEGxfvyumM01BEtsW8xzOahRQ= -google.golang.org/grpc v1.79.3 h1:sybAEdRIEtvcD68Gx7dmnwjZKlyfuc61Dyo9pGXXkKE= -google.golang.org/grpc v1.79.3/go.mod h1:KmT0Kjez+0dde/v2j9vzwoAScgEPx/Bw1CYChhHLrHQ= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= +golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= +golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c= +golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI= +gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= +gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 h1:RmoJA1ujG+/lRGNfUnOMfhCy5EipVMyvUE+KNbPbTlw= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/grpc v1.82.1 h1:NnAxzGRA0677vCa4BUkOAnO5+FfQqVl9iUXeD0IqcGE= +google.golang.org/grpc v1.82.1/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/pkg/agentcard/types.go b/pkg/agentcard/types.go index 3ad4dc0..0b91a5a 100644 --- a/pkg/agentcard/types.go +++ b/pkg/agentcard/types.go @@ -1,6 +1,8 @@ // Package agentcard defines the data structures for the A2A Agent Card. package agentcard +import "encoding/json" + // TransportProtocol defines the supported transport protocols for A2A agents. type TransportProtocol string @@ -32,6 +34,10 @@ type AgentCard struct { SupportsAuthenticatedExtendedCard bool `json:"supportsAuthenticatedExtendedCard,omitempty"` Signatures []Signature `json:"signatures,omitempty"` Extensions []AgentExtension `json:"extensions,omitempty"` + + // raw holds the bytes this card was decoded from, when it was decoded + // rather than constructed. See UnmarshalJSON. + raw []byte } // AgentProvider contains information about the agent's provider. @@ -85,3 +91,39 @@ type AgentExtension struct { Version string `json:"version"` Description string `json:"description,omitempty"` } + +// UnmarshalJSON decodes an Agent Card and retains the exact bytes it was +// decoded from. +// +// Signature verification canonicalizes the document that was signed, and the +// Go struct cannot round-trip it faithfully: `omitempty` on a plain bool +// cannot distinguish a field that was absent from one explicitly set to +// false, and a field without `omitempty` is emitted even when the sender +// omitted it. Either difference changes the canonical payload and fails an +// otherwise valid signature. Keeping the original bytes lets the verifier +// canonicalize what actually arrived. +func (c *AgentCard) UnmarshalJSON(data []byte) error { + // Alias sheds the method set, so the nested decode does not recurse. + type alias AgentCard + var decoded alias + if err := json.Unmarshal(data, &decoded); err != nil { + return err + } + + *c = AgentCard(decoded) + c.raw = append([]byte(nil), data...) + return nil +} + +// Raw returns a copy of the bytes this card was decoded from, or nil when the +// card was constructed programmatically rather than parsed. +// +// The copy is deliberate. This document is what signature verification +// canonicalizes, so handing out the backing array would let any caller change +// what a later verification runs against. +func (c *AgentCard) Raw() []byte { + if c.raw == nil { + return nil + } + return append([]byte(nil), c.raw...) +} diff --git a/pkg/agentcard/types_test.go b/pkg/agentcard/types_test.go index 9609189..79886c5 100644 --- a/pkg/agentcard/types_test.go +++ b/pkg/agentcard/types_test.go @@ -32,6 +32,92 @@ func TestAgentCard_JSONRoundTrip(t *testing.T) { err = json.Unmarshal(data, &decoded) assert.NoError(t, err) - // Compare - assert.Equal(t, original, &decoded) + // A decoded card also retains the bytes it came from, so it is not + // struct-identical to one built in code. Compare the content instead. + assert.Equal(t, *original, decoded.withoutRaw()) +} + +// A decoded card must keep its source document, because signature +// verification canonicalizes what actually arrived rather than the struct's +// rendering of it. +func TestAgentCard_RetainsSourceDocument(t *testing.T) { + const document = `{"name":"Test Agent","capabilities":{"streaming":false}}` + + var decoded AgentCard + assert.NoError(t, json.Unmarshal([]byte(document), &decoded)) + + assert.Equal(t, document, string(decoded.Raw()), + "the raw document must survive decoding byte for byte") +} + +// A card built in code has no source document to retain. +func TestAgentCard_ProgrammaticCardHasNoRaw(t *testing.T) { + card := &AgentCard{Name: "Test Agent"} + + assert.Nil(t, card.Raw()) +} + +// Mutating the caller's buffer after decoding must not change what the card +// retained, or a verifier could canonicalize something the signer never sent. +func TestAgentCard_RawIsCopied(t *testing.T) { + document := []byte(`{"name":"Test Agent"}`) + + var decoded AgentCard + assert.NoError(t, json.Unmarshal(document, &decoded)) + + document[2] = 'X' + + assert.Equal(t, `{"name":"Test Agent"}`, string(decoded.Raw())) +} + +// withoutRaw returns a copy with the retained document cleared, for tests that +// compare card content rather than provenance. +func (c AgentCard) withoutRaw() AgentCard { + c.raw = nil + return c +} + +// Raw must not hand out the backing array. The retained document is what +// signature verification canonicalizes, so a caller holding a reference to it +// could change what a later verification runs against. +func TestAgentCard_RawDoesNotAliasInternalBuffer(t *testing.T) { + var decoded AgentCard + assert.NoError(t, json.Unmarshal([]byte(`{"name":"Test Agent"}`), &decoded)) + + first := decoded.Raw() + first[2] = 'X' + + assert.Equal(t, `{"name":"Test Agent"}`, string(decoded.Raw()), + "mutating the returned slice must not affect the retained document") +} + +// A document that fails to decode must leave the card untouched. Decoding into +// a reused variable must not leave the previous card's retained bytes attached +// to a parse that failed, or a verifier could canonicalize one card's document +// while holding another card's fields. +// +// The cases differ in where they fail. Malformed syntax is rejected by +// encoding/json before UnmarshalJSON is called at all; a type mismatch is +// syntactically valid, so UnmarshalJSON runs and its own decode fails. Both +// must leave the card as it was. +func TestAgentCard_FailedDecodeLeavesCardUntouched(t *testing.T) { + for _, tc := range []struct { + name string + document string + }{ + {"malformed syntax", `{"name": `}, + {"type mismatch", `{"name": 123}`}, + } { + t.Run(tc.name, func(t *testing.T) { + var card AgentCard + assert.NoError(t, json.Unmarshal([]byte(`{"name":"First Agent"}`), &card)) + + err := json.Unmarshal([]byte(tc.document), &card) + + assert.Error(t, err, "the document must not decode") + assert.Equal(t, "First Agent", card.Name, "the previous card must be left intact") + assert.Equal(t, `{"name":"First Agent"}`, string(card.Raw()), + "a failed decode must not attach the rejected document") + }) + } } diff --git a/pkg/crypto/canonical.go b/pkg/crypto/canonical.go index de46996..41bc063 100644 --- a/pkg/crypto/canonical.go +++ b/pkg/crypto/canonical.go @@ -2,38 +2,122 @@ package crypto import ( + "bytes" "encoding/json" + "errors" "fmt" + "io" "github.com/capiscio/capiscio-core/v2/pkg/agentcard" + "github.com/gowebpki/jcs" ) -// CreateCanonicalJSON creates a canonical JSON representation of the Agent Card -// for signature verification. It removes the "signatures" field and ensures -// keys are sorted (which encoding/json does by default). +// CreateCanonicalJSON creates the canonical JSON representation of an Agent +// Card, which is the payload an Agent Card signature is computed over. +// +// A2A specification section 8.4.1 requires the payload to be canonicalized +// with the JSON Canonicalization Scheme (RFC 8785) once the `signatures` +// field has been removed. +// +// Two things this deliberately does not do, both of which previously produced +// a payload that differed from the signer's: +// +// - It does not use encoding/json as a canonicalizer. Go sorts object keys +// by UTF-8 byte order where RFC 8785 sorts by UTF-16 code unit, and Go's +// encoder escapes '<', '>' and '&' where RFC 8785 emits them literally. +// An ampersand in a URL query string was enough to break verification. +// - It does not round-trip the document through the AgentCard struct when +// the original bytes are available. `omitempty` on a plain bool drops a +// field the sender explicitly set to false, and section 8.4.1 requires an +// explicitly set field to survive canonicalization even when it holds a +// default value. +// +// When the card was decoded from JSON, the bytes it was decoded from are used. +// When it was constructed programmatically there is nothing better available +// than the struct, and field presence is then only as faithful as the struct +// tags allow. +// +// Because the received bytes win, this attests to the document that arrived +// and not to the struct's current contents. A caller that decodes a card and +// then mutates its fields gets the payload of the original document, so a +// signature reported valid says the sender signed what was received, not that +// it signed what the struct now holds. Treat a decoded AgentCard as immutable +// up to the point of verification; a card that must be changed should be +// re-encoded and re-decoded first, which is also what a relay forwarding it +// would do. +// +// It returns an error rather than a payload for a document that cannot be +// canonicalized: one that is not a JSON object, or one carrying a number +// outside the IEEE 754 double range that RFC 8785 defines number formatting +// over. Neither can be signed or verified meaningfully. func CreateCanonicalJSON(card *agentcard.AgentCard) ([]byte, error) { - // 1. Marshal to JSON first to get the raw map structure - // This ensures we respect the json tags in the struct - data, err := json.Marshal(card) + source := card.Raw() + if len(source) == 0 { + marshaled, err := json.Marshal(card) + if err != nil { + return nil, fmt.Errorf("failed to marshal agent card: %w", err) + } + source = marshaled + } + + withoutSignatures, err := stripSignatures(source) if err != nil { - return nil, fmt.Errorf("failed to marshal agent card: %w", err) + return nil, err + } + + canonical, err := jcs.Transform(withoutSignatures) + if err != nil { + return nil, fmt.Errorf("failed to canonicalize agent card (RFC 8785): %w", err) + } + + return canonical, nil +} + +// stripSignatures removes the top-level `signatures` field, which is excluded +// from the signed payload to avoid a circular dependency (spec 8.4.1). +// +// Decoding uses UseNumber so numeric literals survive as their original text +// rather than being widened to float64. RFC 8785 defines its own number +// formatting and needs the value, not Go's rendering of it. +func stripSignatures(document []byte) ([]byte, error) { + decoder := json.NewDecoder(bytes.NewReader(document)) + decoder.UseNumber() + + var fields map[string]interface{} + if err := decoder.Decode(&fields); err != nil { + return nil, fmt.Errorf("failed to parse agent card: %w", err) } - // 2. Unmarshal into a map to manipulate fields - var rawMap map[string]interface{} - if err := json.Unmarshal(data, &rawMap); err != nil { - return nil, fmt.Errorf("failed to unmarshal into map: %w", err) + // A JSON `null` decodes into a nil map without error, and would go on to + // canonicalize as the four bytes "null". That is a signable payload for a + // document that is not an agent card, so reject it here rather than hand + // it to a signer or a verifier. + if fields == nil { + return nil, fmt.Errorf("agent card is not a JSON object") } - // 3. Remove the signatures field - delete(rawMap, "signatures") + // One JSON value and nothing after it. Decode stops at the end of the + // first value and would ignore anything following, which on a signature + // path means a document could carry bytes the payload never covers. + // json.Unmarshal already rejects this before the raw bytes are retained, + // so this holds the guarantee here rather than in a caller's invariant. + // + // Reading a token and requiring io.EOF, rather than asking More(). More() + // answers whether another element follows within an array or object, so + // at the top level it reports false for a stray '}' or ']' and lets + // `{"a":1}}` past. + if _, err := decoder.Token(); !errors.Is(err, io.EOF) { + return nil, fmt.Errorf("agent card has trailing data after the JSON object") + } + + delete(fields, "signatures") - // 4. Marshal back to JSON - // encoding/json sorts map keys by default, providing canonicalization - canonical, err := json.Marshal(rawMap) + remarshaled, err := json.Marshal(fields) if err != nil { - return nil, fmt.Errorf("failed to create canonical json: %w", err) + return nil, fmt.Errorf("failed to re-encode agent card: %w", err) } - return canonical, nil + // Any escaping introduced here is undone by jcs.Transform, which reparses + // the document and re-emits it per RFC 8785. + return remarshaled, nil } diff --git a/pkg/crypto/canonical_test.go b/pkg/crypto/canonical_test.go index be9d96d..d98f975 100644 --- a/pkg/crypto/canonical_test.go +++ b/pkg/crypto/canonical_test.go @@ -2,58 +2,236 @@ package crypto import ( "encoding/json" + "strings" "testing" "github.com/capiscio/capiscio-core/v2/pkg/agentcard" ) -func TestCreateCanonicalJSON(t *testing.T) { - // Create a card with fields out of order and with a signature - card := &agentcard.AgentCard{ - Name: "Test Agent", - ProtocolVersion: "0.3.0", - Signatures: []agentcard.Signature{ - {Protected: "abc", Signature: "def"}, - }, - Version: "1.0.0", +// decodeCard parses a card the way a verifier receives one, so the raw bytes +// are retained. +func decodeCard(t *testing.T, document string) *agentcard.AgentCard { + t.Helper() + var card agentcard.AgentCard + if err := json.Unmarshal([]byte(document), &card); err != nil { + t.Fatalf("failed to decode card: %v", err) } + return &card +} - canonical, err := CreateCanonicalJSON(card) +func canonical(t *testing.T, card *agentcard.AgentCard) string { + t.Helper() + out, err := CreateCanonicalJSON(card) if err != nil { t.Fatalf("CreateCanonicalJSON failed: %v", err) } + return string(out) +} + +// The worked example published in A2A specification section 8.4.1. If this +// fails, canonicalization is wrong and every signature check built on it is +// meaningless. +func TestCanonicalJSONMatchesSpecWorkedExample(t *testing.T) { + card := decodeCard(t, `{ + "name": "Example Agent", + "description": "", + "capabilities": { + "streaming": false, + "pushNotifications": false + }, + "skills": [] + }`) + + const want = `{"capabilities":{"pushNotifications":false,"streaming":false},"description":"","name":"Example Agent","skills":[]}` + + if got := canonical(t, card); got != want { + t.Errorf("canonical form does not match the specification's worked example\n got: %s\nwant: %s", got, want) + } +} + +// An `optional` field the sender explicitly set must survive canonicalization +// even when it holds a default (spec 8.4.1). Marshaling the struct dropped +// these, because AgentCapabilities uses plain bools with omitempty. +func TestExplicitlySetFalseSurvives(t *testing.T) { + card := decodeCard(t, `{ + "protocolVersion": "0.3.0", + "name": "Example Agent", + "description": "d", + "url": "https://example.com/a2a", + "version": "1.0.0", + "capabilities": {"streaming": false, "pushNotifications": false}, + "defaultInputModes": ["text/plain"], + "defaultOutputModes": ["text/plain"], + "skills": [] + }`) + + got := canonical(t, card) + + for _, field := range []string{`"streaming":false`, `"pushNotifications":false`} { + if !strings.Contains(got, field) { + t.Errorf("expected %s to survive canonicalization, got: %s", field, got) + } + } +} + +// A field the sender omitted must stay omitted. Struct fields without +// omitempty were emitted as zero values, adding content the signer never +// signed over. +func TestOmittedFieldsAreNotInvented(t *testing.T) { + card := decodeCard(t, `{ + "name": "Example Agent", + "description": "d", + "version": "1.0.0", + "capabilities": {}, + "skills": [] + }`) + + got := canonical(t, card) + + for _, field := range []string{"protocolVersion", "url", "defaultInputModes", "defaultOutputModes"} { + if strings.Contains(got, field) { + t.Errorf("field %q was absent from the source document but appears in the canonical form: %s", field, got) + } + } +} + +// RFC 8785 emits '<', '>' and '&' literally. Go's encoder escapes them, which +// changed the payload for any card carrying an ampersand in a query string. +func TestNoHTMLEscaping(t *testing.T) { + card := decodeCard(t, `{ + "name": "Example Agent", + "description": "planning & routing", + "version": "1.0.0", + "capabilities": {}, + "skills": [], + "url": "https://example.com/a2a?tenant=a&mode=b" + }`) + + got := canonical(t, card) + + // Built from bytes so the source file carries no escape sequence of its own. + unicodeEscape := string([]byte{'\\', 'u'}) + if strings.Contains(got, unicodeEscape) { + t.Errorf("canonical form contains a unicode escape; RFC 8785 emits these characters literally: %s", got) + } + + for _, literal := range []string{"?tenant=a&mode=b", "planning & routing"} { + if !strings.Contains(got, literal) { + t.Errorf("expected %q to survive literally, got: %s", literal, got) + } + } +} + +// The signatures field is excluded from the payload it signs over (spec 8.4.1). +func TestSignaturesAreExcluded(t *testing.T) { + card := decodeCard(t, `{ + "name": "Example Agent", + "description": "d", + "version": "1.0.0", + "capabilities": {}, + "skills": [], + "signatures": [{"protected": "abc", "signature": "def"}] + }`) + + if got := canonical(t, card); strings.Contains(got, "signatures") { + t.Errorf("canonical form must not contain the signatures field: %s", got) + } +} + +// Key order in the received document must not affect the payload. This is the +// property canonicalization exists to provide. +func TestKeyOrderDoesNotChangeOutput(t *testing.T) { + first := decodeCard(t, `{"name":"A","description":"d","version":"1.0.0","capabilities":{},"skills":[]}`) + second := decodeCard(t, `{"skills":[],"capabilities":{},"version":"1.0.0","description":"d","name":"A"}`) + + if canonical(t, first) != canonical(t, second) { + t.Errorf("key order changed the canonical form:\n%s\n%s", canonical(t, first), canonical(t, second)) + } +} + +// Cards built in code carry no raw document, so the struct is the only source +// available. It must still canonicalize rather than error. +func TestProgrammaticCardFallsBackToStruct(t *testing.T) { + card := &agentcard.AgentCard{ + Name: "Test Agent", + ProtocolVersion: "0.3.0", + Version: "1.0.0", + Signatures: []agentcard.Signature{{Protected: "abc", Signature: "def"}}, + } + + got := canonical(t, card) - // Unmarshal to map to check structure - var result map[string]interface{} - if err := json.Unmarshal(canonical, &result); err != nil { - t.Fatalf("Failed to unmarshal canonical JSON: %v", err) + if strings.Contains(got, "signatures") { + t.Errorf("signatures must be excluded: %s", got) } + if !strings.Contains(got, `"name":"Test Agent"`) { + t.Errorf("expected the card contents to survive: %s", got) + } +} - // 1. Check signatures are removed - if _, ok := result["signatures"]; ok { - t.Error("Canonical JSON should not contain 'signatures' field") +// Nested objects are sorted recursively, and array order is preserved because +// arrays are ordered data, not a set. +func TestNestedObjectsAreCanonicalized(t *testing.T) { + card := decodeCard(t, `{ + "name": "A", + "description": "d", + "version": "1.0.0", + "capabilities": {}, + "skills": [{"tags":["b","a"],"name":"S","id":"s","description":"x"}] + }`) + + const wantSkill = `"skills":[{"description":"x","id":"s","name":"S","tags":["b","a"]}]` + + got := canonical(t, card) + if !strings.Contains(got, wantSkill) { + t.Errorf("nested object keys were not sorted, or array order was not preserved\n got: %s\nwant substring: %s", got, wantSkill) } +} - // 2. Check string output is sorted (encoding/json does this, but let's verify the string) - expected := `{"name":"Test Agent","protocolVersion":"0.3.0","version":"1.0.0"}` - // Note: This simple string check works because we only have 3 fields. - // In a real scenario with nested objects, we rely on encoding/json's deterministic sorting. +// A JSON `null` satisfies encoding/json when decoded into an AgentCard, and +// the retained document is then the four bytes "null". Canonicalizing that +// produced a signable payload for something that is not an agent card, so it +// must be refused instead. +func TestRejectsDocumentThatIsNotAnObject(t *testing.T) { + card := decodeCard(t, `null`) - // We need to be careful about default values (empty strings/bools) that might be omitted or included - // based on struct tags. Our struct tags use omitempty for optional fields. - // Required fields like DefaultInputModes are slices, so they might be null or [] depending on init. - // In the test struct above, slices are nil. + if _, err := CreateCanonicalJSON(card); err == nil { + t.Error("a document that is not a JSON object must not canonicalize") + } +} + +// RFC 8785 defines number formatting over IEEE 754 doubles. A literal outside +// that range has no canonical form, so canonicalization must fail rather than +// return a payload a signer and a verifier could disagree about. +func TestRejectsNumbersOutsideIEEE754Range(t *testing.T) { + card := decodeCard(t, `{"name":"Example Agent","extensionNumber":1e400}`) - // Let's just check that the string contains the keys in alphabetical order - jsonStr := string(canonical) + if _, err := CreateCanonicalJSON(card); err == nil { + t.Error("a number outside the IEEE 754 double range must not canonicalize") + } +} - // Simple check: name comes before protocolVersion? No, n comes before p. - // name, protocolVersion, version. - // n, p, v. +// Decode stops at the end of the first JSON value. Anything after it would be +// silently ignored, so a document could carry bytes the signed payload never +// covers. json.Unmarshal rejects trailing data before the raw bytes are +// retained, so this is unreachable through AgentCard today; the guard holds +// the guarantee in the function rather than in a caller's invariant. +func TestRejectsTrailingDataAfterTheObject(t *testing.T) { + // The last two were raised in review. json.Decoder.More() reports whether + // another element follows inside an array or object, so at the top level + // it answers false for a stray closing bracket and waved these through. + for _, document := range []string{ + `{"name":"Example Agent"} trailing`, + `{"name":"Example Agent"}{"name":"Other Agent"}`, + `{"name":"Example Agent"}}`, + `{"name":"Example Agent"}]`, + } { + if _, err := stripSignatures([]byte(document)); err == nil { + t.Errorf("a document with trailing data must not canonicalize: %s", document) + } + } - if jsonStr != expected { - // It might differ due to other fields being present as zero values if not omitempty - // Let's check if it contains the keys we expect - t.Logf("Canonical JSON: %s", jsonStr) + if _, err := stripSignatures([]byte(`{"name":"Example Agent"} `)); err != nil { + t.Errorf("trailing whitespace is not trailing data: %v", err) } } diff --git a/pkg/crypto/interop_test.go b/pkg/crypto/interop_test.go new file mode 100644 index 0000000..2200422 --- /dev/null +++ b/pkg/crypto/interop_test.go @@ -0,0 +1,117 @@ +package crypto + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/capiscio/capiscio-core/v2/pkg/agentcard" + jose "github.com/go-jose/go-jose/v4" +) + +// This fixture was produced by an implementation independent of this package, +// following A2A specification section 8.4 directly: the payload is the card +// with `signatures` removed, canonicalized with RFC 8785, and the signature is +// ES256 over BASE64URL(protected) || "." || BASE64URL(payload). +// +// It deliberately carries the two constructs that this package used to get +// wrong, so a regression fails here rather than in production: +// +// - `"streaming": false` and `"pushNotifications": false` are explicitly set, +// and must survive canonicalization (spec 8.4.1) +// - the description and url contain '<', '>' and '&', which RFC 8785 emits +// literally and Go's encoding/json escapes +const interopSignedCard = `{ + "protocolVersion": "0.3.0", + "name": "Interop Agent", + "description": "planning & routing", + "url": "https://interop.example.com/a2a?tenant=a&mode=b", + "version": "1.0.0", + "capabilities": {"streaming": false, "pushNotifications": false}, + "defaultInputModes": ["text/plain"], + "defaultOutputModes": ["text/plain"], + "skills": [{"id": "plan", "name": "Plan", "description": "Plans routes", "tags": ["geo"]}], + "signatures": [{ + "protected": "eyJhbGciOiJFUzI1NiIsImprdSI6Imh0dHBzOi8vaW50ZXJvcC5leGFtcGxlLmNvbS9qd2tzLmpzb24iLCJraWQiOiJpbnRlcm9wLWtleS0xIiwidHlwIjoiSk9TRSJ9", + "signature": "AR32G971k9vIBf2d698nrsyOTFp2h_BVucwIoaQ26Eha0XuwpSKymPEXkjivxtad68rlGRlibU168bIMskwl4g" + }] +}` + +const interopJWKS = `{"keys":[{"kty":"EC","crv":"P-256","kid":"interop-key-1","x":"BMSUAJn4PU-IAvWhiqymE9p-cEmdh3GFI8hXdyODvTA","y":"xsJNHD_uNUrQ7knDtjmfO2THg_5NyaNjQ96Fkb5ORGo"}]}` + +// The canonical payload the independent signer computed. Recording it makes a +// canonicalization regression report the actual difference rather than only a +// failed signature. +const interopCanonicalPayload = `{"capabilities":{"pushNotifications":false,"streaming":false},"defaultInputModes":["text/plain"],"defaultOutputModes":["text/plain"],"description":"planning & routing","name":"Interop Agent","protocolVersion":"0.3.0","skills":[{"description":"Plans routes","id":"plan","name":"Plan","tags":["geo"]}],"url":"https://interop.example.com/a2a?tenant=a&mode=b","version":"1.0.0"}` + +// stubJWKSFetcher serves a fixed key set, so the test exercises verification +// rather than network behaviour. +type stubJWKSFetcher struct { + jwks string +} + +func (s stubJWKSFetcher) Fetch(_ context.Context, _ string) (*jose.JSONWebKeySet, error) { + var keySet jose.JSONWebKeySet + if err := json.Unmarshal([]byte(s.jwks), &keySet); err != nil { + return nil, err + } + return &keySet, nil +} + +func TestCanonicalPayloadMatchesIndependentSigner(t *testing.T) { + card := decodeCard(t, interopSignedCard) + + if got := canonical(t, card); got != interopCanonicalPayload { + t.Errorf("canonical payload differs from the independent signer's\n got: %s\nwant: %s", got, interopCanonicalPayload) + } +} + +func TestVerifiesCardSignedByIndependentImplementation(t *testing.T) { + var card agentcard.AgentCard + if err := json.Unmarshal([]byte(interopSignedCard), &card); err != nil { + t.Fatalf("failed to decode signed card: %v", err) + } + + verifier := NewVerifierWithFetcher(stubJWKSFetcher{jwks: interopJWKS}) + + result, err := verifier.VerifyAgentCardSignatures(context.Background(), &card) + if err != nil { + t.Fatalf("verification returned an error: %v", err) + } + + if !result.Valid { + t.Errorf("expected the signature to verify, got errors: %v", result.Summary.Errors) + } +} + +// Tampering with the card must break verification. Without this, a test that +// only checks the happy path cannot distinguish a working verifier from one +// that accepts everything. +func TestRejectsTamperedCardFromIndependentImplementation(t *testing.T) { + tampered := decodeTampered(t, interopSignedCard, "Interop Agent", "Evil Agent") + + verifier := NewVerifierWithFetcher(stubJWKSFetcher{jwks: interopJWKS}) + + result, err := verifier.VerifyAgentCardSignatures(context.Background(), tampered) + if err != nil { + t.Fatalf("verification returned an error: %v", err) + } + + if result.Valid { + t.Error("a tampered card must not verify") + } +} + +func decodeTampered(t *testing.T, document, old, replacement string) *agentcard.AgentCard { + t.Helper() + modified := strings.Replace(document, old, replacement, 1) + if modified == document { + t.Fatalf("tampering did not change the document; %q not found", old) + } + var card agentcard.AgentCard + if err := json.Unmarshal([]byte(modified), &card); err != nil { + t.Fatalf("failed to decode tampered card: %v", err) + } + return &card +}