Peer.processCertificateResponse validates against message.requestedCertificates, which the sender never populates — the check is inert

[mitch-burcham]

Summary

Peer.processCertificateResponse passes the inbound message.requestedCertificates to validateCertificates, but sendCertificateResponse never sets that field. The requested-certifier and requested-type checks are therefore skipped on every certificateResponse.

Found while building cross-language conformance vectors for a Rust port against @bsv/sdk 2.4.1.

Detail

In packages/sdk, the two validation paths differ:

• processInitialResponse → validateInitialResponseCertificates passes this.certificatesToRequest — the receiver's own policy. Correct.
• processCertificateResponse passes message.requestedCertificates — a field on the inbound message.

sendCertificateResponse builds the outbound message as {version, messageType, identityKey, nonce, initialNonce, yourNonce, certificates, signature} — there is no requestedCertificates member.

So on the certificateResponse path certificatesRequested is undefined, and validateCertificates skips its entire if (certificatesRequested != null) block — both the certifier check and the type check.

Impact

A peer that requested certificates of type X from certifier C will accept, on the standalone certificateResponse path, any cryptographically valid certificate of any type from any certifier. Subject-match and signature verification still apply, so this is not a forgery issue — but the requested-set policy the caller configured is not enforced on that path.

The initialResponse path is unaffected.

Note for anyone porting

This is easy to reproduce faithfully and then "fix" unilaterally in a port, which creates a silent cross-language divergence — a port that enforces the policy will reject certificates a TS peer accepts, and no same-language test can detect it. We chose to conform to the current behaviour and report it rather than diverge.

Suggested fix

Pass this.certificatesToRequest on the certificateResponse path, matching processInitialResponse. That is a behaviour change for anyone relying on the current leniency, so it may warrant a minor-version note.