From 5752918068c192956de7eb9cc032ac3ad165bb92 Mon Sep 17 00:00:00 2001 From: Radon Rosborough Date: Thu, 7 Jan 2021 21:09:27 -0800 Subject: [PATCH] Make users.js robust to concurrency --- backend/api.js | 10 ++-- backend/sandbox.js | 11 +--- backend/users.js | 125 +++++++++++++++++++++++------------------- backend/util.js | 20 +++++++ docker/app/Dockerfile | 1 + 5 files changed, 99 insertions(+), 68 deletions(-) diff --git a/backend/api.js b/backend/api.js index 1083a4b..f28585c 100644 --- a/backend/api.js +++ b/backend/api.js @@ -28,8 +28,8 @@ export class Session { return this.uidInfo.uid; } - returnUID = async () => { - this.uidInfo && (await this.uidInfo.returnUID()); + returnUser = async () => { + this.uidInfo && (await this.uidInfo.returnUser()); }; get context() { @@ -65,8 +65,8 @@ export class Session { setup = async () => { try { allSessions.add(this); - const { uid, returnUID } = await borrowUser(this.log); - this.uidInfo = { uid, returnUID }; + const { uid, returnUser } = await borrowUser(); + this.uidInfo = { uid, returnUser }; this.log(`Borrowed uid ${this.uid}`); await this.run(this.privilegedSetup()); if (this.config.setup) { @@ -425,7 +425,7 @@ export class Session { allSessions.delete(this); if (this.uidInfo) { await this.run(this.privilegedTeardown()); - await this.returnUID(); + await this.returnUser(); } this.ws.terminate(); } catch (err) { diff --git a/backend/sandbox.js b/backend/sandbox.js index bab18ce..30a7e25 100644 --- a/backend/sandbox.js +++ b/backend/sandbox.js @@ -3,7 +3,7 @@ import { promises as fs } from "fs"; import { v4 as getUUID } from "uuid"; -import { MIN_UID, MAX_UID, borrowUser, ignoreUsers } from "./users.js"; +import { borrowUser } from "./users.js"; import { privilegedSetup, privilegedSpawn, @@ -21,13 +21,8 @@ function log(msg) { } async function main() { - const dirs = await fs.readdir("/tmp/riju"); - const uids = ( - await Promise.all(dirs.map((dir) => fs.stat(`/tmp/riju/${dir}`))) - ).filter((uid) => uid >= MIN_UID && uid < MAX_UID); - await ignoreUsers(uids, log); const uuid = getUUID(); - const { uid, returnUID } = await borrowUser(log); + const { uid, returnUser } = await borrowUser(log); await run(privilegedSetup({ uid, uuid }), log); const args = privilegedSpawn({ uid, uuid }, ["bash"]); const proc = spawn(args[0], args.slice(1), { @@ -38,7 +33,7 @@ async function main() { proc.on("close", resolve); }); await run(privilegedTeardown({ uid, uuid }), log); - await returnUID(); + await returnUser(); } main().catch(die); diff --git a/backend/users.js b/backend/users.js index 2c6dfb6..1abc66e 100644 --- a/backend/users.js +++ b/backend/users.js @@ -1,89 +1,104 @@ import { spawn } from "child_process"; -import fs from "fs"; +import { promises as fs } from "fs"; import os from "os"; import AsyncLock from "async-lock"; import _ from "lodash"; import parsePasswd from "parse-passwd"; -import { privilegedUseradd, run } from "./util.js"; +import { asBool, privilegedUseradd, run, uuidRegexp } from "./util.js"; // Keep in sync with system/src/riju-system-privileged.c export const MIN_UID = 2000; export const MAX_UID = 65000; -const CUR_UID = os.userInfo().uid; +function validUID(uid) { + return uid >= MIN_UID && uid < MAX_UID; +} -let availIds = null; -let nextId = null; +const CUR_UID = os.userInfo().uid; +const ASSUME_SINGLE_PROCESS = asBool( + process.env.RIJU_ASSUME_SINGLE_PROCESS, + false +); + +let initialized = false; +let nextUserToCreate = null; +let locallyBorrowedUsers = new Set(); +let availableUsers = new Set(); let lock = new AsyncLock(); -async function readExistingUsers(log) { - availIds = parsePasswd( - await new Promise((resolve, reject) => - fs.readFile("/etc/passwd", "utf-8", (err, data) => { - if (err) { - reject(err); - } else { - resolve(data); - } - }) +async function getCreatedUsers() { + return new Set( + parsePasswd(await fs.readFile("/etc/passwd", "utf-8")) + .map(({ uid }) => parseInt(uid)) + .filter((uid) => !isNaN(uid) && validUID(uid)) + ); +} + +async function getActiveUsers() { + return new Set( + ( + await Promise.all( + (await fs.readdir("/tmp/riju")) + .filter((name) => name.match(uuidRegexp)) + .map((name) => fs.stat(`/tmp/riju/${name}`)) + ) ) - ) - .filter(({ username }) => username.startsWith("riju")) - .map(({ uid }) => parseInt(uid)) - .filter((uid) => !isNaN(uid) && uid >= MIN_UID && uid < MAX_UID) - .reverse(); - nextId = (_.max(availIds) || MIN_UID - 1) + 1; - log(`Found ${availIds.length} existing users, next is riju${nextId}`); + .map(({ uid }) => uid) + .filter(validUID) + ); } async function createUser(log) { - if (nextId >= MAX_UID) { + if (nextUserToCreate >= MAX_UID) { throw new Error("too many users"); } - const uid = nextId; + const uid = nextUserToCreate; await run(privilegedUseradd(uid), log); - log(`Created new user with ID ${uid}`); - nextId += 1; + nextUserToCreate += 1; return uid; } -export async function ignoreUsers(uids, log) { - await lock.acquire("key", async () => { - if (availIds === null || nextId === null) { - await readExistingUsers(log); - } - const uidSet = new Set(uids); - if (uidSet.size > 0) { - const plural = uidSet.size !== 1 ? "s" : ""; - log( - `Ignoring user${plural} from open session${plural}: ${Array.from(uidSet) - .sort() - .map((uid) => `riju${uid}`) - .join(", ")}` - ); - } - availIds = availIds.filter((uid) => !uidSet.has(uid)); - }); -} - export async function borrowUser(log) { return await lock.acquire("key", async () => { - if (availIds === null || nextId === null) { - await readExistingUsers(log); + if (!initialized || !ASSUME_SINGLE_PROCESS) { + const createdUsers = await getCreatedUsers(); + const activeUsers = await getActiveUsers(); + if (createdUsers.size > 0) { + nextUserToCreate = _.max([...createdUsers]) + 1; + } else { + nextUserToCreate = MIN_UID; + } + // If there are new users created, we want to make them + // available (unless they are already active). Similarly, if + // there are users that have become inactive, we want to make + // them available (unless they are already borrowed locally). + for (const user of createdUsers) { + if (!activeUsers.has(user) && !locallyBorrowedUsers.has(user)) { + availableUsers.add(user); + } + } + // If there are users that have become active, we want to make + // them unavailable. + for (const user of activeUsers) { + availableUsers.delete(user); + } + initialized = true; } - let uid; - if (availIds.length > 0) { - uid = availIds.pop(); - } else { - uid = await createUser(log); + if (availableUsers.size === 0) { + availableUsers.add(await createUser(log)); } + // https://stackoverflow.com/a/32539929/3538165 + const user = availableUsers.values().next().value; + locallyBorrowedUsers.add(user); + availableUsers.delete(user); return { - uid, - returnUID: async () => { + uid: user, + returnUser: async () => { await lock.acquire("key", () => { - availIds.push(uid); + locallyBorrowedUsers.delete(user); + availableUsers.add(user); }); }, }; diff --git a/backend/util.js b/backend/util.js index 4acbd28..42bb783 100644 --- a/backend/util.js +++ b/backend/util.js @@ -129,3 +129,23 @@ export const log = { warn: console.error, error: console.error, }; + +// https://gist.github.com/bugventure/f71337e3927c34132b9a +export const uuidRegexp = /^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$/; + +export function asBool(value, def) { + if (def === undefined) { + throw new Error("asBool needs an explicit default value"); + } + if (!value) { + return def; + } + value = value.toLowerCase().trim(); + if (["y", "yes", "1", "on"].includes(value)) { + return true; + } + if (["n", "no", "0", "off"].includes(value)) { + return false; + } + throw new Error(`asBool doesn't understand value: ${value}`); +} diff --git a/docker/app/Dockerfile b/docker/app/Dockerfile index 59fa923..f3ed3e2 100644 --- a/docker/app/Dockerfile +++ b/docker/app/Dockerfile @@ -11,3 +11,4 @@ RUN chown root:riju system/out/*-privileged && chmod a=,g=rx,u=rwxs system/out/* USER riju CMD ["make", "server"] +ENV RIJU_ASSUME_SINGLE_PROCESS 1