Skip to content

Commit f0d93fd

Browse files
committed
fix(gl): pin secret-file modes at creation instead of chmod after (#354)
identity.pem and ucan.json were written with fs::write and then chmod'd, leaving the private key world-readable between the two syscalls, and ucan.json was never chmod'd at all. A shared helper now opens with mode(0o600) so no permissive window exists, adds O_NOFOLLOW so a pre-planted symlink is refused rather than written through, and re-pins the mode after write so a pre-existing loose file is tightened. The containing directories are created 0700 via DirBuilder and re-pinned the same way. gitlawb-node's own key write gets the same treatment.
1 parent bfc44f9 commit f0d93fd

10 files changed

Lines changed: 174 additions & 73 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/gitlawb-node/src/main.rs‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,18 +1375,40 @@ fn load_or_create_keypair(config: &Config) -> Result<Keypair> {
13751375
.to_pem()
13761376
.map_err(|e| anyhow::anyhow!("failed to serialize key: {e}"))?;
13771377

1378-
if let Some(parent) = key_path.parent() {
1379-
std::fs::create_dir_all(parent)?;
1380-
}
1381-
13821378
#[cfg(unix)]
13831379
{
1384-
use std::os::unix::fs::PermissionsExt;
1385-
std::fs::write(&key_path, pem.as_bytes())?;
1386-
std::fs::set_permissions(&key_path, std::fs::Permissions::from_mode(0o600))?;
1380+
use std::io::Write;
1381+
use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt, PermissionsExt};
1382+
1383+
if let Some(parent) = key_path.parent() {
1384+
std::fs::DirBuilder::new()
1385+
.recursive(true)
1386+
.mode(0o700)
1387+
.create(parent)?;
1388+
}
1389+
1390+
// Pin the mode at creation: a plain write then chmod leaves the
1391+
// key world-readable between the two syscalls, and O_NOFOLLOW
1392+
// refuses to write through a pre-planted symlink.
1393+
let mut f = std::fs::OpenOptions::new()
1394+
.write(true)
1395+
.create(true)
1396+
.truncate(true)
1397+
.mode(0o600)
1398+
.custom_flags(libc::O_NOFOLLOW)
1399+
.open(&key_path)?;
1400+
f.write_all(pem.as_bytes())?;
1401+
// mode() only applies when the file is created; re-pin so a
1402+
// pre-existing loose file is tightened rather than left readable.
1403+
f.set_permissions(std::fs::Permissions::from_mode(0o600))?;
13871404
}
13881405
#[cfg(not(unix))]
1389-
std::fs::write(&key_path, pem.as_bytes())?;
1406+
{
1407+
if let Some(parent) = key_path.parent() {
1408+
std::fs::create_dir_all(parent)?;
1409+
}
1410+
std::fs::write(&key_path, pem.as_bytes())?;
1411+
}
13901412

13911413
info!(path = %key_path.display(), did = %kp.did(), "generated new node identity");
13921414
Ok(kp)

‎crates/gl/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ dirs = "5"
2727
reqwest = { workspace = true }
2828
uuid = { workspace = true }
2929
urlencoding = "2"
30+
libc = "0.2"
3031
alloy = { version = "1", default-features = false, features = [
3132
"contract",
3233
"provider-http",

‎crates/gl/src/identity.rs‎

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -131,23 +131,13 @@ async fn cmd_new_with_reader(
131131
}
132132
}
133133

134-
fs::create_dir_all(&dir)
134+
crate::secret_file::create_dir(&dir)
135135
.with_context(|| format!("failed to create directory {}", dir.display()))?;
136136

137137
let keypair = Keypair::generate();
138138
let pem = keypair.to_pem()?;
139139

140-
// Write with restricted permissions
141-
#[cfg(unix)]
142-
{
143-
use std::os::unix::fs::PermissionsExt;
144-
fs::write(&path, pem.as_bytes())?;
145-
fs::set_permissions(&path, fs::Permissions::from_mode(0o600))?;
146-
}
147-
#[cfg(not(unix))]
148-
{
149-
fs::write(&path, pem.as_bytes())?;
150-
}
140+
crate::secret_file::write(&path, pem.as_bytes())?;
151141

152142
let did = keypair.did();
153143
println!("✓ Generated new identity");
@@ -202,16 +192,7 @@ async fn cmd_backup(out: Option<PathBuf>, dir: Option<PathBuf>) -> Result<()> {
202192
.join("identity.pem.bak")
203193
});
204194

205-
#[cfg(unix)]
206-
{
207-
use std::os::unix::fs::PermissionsExt;
208-
fs::write(&dest, pem.as_bytes())?;
209-
fs::set_permissions(&dest, fs::Permissions::from_mode(0o600))?;
210-
}
211-
#[cfg(not(unix))]
212-
{
213-
fs::write(&dest, pem.as_bytes())?;
214-
}
195+
crate::secret_file::write(&dest, pem.as_bytes())?;
215196

216197
println!("✓ Identity backed up");
217198
println!(" DID: {}", keypair.did());
@@ -262,19 +243,10 @@ async fn cmd_restore_with_reader(
262243
}
263244
}
264245

265-
fs::create_dir_all(&base)
246+
crate::secret_file::create_dir(&base)
266247
.with_context(|| format!("failed to create directory {}", base.display()))?;
267248

268-
#[cfg(unix)]
269-
{
270-
use std::os::unix::fs::PermissionsExt;
271-
fs::write(&dest, pem.as_bytes())?;
272-
fs::set_permissions(&dest, fs::Permissions::from_mode(0o600))?;
273-
}
274-
#[cfg(not(unix))]
275-
{
276-
fs::write(&dest, pem.as_bytes())?;
277-
}
249+
crate::secret_file::write(&dest, pem.as_bytes())?;
278250

279251
println!("✓ Identity restored");
280252
println!(" DID: {}", keypair.did());

‎crates/gl/src/init.rs‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,16 @@ pub async fn run(args: InitArgs) -> Result<()> {
105105
.dir
106106
.clone()
107107
.unwrap_or_else(|| dirs::home_dir().unwrap_or_default().join(".gitlawb"));
108-
std::fs::create_dir_all(&ucan_dir)?;
108+
crate::secret_file::create_dir(&ucan_dir)?;
109109
let record = json!({
110110
"ucan": ucan,
111111
"node": args.node,
112112
"did": did.to_string(),
113113
"saved_at": chrono::Utc::now().to_rfc3339(),
114114
});
115-
std::fs::write(
116-
ucan_dir.join("ucan.json"),
117-
serde_json::to_string_pretty(&record)?,
115+
crate::secret_file::write(
116+
&ucan_dir.join("ucan.json"),
117+
serde_json::to_string_pretty(&record)?.as_bytes(),
118118
)?;
119119
}
120120
}
@@ -228,22 +228,13 @@ fn generate_identity(dir: Option<&std::path::Path>) -> Result<gitlawb_core::iden
228228
.context("could not determine home directory")?
229229
.join(".gitlawb")
230230
};
231-
std::fs::create_dir_all(&base)?;
231+
crate::secret_file::create_dir(&base)?;
232232

233233
let keypair = gitlawb_core::identity::Keypair::generate();
234234
let pem = keypair.to_pem()?;
235235
let path = base.join("identity.pem");
236236

237-
#[cfg(unix)]
238-
{
239-
use std::os::unix::fs::PermissionsExt;
240-
std::fs::write(&path, pem.as_bytes())?;
241-
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600))?;
242-
}
243-
#[cfg(not(unix))]
244-
{
245-
std::fs::write(&path, pem.as_bytes())?;
246-
}
237+
crate::secret_file::write(&path, pem.as_bytes())?;
247238

248239
Ok(keypair)
249240
}

‎crates/gl/src/main.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ mod protect;
2626
mod quickstart;
2727
mod register;
2828
mod repo;
29+
mod secret_file;
2930
mod star;
3031
mod status;
3132
mod sync;

‎crates/gl/src/quickstart.rs‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,17 @@ pub async fn run(args: QuickstartArgs) -> Result<()> {
100100
let payload: Value = resp.json().await.unwrap_or_default();
101101
let ucan = payload["ucan"].as_str().unwrap_or("");
102102
if !ucan.is_empty() {
103-
std::fs::create_dir_all(&dir)?;
103+
crate::secret_file::create_dir(&dir)?;
104104
let record = json!({
105105
"ucan": ucan,
106106
"node": args.node,
107107
"did": did,
108108
"saved_at": chrono::Utc::now().to_rfc3339(),
109109
});
110-
std::fs::write(&ucan_path, serde_json::to_string_pretty(&record)?)?;
110+
crate::secret_file::write(
111+
&ucan_path,
112+
serde_json::to_string_pretty(&record)?.as_bytes(),
113+
)?;
111114
}
112115
let trust = payload["trust_score"].as_f64().unwrap_or(0.0);
113116
println!(" ✓ Registered successfully");
@@ -226,23 +229,15 @@ pub async fn run(args: QuickstartArgs) -> Result<()> {
226229

227230
// ── Helpers ───────────────────────────────────────────────────────────────
228231

229-
fn generate_identity(dir: &PathBuf) -> Result<gitlawb_core::identity::Keypair> {
230-
std::fs::create_dir_all(dir).with_context(|| format!("failed to create {}", dir.display()))?;
232+
fn generate_identity(dir: &std::path::Path) -> Result<gitlawb_core::identity::Keypair> {
233+
crate::secret_file::create_dir(dir)
234+
.with_context(|| format!("failed to create {}", dir.display()))?;
231235

232236
let keypair = gitlawb_core::identity::Keypair::generate();
233237
let pem = keypair.to_pem()?;
234238
let path = dir.join("identity.pem");
235239

236-
#[cfg(unix)]
237-
{
238-
use std::os::unix::fs::PermissionsExt;
239-
std::fs::write(&path, pem.as_bytes())?;
240-
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600))?;
241-
}
242-
#[cfg(not(unix))]
243-
{
244-
std::fs::write(&path, pem.as_bytes())?;
245-
}
240+
crate::secret_file::write(&path, pem.as_bytes())?;
246241

247242
let did = keypair.did();
248243
println!(" ✓ Generated new identity");

‎crates/gl/src/register.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,10 @@ pub async fn run(args: RegisterArgs) -> Result<()> {
7777
"did": did.to_string(),
7878
"saved_at": chrono::Utc::now().to_rfc3339(),
7979
});
80-
std::fs::write(&ucan_path, serde_json::to_string_pretty(&record)?)?;
80+
crate::secret_file::write(
81+
&ucan_path,
82+
serde_json::to_string_pretty(&record)?.as_bytes(),
83+
)?;
8184
tracing::debug!("saved UCAN to {}", ucan_path.display());
8285
}
8386

@@ -113,7 +116,7 @@ fn ucan_path(dir: Option<&std::path::Path>) -> Result<PathBuf> {
113116
.context("could not determine home directory")?
114117
.join(".gitlawb")
115118
};
116-
std::fs::create_dir_all(&base)?;
119+
crate::secret_file::create_dir(&base)?;
117120
Ok(base.join("ucan.json"))
118121
}
119122

‎crates/gl/src/secret_file.rs‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
//! Writes and directories that hold key material or bearer tokens.
2+
3+
use std::io::Write;
4+
use std::path::Path;
5+
6+
/// Write `contents` to `path` with owner-only permissions.
7+
///
8+
/// On unix the mode is pinned at creation, so the file never exists with a
9+
/// permissive mode between open and chmod. `O_NOFOLLOW` refuses to write
10+
/// through a pre-planted symlink. `mode()` applies only when the file is
11+
/// created, so the mode is also set after the write to tighten a
12+
/// pre-existing loose file.
13+
pub(crate) fn write(path: &Path, contents: &[u8]) -> std::io::Result<()> {
14+
#[cfg(unix)]
15+
{
16+
use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
17+
let mut f = std::fs::OpenOptions::new()
18+
.write(true)
19+
.create(true)
20+
.truncate(true)
21+
.mode(0o600)
22+
.custom_flags(libc::O_NOFOLLOW)
23+
.open(path)?;
24+
f.write_all(contents)?;
25+
f.set_permissions(std::fs::Permissions::from_mode(0o600))?;
26+
}
27+
#[cfg(not(unix))]
28+
std::fs::write(path, contents)?;
29+
Ok(())
30+
}
31+
32+
/// Create `path` (and missing parents) as an owner-only directory.
33+
///
34+
/// An existing directory is also re-pinned: a `~/.gitlawb` created before
35+
/// this helper stays group- and world-listable otherwise.
36+
pub(crate) fn create_dir(path: &Path) -> std::io::Result<()> {
37+
#[cfg(unix)]
38+
{
39+
use std::os::unix::fs::{DirBuilderExt, PermissionsExt};
40+
std::fs::DirBuilder::new()
41+
.recursive(true)
42+
.mode(0o700)
43+
.create(path)?;
44+
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700))?;
45+
}
46+
#[cfg(not(unix))]
47+
std::fs::create_dir_all(path)?;
48+
Ok(())
49+
}
50+
51+
#[cfg(test)]
52+
mod tests {
53+
use tempfile::TempDir;
54+
55+
#[test]
56+
#[cfg(unix)]
57+
fn write_creates_file_with_0600() {
58+
use std::os::unix::fs::PermissionsExt;
59+
let dir = TempDir::new().unwrap();
60+
let path = dir.path().join("key.pem");
61+
super::write(&path, b"secret").unwrap();
62+
assert_eq!(std::fs::read(&path).unwrap(), b"secret");
63+
assert_eq!(
64+
std::fs::metadata(&path).unwrap().permissions().mode() & 0o777,
65+
0o600
66+
);
67+
}
68+
69+
#[test]
70+
#[cfg(unix)]
71+
fn write_tightens_preexisting_loose_file() {
72+
use std::os::unix::fs::PermissionsExt;
73+
let dir = TempDir::new().unwrap();
74+
let path = dir.path().join("key.pem");
75+
std::fs::write(&path, b"old").unwrap();
76+
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap();
77+
super::write(&path, b"new").unwrap();
78+
assert_eq!(std::fs::read(&path).unwrap(), b"new");
79+
assert_eq!(
80+
std::fs::metadata(&path).unwrap().permissions().mode() & 0o777,
81+
0o600
82+
);
83+
}
84+
85+
#[test]
86+
#[cfg(unix)]
87+
fn write_refuses_symlink() {
88+
let dir = TempDir::new().unwrap();
89+
let target = dir.path().join("target.pem");
90+
std::fs::write(&target, b"planted").unwrap();
91+
let link = dir.path().join("link.pem");
92+
std::os::unix::fs::symlink(&target, &link).unwrap();
93+
assert!(super::write(&link, b"secret").is_err());
94+
assert_eq!(std::fs::read(&target).unwrap(), b"planted");
95+
}
96+
97+
#[test]
98+
#[cfg(unix)]
99+
fn create_dir_modes_0700_and_tightens_existing() {
100+
use std::os::unix::fs::PermissionsExt;
101+
let dir = TempDir::new().unwrap();
102+
let path = dir.path().join("a").join("b");
103+
super::create_dir(&path).unwrap();
104+
assert_eq!(
105+
std::fs::metadata(&path).unwrap().permissions().mode() & 0o777,
106+
0o700
107+
);
108+
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap();
109+
super::create_dir(&path).unwrap();
110+
assert_eq!(
111+
std::fs::metadata(&path).unwrap().permissions().mode() & 0o777,
112+
0o700
113+
);
114+
}
115+
}

‎crates/gl/src/ucan_cmd.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ async fn cmd_delegate(
9090
let encoded = ucan.encode()?;
9191

9292
if let Some(path) = out {
93-
std::fs::write(&path, &encoded)?;
93+
crate::secret_file::write(&path, encoded.as_bytes())?;
9494
println!("UCAN saved to {}", path.display());
9595
return Ok(());
9696
}

0 commit comments

Comments
 (0)