Skip to content

Commit c16e1d0

Browse files
melvincarvalhomelvincarvalho
committed
review: write privkey AFTER ACLs to close WAC vacuum (JavaScriptSolidServer#444)
Address Copilot pickup on JavaScriptSolidServer#444. The previous reorder (privkey before profile) addressed orphan-VM-on-crash but introduced a separate issue: privkey was written *before* the ACL tree existed, so during the gap between privkey write and /private/.acl write the secret file had no WAC protection. jss's deny-by-default since #f43ecdf mitigates real exposure to 401, but relying on a security default holding is fragile — defence-in-depth beats it. Right ordering: ACLs (already in place) → privkey (born under owner-only WAC) → profile (still last so orphan-VM property preserved on crash). Same fix in createPodStructure (multi-user) and createRootPodStructure (single-user, where the exposure is more acute since the pod URL is the server origin). Both call sites now have a single block-comment block explaining the two distinct concerns the ordering serves: WAC vacuum + orphan VM. 854/854 tests pass.
1 parent d375f18 commit c16e1d0

2 files changed

Lines changed: 87 additions & 60 deletions

File tree

‎src/handlers/container.js‎

Lines changed: 45 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -192,40 +192,18 @@ export async function createPodStructure(name, webId, podUri, issuer, defaultQuo
192192

193193
// Optional: provision a Schnorr secp256k1 owner key. The keypair is
194194
// generated in memory up-front so its VM can be injected into the
195-
// WebID profile, then persisted to /private/privkey.jsonld *before*
196-
// the profile is written. This ordering matters: the WebID
197-
// profile advertises the VM, so a crash between profile and privkey
198-
// would leave the WebID permanently advertising an authentication
199-
// method whose secret was never persisted (#444 review). Writing
200-
// privkey first means the worst-case crash leaves an orphan secret
201-
// file (easy to delete) rather than an orphan VM in a public profile.
202-
// Strict `=== true` (not just truthy) so a misconfigured caller
203-
// passing `'true'` / `1` / etc. doesn't silently activate; matches
204-
// handleCreatePod's HTTP-side check on the body field.
195+
// WebID profile that gets written last. The on-disk persistence of
196+
// the secret is deferred to *after* the ACL tree is in place — see
197+
// the ordering block further below. Strict `=== true` (not just
198+
// truthy) so a misconfigured caller passing `'true'` / `1` / etc.
199+
// doesn't silently activate; matches handleCreatePod's HTTP-side
200+
// check on the body field.
205201
const ownerKey = options.provisionKeys === true
206202
? provisionOwnerKey({ webId })
207203
: null;
208204

209-
if (ownerKey) {
210-
const ok = await storage.write(
211-
`${podPath}private/privkey.jsonld`,
212-
JSON.stringify(ownerKey.document, null, 2),
213-
{ mode: 0o600 }
214-
);
215-
if (!ok) {
216-
throw new Error(
217-
`Failed to write owner key file at ${podPath}private/privkey.jsonld`
218-
);
219-
}
220-
}
221-
222-
// Generate and write WebID profile at /profile/card.jsonld. When
223-
// an owner key was provisioned, its VM lands in the profile so the
224-
// existing LWS-CID verifier (src/auth/lws-cid.js) can authenticate
225-
// JWTs signed with the matching secret. The privkey file already
226-
// exists on disk at this point — see ordering rationale above.
227-
const profile = generateProfile({ webId, name, podUri, issuer, ownerVm: ownerKey?.vm });
228-
await storage.write(`${podPath}profile/card.jsonld`, serialize(profile));
205+
// Profile is written last (see the ACL/privkey block below). Skip
206+
// the write here; we'll do it after privkey lands on disk.
229207

230208
// Generate and write preferences
231209
const prefs = generatePreferences({ webId, podUri });
@@ -281,8 +259,43 @@ export async function createPodStructure(name, webId, podUri, issuer, defaultQuo
281259
await initializeQuota(name, defaultQuota);
282260
}
283261

284-
// (privkey was written above, before the profile, to avoid
285-
// orphan-VM-on-crash. Nothing more to do here.)
262+
// Owner-key persistence + profile write (when --provision-keys is on).
263+
// Order is load-bearing for two distinct concerns (#444 review):
264+
//
265+
// 1. WAC vacuum: write privkey *after* the ACL tree is in place so
266+
// the secret file is born under owner-only WAC. Without this,
267+
// there's a window where the file exists but no /private/.acl
268+
// protects it; jss's deny-by-default since #f43ecdf would
269+
// mitigate to 401, but defence-in-depth beats relying on a
270+
// security default holding.
271+
//
272+
// 2. Orphan-VM: write privkey *before* the profile so a crash
273+
// between the two leaves an orphan secret file (easy to delete)
274+
// rather than an orphan VM in a published WebID profile that
275+
// forever advertises an authentication method whose secret was
276+
// never persisted.
277+
//
278+
// Combined: ACLs (above) → privkey (here) → profile (next).
279+
if (ownerKey) {
280+
const ok = await storage.write(
281+
`${podPath}private/privkey.jsonld`,
282+
JSON.stringify(ownerKey.document, null, 2),
283+
{ mode: 0o600 }
284+
);
285+
if (!ok) {
286+
throw new Error(
287+
`Failed to write owner key file at ${podPath}private/privkey.jsonld`
288+
);
289+
}
290+
}
291+
292+
// Generate and write WebID profile at /profile/card.jsonld. When an
293+
// owner key was provisioned, its VM lands in the profile so the
294+
// existing LWS-CID verifier (src/auth/lws-cid.js) can authenticate
295+
// JWTs signed with the matching secret. Profile is intentionally
296+
// written last — see ordering rationale above.
297+
const profile = generateProfile({ webId, name, podUri, issuer, ownerVm: ownerKey?.vm });
298+
await storage.write(`${podPath}profile/card.jsonld`, serialize(profile));
286299

287300
// Spread `ownerKey` only when set so the field is genuinely absent
288301
// (not `null`) on the no-provisioning path — matches the existing

‎src/server.js‎

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,36 +1046,15 @@ export function createServer(options = {}) {
10461046
await storage.createContainer('/profile/');
10471047

10481048
// Generate the owner key in memory up-front (when --provision-keys
1049-
// is set), persist it to /private/privkey.jsonld *before* the
1050-
// WebID profile is written. The profile advertises the VM, so a
1051-
// crash between profile and privkey would leave the WebID
1052-
// permanently advertising an authentication method whose secret
1053-
// never persisted (#444 review). Writing privkey first means the
1054-
// worst-case crash leaves an orphan secret file (easy to delete)
1055-
// rather than an orphan VM in a public profile.
1049+
// is set) so its VM can be injected into the WebID profile that
1050+
// gets written last. On-disk persistence of the secret happens
1051+
// *after* the ACL tree is in place — see the ordering block
1052+
// further below.
10561053
const ownerKey = provisionKeysEnabled
10571054
? provisionOwnerKey({ webId })
10581055
: null;
10591056

1060-
if (ownerKey) {
1061-
const ok = await storage.write(
1062-
'/private/privkey.jsonld',
1063-
JSON.stringify(ownerKey.document, null, 2),
1064-
{ mode: 0o600 }
1065-
);
1066-
if (!ok) {
1067-
throw new Error(
1068-
'Failed to write owner key file at /private/privkey.jsonld'
1069-
);
1070-
}
1071-
}
1072-
1073-
// Generate profile (with the owner key's VM landed in
1074-
// verificationMethod when --provision-keys is on). The privkey
1075-
// file already exists on disk at this point — see ordering
1076-
// rationale above.
1077-
const profile = generateProfile({ webId, name: displayName, podUri, issuer, ownerVm: ownerKey?.vm });
1078-
await storage.write('/profile/card.jsonld', serialize(profile));
1057+
// Profile is written last (see the ACL/privkey block below).
10791058

10801059
// Preferences and type indexes
10811060
const prefs = generatePreferences({ webId, podUri });
@@ -1118,8 +1097,43 @@ export function createServer(options = {}) {
11181097
const profileAcl = generatePublicFolderAcl('./', owner('profile/'));
11191098
await storage.write('/profile/.acl', serializeAcl(profileAcl));
11201099

1121-
// (privkey was written above, before the profile, to avoid
1122-
// orphan-VM-on-crash. Nothing more to do here.)
1100+
// Owner-key persistence + profile write (when --provision-keys is
1101+
// on). Order is load-bearing for two distinct concerns
1102+
// (#444 review):
1103+
//
1104+
// 1. WAC vacuum: write privkey *after* the ACL tree is in place
1105+
// so the secret file is born under owner-only WAC. Without
1106+
// this, there's a window where the file exists but no
1107+
// /private/.acl protects it; deny-by-default since #f43ecdf
1108+
// would mitigate to 401, but defence-in-depth beats relying
1109+
// on a security default holding. The single-user root pod is
1110+
// especially exposed since the URL is the server origin.
1111+
//
1112+
// 2. Orphan-VM: write privkey *before* the profile so a crash
1113+
// between the two leaves an orphan secret file (easy to
1114+
// delete) rather than an orphan VM in a published WebID
1115+
// profile that forever advertises an authentication method
1116+
// whose secret was never persisted.
1117+
//
1118+
// Combined: ACLs (above) → privkey (here) → profile (next).
1119+
if (ownerKey) {
1120+
const ok = await storage.write(
1121+
'/private/privkey.jsonld',
1122+
JSON.stringify(ownerKey.document, null, 2),
1123+
{ mode: 0o600 }
1124+
);
1125+
if (!ok) {
1126+
throw new Error(
1127+
'Failed to write owner key file at /private/privkey.jsonld'
1128+
);
1129+
}
1130+
}
1131+
1132+
// Generate profile (with the owner key's VM landed in
1133+
// verificationMethod when --provision-keys is on). Written last —
1134+
// see ordering rationale above.
1135+
const profile = generateProfile({ webId, name: displayName, podUri, issuer, ownerVm: ownerKey?.vm });
1136+
await storage.write('/profile/card.jsonld', serialize(profile));
11231137

11241138
// Note: Quota not initialized for root-level pods (no user directory).
11251139
// Spread `ownerKey` only when set so the field is genuinely absent

0 commit comments

Comments
 (0)