Align signature verification with Trusted Server v1.1 by ChristianPavilonis · Pull Request #80 · stackpop/mocktioneer

ChristianPavilonis · 2026-03-03T20:12:03Z

Summary

Align request signature verification with Trusted Server signing v1.1 so signed requests are validated against the canonical payload expected by upstream.
Require and parse all v1.1 ext.trusted_server fields (version, kid, request_host, request_scheme, ts) to prevent partial or ambiguous verification inputs.
Use a fixed CPM bid value ($0.01) for generated OpenRTB and APS responses, and remove request-level bid override behavior from generated ad markup.
Keep signature visibility in creatives consistent while simplifying badge text to signature status only, and document both the v1.1 signing payload and fixed pricing behavior for integrators.

Changes

Crate / File	Change
`crates/mocktioneer-core/src/verification.rs`	Reworked verification to build and verify canonical JSON payload (`version`, `kid`, `host`, `scheme`, `id`, `ts`), required v1.1 ext fields, enforced signing version `1.1`, and added payload/version tests.
`crates/mocktioneer-core/src/render.rs`	Added coverage asserting the signature badge is always rendered in creative HTML.
`crates/mocktioneer-core/static/templates/creative.html.hbs`	Updated badge behavior so it is always shown and displays signature status only (`verified`/`failed`/`not_present`).
`crates/mocktioneer-core/src/auction.rs`	Switched generated OpenRTB and APS bids to fixed CPM (`$0.01`), removed request-supplied bid overrides from response generation, and updated related tests.
`docs/api/openrtb-auction.md`	Updated Trusted Server v1.1 signature docs and aligned pricing docs/examples with fixed CPM behavior.

Closes

Closes #79
Closes #81

Test plan

cargo test --workspace --all-targets
cargo clippy --workspace --all-targets --all-features -- -D warnings
cargo check --workspace --all-targets --features "fastly cloudflare"
Manual testing via cargo run -p mocktioneer-adapter-axum
Playwright e2e tests (cd tests/playwright && npm test)
Other: cargo fmt --all -- --check; cargo test -p mocktioneer-core

Checklist

Changes follow CLAUDE.md conventions
Business logic lives in mocktioneer-core, not in adapter crates
New routes added to both routes.rs and edgezero.toml
Determinism preserved — no randomness or time-dependent logic
Ad markup rendered via templates in render.rs, not inlined in handlers
New code has tests (colocated #[cfg(test)] or in tests/)
No secrets or credentials committed

aram356

PR Review

The v1.1 signing protocol and SigningPayload canonicalization are solid structural improvements. The fixed-pricing simplification makes sense for a mock bidder. A few things need attention before merge — mostly around the trust model for signed fields and consistency across docs.

Summary

Emoji	Count	Meaning
🔧	4	Needs change
♻️	1	Refactoring suggestion
🌱	1	Future consideration
📌	1	Out-of-scope follow-up
💯	1	Positive

Non-inline notes

⛏ test_price_from_ext_and_iframe_bid_param (auction.rs:474) — the test name still implies we're verifying that ext.bid drives the price. It now tests the opposite. Rename to something like test_ext_bid_override_is_ignored.

⛏ log::info!("URI: {}", uri) at verification.rs:73 looks like a debug leftover from development. The line above already logs at debug level. Demote or remove.

📌 The request example in openrtb-auction.md (lines 52–55) still includes "ext": {"mocktioneer": {"bid": 2.5}} in the impression — contradicts the new Pricing section that says bid overrides are ignored. Remove it from the example to avoid confusion.

crates/mocktioneer-core/src/verification.rs

crates/mocktioneer-core/src/auction.rs

docs/api/openrtb-auction.md

crates/mocktioneer-core/static/templates/creative.html.hbs

… with fixed pricing - Cross-check ext.trusted_server host/scheme against server-observed values - Enforce timestamp freshness window (±5 min) to prevent replay attacks - Add field-order warning comment on SigningPayload struct - Fix ts unit mismatch: align tests with docs (milliseconds) - Add positive-path Ed25519 round-trip test with deterministic keypair - Remove debug log leftover in fetch_jwks - Simplify APS size selection to area-based ranking (replaces CPM) - Remove CPM from /sizes endpoint response - Rename test to test_ext_bid_override_is_ignored - Docs sweep: remove bid override references across all doc pages

ChristianPavilonis · 2026-03-06T16:56:53Z

Review feedback addressed in `3bbbbc7`

🔧 Trust model for signed fields (verification.rs)

verify_request_id_signature now accepts trusted_host and trusted_scheme parameters (server-observed values)
After extracting request_host/request_scheme from ext.trusted_server, they are compared against the trusted values — mismatches return VerificationError::InvalidSignature
The handler in routes.rs passes ForwardedHost and extracts scheme from the X-Forwarded-Proto header (defaulting to "https")
Timestamp freshness: enforced via a ±5 minute window (TS_FRESHNESS_WINDOW_MS). Stale or future timestamps are rejected with a descriptive error including the drift amount.
Added tests: verify_host_mismatch_rejected, verify_scheme_mismatch_rejected, verify_stale_timestamp_rejected, verify_future_timestamp_rejected, check_timestamp_freshness_within_window

🌱 Field order comment on `SigningPayload`

Added a prominent comment above the struct explaining that field order defines the canonical signing payload and reordering will silently break verification.

🔧 `ts` unit mismatch

Aligned on milliseconds (matching docs and Trusted Server spec). All test values updated from 1706900000 → 1706900000000. The canonical payload example in docs already used millis.

🔧 Positive-path E2E test

Added verify_ed25519_roundtrip_with_known_keypair: generates a deterministic Ed25519 keypair from a fixed seed, builds a canonical payload, signs it, and verifies the round-trip. Also asserts that a tampered payload produces SignatureVerificationFailed.

🔧 Docs sweep for pricing

openrtb-auction.md: Removed ext.mocktioneer.bid from the Full Request example and removed the "With Price Override" cURL section
prebidjs.md: Removed bid parameter from table, removed Price Override section, removed bid values from all examples, removed High CPM Testing section
prebid-server.md: Removed bid parameter from table, removed With Price Override section, removed bid values from all examples, updated response price to 0.01
aps-bid.md: Updated decoded price example to 0.01, updated amznbid/amznp values to match $0.01 encoding, updated size selection description to area-based
docs/index.md: Changed "price overrides" to "fixed pricing"
docs/api/index.md: Removed CPM column from sizes table, updated /sizes endpoint response docs

♻️ APS size selection simplification (auction.rs)

Replaced get_cpm() ranking with w * h area-based ranking. Since all bids use FIXED_BID_CPM, CPM-based ranking was effectively arbitrary. Largest area is now the explicit selection criterion.
Updated related tests in both auction.rs and tests/aps_endpoints.rs

`/sizes` endpoint consistency (routes.rs)

Removed cpm from the /sizes response since it contradicted fixed pricing. The endpoint now returns only width/height per size.

⛏ Quick fixes

Renamed test_price_from_ext_and_iframe_bid_param → test_ext_bid_override_is_ignored
Removed log::info!("URI: {}", uri) debug leftover from fetch_jwks

💯 Sig allowlist

No changes needed — kept as-is.

All CI gates pass: cargo test --workspace --all-targets (100 tests), cargo fmt, cargo clippy.

Align remaining docs with fixed /bin/zsh.01 bidding and update Playwright /_/sizes assertions to match the cpm-free response, so review cleanups and CI checks reflect the current API behavior.

aram356

Review: Round 2

The follow-up commits addressed the first review well — host/scheme cross-checks, timestamp freshness, v1.1 canonical payload, E2E roundtrip test, docs sweep, and APS size selection are all in. Good work.

Remaining issues below, grouped by severity.

What's done well

Check ordering — cheap validations (field extraction, host/scheme cross-check, timestamp freshness) all run before the expensive JWKS fetch + crypto. Good.
Roundtrip crypto test — verify_ed25519_roundtrip_with_known_keypair proves the full sign→verify pipeline and catches tampered payloads. Most valuable test in the file.
Creative sig param whitelist — sanitizes sig query param against known-safe values instead of trusting arbitrary input. Clean pattern.
Docs consistency — all references to dynamic pricing, price override, and bid param removed across 7 doc files. Thorough.

crates/mocktioneer-core/src/routes.rs

crates/mocktioneer-core/src/verification.rs

crates/mocktioneer-core/src/auction.rs

crates/mocktioneer-core/static/templates/creative.html.hbs

crates/mocktioneer-core/src/verification.rs

aram356

Review: Round 3

Prior rounds addressed host/scheme normalization, dead code removal (get_cpm, DEFAULT_CPM, MAX_AREA_BONUS), unused JS variable, and fragile test assertions. The branch is CI-clean (fmt, clippy, tests, WASM targets all pass).

One blocking item remains from round 2 that was partially addressed — the SIZE_MAP values were documented as legacy but not actually zeroed out.

CI Status

fmt: PASS
clippy: PASS
tests: PASS (all workspace targets)
cargo check --features "fastly cloudflare": PASS
cargo check --target wasm32-unknown-unknown (Cloudflare): PASS

crates/mocktioneer-core/src/auction.rs

crates/mocktioneer-core/src/verification.rs

crates/mocktioneer-core/static/templates/creative.html.hbs

crates/mocktioneer-core/src/verification.rs

crates/mocktioneer-core/src/auction.rs

… with fixed pricing - Cross-check ext.trusted_server host/scheme against server-observed values - Enforce timestamp freshness window (±5 min) to prevent replay attacks - Add field-order warning comment on SigningPayload struct - Fix ts unit mismatch: align tests with docs (milliseconds) - Add positive-path Ed25519 round-trip test with deterministic keypair - Remove debug log leftover in fetch_jwks - Simplify APS size selection to area-based ranking (replaces CPM) - Remove CPM from /sizes endpoint response - Rename test to test_ext_bid_override_is_ignored - Docs sweep: remove bid override references across all doc pages

Align remaining docs with fixed /bin/zsh.01 bidding and update Playwright /_/sizes assertions to match the cpm-free response, so review cleanups and CI checks reflect the current API behavior.

- Normalize X-Forwarded-Proto: split on comma, trim, lowercase, fall back to request URI scheme before defaulting to https - Add canonicalize_host() to strip default ports (:443/:80) and lowercase both sides of host/scheme cross-checks in verification - Log deprecation warning when imp.ext.mocktioneer.bid is present but ignored - Remove dead pricing code: get_cpm(), DEFAULT_CPM, MAX_AREA_BONUS - Update CLAUDE.md key constants to reflect FIXED_BID_CPM - Remove unused JS variable 'b' in creative template - Strengthen timestamp test assertions with matches! on error variant

…calize_host tests

…ot bidder host The host cross-check was comparing ext.trusted_server.request_host (the publisher's domain) against the bidder's own ForwardedHost, which will never match in header bidding. Changed to compare against site.domain from the OpenRTB request instead. Also removed the redundant scheme cross-check since request_scheme is already verified cryptographically as part of the signed payload.

Remove the phf dependency entirely — SIZE_MAP values were zeroed out and only used for key membership checks. Replace with a sorted const array of (i64, i64) tuples. O(n) scan over 13 entries is trivially fast and eliminates the build-time codegen dependency.

Update all references from $0.01 to $0.20 across docs, CLAUDE.md, and inline code comments to match the actual constant value.

aram356

PR Review

Summary

Well-structured PR that reworks signature verification for Trusted Server v1.1 and simplifies pricing to a fixed $0.20 CPM. The verification logic is sound with proper timestamp freshness, host cross-validation against site.domain, and comprehensive Ed25519 roundtrip tests. One blocking issue: incorrect base64-encoded prices in APS docs.

🔧 Needs Change

Incorrect base64-encoded prices in APS docs: MC4wMQ== decodes to "0.01", not the actual fixed bid price. encode_aps_price(0.20) produces MC4y since Rust formats 0.20 as "0.2". The decode example (MC4yMA==) is also wrong. See inline comments on docs/api/aps-bid.md.

⛏ Nitpicks

Stale "highest CPM" comment (routes.rs:905): Says "should select 970x250 with highest CPM" but size selection is now by largest area. Suggest: // should select 970x250 with largest area from [728x90, 970x250]
Stale "highest CPM" comment (tests/aps_endpoints.rs:197): Says "Should bid on the highest CPM size (970x250 = $4.20 > 728x90 = $3.00)" — references old pricing. Suggest: // Should bid on the largest area size (970x250 = 242500 > 728x90 = 65520)
Stale CLAUDE.md reference (CLAUDE.md:154): See inline comment.

♻️ Refactoring

Dead NoJwksDomain variant (verification.rs:67): Defined in VerificationError but never constructed anywhere in the codebase. Consider removing since the verification module is being reworked, or #[allow(dead_code)] if reserved for future use.

🤔 Thoughts

SystemTime::now() WASM compatibility (verification.rs:268): New in this PR. Compiles for WASM targets today, but behavior on wasm32-unknown-unknown is platform-dependent. Consistent with existing Instant::now() usage so not a regression — noting for awareness.

🌱 Seeds

Replay protection: The 5-minute freshness window prevents large-scale replay but permits replay within that window. Fine for a mock bidder; a nonce/seen-ID cache could layer on if production trust enforcement is ever needed.

👍 Praise

Canonical payload design: Struct field order as signing contract + pinned JSON test is the right pattern for security-critical serialization
Timestamp freshness: abs_diff-based check correctly handles both stale and future timestamps
Host cross-check fix: Comparing request_host against site.domain instead of bidder ForwardedHost is the correct header-bidding trust model
Ed25519 roundtrip test: Full sign-verify cycle with tampered payload case provides strong confidence
phf removal: Const array simplification eliminates build-time codegen with negligible runtime cost

CI Status

fmt: PASS
clippy: PASS
tests: PASS (100 tests)

docs/api/aps-bid.md

CLAUDE.md

crates/mocktioneer-core/src/verification.rs

crates/mocktioneer-core/src/auction.rs

prk-Jr

PR Review

Summary

The v1.1 signing and fixed-pricing changes look solid overall, and the required Rust CI gates are green. I found one blocking docs mismatch plus two non-blocking follow-ups worth addressing before merge.

Findings

🔧 Needs Change

Trusted Server example uses mismatched hosts: The request example shows site.domain = "example.com" but ext.trusted_server.request_host = "publisher.example". The implementation now rejects that combination because request_host must match site.domain after canonicalization.

🤔 Thoughts

Floor-testing docs imply Mocktioneer enforces floors: The integration docs read as if Mocktioneer itself rejects bids below imp.bidfloor, but the auction path always emits FIXED_BID_CPM and never evaluates bidfloor. Clarify that the host integration or mediation path is responsible for floor enforcement.

🌱 Seeds

Add a full positive-path verification test: The new tests cover malformed inputs and raw Ed25519 verification, but not a full verify_request_id_signature() success path with JWKS lookup plus a returned kid.

CI Status

fmt: PASS
clippy: PASS
tests: PASS

docs/api/openrtb-auction.md

docs/integrations/index.md

… docs

aram356

PR Review

Summary

Upgrades signature verification to Trusted Server v1.1 (canonical payload, host cross-check, timestamp freshness) and simplifies pricing to fixed $0.20 CPM. Well-structured with thorough tests (100/100 pass). Two stale CLAUDE.md references need fixing; a few questions and observations inline.

Findings

🔧 Needs Change

Stale phf reference in CLAUDE.md: Dependencies Philosophy (line 305) still lists phf (static maps) but the dep was removed in this PR. Remove it.
Stale auction module description in CLAUDE.md: Module table (line 144) says auction.rs — Size pricing, CPM calculation, standard sizes but get_cpm() / DEFAULT_CPM / MAX_AREA_BONUS were all removed. Update to e.g. auction.rs — Fixed-price bidding, standard size validation.
Stale price in encode_aps_price doc comment (auction.rs:186): Still shows echo "Mi41MA==" | base64 -d → 2.50 referencing the old $2.50 CPM. Update to echo "MC4y" | base64 -d → 0.2.

❓ Questions

Redundant domain / site_domain params: Both always receive req.site.domain — is there a planned scenario where they differ? (see inline on routes.rs)
SystemTime::now() on WASM: New current_time_ms() may not work on wasm32-unknown-unknown without a polyfill — has this been validated? (see inline on verification.rs)

🤔 Thoughts

Non-determinism exception: Timestamp freshness check is justified for replay prevention, but contradicts the "same input → same output" rule in CLAUDE.md. Worth a documented exception. (see inline on verification.rs)
Serde field-order contract: SigningPayload correctness depends on declaration-order serialization — the pinning test is great mitigation. (see inline on verification.rs)

⛏ Nitpicks

canonicalize_host port stripping: Only handles :443/:80 — fine for prod, just noting. (see inline on verification.rs)

CI Status

fmt: PASS
clippy: PASS
tests: PASS (100/100)

aram356 · 2026-04-10T06:30:08Z

crates/mocktioneer-core/src/routes.rs

            &req.id,
            req.ext.as_ref(),
            domain,
+            domain,


❓ Redundant parameter: Both domain (JWKS fetch) and the new site_domain (host cross-check) receive the same req.site.domain value. Is there a planned scenario where they'd differ (e.g. JWKS hosted on a separate infra domain)? If not, collapsing to a single parameter would reduce surface area for confusion.

aram356 · 2026-04-10T06:30:08Z

crates/mocktioneer-core/src/verification.rs

+}
+
+/// Strip default ports (:443 for https, :80 for http) and lowercase the host
+/// so that `example.com:443` matches `example.com` from the signer.


❓ WASM compatibility: SystemTime::now() may not work on wasm32-unknown-unknown (Cloudflare Workers) without a polyfill. Instant::now() was already used for JWKS caching (pre-existing), but SystemTime is a new addition. Has this been validated on the Cloudflare target?

aram356 · 2026-04-10T06:30:08Z

crates/mocktioneer-core/src/verification.rs

+        .or_else(|| h.strip_suffix(":80"))
+        .unwrap_or(h)
+        .to_lowercase()
+}


🤔 Non-determinism exception: The freshness check is time-dependent — same request verifies now but fails 6 minutes later. Justified for replay prevention, but CLAUDE.md says "no time-dependent logic" and "same input always produces same output." Consider adding a note near the determinism rule so future contributors don't try to "fix" this.

aram356 · 2026-04-10T06:30:08Z

crates/mocktioneer-core/src/verification.rs


+// IMPORTANT: Field order defines the canonical signing payload.
+// `serde_json::to_string` serializes struct fields in declaration order.
+// Reordering fields will silently break signature verification.


🤔 Serde field-order contract: Canonical payload correctness depends on serde_json::to_string serializing fields in declaration order. The warning comment and build_signing_payload_uses_v11_shape test are great mitigation — just flagging for awareness that adding a field to this struct requires updating the test and coordinating with upstream signers.

aram356 · 2026-04-10T06:30:08Z

crates/mocktioneer-core/src/verification.rs

+
+fn current_time_ms() -> Result<u64, VerificationError> {
+    std::time::SystemTime::now()
+        .duration_since(std::time::UNIX_EPOCH)


⛏ Only strips :443 and :80 — non-standard HTTPS ports like :8443 won't be canonicalized. Fine for production publishers (standard ports), just noting for completeness.

ChristianPavilonis self-assigned this Mar 3, 2026

ChristianPavilonis requested a review from aram356 March 3, 2026 20:16

aram356 reviewed Mar 6, 2026

View reviewed changes

ChristianPavilonis requested a review from aram356 March 11, 2026 16:13

aram356 requested changes Mar 11, 2026

View reviewed changes

ChristianPavilonis requested a review from aram356 March 12, 2026 18:57

aram356 requested changes Mar 13, 2026

View reviewed changes

ChristianPavilonis requested review from aram356 and prk-Jr March 13, 2026 21:23

aram356 requested changes Mar 23, 2026

View reviewed changes

crates/mocktioneer-core/src/auction.rs Outdated Show resolved Hide resolved

ChristianPavilonis added 9 commits March 23, 2026 17:04

Align signature verification with Trusted Server v1.1

f718059

Use fixed CPM bids and align OpenRTB docs #81

49f61f4

Finalize PR #80 follow-ups for fixed-pricing contract

282ed30

Align remaining docs with fixed /bin/zsh.01 bidding and update Playwright /_/sizes assertions to match the cpm-free response, so review cleanups and CI checks reflect the current API behavior.

Address PR review: zero out SIZE_MAP values, lazy error alloc, canoni…

303ae47

…calize_host tests

Align docs with FIXED_BID_CPM of $0.20

4766574

Update all references from $0.01 to $0.20 across docs, CLAUDE.md, and inline code comments to match the actual constant value.

ChristianPavilonis force-pushed the fix/request-verification-issue branch from 00c23d8 to 4766574 Compare March 23, 2026 22:04

ChristianPavilonis requested a review from aram356 March 23, 2026 23:04

format docs

14662fd

aram356 requested changes Apr 1, 2026

View reviewed changes

Address PR review feedback

a95717a

ChristianPavilonis requested a review from aram356 April 2, 2026 00:13

prk-Jr requested changes Apr 2, 2026

View reviewed changes

docs/api/openrtb-auction.md Outdated Show resolved Hide resolved

docs/integrations/index.md Outdated Show resolved Hide resolved

Fix mismatched hosts in OpenRTB example and clarify floor enforcement…

7b3b759

… docs

ChristianPavilonis requested a review from prk-Jr April 3, 2026 17:49

aram356 requested changes Apr 10, 2026

View reviewed changes

Conversation

ChristianPavilonis commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Closes

Test plan

Checklist

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

PR Review

Summary

Non-inline notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChristianPavilonis commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review feedback addressed in 3bbbbc7

🔧 Trust model for signed fields (verification.rs)

🌱 Field order comment on SigningPayload

🔧 ts unit mismatch

🔧 Positive-path E2E test

🔧 Docs sweep for pricing

♻️ APS size selection simplification (auction.rs)

/sizes endpoint consistency (routes.rs)

⛏ Quick fixes

💯 Sig allowlist

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

Review: Round 2

What's done well

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

Review: Round 3

CI Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

PR Review

Summary

🔧 Needs Change

⛏ Nitpicks

♻️ Refactoring

🤔 Thoughts

🌱 Seeds

👍 Praise

CI Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChristianPavilonis commented Mar 3, 2026 •

edited

Loading

ChristianPavilonis commented Mar 6, 2026 •

edited

Loading

Review feedback addressed in `3bbbbc7`

🌱 Field order comment on `SigningPayload`

🔧 `ts` unit mismatch

`/sizes` endpoint consistency (routes.rs)