identity: secret material (PKCS11 PIN, idemix Sk) and unbounded attacker bytes reach error strings and logs

[Effi-S]

Fixes #2069

Summary

Several code paths in the identity stack put secret material (PKCS11 PINs, idemix user secret keys)
into error strings or debug logs, and several others echo unbounded attacker-controlled bytes
directly into log/error messages instead of hashing or truncating them — both are logging-hygiene
issues that this tree otherwise avoids (it already uses utils.Hashable(...) in several places for
exactly this reason).

Where

Secret material in errors/logs:

• token/services/identity/idemix/crypto/config.go:176 — wraps an error with string(configRaw),
  the full serialized IdemixConfig, whose Signer.Sk field is the idemix user secret key.
• token/services/identity/x509/crypto/pkcs11/pkcs11.go:35 — logs opts directly, where
  PKCS11Opts contains a Pin field.
• token/services/identity/x509/crypto/setup.go:64 — logs bccspConfig, which embeds *PKCS11
  (same Pin field).
• token/services/identity/x509/kmp.go:89 — logs opts.PKCS11 twice at debug level.

Unbounded attacker-byte echo into logs/errors (should use utils.Hashable(raw) or lengths, as done
elsewhere in this tree):

• token/services/identity/interop/htlc/deserializer.go:172
• token/services/identity/interop/htlc/validator.go:90
• token/services/identity/idemix/crypto/deserializer.go:115
• token/services/identity/idemix/deserializer.go:184
• token/services/identity/idemixnym/deserializer.go:106
• token/services/identity/x509/crypto/msp.go:245

Impact

• PIN/secret-key exposure: if LogLevel is set to debug in a deployment (a plausible operational
  choice for troubleshooting), the idemix user secret key or the PKCS11 PIN can land in log files,
  which are frequently shipped to less-trusted aggregation systems than the node itself, or retained
  longer than the credential's intended lifetime.
• Unbounded byte echo: logs/error messages become arbitrarily large under attacker control (a crafted
  large or adversarial identity blob), and error strings containing raw untrusted bytes can pollute
  structured-logging pipelines or make log injection easier.

Reproduction

Not yet committed. For the secret-exposure items: set debug logging, trigger the corresponding error
path (e.g. malformed idemix config, PKCS11 setup with a bad slot), and grep the resulting log output
for the PIN/secret-key value. For the byte-echo items: pass an oversized or binary-garbage identity
through the affected deserializer/validator and confirm the raw bytes appear verbatim in the emitted
log/error.

Suggested fix

• For secret-bearing structs, add a custom String()/MarshalLogObject that redacts Sk/Pin
  before it's ever passed to a logger or error constructor, or explicitly select only the
  non-secret fields to log (e.g. log opts.PKCS11.Label/.Slot but never .Pin).
• For attacker-controlled byte fields, replace direct interpolation with
  utils.Hashable(raw) (already imported and used for this exact purpose elsewhere in this package,
  e.g. wallet/service.go:115,118) or log only len(raw).

Severity

MEDIUM — secret exposure requires debug logging to be enabled; byte-echo is a hygiene/DoS-adjacent
issue rather than directly exploitable, but both are inconsistent with the pattern already used
elsewhere in this codebase.

[AkramBitar]

Problem: error and log messages were dumping whole config objects and raw network data, leaking private keys and HSM PINs into log files and letting attacker-supplied bytes bloat them

Fix: the PIN is now printed as [REDACTED] and untrusted bytes are printed as a short SHA-256 fingerprint instead of the bytes themselves.

Review: logging hygiene for secrets and untrusted bytes

Direction is right and the change is low-risk. I verified the head commit (1097bc3) compiles — including the -tags pkcs11 path — and that the affected packages' tests pass. I also confirmed experimentally that the new String() fixes the setup.go and kmp.go leak sites without those files being edited, and that nested/nil cases don't panic.

Four findings, none blocking on their own; the two P1s are worth clearing before merge.

#	Where	Priority	Effort
1	idemix/crypto/config.go:176	P1	1 line
2	x509/crypto/msp.go:245 (not in diff)	P1	1 line, or 1 line of PR text
3	x509/crypto/redact_test.go:33-34	P2	2 lines
4	x509/crypto/config.go:83	P3 — defer	~20 min

Findings 1, 3 and 4 are inline. The remaining two can't be anchored inline:

---

2. P1 — token/services/identity/x509/crypto/msp.go:245 is listed in the PR description but not fixed.

errors.Errorf("could not decode pem bytes [%v]", idBytes)

This is still present, and it's the highest-impact of the six byte-echo sites the description enumerates: %v on a []byte renders a decimal array ([45 45 45 ...]), roughly 4x the input size, and getCertFromPem is reachable from IdentityFactory.DeserializeFullIdentity with bytes supplied by a counterparty — so a 1 MB identity blob becomes a ~4 MB error string that then gets logged.

Severity-wise this is pre-existing rather than a regression, but it's P1 because the PR description claims it's handled: merging as-is closes #2069 with a documented site still open, and the next reader trusts the description. utils.Hashable(idBytes) matches the fix applied elsewhere in this PR. Either fix it or drop it from the description.

---

Out of scope, but relevant to #2069: token/services/identity/x509/kmp.go yaml-marshals the raw opts map into idConfig.Config, persisting the HSM PIN in cleartext in the identity store. That outlives any log rotation, so on severity it's the largest exposure in this area — but it needs its own issue and some design, not a bolt-on to a logging PR.

Also worth noting: the pkcs11/pkcs11.go redaction sits behind //go:build pkcs11, so it compiles only in tagged builds and the new test file never exercises it.

[AkramBitar]

LGTM