Skip to content

Commit 1698466

Browse files
committed
Bound GeoIP security context loading
1 parent 65fa843 commit 1698466

6 files changed

Lines changed: 147 additions & 29 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ behavior when the change improves security or project direction.
3434
`sanitization::SecretVec`, and fix the managed child `PATH`.
3535
- Bind managed PHP-FPM spawn to a trusted executable descriptor and terminate
3636
its complete dedicated process group during shutdown and watchdog recovery.
37+
- Bound GeoIP database reads to an exact admitted-length allocation with a
38+
separate growth probe, and validate every publicly constructed `GeoContext`.
3739

3840
## 1.7.7 - 2026-07-10
3941

crates/fluxheim-geoip/src/geoip_runtime/tests.rs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,45 @@ fn rejects_writable_database_and_parent() {
9898
fn rejects_database_modified_during_read() {
9999
let path = trusted_test_file("geoip-read-change", b"first-database");
100100
let verified = open_verified_mmdb(&path, 64).unwrap();
101-
let error = read_verified_mmdb_with_post_read(verified, || {
101+
let error = read_verified_mmdb_with_post_read(verified, |_| {
102102
std::fs::write(&path, b"other-database").unwrap();
103103
})
104104
.unwrap_err();
105105

106106
assert_eq!(error.kind(), std::io::ErrorKind::InvalidData);
107107
assert!(error.to_string().contains("changed while reading"));
108108
}
109+
110+
#[test]
111+
fn rejects_database_growth_without_growing_admitted_buffer() {
112+
use std::io::Write as _;
113+
114+
let path = trusted_test_file("geoip-read-growth", b"database");
115+
let verified = open_verified_mmdb(&path, 64).unwrap();
116+
let admitted_len = usize::try_from(verified.byte_len).unwrap();
117+
let observed_capacity = std::cell::Cell::new(usize::MAX);
118+
let error = read_verified_mmdb_with_post_read(verified, |capacity| {
119+
observed_capacity.set(capacity);
120+
let mut file = std::fs::OpenOptions::new()
121+
.append(true)
122+
.open(&path)
123+
.unwrap();
124+
file.write_all(b"x").unwrap();
125+
})
126+
.unwrap_err();
127+
128+
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
129+
assert!(error.to_string().contains("grew while reading"));
130+
assert_eq!(observed_capacity.get(), admitted_len);
131+
}
132+
133+
#[test]
134+
fn rejects_database_that_becomes_shorter_before_exact_read() {
135+
let path = trusted_test_file("geoip-read-shorter", b"database");
136+
let verified = open_verified_mmdb(&path, 64).unwrap();
137+
std::fs::write(&path, b"short").unwrap();
138+
let error = read_verified_mmdb_with_post_read(verified, |_| {}).unwrap_err();
139+
140+
assert_eq!(error.kind(), std::io::ErrorKind::UnexpectedEof);
141+
assert!(error.to_string().contains("became shorter while reading"));
142+
}

crates/fluxheim-geoip/src/lib.rs

Lines changed: 81 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,40 @@ pub struct GeoContext {
1010
asn: Option<u32>,
1111
}
1212

13+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
14+
pub enum GeoContextError {
15+
InvalidCountry,
16+
InvalidAsn,
17+
}
18+
19+
impl std::fmt::Display for GeoContextError {
20+
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
21+
match self {
22+
Self::InvalidCountry => {
23+
formatter.write_str("country must contain exactly two ASCII letters")
24+
}
25+
Self::InvalidAsn => formatter.write_str("ASN must be greater than zero"),
26+
}
27+
}
28+
}
29+
30+
impl std::error::Error for GeoContextError {}
31+
1332
impl GeoContext {
14-
pub fn new(country_iso: Option<String>, asn: Option<u32>) -> Self {
15-
Self { country_iso, asn }
33+
pub fn try_new(country_iso: Option<String>, asn: Option<u32>) -> Result<Self, GeoContextError> {
34+
let country_iso = match country_iso {
35+
Some(value)
36+
if value.len() == 2 && value.bytes().all(|byte| byte.is_ascii_alphabetic()) =>
37+
{
38+
Some(value.to_ascii_uppercase())
39+
}
40+
Some(_) => return Err(GeoContextError::InvalidCountry),
41+
None => None,
42+
};
43+
if asn == Some(0) {
44+
return Err(GeoContextError::InvalidAsn);
45+
}
46+
Ok(Self { country_iso, asn })
1647
}
1748

1849
pub fn country_iso(&self) -> Option<&str> {
@@ -149,7 +180,7 @@ mod geoip_runtime {
149180
}
150181
}
151182

152-
let context = GeoContext::new(country, asn);
183+
let context = GeoContext::try_new(country, asn).ok()?;
153184
(!context.is_empty()).then_some(context)
154185
}
155186
}
@@ -438,38 +469,44 @@ mod geoip_runtime {
438469
}
439470

440471
fn read_verified_mmdb(verified: VerifiedMmdbFile) -> io::Result<Vec<u8>> {
441-
read_verified_mmdb_with_post_read(verified, || {})
472+
read_verified_mmdb_with_post_read(verified, |_| {})
442473
}
443474

444475
fn read_verified_mmdb_with_post_read(
445476
mut verified: VerifiedMmdbFile,
446-
post_read: impl FnOnce(),
477+
post_read: impl FnOnce(usize),
447478
) -> io::Result<Vec<u8>> {
448-
let capacity = usize::try_from(verified.byte_len).map_err(|_| {
479+
let expected = usize::try_from(verified.byte_len).map_err(|_| {
449480
io::Error::new(
450481
io::ErrorKind::InvalidInput,
451482
"GeoIP database size cannot be represented on this platform",
452483
)
453484
})?;
454485
let mut contents = Vec::new();
455-
contents.try_reserve_exact(capacity).map_err(|error| {
486+
contents.try_reserve_exact(expected).map_err(|error| {
456487
io::Error::other(format!("failed to reserve GeoIP database buffer: {error}"))
457488
})?;
458-
let mut limited = verified
459-
.file
460-
.by_ref()
461-
.take(verified.byte_len.saturating_add(1));
462-
limited.read_to_end(&mut contents)?;
463-
if contents.len() as u64 != verified.byte_len {
489+
contents.resize(expected, 0);
490+
verified.file.read_exact(&mut contents).map_err(|error| {
491+
io::Error::new(
492+
error.kind(),
493+
format!(
494+
"GeoIP database became shorter while reading {}: {error}",
495+
verified.path.display()
496+
),
497+
)
498+
})?;
499+
post_read(contents.capacity());
500+
let mut extra = [0_u8; 1];
501+
if verified.file.read(&mut extra)? != 0 {
464502
return Err(io::Error::new(
465503
io::ErrorKind::InvalidInput,
466504
format!(
467-
"GeoIP database size changed while reading: {}",
505+
"GeoIP database grew while reading: {}",
468506
verified.path.display()
469507
),
470508
));
471509
}
472-
post_read();
473510
let final_state = MmdbFileState::from_metadata(&verified.file.metadata()?);
474511
if final_state != verified.state {
475512
return Err(io::Error::new(
@@ -486,3 +523,32 @@ mod geoip_runtime {
486523

487524
#[cfg(feature = "runtime")]
488525
pub use geoip_runtime::{GeoIpPolicyUsage, GeoIpRuntime};
526+
527+
#[cfg(test)]
528+
mod geo_context_tests {
529+
use super::{GeoContext, GeoContextError};
530+
531+
#[test]
532+
fn public_constructor_normalizes_valid_security_context() {
533+
let context = GeoContext::try_new(Some("se".to_owned()), Some(64_512)).unwrap();
534+
535+
assert_eq!(context.country_iso(), Some("SE"));
536+
assert_eq!(context.asn(), Some(64_512));
537+
assert!(!context.is_empty());
538+
assert!(GeoContext::try_new(None, None).unwrap().is_empty());
539+
}
540+
541+
#[test]
542+
fn public_constructor_rejects_invalid_security_context() {
543+
for country in ["", "S", "USA", "S1", "SÉ", " SE"] {
544+
assert_eq!(
545+
GeoContext::try_new(Some(country.to_owned()), None),
546+
Err(GeoContextError::InvalidCountry)
547+
);
548+
}
549+
assert_eq!(
550+
GeoContext::try_new(Some("SE".to_owned()), Some(0)),
551+
Err(GeoContextError::InvalidAsn)
552+
);
553+
}
554+
}

docs/config-reference.md

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3165,13 +3165,16 @@ later databases when possible.
31653165
Fluxheim does not download GeoIP databases in-process. Each MMDB file is capped
31663166
at 512 MiB, each loaded GeoIP runtime is capped at 1 GiB total, and at most
31673167
eight databases are accepted. Aggregate capacity is checked from the verified
3168-
open descriptor before its contents are allocated, read, or parsed. Paths must
3169-
be absolute and symlink-free; the file and every parent directory must be owned
3170-
by root, the effective service user, or the platform root-equivalent owner and
3171-
must not be group- or world-writable. Fluxheim rejects descriptor identity or
3172-
metadata changes during loading. GeoIP update jobs should prepare and verify a
3173-
replacement under a trusted directory, apply safe ownership and modes, then
3174-
atomically rename it into place before reloading Fluxheim.
3168+
open descriptor before its contents are allocated, read, or parsed. The loader
3169+
allocates exactly the admitted length, reads it completely, then probes for one
3170+
extra byte separately so concurrent file growth cannot amplify the heap
3171+
allocation. Paths must be absolute and symlink-free; the file and every parent
3172+
directory must be owned by root, the effective service user, or the platform
3173+
root-equivalent owner and must not be group- or world-writable. Fluxheim rejects
3174+
shortened, grown, descriptor-identity-changed, or metadata-changed databases
3175+
during loading. GeoIP update jobs should prepare and verify a replacement under
3176+
a trusted directory, apply safe ownership and modes, then atomically rename it
3177+
into place before reloading Fluxheim.
31753178

31763179
Vhost and route access policies can then use:
31773180

@@ -3184,8 +3187,10 @@ deny_asns = [64512]
31843187
```
31853188

31863189
Country values must be uppercase ISO alpha-2 codes. ASN values are numeric and
3187-
must be greater than zero. Geo allow lists fail closed when no Geo-Context is
3188-
available; Geo deny lists deny only on a resolved match. See
3190+
must be greater than zero. The Geo-Context API also validates these invariants
3191+
and canonicalizes valid country values before they can reach policy. Geo allow
3192+
lists fail closed when no Geo-Context is available; Geo deny lists deny only on
3193+
a resolved match. See
31893194
[`docs/geoip.md`](geoip.md) for the feature boundary and operational update
31903195
pattern.
31913196

docs/geoip.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,20 @@ rejected.
4343

4444
Each MMDB file is capped at 512 MiB at both metadata and read time. Fluxheim
4545
opens and inspects each descriptor, checks its size against the remaining 1 GiB
46-
aggregate allowance, and only then allocates, reads, and parses it. At most
47-
eight databases are accepted by both config validation and the runtime crate.
48-
During hot reload, the old and new runtimes can briefly coexist while in-flight
46+
aggregate allowance, and only then allocates an exact-length input buffer,
47+
reads that admitted length, and probes one extra byte without growing the heap
48+
buffer. Shortened, grown, or metadata-changed files fail loading. At most eight
49+
databases are accepted by both config validation and the runtime crate. During
50+
hot reload, the old and new runtimes can briefly coexist while in-flight
4951
requests finish, so size host/container memory for up to two admitted runtimes
5052
plus parser overhead.
5153

5254
Country fields are decoded as borrowed MMDB strings. Raw values longer than
5355
eight bytes are rejected before normalization or owned allocation; accepted
54-
values must normalize to exactly two ASCII letters.
56+
values must normalize to exactly two ASCII letters. Public `GeoContext`
57+
construction enforces the same invariant, canonicalizes accepted countries to
58+
uppercase, and rejects ASN zero so future policy consumers cannot introduce an
59+
invalid context through the crate API.
5560

5661
```toml
5762
[geoip]

release-notes/RELEASE_NOTES_1.7.8.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ general-purpose WASI application hosting.
5454
- Hold PHP memory bodies and bounded spool-read buffers in
5555
`sanitization::SecretVec`, clear consumed spool buffers immediately, and
5656
clear full buffer capacity on cancellation, error, or drop.
57+
- Read each verified GeoIP database into an exact admitted-length buffer and
58+
probe growth with a separate stack byte, preventing a one-byte in-place
59+
append from triggering large `Vec` capacity growth before rejection.
60+
- Validate public `GeoContext` construction, canonicalize accepted two-letter
61+
ASCII countries to uppercase, and reject ASN zero before policy consumers can
62+
observe malformed security state.
5763
- Replace inherited managed PHP-FPM `PATH` handling with a fixed allowlisted
5864
search path after clearing the child environment.
5965

0 commit comments

Comments
 (0)