Skip to content

Commit 10b05c8

Browse files
refactor(authority): encode unsigned non-zero runtime TTLs (#615)
## Why The schema owns human-readable non-zero duration parsing, while downstream Authority contracts consume exact positive whole-second integers. This internal refactor makes both validated runtime TTL states unsigned and non-zero without changing configuration or the existing signed protobuf request contract. ## What changed - Store private validated `max_ttl_seconds` and `bundle_ttl_seconds` as `std::num::NonZeroU32`. - Convert schema-validated `NonZeroDuration` values once through `AuthorityConfigBuilder`: require whole seconds, retain the `i32::MAX` maximum-token-TTL bound, and checked-narrow to standard non-zero integers. - Keep Authority service state, issuance clamping, and effective TTLs unsigned and non-zero; call `.get()` only at Chrono, Cedar, policy-bundle, library, or wire integer boundaries. - Keep protobuf `requested_ttl_seconds: i32` unchanged. The issuance adapter normalizes it once: `<= 0` selects the configured maximum; a positive request becomes `NonZeroU32` and clamps against that unsigned maximum. - Cover negative, zero, positive, and over-maximum requests plus exact claim and protobuf expiry behavior. ## Contract No user, TOML, environment, or wire change is introduced. Compact duration syntax, `"1h"` / `"30s"` defaults, canonical `FIRMA_AUTHORITY_MAX_TTL` / `FIRMA_AUTHORITY_BUNDLE_TTL` precedence, whole-second and range errors, and effective values remain unchanged. Runtime uses `NonZeroU32` rather than a duration type because its consumers are exact positive whole-second integer boundaries; schema parsing remains owned by `NonZeroDuration`. The remaining signed protobuf request and non-positive sentinel are intentionally deferred to a separate follow-up. ## Atomic revisions 1. [`32539c5c`](32539c5) — accepted plan and plan-review dispositions. 2. [`20f0e090`](20f0e09) — encode maximum-token-TTL runtime/config/service/issuance state as `NonZeroU32`. 3. [`ae6c1c31`](ae6c1c3) — encode bundle-TTL runtime/config/consumer state as `NonZeroU32`. 4. [`ad1c34eb`](ad1c34e) — record the independent implementation review. 5. [`8a6e26aa`](8a6e26a) — record the atomic-commit re-review. 6. [`d01d9c11`](d01d9c1) — mechanically remove the plan artifact. ## Plan and independent review - Accepted plan and plan-review dispositions: [`32539c5c: authority-non-zero-runtime-ttl-plan.md`](https://github.com/Firma-AI/openfirma/blob/32539c5cedb0fe31f08a6d8ca1f8db468df2d15f/docs/architecture/authority-non-zero-runtime-ttl-plan.md). - Independent implementation review: [`ad1c34eb`](https://github.com/Firma-AI/openfirma/blob/ad1c34eb09bfbb2f2e3cd59832e1510427e3827e/docs/architecture/authority-non-zero-runtime-ttl-plan.md#post-implementation-adversarial-review). - Atomic-commit re-review: [`8a6e26aa`](https://github.com/Firma-AI/openfirma/blob/8a6e26aaa4078c3fc4db1f39d008cd1ff5e569a8/docs/architecture/authority-non-zero-runtime-ttl-plan.md). - Immediate mechanical plan removal: [`d01d9c11`](d01d9c1). No plan Markdown remains at the tip or in the PR diff.
1 parent 7fd412b commit 10b05c8

9 files changed

Lines changed: 145 additions & 75 deletions

File tree

crates/firma-authority/src/config.rs

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use firma_config_schema::authority as schema;
2+
use std::num::{NonZeroU32, NonZeroU64};
23
use std::path::{Path, PathBuf};
34

45
/// Authority configuration loaded from TOML file and/or environment variables.
@@ -36,12 +37,12 @@ pub struct AuthorityConfig {
3637
pub(crate) schema_path: Option<PathBuf>,
3738
/// Path to the revocation file (one token ID per line).
3839
pub(crate) revocation_file: PathBuf,
39-
/// Maximum token TTL in seconds (default: 3600).
40-
pub(crate) max_ttl_seconds: i32,
40+
/// Strictly positive maximum token TTL in whole seconds.
41+
max_ttl_seconds: NonZeroU32,
4142
/// Path to the Ed25519 signing key file (64-byte raw or PEM).
4243
pub(crate) key_file: PathBuf,
43-
/// Policy bundle TTL advertised to sidecars in seconds (default: 30).
44-
pub(crate) bundle_ttl_seconds: u32,
44+
/// Strictly positive policy bundle TTL advertised to sidecars in whole seconds.
45+
bundle_ttl_seconds: NonZeroU32,
4546
/// Authority TLS configuration.
4647
pub(crate) tls: AuthorityTlsConfig,
4748
}
@@ -74,9 +75,9 @@ impl AuthorityConfig {
7475
&self.revocation_file
7576
}
7677

77-
/// Maximum token TTL in seconds.
78+
/// Strictly positive maximum token TTL in whole seconds.
7879
#[must_use]
79-
pub fn max_ttl_seconds(&self) -> i32 {
80+
pub const fn max_ttl_seconds(&self) -> NonZeroU32 {
8081
self.max_ttl_seconds
8182
}
8283

@@ -86,9 +87,9 @@ impl AuthorityConfig {
8687
&self.key_file
8788
}
8889

89-
/// Policy bundle TTL advertised to sidecars in seconds.
90+
/// Strictly positive policy bundle TTL advertised to sidecars in whole seconds.
9091
#[must_use]
91-
pub fn bundle_ttl_seconds(&self) -> u32 {
92+
pub const fn bundle_ttl_seconds(&self) -> NonZeroU32 {
9293
self.bundle_ttl_seconds
9394
}
9495

@@ -179,12 +180,26 @@ impl AuthorityConfig {
179180
field: "authority.bundle_ttl",
180181
});
181182
}
182-
let max_ttl_seconds = i32::try_from(max_ttl.as_secs()).map_err(|_| {
183+
let max_ttl_seconds =
184+
NonZeroU64::new(max_ttl.as_secs()).ok_or(AuthorityConfigError::DurationOutOfRange {
185+
field: "authority.max_ttl",
186+
})?;
187+
if max_ttl_seconds.get() > u64::from(i32::MAX.unsigned_abs()) {
188+
return Err(AuthorityConfigError::DurationOutOfRange {
189+
field: "authority.max_ttl",
190+
});
191+
}
192+
let max_ttl_seconds = NonZeroU32::try_from(max_ttl_seconds).map_err(|_| {
183193
AuthorityConfigError::DurationOutOfRange {
184194
field: "authority.max_ttl",
185195
}
186196
})?;
187-
let bundle_ttl_seconds = u32::try_from(bundle_ttl.as_secs()).map_err(|_| {
197+
let bundle_ttl_seconds = NonZeroU64::new(bundle_ttl.as_secs()).ok_or(
198+
AuthorityConfigError::DurationOutOfRange {
199+
field: "authority.bundle_ttl",
200+
},
201+
)?;
202+
let bundle_ttl_seconds = NonZeroU32::try_from(bundle_ttl_seconds).map_err(|_| {
188203
AuthorityConfigError::DurationOutOfRange {
189204
field: "authority.bundle_ttl",
190205
}
@@ -216,18 +231,15 @@ impl AuthorityConfig {
216231
///
217232
/// # Errors
218233
///
219-
/// Returns [`ConfigError::DurationNotPositive`] if either runtime TTL
220-
/// cannot be represented as a non-zero schema duration.
234+
/// This conversion is infallible for builder-produced runtime state. The
235+
/// result remains fallible for API compatibility with existing callers.
221236
pub fn to_schema(&self) -> Result<schema::AuthorityConfig, ConfigError> {
222-
let max_ttl_seconds =
223-
u64::try_from(self.max_ttl_seconds).map_err(|_| ConfigError::DurationNotPositive {
224-
field: "authority.max_ttl",
225-
})?;
226-
let max_ttl = non_zero_duration_from_seconds("authority.max_ttl", max_ttl_seconds)?;
227-
let bundle_ttl = non_zero_duration_from_seconds(
228-
"authority.bundle_ttl",
229-
u64::from(self.bundle_ttl_seconds),
230-
)?;
237+
let max_ttl = firma_config_schema::utils::NonZeroDuration::from(NonZeroU64::from(
238+
self.max_ttl_seconds,
239+
));
240+
let bundle_ttl = firma_config_schema::utils::NonZeroDuration::from(NonZeroU64::from(
241+
self.bundle_ttl_seconds,
242+
));
231243

232244
Ok(schema::AuthorityConfig {
233245
listen_addr: self.listen_addr.clone(),
@@ -247,14 +259,6 @@ impl AuthorityConfig {
247259
}
248260
}
249261

250-
fn non_zero_duration_from_seconds(
251-
field: &'static str,
252-
seconds: u64,
253-
) -> Result<firma_config_schema::utils::NonZeroDuration, ConfigError> {
254-
firma_config_schema::utils::NonZeroDuration::new(std::time::Duration::from_secs(seconds))
255-
.map_err(|_| ConfigError::DurationNotPositive { field })
256-
}
257-
258262
/// Assembles a validated [`AuthorityConfig`].
259263
///
260264
/// The builder retains the schema representation while applying path rebasing
@@ -589,8 +593,8 @@ mod tests {
589593
fn default_config_has_sensible_values() -> anyhow::Result<()> {
590594
let config = AuthorityConfigBuilder::default().build()?;
591595
assert_eq!(config.listen_addr, "[::1]:50051");
592-
assert_eq!(config.max_ttl_seconds, 3600);
593-
assert_eq!(config.bundle_ttl_seconds, 30);
596+
assert_eq!(config.max_ttl_seconds.get(), 3600);
597+
assert_eq!(config.bundle_ttl_seconds.get(), 30);
594598

595599
Ok(())
596600
}
@@ -678,7 +682,7 @@ mod tests {
678682
)
679683
.unwrap();
680684
let c = AuthorityConfig::load_resolved(&p, tmp.path()).unwrap();
681-
assert_eq!(c.max_ttl_seconds, 1800);
685+
assert_eq!(c.max_ttl_seconds.get(), 1800);
682686
assert_eq!(c.policy_dir, tmp.path().join("policies"));
683687
assert_eq!(c.tls.cert, Some(tmp.path().join("authority.crt")));
684688
assert_eq!(c.tls.key, Some(tmp.path().join("authority.key")));
@@ -696,9 +700,9 @@ max_ttl = "30m"
696700
.build()
697701
.unwrap_or_else(|e| panic!("{e}"));
698702
assert_eq!(config.listen_addr, "0.0.0.0:9090");
699-
assert_eq!(config.max_ttl_seconds, 1800);
703+
assert_eq!(config.max_ttl_seconds.get(), 1800);
700704
// Defaults for unspecified fields
701-
assert_eq!(config.bundle_ttl_seconds, 30);
705+
assert_eq!(config.bundle_ttl_seconds.get(), 30);
702706
}
703707

704708
#[test]

crates/firma-authority/src/issuance.rs

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
//! transport (`tonic`) or process-IO dependency so it is straightforward
77
//! to call from a CLI in-process and to unit-test.
88
9+
use std::num::NonZeroU32;
910
use std::sync::Arc;
1011

1112
use chrono::{DateTime, Duration, Utc};
@@ -57,7 +58,7 @@ pub enum IssuanceError {
5758
pub async fn issue_capability(
5859
policy_store: &CedarPolicyStore,
5960
signer: &Arc<PasetoV4Signer>,
60-
max_ttl_seconds: i32,
61+
max_ttl_seconds: NonZeroU32,
6162
req: &IssuanceRequest<'_>,
6263
) -> Result<IssuanceResult, IssuanceError> {
6364
let snapshot = policy_store.snapshot().await;
@@ -79,7 +80,7 @@ pub async fn issue_capability(
7980

8081
let ttl = clamp_ttl(req.requested_ttl_seconds, max_ttl_seconds);
8182
let now: DateTime<Utc> = Utc::now();
82-
let expiry = now + Duration::seconds(i64::from(ttl));
83+
let expiry = now + Duration::seconds(i64::from(ttl.get()));
8384
let token_id = TokenId::generate();
8485
let bundle_version = policy_store.bundle().version;
8586
let agent_id = req.agent_id.to_string();
@@ -111,6 +112,11 @@ mod tests {
111112
use pasetors::keys::{AsymmetricKeyPair, Generate};
112113
use pasetors::version4::V4;
113114

115+
fn max_ttl(seconds: u32) -> NonZeroU32 {
116+
assert!(seconds > 0);
117+
NonZeroU32::new(seconds).unwrap_or(NonZeroU32::MIN)
118+
}
119+
114120
fn fixture_policy_store() -> Arc<CedarPolicyStore> {
115121
let dir = tempfile::tempdir().unwrap().keep();
116122
std::fs::write(
@@ -141,12 +147,44 @@ mod tests {
141147
resource_scope: "wttr.in*",
142148
requested_ttl_seconds: 300,
143149
};
144-
let out = issue_capability(&store, &signer, 600, &req).await.unwrap();
150+
let out = issue_capability(&store, &signer, max_ttl(600), &req)
151+
.await
152+
.unwrap();
145153
assert!(!out.raw_token.is_empty());
146154
assert_eq!(out.claims.action_set, actions);
147155
assert_eq!(out.claims.resource_scope, "wttr.in*");
148156
}
149157

158+
#[tokio::test]
159+
async fn signed_request_ttl_preserves_exact_expiry_semantics() {
160+
let store = fixture_policy_store();
161+
let kp = AsymmetricKeyPair::<V4>::generate().unwrap();
162+
let signer = Arc::new(PasetoV4Signer::try_new(kp.secret.as_bytes()).unwrap());
163+
let agent: AgentId = "agt_01j0000000e008000000000001".parse().unwrap();
164+
let session: SessionId = "sess_1".parse().unwrap();
165+
let actions = vec!["communication.external.send".to_string()];
166+
167+
for (requested_ttl_seconds, expected_ttl_seconds) in
168+
[(-1, 600), (0, 600), (300, 300), (900, 600)]
169+
{
170+
let req = IssuanceRequest {
171+
agent_id: &agent,
172+
session_id: &session,
173+
requested_actions: &actions,
174+
resource_scope: "wttr.in*",
175+
requested_ttl_seconds,
176+
};
177+
let out = issue_capability(&store, &signer, max_ttl(600), &req)
178+
.await
179+
.unwrap();
180+
181+
assert_eq!(
182+
(out.claims.expiry - out.claims.issued_at).num_seconds(),
183+
expected_ttl_seconds
184+
);
185+
}
186+
}
187+
150188
/// Policy store that permits exactly one action class; all others
151189
/// default-deny so the grant narrows to the authorized subset.
152190
fn store_permitting(action: &str) -> Arc<CedarPolicyStore> {
@@ -183,7 +221,9 @@ mod tests {
183221
resource_scope: "*",
184222
requested_ttl_seconds: 300,
185223
};
186-
let out = issue_capability(&store, &signer, 600, &req).await.unwrap();
224+
let out = issue_capability(&store, &signer, max_ttl(600), &req)
225+
.await
226+
.unwrap();
187227
assert_eq!(out.claims.action_set, vec!["code.read".to_string()]);
188228
}
189229

@@ -202,7 +242,7 @@ mod tests {
202242
resource_scope: "*",
203243
requested_ttl_seconds: 300,
204244
};
205-
let err = issue_capability(&store, &signer, 600, &req)
245+
let err = issue_capability(&store, &signer, max_ttl(600), &req)
206246
.await
207247
.unwrap_err();
208248
match err {
@@ -238,7 +278,7 @@ mod tests {
238278
resource_scope: "wttr.in*",
239279
requested_ttl_seconds: 300,
240280
};
241-
let err = issue_capability(&store, &signer, 600, &req)
281+
let err = issue_capability(&store, &signer, max_ttl(600), &req)
242282
.await
243283
.unwrap_err();
244284
match err {

crates/firma-authority/src/server.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,25 +136,25 @@ fn load_authority_service(config: &AuthorityConfig) -> Result<AuthorityServiceIm
136136
let policy_store = CedarPolicyStore::load(
137137
&config.policy_dir,
138138
config.schema_path.clone(),
139-
config.bundle_ttl_seconds,
139+
config.bundle_ttl_seconds().get(),
140140
)?;
141141

142142
tracing::info!(issuance_policy_dir = %config.issuance_policy_dir.display(), "loading issuance policy store");
143143
let issuance_policy_store = CedarPolicyStore::load(
144144
&config.issuance_policy_dir,
145145
config.schema_path.clone(),
146-
config.bundle_ttl_seconds,
146+
config.bundle_ttl_seconds().get(),
147147
)?;
148148

149-
let token_ttl = chrono::Duration::seconds(i64::from(config.max_ttl_seconds));
149+
let token_ttl = chrono::Duration::seconds(i64::from(config.max_ttl_seconds().get()));
150150
let revocation_store = RevocationStore::try_new(&config.revocation_file, token_ttl)?;
151151

152152
AuthorityServiceImpl::try_new(
153153
issuance_policy_store,
154154
policy_store,
155155
revocation_store,
156156
signer,
157-
config.max_ttl_seconds,
157+
config.max_ttl_seconds(),
158158
)
159159
}
160160

crates/firma-authority/src/service.rs

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::collections::BTreeSet;
2+
use std::num::NonZeroU32;
23
use std::pin::Pin;
34
use std::sync::Arc;
45

@@ -37,7 +38,7 @@ pub struct AuthorityServiceImpl {
3738
/// and read in-memory revocation state via `Deref<Target = RevocationStore>`.
3839
revocation_watcher: RevocationStoreWatcher,
3940
signer: Arc<PasetoV4Signer>,
40-
max_ttl_seconds: i32,
41+
max_ttl_seconds: NonZeroU32,
4142
}
4243

4344
impl AuthorityServiceImpl {
@@ -49,7 +50,7 @@ impl AuthorityServiceImpl {
4950
policy_store: CedarPolicyStore,
5051
revocation_store: RevocationStore,
5152
signer: Arc<PasetoV4Signer>,
52-
max_ttl_seconds: i32,
53+
max_ttl_seconds: NonZeroU32,
5354
) -> anyhow::Result<Self> {
5455
let issuance_policy_watcher = issuance_policy_store.watch()?;
5556
let policy_watcher = policy_store.watch()?;
@@ -597,19 +598,22 @@ pub(crate) fn compute_context_hash(
597598
hex::encode(hasher.finalize())
598599
}
599600

600-
/// FR-4: Clamp requested TTL to the configured maximum.
601-
pub(crate) fn clamp_ttl(requested: i32, max: i32) -> i32 {
602-
if requested <= 0 {
603-
max
604-
} else {
605-
requested.min(max)
606-
}
601+
/// FR-4: Normalize a signed protocol request and clamp it to the configured maximum.
602+
pub(crate) fn clamp_ttl(requested: i32, max: NonZeroU32) -> NonZeroU32 {
603+
u32::try_from(requested)
604+
.ok()
605+
.and_then(NonZeroU32::new)
606+
.map_or(max, |requested| requested.min(max))
607607
}
608608

609609
#[cfg(test)]
610610
mod tests {
611611
use super::*;
612612

613+
fn max_ttl() -> NonZeroU32 {
614+
NonZeroU32::new(3600).unwrap_or(NonZeroU32::MIN)
615+
}
616+
613617
fn agent(id: &str) -> AgentId {
614618
id.parse().unwrap()
615619
}
@@ -624,22 +628,22 @@ mod tests {
624628

625629
#[test]
626630
fn clamp_ttl_within_max() {
627-
assert_eq!(clamp_ttl(600, 3600), 600);
631+
assert_eq!(clamp_ttl(600, max_ttl()).get(), 600);
628632
}
629633

630634
#[test]
631635
fn clamp_ttl_exceeds_max() {
632-
assert_eq!(clamp_ttl(7200, 3600), 3600);
636+
assert_eq!(clamp_ttl(7200, max_ttl()).get(), 3600);
633637
}
634638

635639
#[test]
636640
fn clamp_ttl_zero_uses_max() {
637-
assert_eq!(clamp_ttl(0, 3600), 3600);
641+
assert_eq!(clamp_ttl(0, max_ttl()).get(), 3600);
638642
}
639643

640644
#[test]
641645
fn clamp_ttl_negative_uses_max() {
642-
assert_eq!(clamp_ttl(-1, 3600), 3600);
646+
assert_eq!(clamp_ttl(-1, max_ttl()).get(), 3600);
643647
}
644648

645649
fn permit_all() -> PolicySet {

0 commit comments

Comments
 (0)