Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
db4552f4e4 |
No files matched your search
@@ -100,7 +100,7 @@ diffing the two copies and finding nothing but a name between them.
|
|||||||
- **`netif`** — `wg_address()`, which fails closed when the tunnel is down, and `local_addresses()` for the certificate's SANs. The product name is a parameter so the failure reads as advice rather than as a library complaining. Both split the *lookup* from the *decision*, so the failure path can be tested on a machine that has a tunnel — every machine this runs on does.
|
- **`netif`** — `wg_address()`, which fails closed when the tunnel is down, and `local_addresses()` for the certificate's SANs. The product name is a parameter so the failure reads as advice rather than as a library complaining. Both split the *lookup* from the *decision*, so the failure path can be tested on a machine that has a tunnel — every machine this runs on does.
|
||||||
- **`enroll`** — token generation, hex-SHA-256 storage, constant-time comparison, the `<scheme>://enroll?…` URI, and the terminal QR. The URI scheme is the parameter, because it is what routes a scan back to the right app.
|
- **`enroll`** — token generation, hex-SHA-256 storage, constant-time comparison, the `<scheme>://enroll?…` URI, and the terminal QR. The URI scheme is the parameter, because it is what routes a scan back to the right app.
|
||||||
- **`certs`** — the CA generated once and never replaced, the leaf reissued every start. `product` names the organisation and common name and is the whole of what is per-project. Shared because being written twice is worst here: a trust anchor built two ways can be built differently two ways, and the difference surfaces as an opaque handshake failure on a phone.
|
- **`certs`** — the CA generated once and never replaced, the leaf reissued every start. `product` names the organisation and common name and is the whole of what is per-project. Shared because being written twice is worst here: a trust anchor built two ways can be built differently two ways, and the difference surfaces as an opaque handshake failure on a phone.
|
||||||
- **`format`** — the two RON house rules. This was byte-identical in both projects, which made it the clearest thing in the evidence table and the easiest deletion.
|
- **`format`** — the two RON house rules, plus `write`, which renders a value and replaces a file with it atomically and owner-only. This was byte-identical in both projects, which made it the clearest thing in the evidence table and the easiest deletion.
|
||||||
- **`private`** — owner-only files and directories, taken from ai-app's version, with an `append_file` alongside `create_file` because a transcript must never be truncated by being opened.
|
- **`private`** — owner-only files and directories, taken from ai-app's version, with an `append_file` alongside `create_file` because a transcript must never be truncated by being opened.
|
||||||
- **`xdg`** — where each project's state lives: `config_home(product)` and `data_home(product)`, resolving the XDG variable, ignoring it unless absolute, and falling back under `$HOME`. Shared because the *reason* is shared and is not obvious from the code — the repo is a mount that resolves at different absolute paths on each side, so config inside it records paths that work on only one, and a CA private key inside it would let the untrusted side mint a leaf the pinned app trusts.
|
- **`xdg`** — where each project's state lives: `config_home(product)` and `data_home(product)`, resolving the XDG variable, ignoring it unless absolute, and falling back under `$HOME`. Shared because the *reason* is shared and is not obvious from the code — the repo is a mount that resolves at different absolute paths on each side, so config inside it records paths that work on only one, and a CA private key inside it would let the untrusted side mint a leaf the pinned app trusts.
|
||||||
|
|
||||||
@@ -157,7 +157,7 @@ nowhere near the generator. Worth revisiting if it ever needs a second fix.
|
|||||||
**Should follow, in this order.** Each is already near-identical:
|
**Should follow, in this order.** Each is already near-identical:
|
||||||
|
|
||||||
1. ~~The Kotlin `EnrollmentScanActivity`, `PinnedCert`, and the enrollment/Keystore half of `ServerConfig`.~~ Done — see `app/` above. It paid as predicted: ai-app's `ServerConfig` had gained `localNetworkAllowed` and the KTX `edit` block while dev-updater's had not, so the two had drifted in both directions exactly as the evidence suggested.
|
1. ~~The Kotlin `EnrollmentScanActivity`, `PinnedCert`, and the enrollment/Keystore half of `ServerConfig`.~~ Done — see `app/` above. It paid as predicted: ai-app's `ServerConfig` had gained `localNetworkAllowed` and the KTX `edit` block while dev-updater's had not, so the two had drifted in both directions exactly as the evidence suggested.
|
||||||
2. Atomic owner-only config save. Both do temp-file-then-rename with the mode set before the rename; only the schema differs.
|
2. ~~Atomic owner-only config save.~~ Done — `format::write`, since it is the RON house rules and the owner-only rules used together. The subtlety it now states once is that the temp file is *not always new*: a save killed partway leaves one behind, and reopening it keeps whatever mode it had, which is then renamed over the file holding the enrolled token hashes. There is a test for exactly that, because it cannot happen on a machine where nothing has ever crashed mid-save.
|
||||||
|
|
||||||
**Should not.** Naming these is the point of the exercise:
|
**Should not.** Naming these is the point of the exercise:
|
||||||
|
|
||||||
|
|||||||
@@ -35,8 +35,13 @@
|
|||||||
/// `#![enable(implicit_some)]` header every project file would have to
|
/// `#![enable(implicit_some)]` header every project file would have to
|
||||||
/// remember, and matched on the writing side by `skip_serializing_if` so
|
/// remember, and matched on the writing side by `skip_serializing_if` so
|
||||||
/// nothing writes back a `Some(...)` a person didn't type.
|
/// nothing writes back a `Some(...)` a person didn't type.
|
||||||
|
use std::path::Path;
|
||||||
|
|
||||||
|
use anyhow::{Context, Result};
|
||||||
use serde::{Serialize, de::DeserializeOwned};
|
use serde::{Serialize, de::DeserializeOwned};
|
||||||
|
|
||||||
|
use crate::private;
|
||||||
|
|
||||||
fn options() -> ron::Options {
|
fn options() -> ron::Options {
|
||||||
ron::Options::default().with_default_extension(ron::extensions::Extensions::IMPLICIT_SOME)
|
ron::Options::default().with_default_extension(ron::extensions::Extensions::IMPLICIT_SOME)
|
||||||
}
|
}
|
||||||
@@ -51,6 +56,40 @@ pub fn render<T: Serialize>(value: &T) -> Result<String, ron::Error> {
|
|||||||
Ok(unwrap_outer(&text))
|
Ok(unwrap_outer(&text))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Renders `value` and replaces `path` with it, atomically and owner-only.
|
||||||
|
///
|
||||||
|
/// Whole-file-and-rename rather than an in-place edit, because both
|
||||||
|
/// projects' config files are small, are read at startup, and hold the
|
||||||
|
/// enrolled token hashes -- a half-written one would take the server down
|
||||||
|
/// on its next start with no way to fix it from a phone. The rename is
|
||||||
|
/// what makes a reader see either the old file or the new one and never
|
||||||
|
/// part of both.
|
||||||
|
///
|
||||||
|
/// The temp file goes through [`private::write_file`] rather than
|
||||||
|
/// `std::fs::write`, and that is the subtle half: **the temp file is not
|
||||||
|
/// always new.** A save killed partway leaves one behind, and opening that
|
||||||
|
/// again keeps whatever mode it already had -- which is then renamed over
|
||||||
|
/// the file holding the token hashes. Setting the mode as it is opened
|
||||||
|
/// covers both the fresh and the leftover case.
|
||||||
|
pub fn write<T: Serialize>(path: &Path, value: &T) -> Result<()> {
|
||||||
|
if let Some(parent) = path.parent() {
|
||||||
|
private::create_dir(parent)?;
|
||||||
|
}
|
||||||
|
let text = render(value).context("serialize config")?;
|
||||||
|
// Appended rather than substituted, so `config.ron` yields
|
||||||
|
// `config.ron.tmp` and not `config.tmp` -- a name that cannot collide
|
||||||
|
// with a real file and that says what it is a temporary copy of.
|
||||||
|
let tmp = path.with_file_name(format!(
|
||||||
|
"{}.tmp",
|
||||||
|
path.file_name()
|
||||||
|
.unwrap_or_else(|| std::ffi::OsStr::new("config"))
|
||||||
|
.to_string_lossy()
|
||||||
|
));
|
||||||
|
private::write_file(&tmp, text.as_bytes())?;
|
||||||
|
std::fs::rename(&tmp, path)
|
||||||
|
.with_context(|| format!("replace {} with {}", path.display(), tmp.display()))
|
||||||
|
}
|
||||||
|
|
||||||
/// Strips the outer `(`/`)` the writer always emits and removes the
|
/// Strips the outer `(`/`)` the writer always emits and removes the
|
||||||
/// indent level they cost. Deliberately narrow: it accepts only the
|
/// indent level they cost. Deliberately narrow: it accepts only the
|
||||||
/// exact shape `PrettyConfig` produces, and leaves anything else alone
|
/// exact shape `PrettyConfig` produces, and leaves anything else alone
|
||||||
@@ -146,6 +185,57 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn what_is_written_reads_back_and_is_owner_only() {
|
||||||
|
use std::os::unix::fs::PermissionsExt;
|
||||||
|
|
||||||
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
let path = dir.path().join("nested").join("config.ron");
|
||||||
|
let value = Demo {
|
||||||
|
name: "thing".to_string(),
|
||||||
|
note: None,
|
||||||
|
};
|
||||||
|
|
||||||
|
write(&path, &value).expect("write");
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
parse::<Demo>(&std::fs::read_to_string(&path).expect("read")).expect("re-read"),
|
||||||
|
value,
|
||||||
|
);
|
||||||
|
let mode = std::fs::metadata(&path).expect("stat").permissions().mode();
|
||||||
|
assert_eq!(mode & 0o777, 0o600, "config holds token hashes: {mode:o}");
|
||||||
|
assert!(
|
||||||
|
!path.with_extension("ron.tmp").exists(),
|
||||||
|
"the temp file is renamed away, not left behind",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The case the whole thing turns on, and the one that cannot happen on
|
||||||
|
/// a machine where nothing has ever crashed mid-save: a leftover temp
|
||||||
|
/// file from an interrupted write is reopened, and if its mode came
|
||||||
|
/// along it would be renamed straight over the file holding the enrolled
|
||||||
|
/// token hashes.
|
||||||
|
#[test]
|
||||||
|
fn a_leftover_temp_file_cannot_widen_the_config() {
|
||||||
|
use std::os::unix::fs::PermissionsExt;
|
||||||
|
|
||||||
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
let path = dir.path().join("config.ron");
|
||||||
|
let tmp = dir.path().join("config.ron.tmp");
|
||||||
|
|
||||||
|
std::fs::write(&tmp, b"leftover from a save that died").expect("stale temp");
|
||||||
|
std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o644)).expect("widen");
|
||||||
|
|
||||||
|
write(&path, &Demo::default()).expect("write");
|
||||||
|
|
||||||
|
let mode = std::fs::metadata(&path).expect("stat").permissions().mode();
|
||||||
|
assert_eq!(
|
||||||
|
mode & 0o777,
|
||||||
|
0o600,
|
||||||
|
"a world-readable leftover must not become the config: {mode:o}",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// A parse error's line number has to point at the real line, which is
|
/// A parse error's line number has to point at the real line, which is
|
||||||
/// why the opening paren is not followed by a newline.
|
/// why the opening paren is not followed by a newline.
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
Reference in new issue
Block a user