feat(a2acrypto): add AgentCard JWS signing and verification (fixes #141) - #368
feat(a2acrypto): add AgentCard JWS signing and verification (fixes #141)#368kuangmi-bit wants to merge 11 commits into
Conversation
…ixes a2aproject#141) Add a2acrypto package for AgentCard JWS (RFC 7515) signing and verification, with integrations into a2aclient and a2asrv. Core types: - Signer: Creates JWS signatures for AgentCards (ES256/ES384/ES512/ EdDSA/RS256), with automatic algorithm inference - Verifier: Verifies AgentCard signatures via KeyResolver - KeyResolver: Resolves kid/jku to crypto.PublicKey - NewSignedCardProducer: Wraps AgentCardProducer to auto-sign cards Integrations: - a2aclient.WithCardVerifier: Verify signed cards in CreateFromCard - agentcard.Resolver.Verifier: Verify cards after HTTP resolution Tests cover ECDSA P-256, Ed25519, tampered cards, nil signatures, algorithm inference, and protected header fields.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the a2acrypto package to support JWS signing and verification of AgentCards, integrating it into the resolver and factory components. The review feedback highlights several critical improvements: adhering to the A2A specification and RFC 8785 (JCS) by excluding the signatures field and sorting JSON keys lexicographically; including "typ": "JOSE" and validating header fields; resolving security risks related to signature-stripping attacks and key rotation by requiring at least one valid signature; wrapping RSA verification errors consistently; and copying the AgentCard in the producer to avoid data races.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| import ( | ||
| "crypto" | ||
| ) |
There was a problem hiding this comment.
To comply with the A2A specification and RFC 8785 (JSON Canonicalization Scheme / JCS), the signatures field must be excluded from the payload before signing or verifying, and the JSON keys must be sorted lexicographically at all levels.
Since Go's standard json.Marshal sorts map keys alphabetically, we can achieve both requirements without external dependencies by marshaling the card, unmarshaling it into a map[string]any, deleting the "signatures" key, and marshaling it back.
import (
"crypto"
"encoding/json"
"errors"
"github.com/a2aproject/a2a-go/v2/a2a"
)
func preparePayload(card *a2a.AgentCard) ([]byte, error) {
if card == nil {
return nil, errors.New("nil agent card")
}
temp, err := json.Marshal(card)
if err != nil {
return nil, err
}
var m map[string]any
if err := json.Unmarshal(temp, &m); err != nil {
return nil, err
}
delete(m, "signatures")
return json.Marshal(m)
}References
- Prioritize adherence to the official specification over maintaining backward compatibility for undocumented features.
| func (s *Signer) Sign(card *a2a.AgentCard) (*a2a.AgentCardSignature, error) { | ||
| payload, err := json.Marshal(card) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to marshal agent card: %w", err) | ||
| } | ||
|
|
||
| protected := map[string]any{ | ||
| "alg": s.algorithm, | ||
| "kid": s.kid, | ||
| } |
There was a problem hiding this comment.
Use the preparePayload helper to exclude the signatures field and ensure JCS-compliant lexicographical key sorting. Also, include "typ": "JOSE" in the protected header as required by the A2A specification.
func (s *Signer) Sign(card *a2a.AgentCard) (*a2a.AgentCardSignature, error) {
payload, err := preparePayload(card)
if err != nil {
return nil, fmt.Errorf("failed to prepare agent card payload: %w", err)
}
protected := map[string]any{
"alg": s.algorithm,
"typ": "JOSE",
"kid": s.kid,
}References
- Prioritize adherence to the official specification over maintaining backward compatibility for undocumented features.
| alg, _ := protected["alg"].(string) | ||
| kid, _ := protected["kid"].(string) | ||
| jku, _ := protected["jku"].(string) | ||
|
|
||
| if v.kr == nil { | ||
| return fmt.Errorf("%w: no key resolver configured", ErrVerificationFailed) | ||
| } | ||
|
|
||
| pubKey, err := v.kr.ResolveKey(kid, jku) | ||
| if err != nil { | ||
| return fmt.Errorf("%w: key resolution failed: %v", ErrVerificationFailed, err) | ||
| } | ||
|
|
||
| payload, err := json.Marshal(card) | ||
| if err != nil { | ||
| return fmt.Errorf("%w: failed to marshal card for verification: %v", ErrVerificationFailed, err) | ||
| } |
There was a problem hiding this comment.
Use the preparePayload helper to exclude the signatures field and ensure JCS-compliant lexicographical key sorting during verification. Additionally, validate that alg and kid are present in the protected header to prevent nil/empty pointer errors or bypasses.
alg, _ := protected["alg"].(string)
kid, _ := protected["kid"].(string)
jku, _ := protected["jku"].(string)
if alg == "" {
return fmt.Errorf("%w: missing or invalid 'alg' in protected header", ErrVerificationFailed)
}
if kid == "" {
return fmt.Errorf("%w: missing or invalid 'kid' in protected header", ErrVerificationFailed)
}
if v.kr == nil {
return fmt.Errorf("%w: no key resolver configured", ErrVerificationFailed)
}
pubKey, err := v.kr.ResolveKey(kid, jku)
if err != nil {
return fmt.Errorf("%w: key resolution failed: %v", ErrVerificationFailed, err)
}
payload, err := preparePayload(card)
if err != nil {
return fmt.Errorf("%w: failed to prepare card payload for verification: %v", ErrVerificationFailed, err)
}References
- Prioritize adherence to the official specification over maintaining backward compatibility for undocumented features.
| if r.Verifier != nil && len(card.Signatures) > 0 { | ||
| for _, sig := range card.Signatures { | ||
| if err := r.Verifier.Verify(card, &sig); err != nil { | ||
| return nil, fmt.Errorf("agent card signature verification failed: %w", err) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Requiring all signatures to be valid breaks key rotation (where some signatures might use old/expired keys or keys not present in the client's trust store). The A2A specification states that verifying at least one signature is sufficient.
Additionally, allowing unsigned cards to pass through silently when a Verifier is configured makes the client vulnerable to signature-stripping attacks (where an attacker simply removes the signatures field to bypass verification). Consider making signature enforcement configurable or requiring at least one valid signature if a verifier is set.
if r.Verifier != nil && len(card.Signatures) > 0 {
var verified bool
var lastErr error
for _, sig := range card.Signatures {
if err := r.Verifier.Verify(card, &sig); err == nil {
verified = true
break
} else {
lastErr = err
}
}
if !verified {
return nil, fmt.Errorf("agent card signature verification failed: %w", lastErr)
}
}References
- Prioritize adherence to the official specification over maintaining backward compatibility for undocumented features.
| if f.cardVerifier != nil && len(card.Signatures) > 0 { | ||
| for _, sig := range card.Signatures { | ||
| if err := f.cardVerifier.Verify(card, &sig); err != nil { | ||
| return nil, fmt.Errorf("agent card signature verification failed: %w", err) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Requiring all signatures to be valid breaks key rotation. The A2A specification states that verifying at least one signature is sufficient.
Additionally, allowing unsigned cards to pass through silently when a Verifier is configured makes the client vulnerable to signature-stripping attacks. Consider making signature enforcement configurable or requiring at least one valid signature if a verifier is set.
if f.cardVerifier != nil && len(card.Signatures) > 0 {
var verified bool
var lastErr error
for _, sig := range card.Signatures {
if err := f.cardVerifier.Verify(card, &sig); err == nil {
verified = true
break
} else {
lastErr = err
}
}
if !verified {
return nil, fmt.Errorf("agent card signature verification failed: %w", lastErr)
}
}References
- Prioritize adherence to the official specification over maintaining backward compatibility for undocumented features.
| case *rsa.PublicKey: | ||
| if hash == 0 { | ||
| return fmt.Errorf("%w: RSA requires a hash", ErrVerificationFailed) | ||
| } | ||
| h := hash.New() | ||
| h.Write(message) | ||
| digest := h.Sum(nil) | ||
| return rsa.VerifyPKCS1v15(key, hash, digest, sig) |
There was a problem hiding this comment.
Wrap the RSA verification error in ErrVerificationFailed to ensure consistent error reporting across all key types (ECDSA, Ed25519, and RSA).
| case *rsa.PublicKey: | |
| if hash == 0 { | |
| return fmt.Errorf("%w: RSA requires a hash", ErrVerificationFailed) | |
| } | |
| h := hash.New() | |
| h.Write(message) | |
| digest := h.Sum(nil) | |
| return rsa.VerifyPKCS1v15(key, hash, digest, sig) | |
| case *rsa.PublicKey: | |
| if hash == 0 { | |
| return fmt.Errorf("%w: RSA requires a hash", ErrVerificationFailed) | |
| } | |
| h := hash.New() | |
| h.Write(message) | |
| digest := h.Sum(nil) | |
| if err := rsa.VerifyPKCS1v15(key, hash, digest, sig); err != nil { | |
| return fmt.Errorf("%w: %v", ErrVerificationFailed, err) | |
| } | |
| return nil |
| func (p *signedCardProducer) Card(ctx context.Context) (*a2a.AgentCard, error) { | ||
| card, err := p.wrapped.Card(ctx) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| sig, err := p.signer.Sign(card) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to sign agent card: %w", err) | ||
| } | ||
| card.Signatures = append(card.Signatures, *sig) | ||
| return card, nil | ||
| } |
There was a problem hiding this comment.
Mutating the returned AgentCard in-place by appending the signature to card.Signatures can cause data races or unexpected side effects if the underlying card is cached or shared across goroutines. Copy the card and slice before modifying.
| func (p *signedCardProducer) Card(ctx context.Context) (*a2a.AgentCard, error) { | |
| card, err := p.wrapped.Card(ctx) | |
| if err != nil { | |
| return nil, err | |
| } | |
| sig, err := p.signer.Sign(card) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to sign agent card: %w", err) | |
| } | |
| card.Signatures = append(card.Signatures, *sig) | |
| return card, nil | |
| } | |
| func (p *signedCardProducer) Card(ctx context.Context) (*a2a.AgentCard, error) { | |
| card, err := p.wrapped.Card(ctx) | |
| if err != nil { | |
| return nil, err | |
| } | |
| sig, err := p.signer.Sign(card) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to sign agent card: %w", err) | |
| } | |
| cardCopy := *card | |
| cardCopy.Signatures = append([]a2a.AgentCardSignature(nil), card.Signatures...) | |
| cardCopy.Signatures = append(cardCopy.Signatures, *sig) | |
| return &cardCopy, nil | |
| } |
References
- When returning internal state references, ensure that all methods either return a deep copy or handle terminal states appropriately to prevent data integrity issues.
- Exclude 'signatures' field from canonical payload before signing - Sort object keys recursively per RFC 8785 JSON Canonicalization Scheme - Add 'typ: JOSE+JSON' to JWS protected header - Add tests: signatures exclusion, typ header presence Addresses PR a2aproject#368 review feedback from gemini-code-assist.
|
Reviewed the canonicalization layer specifically, since cross-SDK byte compatibility is what makes card signatures verifiable between implementations. We run AgentCard JWS signing in production on this exact discipline, so flagging four things before this hardens. 1. The CI test failure is an import cycle. 2. 3. Key ordering is UTF-8 here, RFC 8785 specifies UTF-16. Go marshals map keys in native byte order; RFC 8785 Section 3.2.3 sorts property names by UTF-16 code units. The orderings diverge when keys mix U+E000..U+FFFF characters with supplementary-plane characters. Edge case for ASCII field names, but worth handling or documenting since the package claims JCS. 4. If a byte-verification target is useful for this: the open conformance corpus at |
…eview - Fix import cycle: move NewSignedCardProducer from a2acrypto to a2asrv (a2acrypto/producer.go imported a2asrv, creating a test import cycle) - Fix HTML escaping: use json.Encoder with SetEscapeHTML(false) per RFC 8785 (Go's json.Marshal escapes &, <, > to unicode escapes by default) - Fix recursive signatures delete: restrict to top-level map per A2A §8.4.1 (sortObjectKeys previously deleted 'signatures' at every nesting depth) - Document UTF-8 vs UTF-16 key ordering limitation (ASCII keys unaffected) Co-authored-by: chopmob-cloud
|
Addressed all 4 issues from @chopmob-cloud's canonicalization review:
Tests: @chopmob-cloud — the conformance vectors at |
- Add Apache 2.0 license header to alg.go, canonical.go, ecdsa.go - Fix tab-indented license URL in a2acrypto.go to match repo convention - Add revive comment for KeyResolver.ResolveKey public method
|
Thanks for turning those four around, the root-only One thing left before this is green: the only red check is Prepending the standard Apache block (the same one every other file in the tree carries) clears all three: // Copyright 2025 The A2A Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.On the conformance vectors: happy to take you up on that. Once this lands I can run your |
verify.go: - Validate alg and kid are non-empty in protected header - Wrap RSA verification error in ErrVerificationFailed for consistency with ECDSA and Ed25519 error paths producer.go: - Copy card before appending signature to prevent data races when the underlying card is cached or shared across goroutines resolver.go + factory.go: - Change signature verification from all-must-be-valid to any-one-must-be-valid, per A2A spec. This supports key rotation where old signatures use expired keys not present in the trust store.
|
Thanks, I ran the byte-level verification you suggested, comparing The first is the supplementary-plane key ordering you already document in The second is not covered and is reachable through ordinary content: a U+2028 (line separator) or U+2029 (paragraph separator) in any text field. Go's
The signature is valid under the current canonicalization, but a conformant verifier in another SDK recomputes the payload with the literal bytes, gets a different signing input, and rejects the signature. I confirmed this end to end: signing such a card with the Ed25519 path verifies under The clean fix is to produce the canonical bytes directly instead of via Happy to open a PR with the serializer and regression tests, or share the patch, whichever is easier for you. |
… 8785 Replace encoding/json-based jcsMarshal with a custom serializer that writes strings with only mandatory escapes (\", \\, \b, \f, \n, \r, \t, \uXXXX for control characters < U+0020). Key difference from Go's encoding/json (even with SetEscapeHTML(false)): U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR are emitted as literal UTF-8 bytes instead of \u2028/\u2029 escape sequences. Also: - Object keys sorted by UTF-16 code unit order (RFC 8785 §3.2.3) - Numbers preserved via json.Number + strconv normalization - TestCanonical_U2028_U2029_literal verifies byte-level compliance
|
Thanks for the thorough RFC 8785 byte-level verification, really appreciate it. Implemented the custom JCS serializer approach — replaced Added |
|
Verified independently. I pulled the current writeJCS / sortedKeys / utf16Compare / canonicalNumber code from this PR verbatim and ran it against the full jcs_edge_v1 corpus (all 10 vectors, not just the two I flagged): 10/10 pass byte-for-byte, including both edge cases from before (U+2028/U+2029 literal, supplementary-plane UTF-16 ordering) and everything else in the set (mandatory short escapes, control-character escapes, no HTML-escaping, no Unicode normalisation, the 1.0-to-1 number case). One honest limit on that result: jcs_edge_v1 does not currently include a case for numbers that require exponential notation (very large or very small floats). Go's strconv.FormatFloat and RFC 8785's ECMAScript-derived number format are a known place implementations diverge. canonicalFloat itself is exercised (the 1.0 case runs through it), but the exponential-notation branch inside it is not, so I can't tell you that path is covered, only that it is untested by what I ran. Given AgentCard's fields, I would not expect this to come up in practice, but flagging it rather than letting a full pass read as broader than it is. Nice fix. Root-only signatures exclusion, SetEscapeHTML(false) plus this custom serializer, and now UTF-16 key ordering is the same discipline the byte-for-byte cross-implementation bar takes to hold, and it is what makes a signature verifiable by an independent SDK rather than only self-consistent. |
golangci-lint errcheck flagged unchecked w.Write and fmt.Fprintf return values in writeJCSObject, writeJCSArray, and writeJCSString.
|
@nahapetyan-serob — gentle ping. This PR has been open for 12 days with green CI and @chopmob-cloud has reviewed the approach. The a2acrypto package adds JWS signing/verification for Agent Cards (fixes #141):
It's also been referenced in A2A #1672 (Agent Identity Verification) where @haroldmalikfrimpong-ops confirmed the KeyResolver interface aligns with AgentID's |
Add AgentIDKeyResolver implementing KeyResolver interface that resolves did:agentid keys by fetching JWKS from getagentid.dev/.well-known/jwks.json. Supports: - Ed25519 (OKP/crv=Ed25519) public keys - ECDSA P-256 (EC/crv=P-256) public keys - Error handling for network failures, HTTP errors, missing keys, invalid JWK formats, and off-curve EC points Includes comprehensive test coverage with httptest-based mock servers and a smoke test against the real JWKS endpoint.
- a2acrypto/resolver_agentid.go: wrap resp.Body.Close() in defer func to suppress errcheck - a2acrypto/resolver_agentid_test.go: add _ = for json.Encode and fmt.Fprint calls to suppress errcheck - a2acrypto/resolver_agentid_test.go: remove empty if branch (SA9003 staticcheck) in TestDefaultJWKSURL
4 participants
Summary
Add
a2acryptopackage for AgentCard JWS (RFC 7515) signing and verification, with integrations intoa2aclientanda2asrv.Follows the interface design from issue #141 and the Python SDK reference (PR #581).
New package:
a2acryptoCore types
KeyResolverkid/jkutocrypto.PublicKeyAgentCardProducerto auto-sign every produced cardIntegrations
a2aclient.WithCardVerifier— verify signed AgentCards inCreateFromCardagentcard.Resolver.Verifier— verify cards after HTTP resolutionTests
6 tests covering ECDSA P-256, Ed25519, tampered card detection, nil signatures, algorithm inference, and protected header roundtrip.
Why zero external dependencies
Implemented with Go stdlib only (
crypto/ecdsa,crypto/ed25519,crypto/rsa,encoding/asn1). ECDSA signatures are converted between Go's DER format and JWS raw R||S format internally.