[policies] Defer signature verification until a principal matches by DorKatzelnick · Pull Request #193 · hyperledger/fabric-x-common

DorKatzelnick · 2026-09-16T15:27:15Z

Type of change

Improvement (improvement to code, performance, etc)
Test update

Description

Add SignatureSetToDeferredVerificationIdentities, which deserializes each signer but leaves the
signature unchecked, returning identities that verify themselves the first time they satisfy a
principal and memoize the outcome. SignatureSetToValidIdentities is untouched, so no existing
caller changes behaviour.
Point cauthdsl's signature policy at it. That is its only caller in this repo, so the change is
confined to the EvaluateSignedData path; EvaluateIdentities already forwarded converted
identities without re-verifying.
The returned identities no longer imply a checked signature — only a principal check does — and
they carry the memoized outcome, so they are mutable and not safe for concurrent use. Both are
documented on the function.
De-duplication now records a signer before verifying rather than after. A set carrying the same
signer twice, the first with an invalid signature and the second valid, previously passed on the
second entry and now fails on the first. It also means repeated copies of one signer's signature
cost one verification instead of one each, which is what the existing de-duplication comment
intends but does not currently achieve. The godoc states this, and a test pins it.
On the 4-organization network [policies] A signature is verified once per sub-policy an implicit meta policy tries #192 measured, one request now costs exactly 1 verification on every
one of 40 independently built bundles, against a mean of 2.80 and a spread of 1 to 5 before. The
policy walk itself is unchanged: leaf evaluations still range 1 to 5 over the same bundles, so the
saving is verifications the walk no longer performs, not a shorter walk.

Additional details (Optional)

Verification moves inside SatisfiesPrincipal, which is the only method the compiled leaf calls on
an identity, and used[i] is set only after it returns nil, so a verification failure neither
admits an identity nor consumes it from the NOutOf accounting.
Tested with the package's existing counterfeiter fakes: the conversion verifies nothing, a matching
principal verifies once and repeats do not, a non-matching principal never verifies, an invalid
signature is refused through the principal check and memoized, an identity that cannot be
deserialized is skipped, and three signers appearing once each are verified once each without one
signer's outcome reaching another.
Two tests in common/cauthdsl cover the case the change is for, end to end through an implicit meta
policy over four signature sub-policies: when one sub-policy requires the signer's principal the
signature is verified once however many sub-policies were tried first, and when none requires it no
signature is verified at all. Both fail before this change, with four verifications where none are
needed and with one per sub-policy tried.

Related issues

resolves [policies] A signature is verified once per sub-policy an implicit meta policy tries #192

coveralls · 2026-09-16T15:31:31Z

coverage: 82.212% (+0.02%) from 82.189% — DorKatzelnick:defer-signature-verification into hyperledger:main

#### Type of change - Improvement (improvement to code, performance, etc) - Test update #### Description - Add `SignatureSetToDeferredVerificationIdentities`, which deserializes each signer but leaves the signature unchecked, returning identities that verify themselves the first time they satisfy a principal and memoize the outcome. `SignatureSetToValidIdentities` is untouched, so no existing caller changes behaviour. - Point `cauthdsl`'s signature policy at it. That is its only caller in this repo, so the change is confined to the `EvaluateSignedData` path; `EvaluateIdentities` already forwarded converted identities without re-verifying. - The returned identities no longer imply a checked signature — only a principal check does — and they carry the memoized outcome, so they are mutable and not safe for concurrent use. Both are documented on the function. - De-duplication now records a signer before verifying rather than after. A set carrying the same signer twice, the first with an invalid signature and the second valid, previously passed on the second entry and now fails on the first. It also means repeated copies of one signer's signature cost one verification instead of one each, which is what the existing de-duplication comment intends but does not currently achieve. The godoc states this, and a test pins it. - On the 4-organization network hyperledger#192 measured, one request now costs exactly 1 verification on every one of 40 independently built bundles, against a mean of 2.80 and a spread of 1 to 5 before. The policy walk itself is unchanged: leaf evaluations still range 1 to 5 over the same bundles, so the saving is verifications the walk no longer performs, not a shorter walk. #### Additional details (Optional) - Verification moves inside `SatisfiesPrincipal`, which is the only method the compiled leaf calls on an identity, and `used[i]` is set only after it returns nil, so a verification failure neither admits an identity nor consumes it from the `NOutOf` accounting. - Tested with the package's existing counterfeiter fakes: the conversion verifies nothing, a matching principal verifies once and repeats do not, a non-matching principal never verifies, an invalid signature is refused through the principal check and memoized, an identity that cannot be deserialized is skipped, and three signers appearing once each are verified once each without one signer's outcome reaching another. - Two tests in `common/cauthdsl` cover the case the change is for, end to end through an implicit meta policy over four signature sub-policies: when one sub-policy requires the signer's principal the signature is verified once however many sub-policies were tried first, and when none requires it no signature is verified at all. Both fail before this change, with four verifications where none are needed and with one per sub-policy tried. #### Related issues - resolves hyperledger#192 Signed-off-by: Dor.Katzelnick <Dor.Katzelnick@ibm.com>

tock-ibm · 2026-10-08T23:48:51Z

 	}

-	ids := policies.SignatureSetToValidIdentities(signatureSet, p.deserializer)
+	ids := policies.SignatureSetToDeferredVerificationIdentities(signatureSet, p.deserializer)


Verdict change on repeated signers, with no capability/version gate (consensus-relevant)

Routing EvaluateSignedData through the new dedup-before-verify converter changes which copy of a repeated signer is kept (first copy instead of first valid copy) and therefore the accept/reject verdict, with no capability or version gate, so nodes on this version and nodes on fabric-x-common <= v0.2.10 disagree on the same consensus-relevant input.

Committer: a tx with namespace endorsements [A junk sig, A valid, B valid] (the junk copy may use the certificate-ID form) passes the sidecar form check and is COMMITTED by verifiers on the pinned v0.2.9/v0.2.10 but ABORTED_SIGNATURE_INVALID on this PR (reproduced end to end through the committer verifier service), so org committers upgraded at different times fork tx_status/world state (a _meta tx under LifecycleEndorsement makes namespace policies diverge; replays re-judge history). Orderer: a ConfigUpdateEnvelope [A0 junk, A0 valid, A1 valid] under MAJORITY Admins passes ProposeConfigUpdate on old consenters and fails on new ones, so mixed followers reject an old leader's proposal (SmartBFT complain/view change). Nested-threshold policies can also flip REJECT->ACCEPT. The PR is not marked [BREAKING], and capabilities are still empty (docs/configtx.md:319).

tock-ibm · 2026-10-08T23:49:11Z

+			continue
+		}
+
+		idMap[key] = struct{}{}


First-entry-wins dedup lets an inserter veto another signer's genuine signature

Recording the dedup key before any signature check makes the first entry that deserializes to a signer's Mspid+Id win, so anyone who can insert an entry carrying a victim's public certificate (full PEM, PEM with trailing bytes, or certificate_id form, all the same key) ahead of the victim's genuine entry vetoes the genuine signature.

fabric-x fxconfig tx endorse appends to existing endorsements (tools/fxconfig/docs/README.md:201): org0 appends {Identity: org1's public cert, Endorsement: "junk"} before handing the file to org1, org1 appends its real endorsement and submits; the committer's _meta verifier (LifecycleEndorsement, MAJORITY over 3 orgs) drops org1's genuine entry as a duplicate and rejects ('1 sub-policies were satisfied, but this policy requires 2'), while main accepts. Same for config updates [A0, junk-A1, A1] under Admins. On main valid duplicates were already verified only once, so keeping later same-key entries as fallbacks (verified in order, each at most once) removes the veto at no cost for honest sets.

tock-ibm · 2026-10-08T23:49:13Z

+		}
+
+		idMap[key] = struct{}{}
+		identities = append(identities, &deferredVerificationIdentity{


Junk-signature entries are now principal-checked by every leaf (DoS cost amplification)

Entries with invalid signatures are no longer dropped before evaluation and only Verify outcomes are memoized, so every SignedBy leaf re-runs SatisfiesPrincipal on every junk entry (same-MSP IDENTITY principals re-deserialize and chain-validate the principal; ROLE runs an uncached Validate), amplifying the cost of adversarial signature sets.

A Byzantine block deliverer (BlockSigVerifier accepts any SignatureHeader creator) or a tx submitter prepends k=300 distinct CA-issued certs with bad signatures: BFT BlockValidation over 4 consenters in one MSP goes 73 ms -> 180 ms (SatisfiesPrincipal calls 6 -> 1206), 7 same-MSP consenters 76 -> 288 ms (3.8x), 2-of-4 orgs .member 72 -> 111 ms, one-consenter-per-org ARMA 71 -> 81 ms (real crypto, cached MSPs); cost stays linear in k but with a larger constant.

tock-ibm · 2026-10-08T23:49:14Z

+		return id.verifyErr
+	}
+
+	id.verified = true


Memo fails open: verified is set before Verify returns

verify() sets verified=true before Identity.Verify returns and assigns verifyErr only on an error return, so the memo fails open: after a Verify panic that the caller recovers, or for a concurrent reader mid-verify, SatisfiesPrincipal returns nil with no signature checked.

A caller of the exported converter evaluates the returned slice under recover and Verify panics; re-evaluating the same slice makes EvaluateIdentities return nil with Verify called once (reproduced); 8 goroutines on one identity with an invalid signature -> 7/8 accept and -race reports the race. Unreachable via cauthdsl today (per-call wrappers, no recover). sync.OnceValue built at conversion fails closed (re-panics), is concurrency-safe and removes the data/signature/index/verified/verifyErr fields, the defensive comment and the QF1008 nolint (+3 allocs, ~40 ns per identity); plain sync.Once would still fail open.

tock-ibm · 2026-10-08T23:49:16Z

+//
+// The returned identities are not safe for concurrent use, and callers must not treat their
+// presence in the slice as evidence that a signature was checked; only a principal check does that.
+func SignatureSetToDeferredVerificationIdentities(


Exported converter returns entries indistinguishable from verified ones

The new exported converter has the exact signature of SignatureSetToValidIdentities but returns forged entries indistinguishable from verified ones, because only the unexported wrapper's own SatisfiesPrincipal checks the signature and every other Identity method (and MSP-level checks) succeed for a forged entry.

With a real cached SampleOrg MSP, a set carrying a victim's cert with signature "garbage" gives len(deferred)=1 vs len(valid)=0, Validate()=nil, GetMSPIdentifier()="SampleOrg", and cachedMSP.SatisfiesPrincipal(forged, member)=nil once warm; a committer/orderer consumer that swaps this in and counts, filters, or uses an MSP-level principal check admits forged signatures. cauthdsl is the only caller, so the converter could be unexported in cauthdsl (no import cycle) or verification made explicit in the SignedBy leaf.

tock-ibm · 2026-10-08T23:49:18Z

+	require.Zero(t, fID.VerifyCallCount())
+}
+
+func TestSignatureSetToDeferredVerificationIdentitiesVerifiesOnceWhenAPrincipalMatches(t *testing.T) {


No test guards that a memoized verification never replaces the principal check

No test guards the new memo's key invariant that a successful verification never stands in for the principal check: the fakes satisfy every principal, the tests reuse one empty principal, and the cauthdsl tests build fresh wrappers per leaf.

Adding if id.verified && id.verifyErr == nil { return nil } at the top of deferredVerificationIdentity.SatisfiesPrincipal makes OR(AND(SignedBy(A), SignedBy(B)), SignedBy(C)) accept a set signed only by A (head rejects), yet every test in common/..., protoutil, core/... and the committer's utils/signature still passes, so this privilege-escalation regression would ship. A test that verifies an identity against one principal and then offers it a principal it does not satisfy (plus the nested OR/AND case through EvaluateSignedData) would catch it.

tock-ibm · 2026-10-08T23:49:19Z

+}
+
+// deferredVerificationIdentity checks its signature the first time it satisfies a principal, and
+// remembers the outcome so that a signature is verified at most once however many principals the


Memo is per leaf, so the godoc's "at most once" / "independent of them" claims don't hold for policy trees

The memo and deserialization are scoped to one cauthdsl leaf (ImplicitMetaPolicy still passes the raw set to every sub-policy), so the godoc's 'verified at most once however many principals the policy tries it against' and 'independent of [the number of organizations]' (line 369) do not hold for policy trees.

Shipped TwoOrgsSampleFabricX: /Channel/Admins (MAJORITY over Orderer and Application MAJORITY Admins, both containing Org1 and Org2) for a config update signed by both admins costs 4 verifications + 8 deserializations (~420 us) vs 2 + 2 (~200 us) with one conversion at the ImplicitMeta root driving sub-policies via EvaluateIdentities (main: 8 + 8, ~780 us); ANY over 100 leaves still pays 100 deserializations (~1.05 ms vs ~0.41 ms). Either convert once at the root or scope the godoc to a single signature-policy evaluation and to the verification count.

tock-ibm · 2026-10-08T23:49:21Z

+	// correct if a Verify method is ever added to the wrapper, which would otherwise recurse.
+	if err := id.Identity.Verify(id.data, id.signature); err != nil { //nolint:staticcheck // QF1008
+		id.verifyErr = fmt.Errorf("signature verification failed: %w", err)
+		logger.Warningf("signature for identity %d is invalid: %s", id.index, err)


Invalid signatures are no longer logged unless a principal matched in an evaluated leaf

Invalid signatures are now WARN-logged only when the signer matched a principal in a leaf that was actually evaluated, so forged signatures from non-matching signers, after a leaf is satisfied, in sub-policies skipped by the ImplicitMeta short-circuit, or in duplicate copies leave no 'signature for identity N is invalid' or msp.identity 'The signature is invalid for (certificate subject=...)' WARN.

ANY[Org1, Org2] (e.g. Writers/BlockValidation) with {User1@Org1 valid, User1@Org2 corrupted signature} and Org1's sub-policy first: the policy passes; main logs both WARNs naming the certificate, head logs nothing, and because sub-policy order comes from map iteration, whether it is logged varies across nodes and restarts, hiding a misbehaving party's corrupt block signatures. At minimum, document this observability change.

tock-ibm · 2026-10-08T23:49:23Z

+	t.Parallel()
+
+	// Scenario:
+	// - an implicit meta ANY policy over four sub-policies, only the last of which requires the


"Only the last sub-policy matches" test is order-dependent

The test assumes the matching sub-policy is 'the last of' four, but NewImplicitMetaPolicy orders sub-policies by ranging over the managers map, so the match is tried first about 1 run in 8 and the test then passes against the pre-PR eager code.

With policy.go:93 reverted to SignatureSetToValidIdentities the test passes 45/400 runs (verifies==1), so it does not deterministically guard 'verified once however many sub-policies were tried before it', and 'Both fail before this change' is not reliably true (the no-match companion does fail 400/400). Building &policies.ImplicitMetaPolicy{Threshold: 1, SubPolicies: []policies.Policy{p0, p1, p2, pMatch}} directly fails 500/500 on main and passes 500/500 on head.

tock-ibm · 2026-10-08T23:49:24Z

+}
+
+func (id *deferredVerificationIdentity) SatisfiesPrincipal(principal *msp.MSPPrincipal) error {
+	if err := id.Identity.SatisfiesPrincipal(principal); err != nil {


Principal-first order inverts cost for idemix identities (latent)

Checking the principal before verifying inverts the cost order for idemix identities, whose inner SatisfiesPrincipal runs Validate -> verifyProof (~27 ms, uncached) before comparing MSP IDs, so a replayed idemix identity with a junk signature pays a proof verification per leaf instead of a cheap failed Verify.

Hand-built MSP manager with an idemix MSP (v1_3) and three X.509 orgs, ANY over 4 orgs x OR(member, admin), junk signature: 0.27 ms on main vs 217 ms on head (~800x). Latent: channelconfig floors the MSP version to v3_0, which msp.New rejects for idemix, so no Fabric-X channel can hold an idemix MSP today; an MSP-ID pre-check for ROLE/OU principals before the inner call would restore the old order.

tock-ibm · 2026-10-08T23:49:26Z

+		idMap[key] = struct{}{}
+		identities = append(identities, &deferredVerificationIdentity{
+			Identity:  identity,
+			data:      sd.Data,


Wrapper retains sd.Data/sd.Signature and verifies them later; contract undocumented

The wrapper keeps sd.Data and sd.Signature by reference and verifies them only at principal-check time, and the exported function's godoc does not say callers must leave those buffers unchanged until evaluation.

A direct caller converts a set, recycles the Data/Signature buffer (e.g. a pooled marshal buffer), then evaluates: Verify runs over the mutated bytes (reproduced: Verify saw "mutated!"), so a signature valid for other content can be accepted or a genuine one rejected; the eager function verified during conversion. No such caller exists today; a godoc note (or a copy) closes it.

tock-ibm · 2026-10-08T23:49:27Z

+	require.Equal(t, 1, fID.VerifyCallCount())
+}
+
+func TestSignatureSetToDeferredVerificationIdentitiesSkipsAnIdentityItCannotDeserialize(t *testing.T) {


Copied "invalid identity" Warnw on the live path has no log assertion; overlapping tests

SkipsAnIdentityItCannotDeserialize repeats case 1 of TestSignatureSetToValidIdentitiesDeserializeErr without its log assertions, so the copied logger.Warnw("invalid identity", ...) on what is now the only production path is unasserted, while VerifiesEachIdentityOnce already subsumes VerifiesOnceWhenAPrincipalMatches and RefusesAnInvalidSignature.

Deleting or rewording the Warnw at policy.go:388-389 leaves every test in common/policies and common/cauthdsl green; adding ids = SignatureSetToDeferredVerificationIdentities(tc.signedData, fakeIdentityDeserializer); require.Empty(t, ids); assertLogContains(t, recorder, tc.expectedLogEntryContains...) to the existing non-parallel table covers it and catches that mutation.

tock-ibm · 2026-10-08T23:49:29Z

+			&cb.SignaturePolicyEnvelope{
+				Version:    0,
+				Rule:       policydsl.SignedBy(0),
+				Identities: []*mb.MSPPrincipal{{Principal: principal}},


ROLE classification for a serialized-identity principal; helpers duplicate existing ones

The hand-built sub-policy envelope leaves PrincipalClassification at ROLE for a serialized-identity principal (it only works because MockIdentity ignores the classification), and the new test plumbing re-implements existing helpers (signedDataFromSigner == newSignedData("org1", "identity1", "data1", "signature1"); signerPrincipal == msp.NewSerializedIdentity; countingDeserializer vs MockIdentityDeserializer / the common/policies/mocks fakes).

With a real MSP the serialized identity unmarshals as MSPRole{Org1, MEMBER}, so the fixture silently means 'any Org1 member' (User2@Org1 satisfies a principal built from User1's bytes, while policydsl.Envelope's IDENTITY principal refuses it); countingDeserializer ignores its argument, so the test still passes when the signed data names a different signer. policydsl.Envelope(policydsl.SignedBy(0), [][]byte{principal}) fixes the classification and drops the mb import.

tock-ibm · 2026-10-08T23:49:30Z

+	idMap := map[string]struct{}{}
+	identities := make([]mspi.Identity, 0, len(signedData))
+
+	for i, sd := range signedData {


Second exported converter duplicates SignatureSetToValidIdentities with a conflicting dedup rule

The new exported converter copies 22 of the 30 loop lines of SignatureSetToValidIdentities (deserialize+warn, Mspid+Id key, dedup check+warn) with a conflicting dedup rule, leaving two exported converters that disagree on the same set while the original has no production caller left.

For P = SignedBy(alice) and S = [alice bad sig, alice good sig] on the PR head, P.EvaluateSignedData(S) rejects while P.EvaluateIdentities(policies.SignatureSetToValidIdentities(S, d)) accepts; any later fix to the dedup key or log lines must be made twice. The export exists only because the call site is in cauthdsl; moving the converter there unexported, or sharing one loop with an admit callback, avoids the drift.

tock-ibm · 2026-10-08T23:49:32Z

+func implicitMetaOverSignaturePolicies( //nolint:ireturn // policies.NewImplicitMetaPolicy returns this interface
+	t *testing.T,
+	rule cb.ImplicitMetaPolicy_Rule,
+	principals [][]byte,
+	deserializer msp.IdentityDeserializer,
+) policies.Policy {


False nolint:ireturn justification

The //nolint:ireturn // policies.NewImplicitMetaPolicy returns this interface justification is false: NewImplicitMetaPolicy returns the concrete *policies.ImplicitMetaPolicy (implicitmeta.go:31), so the helper can return that type and needs no suppression.

A misleading suppression against the development skill's 'No premature interfaces ... return concrete types'; returning *policies.ImplicitMetaPolicy compiles, passes both tests and lints clean, and ireturn fires only for the interface version.

Suggested change

func implicitMetaOverSignaturePolicies( //nolint:ireturn // policies.NewImplicitMetaPolicy returns this interface

t *testing.T,

rule cb.ImplicitMetaPolicy_Rule,

principals [][]byte,

deserializer msp.IdentityDeserializer,

) policies.Policy {

func implicitMetaOverSignaturePolicies(

t *testing.T,

rule cb.ImplicitMetaPolicy_Rule,

principals [][]byte,

deserializer msp.IdentityDeserializer,

) *policies.ImplicitMetaPolicy {

tock-ibm force-pushed the defer-signature-verification branch from eff6548 to a1058b3 Compare October 7, 2026 08:05

tock-ibm reviewed Oct 8, 2026

View reviewed changes

Conversation

DorKatzelnick commented Sep 16, 2026

Type of change

Description

Additional details (Optional)

Related issues

Uh oh!

coveralls commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented Sep 16, 2026 •

edited

Loading