diff --git a/.gitignore b/.gitignore index 5bd523e..7384878 100644 --- a/.gitignore +++ b/.gitignore @@ -94,5 +94,5 @@ coverage/ pkg/ wasm-pack.log -# generated API reference (build artifact — never commit/publish pre-managed-infra) +# generated API reference (build artifact — never commit/publish pre-managed-infra) api/openapi.html diff --git a/cli/src/cli_args.rs b/cli/src/cli_args.rs index ada61ea..f076ed1 100644 --- a/cli/src/cli_args.rs +++ b/cli/src/cli_args.rs @@ -53,6 +53,11 @@ pub enum Commands { did_web_url: Option, #[arg(long)] admin_user: Option, + /// Prefer the `ADMIN_PASSWORD` environment variable instead: a value + /// passed here lands in shell history and is readable by other local + /// users via `ps`/`/proc//cmdline` for the process lifetime. + /// `bootstrap` is the scripting/CI entrypoint (no interactive prompt); + /// interactive operators should run `odal` instead. #[arg(long)] admin_pass: Option, /// Mint an additional key even if the node is already bootstrapped @@ -278,8 +283,9 @@ pub enum KeyCommands { /// variable or the interactive prompt (omit the argument) so it does not /// land in shell history or `ps`/`/proc//cmdline`. Use { - /// The `odal_sk_…` secret to save (prefer the env var / prompt instead). - #[arg(value_name = "SECRET")] + /// The `odal_sk_…` secret to save. If omitted, it is read from + /// `ODAL_API_SECRET` or prompted for without echoing to the terminal — + /// keeping the secret out of shell history and the process table. secret: Option, }, } diff --git a/cli/src/core/passport/import.rs b/cli/src/core/passport/import.rs index efe30d5..d1a96f4 100644 --- a/cli/src/core/passport/import.rs +++ b/cli/src/core/passport/import.rs @@ -83,7 +83,7 @@ async fn import_json_records( "Record {}: HTTP {} — {}", i + 1, status, - &body[..body.len().min(200)] + crate::stateless::render::truncate(&body, 200) )); } Err(e) => { @@ -174,7 +174,7 @@ async fn summarize_import_response( if !status.is_success() { anyhow::bail!( "Import failed (HTTP {status}): {}", - &body[..body.len().min(300)] + crate::stateless::render::truncate(body, 300) ); } let resp: serde_json::Value = @@ -197,7 +197,7 @@ async fn poll_import_job(client: &OdalClient, cfg: &Config, job_id: &str) -> Res if !status.is_success() { anyhow::bail!( "Failed to poll import job (HTTP {status}): {}", - &body[..body.len().min(200)] + crate::stateless::render::truncate(&body, 200) ); } let resp: serde_json::Value = diff --git a/cli/src/core/passport/publish.rs b/cli/src/core/passport/publish.rs index 6b4fa01..9058f39 100644 --- a/cli/src/core/passport/publish.rs +++ b/cli/src/core/passport/publish.rs @@ -50,7 +50,7 @@ async fn publish_one(client: &OdalClient, vault_url: &str, id: &str) -> Result

Result { let err = format!( "{name}: HTTP {status} — {}", - &resp_body[..resp_body.len().min(200)] + crate::stateless::render::truncate(&resp_body, 200) ); errors.push(err.clone()); items.push(PassportPublishResult { diff --git a/cli/src/stateless/render.rs b/cli/src/stateless/render.rs index 63a3cab..ecc48e3 100644 --- a/cli/src/stateless/render.rs +++ b/cli/src/stateless/render.rs @@ -17,11 +17,16 @@ use crate::core::types::{ // ── Helpers ────────────────────────────────────────────────────────────────── -fn truncate(s: &str, max: usize) -> String { - if s.len() <= max { +/// Truncate to at most `max` characters (not bytes), appending `…` when cut. +/// +/// Counts and slices by `char` so a multi-byte UTF-8 sequence landing on the +/// cutoff can never panic the CLI (the byte-index `&s[..n]` form does). +pub(crate) fn truncate(s: &str, max: usize) -> String { + if s.chars().count() <= max { s.to_owned() } else { - format!("{}…", &s[..max.saturating_sub(1)]) + let head: String = s.chars().take(max.saturating_sub(1)).collect(); + format!("{head}…") } } diff --git a/crates/dpp-common/src/request_id.rs b/crates/dpp-common/src/request_id.rs index 81efe8f..cfd09b9 100644 --- a/crates/dpp-common/src/request_id.rs +++ b/crates/dpp-common/src/request_id.rs @@ -9,6 +9,18 @@ use axum::{ }; use tower_http::request_id::{MakeRequestId, RequestId}; +/// Maximum error-body size buffered to inject `requestId`. Larger bodies are +/// passed through unmodified (the id remains in the `x-request-id` header). +const MAX_INJECT_BYTES: usize = 64 * 1024; + +/// Whether a `Content-Type` names a JSON media type: plain `application/json` +/// or any RFC 6839 `+json` structured suffix — including this platform's +/// `application/problem+json` error type, which `contains("application/json")` +/// alone does **not** match. +fn is_json_content_type(content_type: &str) -> bool { + content_type.contains("application/json") || content_type.contains("+json") +} + /// Generates a UUIDv7-based request ID for `SetRequestIdLayer`. #[derive(Clone, Default)] pub struct UuidRequestId; @@ -48,7 +60,7 @@ pub async fn inject_request_id(request: Request, next: Next) -> impl IntoRespons .headers() .get(axum::http::header::CONTENT_TYPE) .and_then(|v| v.to_str().ok()) - .is_some_and(|v| v.contains("application/json")); + .is_some_and(is_json_content_type); if !is_json { return response; @@ -56,9 +68,35 @@ pub async fn inject_request_id(request: Request, next: Next) -> impl IntoRespons let (parts, body) = response.into_parts(); - let bytes = match axum::body::to_bytes(body, 64 * 1024).await { + // A large error body (e.g. a batch-import validation report) can't be + // buffered to inject `requestId` without risking unbounded memory. When + // `Content-Length` already says it exceeds the limit, pass it through + // unmodified rather than dropping it — the id is still in the `x-request-id` + // header regardless. + if parts + .headers + .get(axum::http::header::CONTENT_LENGTH) + .and_then(|v| v.to_str().ok()) + .and_then(|v| v.parse::().ok()) + .is_some_and(|len| len > MAX_INJECT_BYTES) + { + return Response::from_parts(parts, body); + } + + let bytes = match axum::body::to_bytes(body, MAX_INJECT_BYTES).await { Ok(b) => b, - Err(_) => return Response::from_parts(parts, Body::empty()), + Err(e) => { + // A streamed body with no Content-Length that overran the limit; it + // is consumed and can't be recovered, so return empty — but never + // silently, so the truncation is visible in logs. + tracing::warn!( + error = %e, + status = %parts.status, + "error response body exceeded {MAX_INJECT_BYTES} bytes; \ + requestId not injected and body dropped" + ); + return Response::from_parts(parts, Body::empty()); + } }; let mut json: serde_json::Value = match serde_json::from_slice(&bytes) { @@ -73,3 +111,24 @@ pub async fn inject_request_id(request: Request, next: Next) -> impl IntoRespons let new_body = serde_json::to_vec(&json).unwrap_or_else(|_| bytes.to_vec()); Response::from_parts(parts, Body::from(new_body)) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn json_content_type_matches_problem_json() { + assert!(is_json_content_type("application/json")); + assert!(is_json_content_type("application/json; charset=utf-8")); + // The platform's actual error media type — the case the old + // `contains("application/json")` check silently missed. + assert!(is_json_content_type("application/problem+json")); + assert!(is_json_content_type( + "application/problem+json; charset=utf-8" + )); + assert!(is_json_content_type("application/ld+json")); + // Non-JSON must not match. + assert!(!is_json_content_type("text/html")); + assert!(!is_json_content_type("text/plain; charset=utf-8")); + } +} diff --git a/crates/dpp-dal/src/pg/repo_api_key.rs b/crates/dpp-dal/src/pg/repo_api_key.rs index eb42c0a..6e30860 100644 --- a/crates/dpp-dal/src/pg/repo_api_key.rs +++ b/crates/dpp-dal/src/pg/repo_api_key.rs @@ -25,8 +25,9 @@ impl PgApiKeyRepo { } fn key_from_row(r: &sqlx::postgres::PgRow) -> ApiKey { - // `scopes TEXT[]` may be NULL for keys written before scopes were - // enforced; `from_scopes` maps NULL/empty to Admin (pre-scope behaviour). + // `scopes TEXT[]` may be NULL for legacy pre-scope rows; `from_scopes` + // fails closed, mapping NULL/empty (and any unrecognized value) to the + // least-privilege Read rather than Admin. let scope = ApiKeyScope::from_scopes( &r.try_get::>, _>("scopes") .ok() diff --git a/crates/dpp-dal/src/pg/repo_passport.rs b/crates/dpp-dal/src/pg/repo_passport.rs index 1a81b11..67dc645 100644 --- a/crates/dpp-dal/src/pg/repo_passport.rs +++ b/crates/dpp-dal/src/pg/repo_passport.rs @@ -23,6 +23,30 @@ use dpp_domain::{ use super::{PgDal, db_err}; +/// Fields `patch_fields` refuses to modify: passport identity, lifecycle state, +/// retention lock, signatures, and seal. Each is governed by the publish +/// pipeline / `update_status` and backs a scalar column that this JSONB-merge +/// path does not rewrite — allowing them here would both bypass the state +/// machine and desync the doc from its enforcing column (e.g. flipping +/// `retentionLocked` in the doc while the `retention_locked` column stays +/// `false`). Mirrors the `PassportRepository` default-impl guard in dpp-core. +/// Serialized (camelCase) names. +const PROTECTED_PATCH_FIELDS: [&str; 13] = [ + "id", + "sector", + "status", + "retentionLocked", + "retentionUntil", + "jwsSignature", + "publicJwsSignature", + "seal", + "version", + "publishedAt", + "createdAt", + "supersedesId", + "schemaVersion", +]; + /// Apply a passport update (scalar columns + `doc`) inside a caller-supplied /// transaction. Shared by [`PgPassportRepo::update`] and the transactional /// outbox's `commit_publish`, so the publish-write and the outbox insert commit @@ -139,6 +163,13 @@ impl PassportRepository for PgPassportRepo { /// /// O(n) over active passports — acceptable for single-tenant MVP scale. async fn find_published_by_gtin(&self, gtin: &str) -> Result, DppError> { + // A GTIN is purely numeric. Reject anything else so LIKE metacharacters + // (`%`/`_`) in an untrusted value can't widen the pattern to match — and + // return — an arbitrary passport. A non-numeric value can never match a + // real GS1 Digital Link URL anyway. + if gtin.is_empty() || !gtin.bytes().all(|b| b.is_ascii_digit()) { + return Ok(None); + } // Battery GS1 DL URL: https://id.odal-node.io/01/{gtin}/21/{serialId} let row = sqlx::query( "SELECT doc FROM odal.passport \ @@ -216,6 +247,28 @@ impl PassportRepository for PgPassportRepo { id: PassportId, delta: serde_json::Value, ) -> Result { + // Reject protected/state-machine fields up front: they are set only via + // the publish pipeline / update_status and back scalar columns this path + // does not rewrite, so patching them would bypass the state machine and + // desync the doc from its enforcing column. + if let Some(obj) = delta.as_object() { + let mut forbidden: Vec<&str> = PROTECTED_PATCH_FIELDS + .iter() + .copied() + .filter(|k| obj.contains_key(*k)) + .collect(); + if !forbidden.is_empty() { + forbidden.sort_unstable(); + return Err(DppError::Validation( + format!( + "patch_fields cannot modify protected field(s): {}", + forbidden.join(", ") + ) + .into(), + )); + } + } + let mut tx = self.dal.begin().await?; // Row lock makes concurrent patches serialise instead of clobbering. let row = sqlx::query("SELECT doc FROM odal.passport WHERE id = $1 FOR UPDATE") @@ -237,17 +290,14 @@ impl PassportRepository for PgPassportRepo { } } let passport = Self::from_doc(doc.clone())?; - sqlx::query( - r#"UPDATE odal.passport SET - status = COALESCE($2->>'status', status), - doc = $2 - WHERE id = $1"#, - ) - .bind(Self::uuid_of(id)) - .bind(&doc) - .execute(&mut *tx) - .await - .map_err(db_err)?; + // Doc-only write: the scalar columns are all protected fields (rejected + // above), so they can never drift from the doc via this path. + sqlx::query("UPDATE odal.passport SET doc = $2 WHERE id = $1") + .bind(Self::uuid_of(id)) + .bind(&doc) + .execute(&mut *tx) + .await + .map_err(db_err)?; tx.commit().await.map_err(db_err)?; Ok(passport) } diff --git a/crates/dpp-dal/src/pg/repo_registry_sync.rs b/crates/dpp-dal/src/pg/repo_registry_sync.rs index 7ca1dd6..324ad74 100644 --- a/crates/dpp-dal/src/pg/repo_registry_sync.rs +++ b/crates/dpp-dal/src/pg/repo_registry_sync.rs @@ -19,6 +19,22 @@ use dpp_types::{RegistrySyncCounts, RegistrySyncOutbox, RegistrySyncRow, Registr use super::{PgDal, db_err, repo_passport::update_passport_in_tx}; +/// Turn an `UPDATE` that matched no row into a `NotFound` rather than a silent +/// `Ok(())` — a status update against an absent `passport_id` is a real error, +/// not a no-op success. +fn require_updated( + res: &sqlx::postgres::PgQueryResult, + passport_id: PassportId, +) -> Result<(), DppError> { + if res.rows_affected() == 0 { + return Err(DppError::NotFound(format!( + "registry_sync row for passport {}", + passport_id.0 + ))); + } + Ok(()) +} + /// PostgreSQL implementation of [`RegistrySyncOutbox`]. pub struct PgRegistrySyncRepo { dal: PgDal, @@ -53,10 +69,21 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo { let mut tx = self.dal.begin().await?; // Same transaction as the passport write — the atomicity guarantee. update_passport_in_tx(&mut tx, passport).await?; + // A previously *rejected* row is re-queued with the corrected payload so + // a fixed passport can actually be resubmitted (and `due()`, which only + // selects 'pending', picks it back up). Rows already 'registered', + // 'pending', or carrying a status-intent are left untouched. sqlx::query( r#"INSERT INTO odal.registry_sync (passport_id, payload, status) VALUES ($1, $2, 'pending') - ON CONFLICT (passport_id) DO NOTHING"#, + ON CONFLICT (passport_id) DO UPDATE SET + payload = EXCLUDED.payload, + status = 'pending', + attempts = 0, + next_attempt_at = now(), + message = NULL, + updated_at = now() + WHERE odal.registry_sync.status = 'rejected'"#, ) .bind(passport.id.0) .bind(&payload) @@ -109,7 +136,7 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo { passport_id: PassportId, registry_id: String, ) -> Result<(), DppError> { - sqlx::query( + let res = sqlx::query( r#"UPDATE odal.registry_sync SET status = 'registered', registry_id = $2, @@ -124,7 +151,7 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo { .execute(self.dal.pool()) .await .map_err(db_err)?; - Ok(()) + require_updated(&res, passport_id) } async fn mark_rejected( @@ -132,7 +159,7 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo { passport_id: PassportId, message: String, ) -> Result<(), DppError> { - sqlx::query( + let res = sqlx::query( r#"UPDATE odal.registry_sync SET status = 'rejected', message = $2, @@ -145,7 +172,7 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo { .execute(self.dal.pool()) .await .map_err(db_err)?; - Ok(()) + require_updated(&res, passport_id) } async fn mark_attempt_failed( @@ -156,7 +183,7 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo { // Exponential backoff on the *new* attempt count, capped at 1h, with // 0.75–1.25× jitter to avoid thundering-herd retries. `attempts` in the // expression is the pre-increment value the row already holds. - sqlx::query( + let res = sqlx::query( r#"UPDATE odal.registry_sync SET attempts = attempts + 1, message = $2, @@ -171,7 +198,7 @@ impl RegistrySyncOutbox for PgRegistrySyncRepo { .execute(self.dal.pool()) .await .map_err(db_err)?; - Ok(()) + require_updated(&res, passport_id) } async fn pending_for( diff --git a/crates/dpp-dal/tests/pg_integration.rs b/crates/dpp-dal/tests/pg_integration.rs index c8432cf..3139ff8 100644 --- a/crates/dpp-dal/tests/pg_integration.rs +++ b/crates/dpp-dal/tests/pg_integration.rs @@ -455,6 +455,52 @@ async fn t6_patch_fields_merge() { assert_eq!(reread.schema_version, "2.0.0", "untouched fields survive"); } +// patch_fields must reject state-machine / integrity fields so it can't bypass +// the state machine or desync the doc from its enforcing scalar column. +#[tokio::test] +async fn patch_fields_rejects_protected_fields() { + let pg = start_pg().await; + let repo = PgPassportRepo::new(pg.dal.clone()); + let p = make_passport(); + let id = p.id; + repo.create(p).await.expect("create"); + + let err = repo + .patch_fields( + id, + serde_json::json!({ "retentionLocked": true, "status": "active" }), + ) + .await + .expect_err("protected fields must be rejected"); + assert!( + matches!(err, dpp_domain::DppError::Validation(_)), + "got: {err:?}" + ); + + // The passport is untouched — still a retention-unlocked draft. + let reread = repo.find_by_id(id).await.unwrap().unwrap(); + assert_eq!(reread.status, PassportStatus::Draft); + assert!(!reread.retention_locked); +} + +// A LIKE wildcard in the GTIN must not widen the match to arbitrary passports. +#[tokio::test] +async fn find_published_by_gtin_rejects_like_metacharacters() { + let pg = start_pg().await; + let repo = PgPassportRepo::new(pg.dal.clone()); + // Publish a passport so there's an active row a wildcard could otherwise hit. + let mut p = make_passport(); + p.status = PassportStatus::Published; + repo.create(p).await.expect("create"); + + for bad in ["%", "_", "not-a-gtin", ""] { + assert!( + repo.find_published_by_gtin(bad).await.unwrap().is_none(), + "non-numeric gtin {bad:?} must never match" + ); + } +} + // T8 — grant coverage: the app role can read every table a migration creates. // Catches the "0010's grants were a snapshot" lesson (0017 had to re-grant): a // migration that adds a table after 0010 must ship its own odal_app grant, or diff --git a/crates/dpp-identity/src/handlers/did_document.rs b/crates/dpp-identity/src/handlers/did_document.rs index e9a1a4a..46bef2f 100644 --- a/crates/dpp-identity/src/handlers/did_document.rs +++ b/crates/dpp-identity/src/handlers/did_document.rs @@ -1,6 +1,6 @@ use axum::{ Json, - extract::{Path, State}, + extract::State, http::{HeaderValue, StatusCode, header::CACHE_CONTROL}, response::IntoResponse, }; @@ -14,18 +14,16 @@ use crate::state::AppState; /// `must-revalidate` so nothing serves a stale key past that window. const DID_DOCUMENT_CACHE_CONTROL: &str = "public, max-age=60, must-revalidate"; -/// Serve the `did:web` DID document for a given operator. -/// Accessible at `/.well-known/did.json` (root operator) or `/operators/{id}/did.json`. -pub async fn did_document_handler( - State(state): State, - operator_path: Option>, -) -> impl IntoResponse { - let operator_id = match operator_path { - Some(Path(id)) => id, - None => "root".to_owned(), - }; +/// Serve the `did:web` DID document for this node's operator. +/// +/// Mounted only at `/.well-known/did.json`. The node is single-tenant, so there +/// is one operator (`root`) and no per-operator DID route — the previous +/// `Option` per-operator branch was dead code (no such route was ever +/// registered). +pub async fn did_document_handler(State(state): State) -> impl IntoResponse { + let operator_id = "root"; - match did_builder::build_did_document(&state.store, &state.did_web_base_url, &operator_id) { + match did_builder::build_did_document(&state.store, &state.did_web_base_url, operator_id) { Ok(doc) => ( StatusCode::OK, [( diff --git a/crates/dpp-identity/src/handlers/rotate_key.rs b/crates/dpp-identity/src/handlers/rotate_key.rs index 117208d..52c9aeb 100644 --- a/crates/dpp-identity/src/handlers/rotate_key.rs +++ b/crates/dpp-identity/src/handlers/rotate_key.rs @@ -37,13 +37,22 @@ pub async fn rotate_key_handler( State(state): State, Json(body): Json, ) -> impl IntoResponse { - if body.operator_id.is_empty() { - return http_problem::unprocessable("operator_id is required").into_response(); + if !super::sign::is_valid_operator_id(&body.operator_id) { + return http_problem::unprocessable( + "operator_id must be 1-64 characters of [A-Za-z0-9._:-]", + ) + .into_response(); } - // Archive first — best effort; log but don't abort on failure. + // Archive first — and ABORT if it fails. Generating a new key overwrites the + // current record, so proceeding without a successful archive would destroy + // the old key with no backup and permanently invalidate every JWS it signed. if let Err(e) = state.store.archive_key(&body.operator_id) { - tracing::warn!(operator_id = %body.operator_id, error = %e, "failed to archive old key before rotation"); + tracing::error!(operator_id = %body.operator_id, error = %e, "aborting rotation — could not archive the current signing key"); + return http_problem::internal_error( + "could not archive the current signing key; rotation aborted to avoid losing it", + ) + .into_response(); } let new_key = match state.store.generate_key(&body.operator_id) { diff --git a/crates/dpp-identity/src/handlers/sign.rs b/crates/dpp-identity/src/handlers/sign.rs index edd429c..13abb08 100644 --- a/crates/dpp-identity/src/handlers/sign.rs +++ b/crates/dpp-identity/src/handlers/sign.rs @@ -12,6 +12,19 @@ use dpp_crypto::jws::signer; use crate::state::AppState; +/// Whether `operator_id` is a well-formed identifier safe to key the on-disk +/// key store by: 1–64 characters from `[A-Za-z0-9._:-]`. +/// +/// Rejecting anything else bounds key auto-provisioning — a typo, retry, or +/// garbage/oversized value can no longer silently grow the (never-pruned) key +/// store with a brand-new key. +pub(crate) fn is_valid_operator_id(id: &str) -> bool { + (1..=64).contains(&id.len()) + && id + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'_' | b':' | b'-')) +} + /// Request body for the signing endpoint. #[derive(Debug, Deserialize)] pub struct SignRequest { @@ -31,9 +44,14 @@ pub async fn sign_handler( State(state): State, Json(body): Json, ) -> impl IntoResponse { - if body.operator_id.is_empty() || body.passport_id.is_empty() || body.payload.is_empty() { - return http_problem::unprocessable("operator_id, passport_id, and payload are required") - .into_response(); + if !is_valid_operator_id(&body.operator_id) { + return http_problem::unprocessable( + "operator_id must be 1-64 characters of [A-Za-z0-9._:-]", + ) + .into_response(); + } + if body.passport_id.is_empty() || body.payload.is_empty() { + return http_problem::unprocessable("passport_id and payload are required").into_response(); } // Decode payload from base64 @@ -67,3 +85,20 @@ pub async fn sign_handler( } } } + +#[cfg(test)] +mod tests { + use super::is_valid_operator_id; + + #[test] + fn operator_id_validation_bounds_provisioning() { + assert!(is_valid_operator_id("self_hosted")); + assert!(is_valid_operator_id("did:web:acme.example")); + // Rejected: empty, too long, or unsafe characters. + assert!(!is_valid_operator_id("")); + assert!(!is_valid_operator_id(&"x".repeat(65))); + for bad in ["../etc", "a b", "a/b", "a\nb", "a;b"] { + assert!(!is_valid_operator_id(bad), "should reject: {bad}"); + } + } +} diff --git a/crates/dpp-identity/src/middleware/mtls.rs b/crates/dpp-identity/src/middleware/mtls.rs index 2258f8d..7dbad5c 100644 --- a/crates/dpp-identity/src/middleware/mtls.rs +++ b/crates/dpp-identity/src/middleware/mtls.rs @@ -52,6 +52,46 @@ fn required_issuer_cn() -> String { std::env::var("MTLS_REQUIRED_ISSUER_CN").unwrap_or_else(|_| "Odal Internal CA".to_owned()) } +/// Header the terminating proxy sets to prove that *it* — not a client that +/// reached this listener directly — is the origin of the forwarded cert headers. +pub const PROXY_AUTH_HEADER: &str = "X-Proxy-Auth"; + +/// Constant-time byte comparison, so a wrong secret can't be recovered by timing. +fn ct_eq(a: &[u8], b: &[u8]) -> bool { + if a.len() != b.len() { + return false; + } + let mut diff = 0u8; + for (x, y) in a.iter().zip(b) { + diff |= x ^ y; + } + diff == 0 +} + +/// Bind trust in the forwarded `X-Client-Cert-*` headers to the terminating +/// proxy. When `MTLS_PROXY_SHARED_SECRET` is configured, the request must carry +/// a matching [`PROXY_AUTH_HEADER`], so a caller that reaches this listener +/// directly (bypassing the proxy, e.g. a network misconfiguration) cannot forge +/// the cert headers. When the secret is unset, binding is disabled — logged as a +/// warning so the deployment gap is visible rather than silent. +fn proxy_binding_ok(request: &Request) -> bool { + let secret = match std::env::var("MTLS_PROXY_SHARED_SECRET") { + Ok(s) if !s.is_empty() => s, + _ => { + tracing::warn!( + "mTLS: MTLS_PROXY_SHARED_SECRET is not set — forwarded client-certificate \ + headers are trusted without proxy binding (set it in production)" + ); + return true; + } + }; + request + .headers() + .get(PROXY_AUTH_HEADER) + .and_then(|h| h.to_str().ok()) + .is_some_and(|presented| ct_eq(presented.as_bytes(), secret.as_bytes())) +} + /// Extract the value of the `CN=` component from an RFC 4514 subject DN string. /// /// Matches both `CN=foo` and `cn=foo` (case-insensitive key). @@ -82,6 +122,15 @@ pub async fn mtls_middleware(request: Request, next: Next) -> Response { .unwrap_or(false); let enforce = !allow_insecure; + // Before trusting any forwarded cert header, confirm the request actually + // came through the terminating proxy (when a binding secret is configured). + if enforce && !proxy_binding_ok(&request) { + tracing::warn!("mTLS: rejecting request — missing or invalid proxy binding secret"); + return Problem::new(StatusCode::UNAUTHORIZED, "Unauthorized") + .with_detail("Request did not arrive through the trusted terminating proxy.") + .into_response(); + } + match request.headers().get(CLIENT_CERT_SUBJECT_HEADER) { Some(subject) => { let subject_str = subject.to_str().unwrap_or(""); @@ -259,6 +308,52 @@ mod http_tests { assert_eq!(response.status(), StatusCode::FORBIDDEN); } + /// Proxy secret configured, but the request carries no `X-Proxy-Auth` — even + /// with otherwise-valid cert headers it is rejected (didn't come via proxy). + #[tokio::test] + #[serial] + async fn proxy_secret_configured_rejects_unbound_request() { + unsafe { std::env::set_var("MTLS_PROXY_SHARED_SECRET", "s3cr3t") }; + unsafe { std::env::set_var("MTLS_REQUIRED_ISSUER_CN", "Odal Internal CA") }; + let response = build_test_router() + .oneshot( + Request::builder() + .uri("/test") + .header(CLIENT_CERT_SUBJECT_HEADER, "CN=odal-vault, O=Odal") + .header(CLIENT_CERT_ISSUER_HEADER, "CN=Odal Internal CA, O=Odal") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + unsafe { std::env::remove_var("MTLS_PROXY_SHARED_SECRET") }; + unsafe { std::env::remove_var("MTLS_REQUIRED_ISSUER_CN") }; + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + } + + /// Proxy secret configured and the matching `X-Proxy-Auth` present → 200. + #[tokio::test] + #[serial] + async fn proxy_secret_configured_allows_bound_request() { + unsafe { std::env::set_var("MTLS_PROXY_SHARED_SECRET", "s3cr3t") }; + unsafe { std::env::set_var("MTLS_REQUIRED_ISSUER_CN", "Odal Internal CA") }; + let response = build_test_router() + .oneshot( + Request::builder() + .uri("/test") + .header(PROXY_AUTH_HEADER, "s3cr3t") + .header(CLIENT_CERT_SUBJECT_HEADER, "CN=odal-vault, O=Odal") + .header(CLIENT_CERT_ISSUER_HEADER, "CN=Odal Internal CA, O=Odal") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + unsafe { std::env::remove_var("MTLS_PROXY_SHARED_SECRET") }; + unsafe { std::env::remove_var("MTLS_REQUIRED_ISSUER_CN") }; + assert_eq!(response.status(), StatusCode::OK); + } + /// Correct CN and correct issuer → 200. #[tokio::test] #[serial] diff --git a/crates/dpp-integrator/src/domain/batch_runner.rs b/crates/dpp-integrator/src/domain/batch_runner.rs index 5dede92..38fdde9 100644 --- a/crates/dpp-integrator/src/domain/batch_runner.rs +++ b/crates/dpp-integrator/src/domain/batch_runner.rs @@ -116,13 +116,16 @@ pub async fn run_batch( .await .map(|_| RowOutcome::Updated(id)) } - _ => retry_create(&client, &req, &token).await.map(|body| { - let id = body - .get("id") - .and_then(|v| v.as_str()) - .unwrap_or("") - .to_owned(); - RowOutcome::Created(id) + _ => retry_create(&client, &req, &token).await.and_then(|body| { + match body.get("id").and_then(|v| v.as_str()) { + // A 2xx response must carry a non-empty passport id; + // recording a missing/empty id as a success would report + // an unusable empty id and overstate success_count. + Some(id) if !id.is_empty() => Ok(RowOutcome::Created(id.to_owned())), + _ => Err(VaultClientError::Parse( + "vault returned success without a passport id".into(), + )), + } }), }; (row_num, outcome) diff --git a/crates/dpp-integrator/src/domain/fields.rs b/crates/dpp-integrator/src/domain/fields.rs index 8a98895..503f614 100644 --- a/crates/dpp-integrator/src/domain/fields.rs +++ b/crates/dpp-integrator/src/domain/fields.rs @@ -2,10 +2,31 @@ use std::collections::HashMap; +use dpp_domain::domain::gtin::Gtin; use dpp_domain::domain::passport::MaterialEntry; use super::request::RowError; +/// Push a `RowError` if `gtin` (when present) is not a structurally valid GS1 +/// GTIN-14 (14 digits + mod-10 check digit). Shared by the sector importers so +/// steel/aluminium/tyre validate the checksum the same way the battery importer +/// already does — a bad checksum must not pass through the pipeline unchecked. +pub(super) fn validate_gtin_checksum( + gtin: Option<&str>, + row_num: usize, + errors: &mut Vec, +) { + if let Some(g) = gtin + && let Err(e) = Gtin::parse(g) + { + errors.push(RowError { + row: row_num, + field: "gtin".into(), + message: e.to_string(), + }); + } +} + /// Normalize a header key for case/separator-insensitive matching: drop /// non-alphanumerics (`_`, `-`, spaces) and lowercase. So `manufacturerName`, /// `manufacturer_name`, and `Manufacturer Name` all map to `manufacturername`. diff --git a/crates/dpp-integrator/src/domain/validate/aluminium.rs b/crates/dpp-integrator/src/domain/validate/aluminium.rs index f67b3b3..d5cf9bd 100644 --- a/crates/dpp-integrator/src/domain/validate/aluminium.rs +++ b/crates/dpp-integrator/src/domain/validate/aluminium.rs @@ -7,7 +7,9 @@ use dpp_domain::domain::{ sector::{AluminiumData, ProductionRoute, Sector, SectorData}, }; -use crate::domain::fields::{optional_f64, optional_str, require_f64, require_str}; +use crate::domain::fields::{ + optional_f64, optional_str, require_f64, require_str, validate_gtin_checksum, +}; use crate::domain::request::{CreatePassportRequest, RowError}; /// Validate a single aluminium row and convert it to a vault `CreatePassportRequest`. @@ -27,6 +29,7 @@ pub fn validate_aluminium_row( let manufacturer_name = require_str(row, "manufacturerName", row_num, &mut errors); let manufacturer_country = require_str(row, "manufacturerCountry", row_num, &mut errors); let gtin = require_str(row, "gtin", row_num, &mut errors); + validate_gtin_checksum(gtin.as_deref(), row_num, &mut errors); let alloy_grade = require_str(row, "alloyGrade", row_num, &mut errors); let production_route_raw = require_str(row, "productionRoute", row_num, &mut errors); let co2e = require_f64(row, "co2ePerTonneKg", row_num, &mut errors); diff --git a/crates/dpp-integrator/src/domain/validate/steel.rs b/crates/dpp-integrator/src/domain/validate/steel.rs index c122086..878f260 100644 --- a/crates/dpp-integrator/src/domain/validate/steel.rs +++ b/crates/dpp-integrator/src/domain/validate/steel.rs @@ -7,7 +7,9 @@ use dpp_domain::domain::{ sector::{ProductionRoute, Sector, SectorData, SteelData}, }; -use crate::domain::fields::{optional_f64, optional_str, require_f64, require_str}; +use crate::domain::fields::{ + optional_f64, optional_str, require_f64, require_str, validate_gtin_checksum, +}; use crate::domain::request::{CreatePassportRequest, RowError}; /// Validate a single steel row and convert it to a vault `CreatePassportRequest`. @@ -27,6 +29,7 @@ pub fn validate_steel_row( let manufacturer_name = require_str(row, "manufacturerName", row_num, &mut errors); let manufacturer_country = require_str(row, "manufacturerCountry", row_num, &mut errors); let gtin = require_str(row, "gtin", row_num, &mut errors); + validate_gtin_checksum(gtin.as_deref(), row_num, &mut errors); let co2e = require_f64(row, "co2ePerTonneSteel", row_num, &mut errors); let recycled = require_f64(row, "recycledScrapContentPct", row_num, &mut errors); let product_category = require_str(row, "productCategory", row_num, &mut errors); @@ -119,4 +122,14 @@ mod tests { let errs = validate_steel_row(&row, 2).expect_err("should fail"); assert!(errs.iter().any(|e| e.field == "co2ePerTonneSteel")); } + + #[test] + fn steel_row_bad_gtin_checksum_returns_error() { + // Right length/digits but a wrong GS1 mod-10 check digit — this must be + // caught (matching the battery importer), not passed through. + let mut row = steel_row(); + row.insert("gtin".into(), "09506000134353".into()); // valid is ...352 + let errs = validate_steel_row(&row, 3).expect_err("bad GTIN checksum must fail"); + assert!(errs.iter().any(|e| e.field == "gtin")); + } } diff --git a/crates/dpp-integrator/src/domain/validate/tyre.rs b/crates/dpp-integrator/src/domain/validate/tyre.rs index 3022601..1aca276 100644 --- a/crates/dpp-integrator/src/domain/validate/tyre.rs +++ b/crates/dpp-integrator/src/domain/validate/tyre.rs @@ -7,7 +7,9 @@ use dpp_domain::domain::{ sector::{Sector, SectorData, TyreData}, }; -use crate::domain::fields::{optional_f64, optional_str, require_f64, require_str}; +use crate::domain::fields::{ + optional_f64, optional_str, require_f64, require_str, validate_gtin_checksum, +}; use crate::domain::request::{CreatePassportRequest, RowError}; /// Validate a single tyre row and convert it to a vault `CreatePassportRequest`. @@ -28,6 +30,7 @@ pub fn validate_tyre_row( let manufacturer_name = require_str(row, "manufacturerName", row_num, &mut errors); let manufacturer_country = require_str(row, "manufacturerCountry", row_num, &mut errors); let gtin = require_str(row, "gtin", row_num, &mut errors); + validate_gtin_checksum(gtin.as_deref(), row_num, &mut errors); let tyre_class = require_str(row, "tyreClass", row_num, &mut errors); let fuel_class = require_str(row, "fuelEfficiencyClass", row_num, &mut errors); let wet_class = require_str(row, "wetGripClass", row_num, &mut errors); diff --git a/crates/dpp-node/src/infra/registry/mapping.rs b/crates/dpp-node/src/infra/registry/mapping.rs index 4f073ca..86cff6b 100644 --- a/crates/dpp-node/src/infra/registry/mapping.rs +++ b/crates/dpp-node/src/infra/registry/mapping.rs @@ -142,8 +142,12 @@ impl RegistrySyncPort for EuRegistrySync { operator_id: OperatorIdentifier { scheme: "did".into(), value: request.operator_identifier.clone(), + // Wire the operator country that the request already carries + // (sourced from OperatorConfig) instead of dropping it. The + // operator legal `name` is not yet threaded through the port — + // see the payload-validation note below. name: String::new(), - country: String::new(), + country: request.country_code.clone(), did: Some(request.operator_identifier.clone()), }, sector: request.product_category.clone(), diff --git a/crates/dpp-node/src/infra/registry/token.rs b/crates/dpp-node/src/infra/registry/token.rs index 84a9fd3..86a3140 100644 --- a/crates/dpp-node/src/infra/registry/token.rs +++ b/crates/dpp-node/src/infra/registry/token.rs @@ -20,7 +20,34 @@ pub(super) struct CachedToken { impl CachedToken { pub(super) fn is_expired(&self) -> bool { - // Refresh 30 seconds before actual expiry to avoid edge-case failures. - Instant::now() >= self.expires_at - Duration::from_secs(30) + // Refresh 30 seconds before actual expiry. Compare additively + // (`now + 30s >= expires_at`) rather than subtracting from an `Instant`, + // which panics on underflow when the token expires in under ~30s (a fresh + // restart racing a short-lived token refresh). + Instant::now() + Duration::from_secs(30) >= self.expires_at + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn near_immediate_expiry_reports_expired_without_panicking() { + // Expiry within the 30s refresh window must not underflow-panic. + let t = CachedToken { + access_token: "x".into(), + expires_at: Instant::now(), + }; + assert!(t.is_expired()); + } + + #[test] + fn far_future_token_is_not_expired() { + let t = CachedToken { + access_token: "x".into(), + expires_at: Instant::now() + Duration::from_secs(3600), + }; + assert!(!t.is_expired()); } } diff --git a/crates/dpp-node/src/infra/ruleset.rs b/crates/dpp-node/src/infra/ruleset.rs index cd1a5d0..7d0f9e1 100644 --- a/crates/dpp-node/src/infra/ruleset.rs +++ b/crates/dpp-node/src/infra/ruleset.rs @@ -55,7 +55,7 @@ pub fn sign_bundle( effective_date, act_citations, schema_versions, - content_sha256: content_hash(&content), + content_sha256: content_hash(&content)?, }; let manifest_value = serde_json::to_value(&manifest)?; let manifest_jws = jws::sign(store, key_id, &manifest_value)?; @@ -91,7 +91,8 @@ impl ActiveRuleset { effective_date: Utc::now(), act_citations: vec![], schema_versions: BTreeMap::new(), - content_sha256: content_hash(&content), + content_sha256: content_hash(&content) + .expect("baseline content is a static empty object; hashing cannot fail"), }; Self { current: RwLock::new(Arc::new(VerifiedRuleset { manifest, content })), diff --git a/crates/dpp-node/src/main.rs b/crates/dpp-node/src/main.rs index ebf26f5..9ebb55c 100644 --- a/crates/dpp-node/src/main.rs +++ b/crates/dpp-node/src/main.rs @@ -195,14 +195,23 @@ async fn main() -> anyhow::Result<()> { tracing::info!(version = %active_ruleset.version(), "active ruleset"); // ── Vault service state ──────────────────────────────────────────────────── - let operator_country = db + let operator_country = match db .operator_repo .get(dpp_types::STANDALONE_OPERATOR_ID) .await - .ok() - .flatten() - .map(|c| c.country) - .unwrap_or_default(); + { + Ok(cfg) => cfg.map(|c| c.country).unwrap_or_default(), + Err(e) => { + // Don't silently bake an empty country into every registry payload + // for the life of the process — a transient DB hiccup at boot must + // be visible, like every other fallible boot step in this file. + tracing::warn!( + error = %e, + "could not read operator config at boot — operator country left empty this run" + ); + String::new() + } + }; let compliance: Arc = plugin_host.clone(); // Clone the registry-sync port for the outbox drain task before it is moved diff --git a/crates/dpp-plugin-host/src/host.rs b/crates/dpp-plugin-host/src/host.rs index 7cc69e4..86a5d82 100644 --- a/crates/dpp-plugin-host/src/host.rs +++ b/crates/dpp-plugin-host/src/host.rs @@ -70,7 +70,7 @@ impl WasmPluginHost { key, ); - let payload = plugin + let mut payload = plugin .invoke_generate_passport(&input) .map_err(|e| ComplianceError { kind: ComplianceErrorKind::Internal, @@ -93,6 +93,17 @@ impl WasmPluginHost { }); } + // Host-side backstop mirroring compute()'s determination gate: a + // provisional (not-in-force) sector can never surface a binding + // compliance claim, even if the plugin ignores the advisory __isInForce + // flag and injects one into its generated output. + if !catalog().is_in_force(key) + && let Some(obj) = payload.as_object_mut() + { + obj.remove("complianceStatus"); + obj.remove("complianceResult"); + } + Ok(payload) } } @@ -136,11 +147,12 @@ impl PluginHost for WasmPluginHost { Ok(r) => r, Err(e) => { let msg = e.to_string(); - // Fuel exhaustion is a distinct sandbox event — track it separately - // so the alert rule "any increment = sandbox limit actually firing" - // is unambiguous. - if msg.to_lowercase().contains("fuel") || msg.to_lowercase().contains("out of fuel") - { + // Classify sandbox events from HOST-controlled error prefixes, + // never from plugin-controlled text. A fuel trap is remapped to a + // "fuel exhausted" prefix and the memory cap bail to "memory cap + // exceeded"; a plugin's own error surfaces as "plugin reported an + // error: …", so it cannot spoof either metric/alert. + if msg.starts_with("fuel exhausted") { metrics::counter!( "plugin_fuel_exhausted_total", "sector" => key.to_owned() @@ -152,9 +164,7 @@ impl PluginHost for WasmPluginHost { "Wasm plugin exhausted fuel budget" ); } - // Memory cap is signalled by ResourceLimiter::memory_growing returning Err - // with a message containing "memory cap" (see runtime.rs). - if msg.contains("memory cap") { + if msg.starts_with("memory cap exceeded") { metrics::counter!( "plugin_mem_capped_total", "sector" => key.to_owned() diff --git a/crates/dpp-plugin-host/src/loader/plugin.rs b/crates/dpp-plugin-host/src/loader/plugin.rs index d166f5e..978da42 100644 --- a/crates/dpp-plugin-host/src/loader/plugin.rs +++ b/crates/dpp-plugin-host/src/loader/plugin.rs @@ -18,6 +18,26 @@ use crate::runtime::{DEFAULT_FUEL, DEFAULT_MEMORY_CAP_BYTES, HostState, build_st /// Maximum bytes accepted from any plugin ABI output (4 MiB). const MAX_ABI_OUTPUT_BYTES: usize = 4 * 1024 * 1024; +/// Whether unsigned plugin loading is explicitly opted into, from the +/// `DPP_ALLOW_UNSIGNED_PLUGINS` value. Pure (takes the value) so it is testable +/// without mutating the process-global environment. +fn unsigned_allowed(env_value: Option<&str>) -> bool { + matches!(env_value, Some(v) if v.eq_ignore_ascii_case("true")) +} + +/// Map a wasmtime call error, giving a fuel-exhaustion trap a distinct, +/// host-controlled `"fuel exhausted"` prefix. Sandbox metrics are then classified +/// from this prefix (and the host's `"memory cap exceeded"` bail), never by +/// string-matching plugin-controlled error text — a plugin's structured error +/// surfaces as `"plugin reported an error: …"` and so cannot spoof either signal. +fn map_call_error(e: wasmtime::Error) -> anyhow::Error { + if e.downcast_ref::() == Some(&wasmtime::Trap::OutOfFuel) { + anyhow::anyhow!("fuel exhausted: plugin exceeded its fuel budget") + } else { + anyhow::anyhow!("{e}") + } +} + /// A compiled, in-memory sector plugin ready to be instantiated per-request. pub struct LoadedPlugin { engine: Engine, @@ -57,9 +77,23 @@ impl LoadedPlugin { return Err(e); } } else { + // No trusted key configured. Unsigned loading is a development-only + // convenience and must be explicitly opted into — otherwise a + // misconfigured production deploy would silently run unverified + // plugins. Fail closed unless `DPP_ALLOW_UNSIGNED_PLUGINS=true`. + let allow_unsigned = + unsigned_allowed(std::env::var("DPP_ALLOW_UNSIGNED_PLUGINS").ok().as_deref()); + if !allow_unsigned { + return Err(anyhow::anyhow!( + "refusing to load unsigned plugin {}: no trusted key is configured and \ + DPP_ALLOW_UNSIGNED_PLUGINS is not set (production must provide a key)", + path.display() + )); + } tracing::warn!( path = %path.display(), - "loading Wasm plugin WITHOUT signature verification — not safe for production" + "loading Wasm plugin WITHOUT signature verification \ + (DPP_ALLOW_UNSIGNED_PLUGINS=true) — not safe for production" ); } @@ -277,7 +311,7 @@ fn call_generate_passport( let packed = generate .call(&mut *store, (input_ptr, input_len)) - .map_err(w)?; + .map_err(map_call_error)?; let out_ptr = (packed >> 32) as usize; let out_len = (packed & 0xFFFF_FFFF) as usize; @@ -335,7 +369,7 @@ fn call_calculate( // Returns a packed (ptr << 32 | len) u64 let packed = calculate .call(&mut *store, (input_ptr, input_len)) - .map_err(w)?; + .map_err(map_call_error)?; let out_ptr = (packed >> 32) as usize; let out_len = (packed & 0xFFFF_FFFF) as usize; @@ -450,6 +484,26 @@ mod tests { build_plugin_wasm(DESCRIBE_JSON, Some(CALC_OK_JSON), None, Some(GEN_OK_JSON)) } + /// Load a plugin without a trusted key — the development path. Opts into + /// unsigned loading explicitly, mirroring what a real dev/CI deploy must do. + fn load_unsigned(engine: &Engine, path: &Path, sector: &str) -> Result { + // Safe: every caller sets the same value, so concurrent sets are benign. + unsafe { std::env::set_var("DPP_ALLOW_UNSIGNED_PLUGINS", "true") }; + LoadedPlugin::from_file(engine, path, sector, None) + } + + #[test] + fn unsigned_allowed_only_when_explicitly_opted_in() { + // The refusal decision is a pure function of the env value, so it can be + // asserted deterministically without racing the process-global env. + assert!(super::unsigned_allowed(Some("true"))); + assert!(super::unsigned_allowed(Some("TRUE"))); + assert!(!super::unsigned_allowed(Some("false"))); + assert!(!super::unsigned_allowed(Some("1"))); + assert!(!super::unsigned_allowed(Some(""))); + assert!(!super::unsigned_allowed(None)); + } + #[test] fn from_file_refuses_when_signature_verification_fails() { let engine = build_engine().unwrap(); @@ -476,7 +530,7 @@ mod tests { let wasm = wat::parse_str("(module)").unwrap(); let path = write_plugin_file(&dir, &wasm); - let plugin = LoadedPlugin::from_file(&engine, &path, "battery", None) + let plugin = load_unsigned(&engine, &path, "battery") .expect("dev-mode load without a key must succeed even without describe()"); assert!(plugin.capabilities.supported_schemas.is_empty()); assert!(plugin.capabilities.capabilities.is_empty()); @@ -490,7 +544,7 @@ mod tests { let wasm = build_plugin_wasm(DESCRIBE_JSON, Some(CALC_OK_JSON), None, None); let path = write_plugin_file(&dir, &wasm); - let plugin = LoadedPlugin::from_file(&engine, &path, "battery", None).unwrap(); + let plugin = load_unsigned(&engine, &path, "battery").unwrap(); assert_eq!(plugin.capabilities.abi_version.major, 1); assert_eq!( @@ -506,7 +560,7 @@ mod tests { let engine = build_engine().unwrap(); let dir = TempDir::new().unwrap(); let path = write_plugin_file(&dir, &full_plugin_wasm()); - let plugin = LoadedPlugin::from_file(&engine, &path, "battery", None).unwrap(); + let plugin = load_unsigned(&engine, &path, "battery").unwrap(); let result = plugin .invoke_calculate(&serde_json::json!({"gtin": "irrelevant — fixture ignores input"})) @@ -521,7 +575,7 @@ mod tests { let engine = build_engine().unwrap(); let dir = TempDir::new().unwrap(); let path = write_plugin_file(&dir, &full_plugin_wasm()); - let plugin = LoadedPlugin::from_file(&engine, &path, "battery", None).unwrap(); + let plugin = load_unsigned(&engine, &path, "battery").unwrap(); let payload = plugin .invoke_generate_passport(&serde_json::json!({})) @@ -536,7 +590,7 @@ mod tests { let dir = TempDir::new().unwrap(); let wasm = build_plugin_wasm(DESCRIBE_JSON, Some(CALC_ERR_JSON), None, None); let path = write_plugin_file(&dir, &wasm); - let plugin = LoadedPlugin::from_file(&engine, &path, "battery", None).unwrap(); + let plugin = load_unsigned(&engine, &path, "battery").unwrap(); let err = plugin .invoke_calculate(&serde_json::json!({})) @@ -550,7 +604,7 @@ mod tests { let dir = TempDir::new().unwrap(); let wasm = build_plugin_wasm(DESCRIBE_JSON, Some("not json at all"), None, None); let path = write_plugin_file(&dir, &wasm); - let plugin = LoadedPlugin::from_file(&engine, &path, "battery", None).unwrap(); + let plugin = load_unsigned(&engine, &path, "battery").unwrap(); let err = plugin .invoke_calculate(&serde_json::json!({})) @@ -567,7 +621,7 @@ mod tests { let oversized_len = (MAX_ABI_OUTPUT_BYTES + 1) as u32; let wasm = build_plugin_wasm(DESCRIBE_JSON, Some(CALC_OK_JSON), Some(oversized_len), None); let path = write_plugin_file(&dir, &wasm); - let plugin = LoadedPlugin::from_file(&engine, &path, "battery", None).unwrap(); + let plugin = load_unsigned(&engine, &path, "battery").unwrap(); let err = plugin .invoke_calculate(&serde_json::json!({})) @@ -583,7 +637,7 @@ mod tests { // never exported — a partial/malformed plugin must fail predictably, not panic. let wasm = build_plugin_wasm(DESCRIBE_JSON, None, None, Some(GEN_OK_JSON)); let path = write_plugin_file(&dir, &wasm); - let plugin = LoadedPlugin::from_file(&engine, &path, "battery", None).unwrap(); + let plugin = load_unsigned(&engine, &path, "battery").unwrap(); let err = plugin .invoke_calculate(&serde_json::json!({})) diff --git a/crates/dpp-plugin-host/src/runtime.rs b/crates/dpp-plugin-host/src/runtime.rs index e8097ab..52a310f 100644 --- a/crates/dpp-plugin-host/src/runtime.rs +++ b/crates/dpp-plugin-host/src/runtime.rs @@ -63,13 +63,23 @@ impl ResourceLimiter for HostState { fn table_growing( &mut self, _current: usize, - _desired: usize, + desired: usize, _maximum: Option, ) -> Result { - Ok(true) + // Meter table growth like linear memory. Otherwise a single `table.grow` + // with a huge element count forces a large host-side allocation that + // fuel counts as only one instruction — a resource-limit bypass distinct + // from the (capped) linear-memory path. Ok(false) makes `table.grow` + // return -1 (the Wasm denial signal). Sector plugins use tiny indirect + // tables, so this ceiling is generous. + Ok(desired <= MAX_TABLE_ELEMENTS) } } +/// Maximum number of elements a plugin's tables may grow to. A funcref/externref +/// element is a few host bytes, so this caps table growth well under a MiB. +const MAX_TABLE_ELEMENTS: usize = 100_000; + /// Create a sandboxed `Store` with WASI disabled for filesystem and network. /// /// `fuel` and `memory_cap` override the defaults; the host always clamps them diff --git a/crates/dpp-plugin-host/tests/integration.rs b/crates/dpp-plugin-host/tests/integration.rs index aebbff5..9285e40 100644 --- a/crates/dpp-plugin-host/tests/integration.rs +++ b/crates/dpp-plugin-host/tests/integration.rs @@ -117,6 +117,23 @@ fn compile_wat_to_temp_file(wat_src: &str) -> tempfile::NamedTempFile { f } +/// Load a plugin the same way [`LoadedPlugin::from_file`] with `trusted_key: +/// None` used to, before unsigned loading required an explicit opt-in. These +/// tests load throwaway WAT fixtures, not real sector plugins, so unsigned +/// loading is the correct mode here — opt in for the duration of the call. +/// +/// SAFETY: mutates a process-global env var; sound because nextest runs each +/// `#[test]` in its own process (unlike `cargo test`, which would race this +/// across threads in the same binary). +fn load_unsigned_test_plugin( + engine: &wasmtime::Engine, + path: &std::path::Path, + sector_key: &str, +) -> anyhow::Result { + unsafe { std::env::set_var("DPP_ALLOW_UNSIGNED_PLUGINS", "true") }; + LoadedPlugin::from_file(engine, path, sector_key, None) +} + fn battery_sector_data() -> SectorData { // Minimal battery input — the passthrough plugin ignores it but the host // still serialises it and writes it to Wasm memory, exercising that path. @@ -184,7 +201,7 @@ fn load_wat_plugin_and_invoke_calculate() { let engine = build_engine().expect("build engine failed"); let tmp = compile_wat_to_temp_file(PASSTHROUGH_WAT); - let plugin = LoadedPlugin::from_file(&engine, tmp.path(), "battery", None) + let plugin = load_unsigned_test_plugin(&engine, tmp.path(), "battery") .expect("LoadedPlugin::from_file failed"); let input = serde_json::json!({"chemistry": "LFP", "capacityKwh": 10.0}); @@ -207,7 +224,7 @@ fn register_plugin_and_compute_via_host() { let engine = build_engine().expect("build engine"); let tmp = compile_wat_to_temp_file(PASSTHROUGH_WAT); - let plugin = LoadedPlugin::from_file(&engine, tmp.path(), "battery", None) + let plugin = load_unsigned_test_plugin(&engine, tmp.path(), "battery") .expect("LoadedPlugin::from_file failed"); let host = WasmPluginHost::new(); @@ -234,7 +251,7 @@ fn fuel_exhaustion_returns_error_not_panic() { let engine = build_engine().expect("build engine"); let tmp = compile_wat_to_temp_file(INFINITE_LOOP_WAT); - let plugin = LoadedPlugin::from_file(&engine, tmp.path(), "battery", None) + let plugin = load_unsigned_test_plugin(&engine, tmp.path(), "battery") .expect("LoadedPlugin::from_file failed"); let input = serde_json::json!({}); @@ -269,7 +286,7 @@ fn memory_cap_rejects_over_budget_allocation() { let engine = build_engine().expect("build engine"); let tmp = compile_wat_to_temp_file(MEMORY_OVERFLOW_WAT); - let plugin = LoadedPlugin::from_file(&engine, tmp.path(), "battery", None) + let plugin = load_unsigned_test_plugin(&engine, tmp.path(), "battery") .expect("LoadedPlugin::from_file failed"); let input = serde_json::json!({}); diff --git a/crates/dpp-resolver/src/domain/id.rs b/crates/dpp-resolver/src/domain/id.rs index f0d4e33..c0b3412 100644 --- a/crates/dpp-resolver/src/domain/id.rs +++ b/crates/dpp-resolver/src/domain/id.rs @@ -10,6 +10,17 @@ pub fn is_valid_dpp_id(id: &str) -> bool { uuid::Uuid::parse_str(id).is_ok() } +/// Whether `gtin` is a syntactically valid GTIN for resolution: 8–14 ASCII +/// digits and nothing else. +/// +/// Validated at the resolver edge for the same reason as [`is_valid_dpp_id`]: +/// a value like `../admin` (from a percent-encoded `/01/{gtin}` path segment, +/// which Axum decodes after routing) must never reach the server-to-server +/// vault URL, closing a path-traversal / SSRF surface. +pub fn is_valid_gtin(gtin: &str) -> bool { + (8..=14).contains(>in.len()) && gtin.bytes().all(|b| b.is_ascii_digit()) +} + #[cfg(test)] mod tests { use super::is_valid_dpp_id; @@ -34,4 +45,22 @@ mod tests { assert!(!is_valid_dpp_id(bad), "should reject: {bad}"); } } + + #[test] + fn gtin_accepts_digits_rejects_traversal() { + use super::is_valid_gtin; + assert!(is_valid_gtin("09506000134352")); // GTIN-14 + assert!(is_valid_gtin("12345678")); // GTIN-8 + for bad in [ + "../admin", + "%2E%2E", + "0950/6000", + "abc", + "", + "1234567", + "123456789012345", + ] { + assert!(!is_valid_gtin(bad), "should reject: {bad}"); + } + } } diff --git a/crates/dpp-resolver/src/domain/mod.rs b/crates/dpp-resolver/src/domain/mod.rs index 2196c11..ea3c9b6 100644 --- a/crates/dpp-resolver/src/domain/mod.rs +++ b/crates/dpp-resolver/src/domain/mod.rs @@ -5,4 +5,4 @@ mod id; -pub use id::is_valid_dpp_id; +pub use id::{is_valid_dpp_id, is_valid_gtin}; diff --git a/crates/dpp-resolver/src/handlers/resolve_by_gtin.rs b/crates/dpp-resolver/src/handlers/resolve_by_gtin.rs index b7cf54e..9ed7f39 100644 --- a/crates/dpp-resolver/src/handlers/resolve_by_gtin.rs +++ b/crates/dpp-resolver/src/handlers/resolve_by_gtin.rs @@ -11,6 +11,11 @@ use serde_json::Value; use crate::state::AppState; +/// The resolver's own public base URL (its GS1 Digital Link host). Hardcoded +/// rather than derived from mutable passport data, matching the QR-code handler +/// — this is what closes the open-redirect surface. +const RESOLVER_BASE_URL: &str = "https://id.odal-node.io"; + /// Query parameters for the GS1 Digital Link resolver endpoint (`/01/{gtin}`). #[derive(Deserialize)] pub struct ByGtinQuery { @@ -33,6 +38,17 @@ pub async fn resolve_by_gtin_handler( Query(query): Query, headers: HeaderMap, ) -> impl IntoResponse { + // Validate the GTIN at the edge before it reaches the server-to-server vault + // URL — a percent-decoded `../admin` must not path-traverse/SSRF the vault. + if !crate::domain::is_valid_gtin(>in) { + return ( + StatusCode::NOT_FOUND, + [(header::CONTENT_TYPE, "application/json")], + r#"{"error":"NOT_FOUND","message":"No published DPP for this GTIN"}"#.to_owned(), + ) + .into_response(); + } + let passport = match fetch_by_gtin(&state, >in).await { Ok(v) => v, Err(status) => { @@ -52,13 +68,12 @@ pub async fn resolve_by_gtin_handler( } }; - // Derive the resolver's public base URL from qrCodeUrl stored in the passport. - // Battery GS1 DL URL: https://id.odal-node.io/01/{gtin}/21/{uuid} - let base_url = passport - .get("qrCodeUrl") - .and_then(Value::as_str) - .and_then(resolver_base) - .unwrap_or_else(|| "https://id.odal-node.io".to_owned()); + // The resolver's own trusted base URL. It is NOT derived from the passport's + // `qrCodeUrl`: that field is mutable and content-binding-exempt (tamperable), + // so trusting it would let an altered value 307-redirect the scan to an + // attacker host — the exact open redirect the QR-code handler hardcodes + // against. + let base_url = RESOLVER_BASE_URL; // Linkset request: ?linkType=linkset OR Accept: application/linkset+json let wants_linkset = query.link_type.as_deref() == Some("linkset") @@ -69,7 +84,7 @@ pub async fn resolve_by_gtin_handler( .unwrap_or(false); if wants_linkset { - let body = serde_json::to_string(&build_linkset(&base_url, >in, &passport_id, &passport)) + let body = serde_json::to_string(&build_linkset(base_url, >in, &passport_id, &passport)) .unwrap_or_default(); return ( StatusCode::OK, @@ -115,16 +130,6 @@ fn redirect(location: &str) -> axum::response::Response { .into_response() } -/// Extract `https://host` from a GS1 DL URL like -/// `https://id.odal-node.io/01/{gtin}/21/{uuid}`. -fn resolver_base(qr_code_url: &str) -> Option { - let without_scheme = qr_code_url - .strip_prefix("https://") - .or_else(|| qr_code_url.strip_prefix("http://"))?; - let host = without_scheme.split('/').next()?; - Some(format!("https://{host}")) -} - /// Build a GS1 linkset per RFC 9264 / GS1 Digital Link standard. When the /// passport cites a predecessor (second-life successor linkage), the linkset /// also advertises an Odal `predecessor` relation pointing at the source @@ -211,15 +216,6 @@ async fn fetch_by_gtin(state: &AppState, gtin: &str) -> Result SealCapabilities { - GhostSeal.capabilities() + if self.qtsp_url.is_none() { + // Ghost-backed placeholder path — report exactly what `GhostSeal` + // actually does (its seal()/verify() succeed with synthetic values). + GhostSeal.capabilities() + } else { + // Configured, but the real CSC flow isn't implemented — seal() and + // verify() both return errors. Report no capability rather than + // advertising JAdES sealing the adapter cannot deliver, so a caller + // that checks capabilities() first isn't contradicted by seal(). + SealCapabilities { + supported_formats: Vec::new(), + supported_modes: Vec::new(), + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unconfigured_reports_ghost_capabilities() { + // The placeholder path genuinely produces (synthetic) seals. + let caps = QtspSealAdapter::new(None).capabilities(); + assert!(!caps.supported_formats.is_empty()); + } + + #[test] + fn configured_but_unimplemented_reports_no_capability() { + // capabilities() must not contradict seal()/verify(), which error here. + let caps = QtspSealAdapter::new(Some("https://qtsp.example".into())).capabilities(); + assert!( + caps.supported_formats.is_empty(), + "must not advertise sealing it cannot deliver" + ); + assert!(caps.supported_modes.is_empty()); } } diff --git a/crates/dpp-types/src/api_key.rs b/crates/dpp-types/src/api_key.rs index 9501d25..9879949 100644 --- a/crates/dpp-types/src/api_key.rs +++ b/crates/dpp-types/src/api_key.rs @@ -17,8 +17,11 @@ use uuid::Uuid; /// NOT key management or operator-config mutation. /// - `Admin` — everything, including `/api-keys` and operator-config `PATCH`. /// -/// The default is `Admin` for backward compatibility (pre-scope keys and the -/// bootstrap key remain fully capable). Issue `Write`/`Read` keys to integrations. +/// The `#[default]` is `Admin`: creating a key without specifying a scope (the +/// bootstrap key, or an omitted `scope` in a create request) is fully capable. +/// A NULL/empty `scopes` *column*, by contrast, fails closed to `Read` (see +/// [`ApiKeyScope::from_scopes`]) — only an explicit `admin` grants admin. +/// Issue `Write`/`Read` keys to integrations. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum ApiKeyScope { @@ -54,20 +57,20 @@ impl ApiKeyScope { /// Collapse the DB `scopes TEXT[]` array into the effective (highest) scope. /// - /// A NULL/empty array — i.e. a key written before scopes were enforced — - /// maps to `Admin`, preserving the pre-scope "full access" behaviour. The - /// service only ever writes a single-element array, but reading the highest - /// privilege present keeps this robust to manual edits. + /// **Fails closed:** an empty/NULL array or any unrecognized scope string + /// maps to the least-privilege `Read`, never `Admin`. The service always + /// writes an explicit single-element scope, so a NULL/empty column only + /// arises from a legacy pre-scope row or a manual edit — neither of which + /// should silently grant admin. Reading the highest recognized privilege + /// present keeps this robust to manual multi-element edits. #[must_use] pub fn from_scopes(scopes: &[String]) -> Self { - if scopes.is_empty() || scopes.iter().any(|s| s == "admin") { + if scopes.iter().any(|s| s == "admin") { ApiKeyScope::Admin } else if scopes.iter().any(|s| s == "write") { ApiKeyScope::Write - } else if scopes.iter().any(|s| s == "read") { - ApiKeyScope::Read } else { - ApiKeyScope::Admin + ApiKeyScope::Read } } } @@ -154,3 +157,51 @@ pub trait ApiKeyRepository: Send + Sync { /// Revoke a key by id. Returns `true` if the key existed and was revoked. async fn revoke(&self, id: Uuid) -> Result; } + +#[cfg(test)] +mod tests { + use super::*; + + fn scopes(vals: &[&str]) -> Vec { + vals.iter().map(|s| (*s).to_owned()).collect() + } + + #[test] + fn from_scopes_fails_closed_to_read() { + // Empty/NULL, unrecognized, and wrong-case values must all fail closed. + assert_eq!(ApiKeyScope::from_scopes(&[]), ApiKeyScope::Read); + assert_eq!( + ApiKeyScope::from_scopes(&scopes(&["superuser"])), + ApiKeyScope::Read + ); + assert_eq!( + ApiKeyScope::from_scopes(&scopes(&["ADMIN"])), // not the literal "admin" + ApiKeyScope::Read + ); + } + + #[test] + fn from_scopes_recognizes_known_values_highest_wins() { + assert_eq!( + ApiKeyScope::from_scopes(&scopes(&["read"])), + ApiKeyScope::Read + ); + assert_eq!( + ApiKeyScope::from_scopes(&scopes(&["write"])), + ApiKeyScope::Write + ); + assert_eq!( + ApiKeyScope::from_scopes(&scopes(&["admin"])), + ApiKeyScope::Admin + ); + // Highest recognized privilege present wins. + assert_eq!( + ApiKeyScope::from_scopes(&scopes(&["read", "admin"])), + ApiKeyScope::Admin + ); + assert_eq!( + ApiKeyScope::from_scopes(&scopes(&["read", "write"])), + ApiKeyScope::Write + ); + } +} diff --git a/crates/dpp-types/src/operator.rs b/crates/dpp-types/src/operator.rs index 39f3dd8..a3ecd30 100644 --- a/crates/dpp-types/src/operator.rs +++ b/crates/dpp-types/src/operator.rs @@ -65,8 +65,12 @@ fn default_data_residency() -> String { "EU".to_owned() } +/// ESPR-driven minimum data-retention floor (days, ~10 years). A configured +/// retention shorter than this would violate the minimum-retention guarantee. +pub const MIN_RETENTION_DAYS: i64 = 3650; + fn default_retention_days() -> i64 { - 3650 + MIN_RETENTION_DAYS } impl OperatorConfig { @@ -146,6 +150,24 @@ pub struct UpdateOperatorConfig { } impl UpdateOperatorConfig { + /// Validate the patch's invariants before it is applied. + /// + /// # Errors + /// Returns a message if `retention_policy_days` is present and below + /// [`MIN_RETENTION_DAYS`] (the ESPR minimum-retention floor) — the config + /// must never silently drop below the documented minimum-retention guarantee. + pub fn validate(&self) -> Result<(), String> { + if let Some(days) = self.retention_policy_days + && days < MIN_RETENTION_DAYS + { + return Err(format!( + "retentionPolicyDays must be at least {MIN_RETENTION_DAYS} \ + (ESPR minimum retention); got {days}" + )); + } + Ok(()) + } + /// Apply all `Some` fields from `self` onto `cfg` in-place. pub fn apply(&self, cfg: &mut OperatorConfig) { if let Some(ref v) = self.legal_name { @@ -261,4 +283,45 @@ mod tests { assert!(!cfg.is_complete()); assert_eq!(cfg.missing_fields(), vec!["address"]); } + + fn patch_with_retention(days: Option) -> UpdateOperatorConfig { + UpdateOperatorConfig { + legal_name: None, + trade_name: None, + address: None, + country: None, + contact_email: None, + did_web_url: None, + product_categories: None, + brand_primary: None, + brand_secondary: None, + brand_logo_url: None, + custom_domain: None, + data_residency: None, + retention_policy_days: days, + feature_flags: None, + } + } + + #[test] + fn validate_rejects_retention_below_minimum() { + assert!(patch_with_retention(Some(-1)).validate().is_err()); + assert!(patch_with_retention(Some(0)).validate().is_err()); + assert!( + patch_with_retention(Some(MIN_RETENTION_DAYS - 1)) + .validate() + .is_err() + ); + } + + #[test] + fn validate_accepts_retention_at_or_above_minimum_and_absent() { + assert!( + patch_with_retention(Some(MIN_RETENTION_DAYS)) + .validate() + .is_ok() + ); + assert!(patch_with_retention(Some(5475)).validate().is_ok()); // ~15y + assert!(patch_with_retention(None).validate().is_ok()); + } } diff --git a/crates/dpp-vault/src/domain/registry_identity_service.rs b/crates/dpp-vault/src/domain/registry_identity_service.rs index f6a82cb..9b97818 100644 --- a/crates/dpp-vault/src/domain/registry_identity_service.rs +++ b/crates/dpp-vault/src/domain/registry_identity_service.rs @@ -160,9 +160,10 @@ impl RegistryIdentityService { pub async fn add_operator_identifier( &self, req: CreateOperatorIdentifierRequest, + operator_country: &str, actor: &str, ) -> Result { - validate_operator_identifier(&req)?; + validate_operator_identifier(&req, operator_country)?; let identifier = OperatorIdentifier { id: Uuid::now_v7(), scheme: req.scheme.trim().to_lowercase(), @@ -299,7 +300,15 @@ fn validate_facility(req: &CreateFacilityRequest) -> Result<(), DppError> { /// Validate an operator identifier via the `dpp-registry` scheme check /// (LEI ISO 7064, DUNS length, EORI/VAT prefix), plus non-empty fields. -fn validate_operator_identifier(req: &CreateOperatorIdentifierRequest) -> Result<(), DppError> { +/// +/// `operator_country` is the operator's own registered country (`OperatorConfig`): +/// an Art. 13 identifier belongs to the operator, not a location, so it has no +/// per-entry country of its own — the operator's country is reused for the +/// `dpp-registry` validation, which requires a non-empty ISO code. +fn validate_operator_identifier( + req: &CreateOperatorIdentifierRequest, + operator_country: &str, +) -> Result<(), DppError> { if req.scheme.trim().is_empty() || req.value.trim().is_empty() { return Err(DppError::Validation("scheme and value are required".into())); } @@ -310,9 +319,7 @@ fn validate_operator_identifier(req: &CreateOperatorIdentifierRequest) -> Result .label .clone() .unwrap_or_else(|| req.value.trim().to_owned()), - // Per-identifier country is not modelled; empty skips the country check, - // leaving the scheme/value structural validation (the part that matters here). - country: String::new(), + country: operator_country.trim().to_uppercase(), did: None, }; oid.validate() diff --git a/crates/dpp-vault/src/domain/verify/jws.rs b/crates/dpp-vault/src/domain/verify/jws.rs index 41c3f7a..37851d4 100644 --- a/crates/dpp-vault/src/domain/verify/jws.rs +++ b/crates/dpp-vault/src/domain/verify/jws.rs @@ -11,16 +11,19 @@ use dpp_crypto::jws::{ verify_jws, }; -/// Resolve the public key to verify `jws` against, from a DID document: try -/// the `kid`-fingerprint match first (supports rotation-archived keys), then -/// fall back to the primary key for JWS tokens signed before `kid` was added. +/// Resolve the public key to verify `jws` against, from a DID document. +/// +/// If the JWS carries a `kid`, it must resolve to a key by fingerprint (this +/// includes rotation-archived keys). A present-but-unresolvable `kid` (revoked, +/// rotated out, or simply wrong) returns `None` so the caller surfaces an +/// accurate "kid does not resolve" diagnosis — it does **not** silently +/// substitute the primary key. The primary-key fallback is reserved for legacy +/// tokens signed before `kid` was added, i.e. tokens carrying no `kid` at all. pub(crate) fn resolve_public_key(jws: &str, did_document: &serde_json::Value) -> Option { - if let Some(kid) = extract_kid_from_jws(jws) - && let Some(key) = extract_key_by_fingerprint(did_document, &kid) - { - return Some(key); + match extract_kid_from_jws(jws) { + Some(kid) => extract_key_by_fingerprint(did_document, &kid), + None => extract_primary_public_key(did_document), } - extract_primary_public_key(did_document) } /// Decode the payload segment of a compact JWS to raw bytes (post-base64, @@ -72,6 +75,16 @@ mod tests { format!("{signing_input}.{}", b64.encode(sig.to_bytes())) } + fn sign_with_kid(signing_key: &SigningKey, payload: &serde_json::Value, kid: &str) -> String { + let b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD; + let header = b64 + .encode(serde_json::to_vec(&serde_json::json!({"alg": "EdDSA", "kid": kid})).unwrap()); + let body = b64.encode(canonicalize(payload).unwrap()); + let signing_input = format!("{header}.{body}"); + let sig = signing_key.sign(signing_input.as_bytes()); + format!("{signing_input}.{}", b64.encode(sig.to_bytes())) + } + fn did_doc_for(signing_key: &SigningKey) -> serde_json::Value { let b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD; let x = b64.encode(signing_key.verifying_key().to_bytes()); @@ -111,6 +124,24 @@ mod tests { assert!(key.is_some(), "must fall back to the primary key"); } + #[test] + fn resolve_returns_none_for_present_but_unresolvable_kid() { + // A kid that resolves to no key in the DID document (revoked/rotated + // out/wrong) must NOT silently substitute the primary key — return None + // so the caller reports the accurate "kid does not resolve" diagnosis. + let signing_key = SigningKey::from_bytes(&[7u8; 32]); + let did_doc = did_doc_for(&signing_key); + let jws = sign_with_kid( + &signing_key, + &serde_json::json!({"a": 1}), + "not-a-real-fingerprint", + ); + assert!( + resolve_public_key(&jws, &did_doc).is_none(), + "an unresolvable kid must not fall back to the primary key" + ); + } + #[test] fn decode_payload_bytes_rejects_segmentless_input() { assert!(decode_payload_bytes("no-dots-here").is_err()); diff --git a/crates/dpp-vault/src/domain/verify/transfer_chain.rs b/crates/dpp-vault/src/domain/verify/transfer_chain.rs index 52dbe32..aae4eb9 100644 --- a/crates/dpp-vault/src/domain/verify/transfer_chain.rs +++ b/crates/dpp-vault/src/domain/verify/transfer_chain.rs @@ -35,36 +35,68 @@ pub struct TransferChainBreak { /// Verify every completed transfer record's signatures against the DID /// documents available in `did_documents` (keyed by DID). /// -/// A record with no signature yet (still `Initiated`) is skipped — it has -/// nothing to verify. A record whose signer's DID document is missing from -/// `did_documents` fails closed (reported, not silently skipped) so a -/// verifier never reports false-green on an unresolvable cross-operator DID. +/// A **completed** record (has `completed_at`, not rejected/cancelled) must +/// carry both operator signatures — an absent signature fails closed rather than +/// being treated as "nothing to verify", so a record a producing node marked +/// completed without signing can never pass with zero cryptographic checks. A +/// still-`Initiated` record's not-yet-present signature is skipped. A record +/// whose signer's DID document is missing from `did_documents` fails closed +/// (reported, not silently skipped) so a verifier never reports false-green on +/// an unresolvable cross-operator DID. /// /// # Errors -/// [`TransferChainBreak`] at the first record with a bad or unverifiable -/// signature. +/// [`TransferChainBreak`] at the first record with a missing (on a completed +/// record), bad, or unverifiable signature. pub fn verify_transfer_chain( chain: &TransferChain, did_documents: &BTreeMap, ) -> Result<(), TransferChainBreak> { for (index, record) in chain.transfers.iter().enumerate() { + // A completed transfer must be fully signed by both parties. + let is_completed = record.completed_at.is_some() + && record.rejected_at.is_none() + && record.cancelled_at.is_none(); + let payload = record.signing_payload(); - if let Some(sig) = &record.from_signature { - check_signature(&record.from_operator.did, sig, &payload, did_documents).map_err( - |reason| TransferChainBreak { + match &record.from_signature { + Some(sig) => { + check_signature(&record.from_operator.did, sig, &payload, did_documents).map_err( + |reason| TransferChainBreak { + index, + issue: TransferSignatureIssue::From(reason), + }, + )?; + } + None if is_completed => { + return Err(TransferChainBreak { index, - issue: TransferSignatureIssue::From(reason), - }, - )?; + issue: TransferSignatureIssue::From( + "completed transfer is missing the from-operator signature".into(), + ), + }); + } + None => {} } - if let Some(sig) = &record.to_signature { - check_signature(&record.to_operator.did, sig, &payload, did_documents).map_err( - |reason| TransferChainBreak { + + match &record.to_signature { + Some(sig) => { + check_signature(&record.to_operator.did, sig, &payload, did_documents).map_err( + |reason| TransferChainBreak { + index, + issue: TransferSignatureIssue::To(reason), + }, + )?; + } + None if is_completed => { + return Err(TransferChainBreak { index, - issue: TransferSignatureIssue::To(reason), - }, - )?; + issue: TransferSignatureIssue::To( + "completed transfer is missing the to-operator signature".into(), + ), + }); + } + None => {} } } Ok(()) @@ -231,4 +263,68 @@ mod tests { let brk = verify_transfer_chain(&chain, &docs).expect_err("must fail closed"); assert!(matches!(brk.issue, TransferSignatureIssue::To(_))); } + + #[test] + fn completed_record_without_signatures_fails_closed() { + // A record marked completed but carrying no signatures (a producing-node + // workflow bug) must fail closed, not pass with zero cryptographic checks. + let record = TransferRecord { + transfer_id: Uuid::now_v7(), + passport_id: PassportId::new(), + from_operator: operator("did:web:from.example"), + to_operator: operator("did:web:to.example"), + reason: TransferReason::Sale, + from_signature: None, + to_signature: None, + initiated_at: Utc::now(), + completed_at: Some(Utc::now()), + rejected_at: None, + cancelled_at: None, + notes: None, + }; + let chain = TransferChain { + passport_id: record.passport_id, + original_operator: operator("did:web:from.example"), + transfers: vec![record], + }; + // No DID docs needed — it must fail on the missing signature first. + let brk = verify_transfer_chain(&chain, &BTreeMap::new()) + .expect_err("a completed but unsigned record must fail closed"); + assert_eq!(brk.index, 0); + assert!(matches!(brk.issue, TransferSignatureIssue::From(_))); + } + + #[test] + fn initiated_record_pending_countersignature_is_skipped() { + // Still-Initiated (not completed): from signed, awaiting the to-operator. + // The absent to-signature is skipped, not treated as a failure. + let from_key = SigningKey::from_bytes(&[1u8; 32]); + let mut record = TransferRecord { + transfer_id: Uuid::now_v7(), + passport_id: PassportId::new(), + from_operator: operator("did:web:from.example"), + to_operator: operator("did:web:to.example"), + reason: TransferReason::Sale, + from_signature: None, + to_signature: None, + initiated_at: Utc::now(), + completed_at: None, + rejected_at: None, + cancelled_at: None, + notes: None, + }; + let payload = record.signing_payload(); + record.from_signature = Some(sign(&from_key, &payload)); + let chain = TransferChain { + passport_id: record.passport_id, + original_operator: operator("did:web:from.example"), + transfers: vec![record], + }; + let mut docs = BTreeMap::new(); + docs.insert("did:web:from.example".to_string(), did_doc_for(&from_key)); + assert!( + verify_transfer_chain(&chain, &docs).is_ok(), + "an initiated (uncompleted) record must not fail on its pending countersignature" + ); + } } diff --git a/crates/dpp-vault/src/handlers/archive.rs b/crates/dpp-vault/src/handlers/archive.rs index 7042988..2cd4744 100644 --- a/crates/dpp-vault/src/handlers/archive.rs +++ b/crates/dpp-vault/src/handlers/archive.rs @@ -20,6 +20,13 @@ pub async fn archive_handler( Extension(auth): Extension, Path(dpp_id): Path, ) -> impl IntoResponse { + if !auth.scope.can_write() { + return api_error( + StatusCode::FORBIDDEN, + "FORBIDDEN", + "Archiving a passport requires a write-scoped credential.", + ); + } let passport_id = match parse_passport_id(&dpp_id) { Ok(id) => id, Err(e) => return e, diff --git a/crates/dpp-vault/src/handlers/create.rs b/crates/dpp-vault/src/handlers/create.rs index da8e081..6e1eb03 100644 --- a/crates/dpp-vault/src/handlers/create.rs +++ b/crates/dpp-vault/src/handlers/create.rs @@ -60,6 +60,13 @@ pub async fn create_handler( Extension(auth): Extension, Json(body): Json, ) -> impl IntoResponse { + if !auth.scope.can_write() { + return api_error( + StatusCode::FORBIDDEN, + "FORBIDDEN", + "Creating a passport requires a write-scoped credential.", + ); + } if body.product_name.trim().is_empty() { return api_error( StatusCode::UNPROCESSABLE_ENTITY, diff --git a/crates/dpp-vault/src/handlers/eol.rs b/crates/dpp-vault/src/handlers/eol.rs index 8103761..3650e2e 100644 --- a/crates/dpp-vault/src/handlers/eol.rs +++ b/crates/dpp-vault/src/handlers/eol.rs @@ -40,6 +40,13 @@ pub async fn eol_handler( Path(dpp_id): Path, Json(body): Json, ) -> impl IntoResponse { + if !auth.scope.can_write() { + return api_error( + StatusCode::FORBIDDEN, + "FORBIDDEN", + "Declaring a passport end-of-life requires a write-scoped credential.", + ); + } let passport_id = match parse_passport_id(&dpp_id) { Ok(id) => id, Err(e) => return e, diff --git a/crates/dpp-vault/src/handlers/operator.rs b/crates/dpp-vault/src/handlers/operator.rs index cff4de2..fb5cab1 100644 --- a/crates/dpp-vault/src/handlers/operator.rs +++ b/crates/dpp-vault/src/handlers/operator.rs @@ -40,6 +40,9 @@ pub async fn operator_patch_handler( "Updating operator config requires an admin-scoped credential.", ); } + if let Err(msg) = patch.validate() { + return api_error(StatusCode::BAD_REQUEST, "INVALID_CONFIG", &msg); + } match state .operator_service .update(STANDALONE_OPERATOR_ID, patch) diff --git a/crates/dpp-vault/src/handlers/publish.rs b/crates/dpp-vault/src/handlers/publish.rs index b441038..16ece15 100644 --- a/crates/dpp-vault/src/handlers/publish.rs +++ b/crates/dpp-vault/src/handlers/publish.rs @@ -21,6 +21,13 @@ pub async fn publish_handler( Extension(auth): Extension, Path(dpp_id): Path, ) -> impl IntoResponse { + if !auth.scope.can_write() { + return api_error( + StatusCode::FORBIDDEN, + "FORBIDDEN", + "Publishing a passport requires a write-scoped credential.", + ); + } let passport_id = match parse_passport_id(&dpp_id) { Ok(id) => id, Err(e) => return e, diff --git a/crates/dpp-vault/src/handlers/registry_identity.rs b/crates/dpp-vault/src/handlers/registry_identity.rs index a2ec43c..da79a1c 100644 --- a/crates/dpp-vault/src/handlers/registry_identity.rs +++ b/crates/dpp-vault/src/handlers/registry_identity.rs @@ -10,6 +10,7 @@ use axum::{ use uuid::Uuid; use dpp_domain::domain::error::DppError; +use dpp_types::STANDALONE_OPERATOR_ID; use dpp_types::registry_identity::{CreateFacilityRequest, CreateOperatorIdentifierRequest}; use crate::{middleware::auth::AuthContext, state::AppState}; @@ -194,9 +195,16 @@ pub async fn operator_ids_create_handler( if let Some(resp) = require_admin(&auth) { return resp; } + // The identifier itself carries no per-entry country (an Art. 13 economic- + // operator identifier belongs to the operator, not a location) — reuse the + // operator's own registered country for the `dpp-registry` validation. + let operator_country = match state.operator_service.get(STANDALONE_OPERATOR_ID).await { + Ok(cfg) => cfg.country, + Err(e) => return internal_error(e), + }; match state .registry_identity_service - .add_operator_identifier(body, &auth.user_id) + .add_operator_identifier(body, &operator_country, &auth.user_id) .await { Ok(o) => (StatusCode::CREATED, Json(o)).into_response(), diff --git a/crates/dpp-vault/src/handlers/suspend.rs b/crates/dpp-vault/src/handlers/suspend.rs index f0824e3..74df2bc 100644 --- a/crates/dpp-vault/src/handlers/suspend.rs +++ b/crates/dpp-vault/src/handlers/suspend.rs @@ -30,6 +30,13 @@ pub async fn suspend_handler( Path(dpp_id): Path, body: Option>, ) -> impl IntoResponse { + if !auth.scope.can_write() { + return api_error( + StatusCode::FORBIDDEN, + "FORBIDDEN", + "Suspending a passport requires a write-scoped credential.", + ); + } let passport_id = match parse_passport_id(&dpp_id) { Ok(id) => id, Err(e) => return e, diff --git a/crates/dpp-vault/src/handlers/transfer.rs b/crates/dpp-vault/src/handlers/transfer.rs index 807008a..7ea38da 100644 --- a/crates/dpp-vault/src/handlers/transfer.rs +++ b/crates/dpp-vault/src/handlers/transfer.rs @@ -38,6 +38,13 @@ pub async fn transfer_initiate_handler( Path(dpp_id): Path, Json(body): Json, ) -> impl IntoResponse { + if !auth.scope.can_write() { + return api_error( + StatusCode::FORBIDDEN, + "FORBIDDEN", + "Initiating a transfer requires a write-scoped credential.", + ); + } let id = match parse_passport_id(&dpp_id) { Ok(i) => i, Err(e) => return e, @@ -79,6 +86,13 @@ pub async fn transfer_accept_handler( Extension(auth): Extension, Path(dpp_id): Path, ) -> impl IntoResponse { + if !auth.scope.can_write() { + return api_error( + StatusCode::FORBIDDEN, + "FORBIDDEN", + "Accepting a transfer requires a write-scoped credential.", + ); + } let id = match parse_passport_id(&dpp_id) { Ok(i) => i, Err(e) => return e, diff --git a/crates/dpp-vault/src/handlers/update.rs b/crates/dpp-vault/src/handlers/update.rs index f3bcfa4..1bc094c 100644 --- a/crates/dpp-vault/src/handlers/update.rs +++ b/crates/dpp-vault/src/handlers/update.rs @@ -20,6 +20,13 @@ pub async fn update_handler( Path(dpp_id): Path, Json(body): Json, ) -> impl IntoResponse { + if !auth.scope.can_write() { + return api_error( + StatusCode::FORBIDDEN, + "FORBIDDEN", + "Updating a passport requires a write-scoped credential.", + ); + } let passport_id = match parse_passport_id(&dpp_id) { Ok(id) => id, Err(e) => return e, diff --git a/crates/dpp-vault/tests/api_key_scope.rs b/crates/dpp-vault/tests/api_key_scope.rs index 745f9c9..3b68353 100644 --- a/crates/dpp-vault/tests/api_key_scope.rs +++ b/crates/dpp-vault/tests/api_key_scope.rs @@ -61,6 +61,44 @@ async fn write_scoped_credential_cannot_escalate() { ); } +#[tokio::test(flavor = "multi_thread")] +async fn read_scoped_credential_cannot_mutate_passports() { + let pg = start_postgres().await; + let vault_url = start_vault(pg.dal.clone()).await; + + // A least-privilege read-only key (documented for GET-only integrations). + let reader = TestClient::new(&vault_url, make_jwt_scoped("op", "read")); + let id = "00000000-0000-4000-8000-000000000000"; + + // Every passport-lifecycle mutation must reject a Read-scoped credential with + // 403 — the `can_write()` gate runs first, before any state change. Bodies are + // valid so each request reaches the handler rather than failing extraction. + let create = reader + .post_json( + "/api/v1/dpp", + json!({ "productName": "x", "manufacturer": { "name": "n", "address": "a" } }), + ) + .await; + assert_eq!(create.status(), 403, "create must require write scope"); + + let update = reader + .put_json(&format!("/api/v1/dpp/{id}"), json!({})) + .await; + assert_eq!(update.status(), 403, "update must require write scope"); + + // publish / suspend / archive / transfer-accept take no request body of their + // own; eol and transfer-initiate share the identical first-line gate. + for path in [ + format!("/api/v1/dpp/{id}/publish"), + format!("/api/v1/dpp/{id}/suspend"), + format!("/api/v1/dpp/{id}/archive"), + format!("/api/v1/dpp/{id}/transfer/accept"), + ] { + let r = reader.post_json(&path, json!({})).await; + assert_eq!(r.status(), 403, "{path} must require write scope"); + } +} + #[tokio::test(flavor = "multi_thread")] async fn admin_can_mint_least_privilege_key_and_scope_round_trips() { let pg = start_postgres().await;