fix(identity): require canonical DER in every identity decode path

Identity.UniqueID() hashes the raw identity bytes rather than a canonicalised
form of the decoded value, and it is the cache key throughout the identity and
wallet layers (role/registry.go's fast path, provider.go's signer cache). Any
two byte strings that decode to the same logical identity but hash differently
give that one identity two cache slots: a token paid to the second spelling
still verifies, because verification works on the decoded value, but never
resolves to its owner's wallet, because the lookup works on UniqueID().

Every identity decode path allowed exactly that. The four encoding/asn1 sites
(MultiIdentity.Deserialize, PolicyIdentity.Deserialize, MultiSignature.FromBytes,
PolicySignature.FromBytes) discarded asn1.Unmarshal's "rest" return, and
DecodeIdentity read the outer SEQUENCE length only to throw it away:

  canonical : 300e 300c 0405 616c696365 0403 626f62
  variant   : 3011 300c 0405 616c696365 0403 626f62 02012a

Both decode to MultiIdentity{alice, bob}; both were accepted; their UniqueID()s
differ.

Four spellings of one identity were reachable:

  - garbage appended after the value
  - garbage smuggled inside the SEQUENCE with the outer length grown to cover
    it. This is the one a "rest" check cannot see: rest reports only bytes after
    the top-level TLV, and encoding/asn1 silently discards SEQUENCE elements the
    destination struct has no field for, so the outer TLV consumes the whole
    input and the parse looks clean
  - T61String/IA5String/GeneralString where UTF8String was declared, which
    encoding/asn1 accepts
  - non-minimal lengths (0x81 0x06) and non-minimal INTEGER contents
    (02 02 00 05)

marshal.UnmarshalStrict, which the four asn1 sites now route through, checks
rest and then re-encodes the decoded value and requires it to reproduce the
input byte-for-byte (ErrNonCanonical). One check covers the first three and any
further encoding/asn1 leniency, for one extra marshal of a small struct per
decode. DecodeIdentity — the TypedIdentity envelope and the hot path, which
hand-walks the TLVs and does not re-marshal — pins the outer SEQUENCE length to
len(b), requires the OCTET STRING to end exactly at len(b), and rejects
non-minimal lengths (ErrNonMinimalLen) and non-minimal integers
(ErrNonMinimalInt).

Nothing previously written can be rejected. The envelope is single-sourced: all
seven construction sites funnel through TypedIdentity.Bytes() ->
marshal.EncodeIdentity, and the four inner envelopes only through their own
asn1.Marshal. Every one of those encoders is canonical — appendTLV emits minimal
lengths, encodeInt32 strips exactly the bytes parseInt32 now rejects, and
asn1.Marshal is canonical by construction, including the legacy string-typed
spellings written by older versions of this SDK. There is no non-Go producer of
these bytes, so no identity on a ledger or in storage becomes undecodable and
old and new nodes cannot disagree during a rolling upgrade. Key material is
untouched: x509 certs and idemix credentials live inside the OCTET STRING and
never reach these decoders.

Scope is the DER envelopes. Signature parsing (x509/crypto/ecdsa.go,
idemixnym/nym/signer.go) stays lenient for external signers and HSMs; the
MultiSignature/PolicySignature envelopes are strict because we always produce
them, the signatures they carry are not. Two things remain malleable and are now
documented rather than implied away: the protobuf payload inside the OCTET
STRING, which is not DER at all, and the legacy type fold, where INTEGER 2,
UTF8String "x509" and PrintableString "x509" still give one x509 identity three
UniqueID()s. This change narrows that set from unbounded to exactly three;
closing it to one needs a rule at the validator boundary, since the older
spellings may exist in persisted data, and is tracked separately.

UnmarshalStrict's round-trip means "b is what asn1.Marshal would emit", which is
narrower than "b is valid DER": a field tagged optional or omitempty, or a
time.Time, would false-reject legal encodings. None of the four types has one,
and TestUnmarshalStrict_FourCallSitesHaveNoOptionalFields reflects over them so
a future field trips there rather than in production.

Tests cover every vector at each affected site, asserting the bypass really did
decode to the same value with an empty rest and a different UniqueID() rather
than only that it now errors, alongside guards that our own encoders' output is
never rejected. The five fuzz targets also assert canonicality; at the four asn1
sites that is a contract-level regression guard rather than an independent check
(UnmarshalStrict enforces it the way Bytes() computes it), and it is a real
check only in FuzzDecodeIdentityNoPanic, where DecodeIdentity and EncodeIdentity
are separate implementations. boolpolicy's two targets are new and wired into
the nightly-fuzz matrix. Two TestDecodeErrors fixtures had an outer length
disagreeing with their own buffer, so the new check fired before the inner
failure they were named for; their lengths are corrected rather than their
expectations.

Signed-off-by: AkramBitar <akram@il.ibm.com>

Author: AkramBitar

.github/workflows/nightly-fuzz.yml

@@ -54,6 +54,12 @@ jobs:
           - name: identity-multisig-signature-from-bytes
             pkg: ./token/services/identity/multisig
             func: FuzzMultiSignatureFromBytesNoPanic
+          - name: identity-boolpolicy-identity-deserializer
+            pkg: ./token/services/identity/boolpolicy
+            func: FuzzPolicyIdentityDeserializeNoPanic
+          - name: identity-boolpolicy-signature-from-bytes
+            pkg: ./token/services/identity/boolpolicy
+            func: FuzzPolicySignatureFromBytesNoPanic
           - name: identity-idemix-audit-info-deserializer
             pkg: ./token/services/identity/idemix/crypto
             func: FuzzDeserializeAuditInfoNoPanic

docs/services/identity.md

@@ -177,6 +177,30 @@ It wraps the raw identity bytes with a type label, enabling the system to verify
     - `Type` (string): The identifier of the identity scheme (e.g., `"x509"`, `"idemix"`).
     - `Identity` (bytes): The raw payload of the identity, specific to the key manager.
 
+#### Canonical encoding requirement
+
+The **DER envelopes** of an identity must be the canonical encoding of the value they decode to — exactly one byte string per logical envelope:
+
+*   `marshal.DecodeIdentity` (the `TypedIdentity` envelope decoder in `token/services/identity/marshal`) pins the outer `SEQUENCE`'s declared length to the end of the buffer, requires the read position to land exactly on the last byte after the final field (`ErrTrailingBytes`), rejects non-minimal DER length encodings — a length that fits the short form written in the long form, or a long form with leading zero bytes (`ErrNonMinimalLen`) — and rejects non-minimal `INTEGER` contents, i.e. a redundant leading `0x00`/`0xFF` in the type field (`ErrNonMinimalInt`).
+*   Envelopes decoded with `encoding/asn1` (`MultiIdentity`, `PolicyIdentity`, `MultiSignature`, `PolicySignature`) go through `marshal.UnmarshalStrict`, which rejects any bytes left over after the top-level value **and** re-encodes the decoded value to require it reproduces the input byte-for-byte (`ErrNonCanonical`). The second check is the load-bearing one: `asn1.Unmarshal`'s `rest` return only reports bytes *after* the top-level TLV, while `encoding/asn1` silently discards `SEQUENCE` elements the destination struct has no field for and accepts `T61String`/`IA5String`/`GeneralString` where a `UTF8String` was declared — neither of which leaves anything in `rest`.
+
+The reason is `Identity.UniqueID()`: it hashes the **raw** identity bytes rather than a canonicalised form of the decoded value, and it is the cache key throughout the identity and wallet layers (`role/registry.go`'s fast-path cache, `provider.go`'s signer cache, and so on). Any two byte strings that decode to the same logical identity but hash differently give that one identity two cache slots — a token paid to the second spelling still verifies, because verification works on the decoded value, but never resolves to its owner's wallet, because the lookup works on `UniqueID()`. Both producers of these bytes — `appendTLV` in the `marshal` package and `encoding/asn1.Marshal` for legacy encodings — already emit minimal lengths, minimal integers and no undeclared elements, so the stricter decode rejects nothing this tree ever writes.
+
+**What this does not cover.** The guarantee is about the envelopes, not about everything reachable through them:
+
+*   **The legacy type spellings remain a `UniqueID()` split, and it is the same class of problem as the one above.** `DecodeIdentity` folds `INTEGER 2`, `UTF8String "x509"` and `PrintableString "x509"` onto the same type for compatibility with identities written by older versions of this SDK. For one x509 identity that is three accepted byte strings and therefore three `UniqueID()`s:
+
+    ```
+    3006 020102       040150   -> {Type: 2, "P"}   uid NQ5fFHOcay5c
+    3009 0c0478353039 040150   -> {Type: 2, "P"}   uid FK3RQ1kfFhZG
+    3009 1304783530 39 040150  -> {Type: 2, "P"}   uid f4VvsY1uwzTB
+    ```
+
+    A token paid to the second or third spelling of a victim's identity verifies — validation decodes type 2 and checks the payload's cert — but does not resolve to that owner's wallet. The checks above reduce this set from unbounded to exactly three; closing it to one cannot be done in the decoder, because the older spellings may exist in persisted data. It needs a rule at the validator boundary: require the `INTEGER` spelling for identities in *new* transactions while still decoding the others for reads. Out of scope here, tracked separately.
+*   The payload inside the `TypedIdentity` `OCTET STRING` is **protobuf** for x509 and idemix identities (`x509/crypto/config.go`, `idemix/crypto/deserializer.go`), not DER. Protobuf permits field reordering and redundant varints, so those payload bytes remain malleable and the checks above say nothing about them.
+
+This applies to identity decoding only. Signature parsing (`x509/crypto/ecdsa.go`, `idemixnym/nym/signer.go`) stays deliberately lenient: those bytes come from external signers and HSMs whose DER encoders are routinely non-minimal in ways that are still valid for signature purposes. The `MultiSignature` / `PolicySignature` *envelopes* are strict, because we always produce them ourselves; the individual signatures they carry are not.
+
 ### Default Key Managers
 
 The identity service includes two primary implementations for concrete identities:
@@ -388,8 +412,10 @@ type MyNewIdentity struct {
 
 func (m *MyNewIdentity) Serialize() ([]byte, error) { return asn1.Marshal(*m) }
 func (m *MyNewIdentity) Deserialize(raw []byte) error {
-    _, err := asn1.Unmarshal(raw, m)
-    return err
+    // Never `_, err := asn1.Unmarshal(raw, m)`: discarding the "rest" return
+    // accepts trailing garbage, which breaks the canonical encoding
+    // requirement described under TypedIdentity above.
+    return marshal.UnmarshalStrict(raw, m)
 }
 ```
 

token/services/identity/boolpolicy/boolpolicy_fuzz_test.go

@@ -0,0 +1,108 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package boolpolicy
+
+import (
+	"encoding/asn1"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+const maxFuzzBoolPolicyBytes = 64 << 10
+
+// FuzzPolicyIdentityDeserializeNoPanic hunts for malformed ASN.1 that panics
+// PolicyIdentity.Deserialize instead of returning an error. This is the
+// deserialization entry point for policy identities across DeserializeVerifier,
+// GetAuditInfoMatcher, Recipients and Unwrap, and it mirrors
+// FuzzMultiIdentityDeserializeNoPanic in the structurally identical multisig
+// package.
+func FuzzPolicyIdentityDeserializeNoPanic(f *testing.F) {
+	valid, err := (&PolicyIdentity{
+		Policy:     "$0 OR ($1 AND $2)",
+		Identities: [][]byte{[]byte("alice"), []byte("bob"), []byte("carol")},
+	}).Bytes()
+	require.NoError(f, err)
+	f.Add(valid)
+	// Trailing bytes after the canonical encoding — the laxness this decode
+	// path was hardened against; it must now be a clean error, never a panic.
+	f.Add(append(append([]byte{}, valid...), 0xDE, 0xAD))
+	// An element smuggled *inside* the SEQUENCE rather than appended after it.
+	// A rest check cannot see this one, so it is the shape most likely to hide
+	// a regression.
+	smuggled, err := asn1.Marshal(policyIdentityWithExtra{
+		Policy:     "$0 OR ($1 AND $2)",
+		Identities: [][]byte{[]byte("alice"), []byte("bob"), []byte("carol")},
+		Extra:      42,
+	})
+	require.NoError(f, err)
+	f.Add(smuggled)
+	f.Add([]byte{})
+	f.Add([]byte("not asn1"))
+	f.Add([]byte{0x30, 0x81})
+
+	f.Fuzz(func(t *testing.T, raw []byte) {
+		if len(raw) > maxFuzzBoolPolicyBytes {
+			t.Skip()
+		}
+		pi := &PolicyIdentity{}
+		var err error
+		require.NotPanics(t, func() {
+			err = pi.Deserialize(raw)
+		})
+		if err != nil {
+			return
+		}
+		// Canonicality contract: raw was accepted, so it must be the one
+		// encoding of what it decoded to — any other accepted spelling is a
+		// second Identity.UniqueID() cache slot for one identity. This is a
+		// regression guard stated in terms of the contract, not an independent
+		// check: UnmarshalStrict enforces it by re-marshalling and comparing,
+		// which is what Bytes() does, so it cannot fail against the current
+		// implementation. security_test.go carries the actual vectors.
+		reencoded, err := pi.Bytes()
+		require.NoError(t, err, "an accepted PolicyIdentity must be re-serializable")
+		require.Equal(t, raw, reencoded,
+			"accepted bytes must be the canonical encoding of the decoded identity")
+	})
+}
+
+// FuzzPolicySignatureFromBytesNoPanic hunts for malformed ASN.1 that panics
+// PolicySignature.FromBytes instead of returning an error. This is the
+// deserialization entry point invoked directly on peer-supplied signature bytes
+// in PolicyVerifier.Verify.
+func FuzzPolicySignatureFromBytesNoPanic(f *testing.F) {
+	sigs := [][]byte{[]byte("sig1"), nil, []byte("sig3")}
+	valid, err := (&PolicySignature{Signatures: sigs}).Bytes()
+	require.NoError(f, err)
+	f.Add(valid)
+	f.Add(append(append([]byte{}, valid...), 0x00))
+	smuggled, err := asn1.Marshal(policySignatureWithExtra{Signatures: sigs, Extra: 7})
+	require.NoError(f, err)
+	f.Add(smuggled)
+	f.Add([]byte{})
+	f.Add([]byte("not asn1"))
+
+	f.Fuzz(func(t *testing.T, raw []byte) {
+		if len(raw) > maxFuzzBoolPolicyBytes {
+			t.Skip()
+		}
+		sig := &PolicySignature{}
+		var err error
+		require.NotPanics(t, func() {
+			err = sig.FromBytes(raw)
+		})
+		if err != nil {
+			return
+		}
+		reencoded, err := sig.Bytes()
+		require.NoError(t, err, "an accepted PolicySignature must be re-serializable")
+		// Contract-level regression guard; see FuzzPolicyIdentityDeserializeNoPanic.
+		require.Equal(t, raw, reencoded,
+			"accepted bytes must be the canonical encoding of the decoded signature")
+	})
+}

token/services/identity/boolpolicy/identity.go

@@ -39,6 +39,7 @@ import (
 	"github.com/LFDT-Panurus/panurus/token/core/common/encoding/json"
 	tdriver "github.com/LFDT-Panurus/panurus/token/driver"
 	"github.com/LFDT-Panurus/panurus/token/services/identity"
+	"github.com/LFDT-Panurus/panurus/token/services/identity/marshal"
 	"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
 )
 
@@ -74,11 +75,13 @@ func (p *PolicyIdentity) Serialize() ([]byte, error) {
 	return asn1.Marshal(*p)
 }
 
-// Deserialize decodes raw DER bytes into the receiver.
+// Deserialize decodes raw DER bytes into the receiver. It rejects trailing
+// bytes after the encoded value: raw comes off the wire, and accepting a
+// non-canonical re-encoding of an identity would make two distinct byte
+// strings decode to the same PolicyIdentity while hashing to different
+// Identity.UniqueID()s. See marshal.UnmarshalStrict.
 func (p *PolicyIdentity) Deserialize(raw []byte) error {
-	_, err := asn1.Unmarshal(raw, p)
-
-	return err
+	return marshal.UnmarshalStrict(raw, p)
 }
 
 // Bytes is an alias for Serialize, provided for symmetry with MultiIdentity.

token/services/identity/boolpolicy/security_test.go

@@ -8,6 +8,7 @@ package boolpolicy
 
 import (
 	"context"
+	"encoding/asn1"
 	"testing"
 
 	"github.com/LFDT-Panurus/panurus/token"
@@ -99,3 +100,164 @@ func TestDuplicateIdentityRejectedAtDeserializeTime(t *testing.T) {
 	_, err = d.DeserializeVerifier(ctx, Policy, raw)
 	require.Error(t, err, "a policy identity with a duplicated component identity must be rejected at deserialize time")
 }
+
+// TestTrailingBytesRejectedOnIdentityDeserialize is the reproduction from the
+// issue: PolicyIdentity.Deserialize discarded encoding/asn1.Unmarshal's "rest"
+// return, so a canonical PolicyIdentity with garbage appended still
+// deserialized successfully — while hashing to a *different*
+// Identity.UniqueID(), because UniqueID() hashes the raw bytes rather than a
+// canonicalised form of the decoded value. Since UniqueID() is the cache key
+// throughout the identity/wallet layers (role/registry.go's fast path,
+// provider.go's signer cache, …), one logical identity could occupy two cache
+// slots.
+func TestTrailingBytesRejectedOnIdentityDeserialize(t *testing.T) {
+	canonical, err := (&PolicyIdentity{Policy: "$0 OR $1", Identities: [][]byte{id0, id1}}).Bytes()
+	require.NoError(t, err)
+
+	require.NoError(t, (&PolicyIdentity{}).Deserialize(canonical),
+		"the canonical encoding must still deserialize")
+
+	padded := append(append([]byte{}, canonical...), 0xDE, 0xAD, 0xBE, 0xEF)
+
+	require.Error(t, (&PolicyIdentity{}).Deserialize(padded),
+		"a PolicyIdentity with trailing bytes appended must be rejected")
+
+	// This inequality is why the lenient parse mattered: the two byte strings
+	// decoded to the same logical identity but keyed the caches differently.
+	require.NotEqual(t, token.Identity(canonical).UniqueID(), token.Identity(padded).UniqueID(),
+		"UniqueID() hashes the raw bytes, so the padded form was a distinct cache key")
+}
+
+// TestTrailingBytesRejectedOnSignatureFromBytes covers the same laxness in
+// PolicySignature.FromBytes, which is invoked directly on peer-supplied bytes
+// in PolicyVerifier.Verify. Only the policy *envelope* is tightened here; the
+// individual signatures it carries are still parsed by their own verifiers,
+// which must stay lenient for external signers and HSMs.
+func TestTrailingBytesRejectedOnSignatureFromBytes(t *testing.T) {
+	canonical, err := (&PolicySignature{Signatures: [][]byte{
+		[]byte("sig1"), []byte("sig2"),
+	}}).Bytes()
+	require.NoError(t, err)
+
+	require.NoError(t, (&PolicySignature{}).FromBytes(canonical),
+		"the canonical encoding must still deserialize")
+
+	padded := append(append([]byte{}, canonical...), 0x00)
+
+	require.Error(t, (&PolicySignature{}).FromBytes(padded),
+		"a PolicySignature with trailing bytes appended must be rejected")
+}
+
+// policyIdentityWithExtra is PolicyIdentity plus one undeclared trailing
+// element. Marshalling it is how an attacker writes the smuggled form:
+// encoding/asn1 consumes only as many SEQUENCE elements as the destination
+// struct has fields and silently drops the rest, so these bytes decode into a
+// plain PolicyIdentity with nothing left over.
+type policyIdentityWithExtra struct {
+	Policy     string `asn1:"utf8"`
+	Identities [][]byte
+	Extra      int32
+}
+
+// TestExtraElementInsideSequenceRejectedOnIdentityDeserialize covers the vector
+// the trailing-byte check alone cannot see. Appending garbage *after* the
+// top-level TLV is caught by asn1.Unmarshal's "rest"; moving that same garbage
+// *inside* the SEQUENCE, with the outer length grown to cover it, leaves rest
+// empty — so the identity decodes to the same policy over the same components
+// under raw bytes that hash to a different UniqueID(). Concretely: a token
+// transferred to the smuggled form verifies (the verifier works on the decoded
+// policy) but never resolves to a component owner's wallet (the lookup works on
+// UniqueID()), leaving a valid token none of the owners can see.
+func TestExtraElementInsideSequenceRejectedOnIdentityDeserialize(t *testing.T) {
+	const policy = "$0 OR $1"
+	ids := [][]byte{id0, id1}
+
+	canonical, err := (&PolicyIdentity{Policy: policy, Identities: ids}).Bytes()
+	require.NoError(t, err)
+	require.NoError(t, (&PolicyIdentity{}).Deserialize(canonical),
+		"the canonical encoding must still deserialize")
+
+	smuggled, err := asn1.Marshal(policyIdentityWithExtra{Policy: policy, Identities: ids, Extra: 42})
+	require.NoError(t, err)
+	require.NotEqual(t, canonical, smuggled, "the two encodings must really differ")
+
+	// Baseline: the lax decode accepts it, yields the same policy identity, and
+	// has nothing left over for a rest check to catch.
+	var lenient PolicyIdentity
+	rest, err := asn1.Unmarshal(smuggled, &lenient)
+	require.NoError(t, err)
+	require.Empty(t, rest, "the extra element is inside the SEQUENCE, so there is no tail to reject")
+	require.Equal(t, policy, lenient.Policy)
+	require.Equal(t, ids, lenient.Identities, "same logical identity, different bytes")
+
+	require.NotEqual(t, token.Identity(canonical).UniqueID(), token.Identity(smuggled).UniqueID(),
+		"UniqueID() hashes the raw bytes, so the smuggled form is a distinct cache key")
+
+	require.Error(t, (&PolicyIdentity{}).Deserialize(smuggled),
+		"a PolicyIdentity with an element smuggled inside the SEQUENCE must be rejected")
+}
+
+// TestAlternateStringTagRejectedOnIdentityDeserialize covers a third spelling,
+// unique to PolicyIdentity because it is the only one of the four envelopes
+// carrying a string field: encoding/asn1 accepts T61String, IA5String and
+// GeneralString wherever `asn1:"utf8"` was declared. Flipping that one tag byte
+// leaves the decoded Policy identical and the raw bytes — hence the UniqueID()
+// — different, with no trailing bytes and no length anomaly for either earlier
+// check to catch.
+func TestAlternateStringTagRejectedOnIdentityDeserialize(t *testing.T) {
+	canonical, err := (&PolicyIdentity{Policy: "$0 OR $1", Identities: [][]byte{id0, id1}}).Bytes()
+	require.NoError(t, err)
+
+	// The Policy field is the first element of the SEQUENCE, so its tag is the
+	// first byte of the body: [SEQUENCE, len, tag, ...].
+	require.Equal(t, byte(asn1.TagUTF8String), canonical[2],
+		"fixture assumption: canonical[2] is the Policy field's UTF8String tag")
+
+	for _, tag := range []byte{
+		byte(asn1.TagT61String),
+		byte(asn1.TagIA5String),
+		byte(asn1.TagGeneralString),
+	} {
+		retagged := append([]byte{}, canonical...)
+		retagged[2] = tag
+
+		// Baseline: same policy string, no leftovers.
+		var lenient PolicyIdentity
+		rest, err := asn1.Unmarshal(retagged, &lenient)
+		require.NoError(t, err, "tag 0x%02X: encoding/asn1 accepts this where UTF8String was declared", tag)
+		require.Empty(t, rest)
+		require.Equal(t, "$0 OR $1", lenient.Policy, "tag 0x%02X: same policy, different bytes", tag)
+
+		require.NotEqual(t, token.Identity(canonical).UniqueID(), token.Identity(retagged).UniqueID(),
+			"tag 0x%02X: the retagged form is a distinct cache key", tag)
+
+		require.Error(t, (&PolicyIdentity{}).Deserialize(retagged),
+			"tag 0x%02X: a non-UTF8String spelling of the same policy must be rejected", tag)
+	}
+}
+
+// policySignatureWithExtra is PolicySignature plus one undeclared element, the
+// signature-envelope analog of policyIdentityWithExtra.
+type policySignatureWithExtra struct {
+	Signatures [][]byte
+	Extra      int32
+}
+
+// TestExtraElementInsideSequenceRejectedOnSignatureFromBytes covers the same
+// vector in PolicySignature.FromBytes, which PolicyVerifier.Verify calls
+// directly on peer-supplied bytes. The nil slot mirrors an unsigned OR branch,
+// which must keep decoding.
+func TestExtraElementInsideSequenceRejectedOnSignatureFromBytes(t *testing.T) {
+	sigs := [][]byte{[]byte("sig1"), nil, []byte("sig3")}
+
+	canonical, err := (&PolicySignature{Signatures: sigs}).Bytes()
+	require.NoError(t, err)
+	require.NoError(t, (&PolicySignature{}).FromBytes(canonical),
+		"the canonical encoding, unsigned slot included, must still deserialize")
+
+	smuggled, err := asn1.Marshal(policySignatureWithExtra{Signatures: sigs, Extra: 7})
+	require.NoError(t, err)
+
+	require.Error(t, (&PolicySignature{}).FromBytes(smuggled),
+		"a PolicySignature with an element smuggled inside the SEQUENCE must be rejected")
+}

token/services/identity/boolpolicy/sig.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/LFDT-Panurus/panurus/token"
 	"github.com/LFDT-Panurus/panurus/token/driver"
+	"github.com/LFDT-Panurus/panurus/token/services/identity/marshal"
 	"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
 )
 
@@ -29,11 +30,15 @@ func (s *PolicySignature) Bytes() ([]byte, error) {
 	return asn1.Marshal(*s)
 }
 
-// FromBytes deserialises raw ASN.1 DER into the receiver.
+// FromBytes deserialises raw ASN.1 DER into the receiver. It rejects trailing
+// bytes after the encoded value: raw is peer-supplied, and the envelope itself
+// is always produced by our own canonical asn1.Marshal, so leftover bytes only
+// ever mean a mangled or deliberately padded signature. (This tightens the
+// PolicySignature *envelope* only — the individual signatures it carries are
+// parsed by their own verifiers, which must stay lenient for external
+// signers/HSMs.)
 func (s *PolicySignature) FromBytes(raw []byte) error {
-	_, err := asn1.Unmarshal(raw, s)
-
-	return err
+	return marshal.UnmarshalStrict(raw, s)
 }
 
 // JoinSignatures builds a PolicySignature from a map of per-identity