mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-10-24 01:37:02 -06:00
Security: private temp paths for the signing certificate and test output; web font licences stay inside the checkout (#3871)
* Audit: private temp paths for the signing certificate and testkit output; confine web font licences to the checkout * testkit: refuse a pre-made private temp directory this user cannot write to
This commit is contained in:
@@ -39,10 +39,24 @@ pub use vectorcraft_engine::{Session, doc::NodeId};
|
|||||||
/// Re-exports so dependents without direct dependencies (e.g. app test crates) can use them.
|
/// Re-exports so dependents without direct dependencies (e.g. app test crates) can use them.
|
||||||
pub use {vectorcraft_doc as doc, vectorcraft_format as format, vectorcraft_render as render, vectorcraft_svg as svg};
|
pub use {vectorcraft_doc as doc, vectorcraft_format as format, vectorcraft_render as render, vectorcraft_svg as svg};
|
||||||
|
|
||||||
/// A per-process temporary directory for test output (created on first use).
|
/// A per-process temporary directory for test output (created on first use). It is made with
|
||||||
|
/// `create_dir` (owner-only on Unix), and a directory that already exists is reused only when it is a
|
||||||
|
/// real directory, not a link, with no access for others and writable by this user: one pre-made in a shared temp folder is refused.
|
||||||
pub fn temp_dir(tag: &str) -> std::path::PathBuf {
|
pub fn temp_dir(tag: &str) -> std::path::PathBuf {
|
||||||
let dir = std::env::temp_dir().join(format!("vectorcraft-testkit-{tag}-{}", std::process::id()));
|
let dir = std::env::temp_dir().join(format!("vectorcraft-testkit-{tag}-{}", std::process::id()));
|
||||||
std::fs::create_dir_all(&dir).expect("create temp dir");
|
let mut b = std::fs::DirBuilder::new();
|
||||||
|
#[cfg(unix)]
|
||||||
|
std::os::unix::fs::DirBuilderExt::mode(&mut b, 0o700);
|
||||||
|
if let Err(e) = b.create(&dir) {
|
||||||
|
let m = std::fs::symlink_metadata(&dir).expect("temp dir");
|
||||||
|
assert!(e.kind() == std::io::ErrorKind::AlreadyExists && m.is_dir(), "create temp dir: {e}");
|
||||||
|
#[cfg(unix)]
|
||||||
|
assert!(std::os::unix::fs::PermissionsExt::mode(&m.permissions()) & 0o077 == 0, "{}: not private", dir.display());
|
||||||
|
// A private directory made by another user passes the mode check but is closed to this one:
|
||||||
|
// writing a probe file tells it apart from ours without needing the user ids.
|
||||||
|
let probe = dir.join(".probe");
|
||||||
|
assert!(std::fs::write(&probe, b"").and_then(|()| std::fs::remove_file(&probe)).is_ok(), "{}: not owned by this user", dir.display());
|
||||||
|
}
|
||||||
dir
|
dir
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -19,10 +19,12 @@ fi
|
|||||||
|
|
||||||
TMP="${RUNNER_TEMP:-${TMPDIR:-/tmp}}"
|
TMP="${RUNNER_TEMP:-${TMPDIR:-/tmp}}"
|
||||||
KEYCHAIN="$TMP/vectorcraft-signing.keychain-db"
|
KEYCHAIN="$TMP/vectorcraft-signing.keychain-db"
|
||||||
CERT="$TMP/vectorcraft-signing.p12"
|
# A fresh private directory, so a pre-made symlink in a shared $TMP cannot redirect the secret.
|
||||||
|
CERT_DIR="$(mktemp -d "$TMP/vectorcraft-signing.XXXXXX")"
|
||||||
|
CERT="$CERT_DIR/vectorcraft-signing.p12"
|
||||||
KC_PASS="${KEYCHAIN_PASSWORD:-$(openssl rand -hex 24)}"
|
KC_PASS="${KEYCHAIN_PASSWORD:-$(openssl rand -hex 24)}"
|
||||||
|
|
||||||
trap 'rm -f "$CERT"' EXIT
|
trap 'rm -rf "$CERT_DIR"' EXIT
|
||||||
printf '%s' "$APPLE_CERTIFICATE" | base64 --decode >"$CERT"
|
printf '%s' "$APPLE_CERTIFICATE" | base64 --decode >"$CERT"
|
||||||
security create-keychain -p "$KC_PASS" "$KEYCHAIN"
|
security create-keychain -p "$KC_PASS" "$KEYCHAIN"
|
||||||
security set-keychain-settings -lut 21600 "$KEYCHAIN"
|
security set-keychain-settings -lut 21600 "$KEYCHAIN"
|
||||||
|
|||||||
+18
-1
@@ -61,7 +61,13 @@ fn copy(list_path: &Path, dir: &Path, site: &Path) -> Result<(usize, Vec<String>
|
|||||||
std::fs::create_dir_all(folder).map_err(|err| format!("{}: {err}", folder.display()))?;
|
std::fs::create_dir_all(folder).map_err(|err| format!("{}: {err}", folder.display()))?;
|
||||||
std::fs::write(&dest, &bytes).map_err(|err| format!("{}: {err}", dest.display()))?;
|
std::fs::write(&dest, &bytes).map_err(|err| format!("{}: {err}", dest.display()))?;
|
||||||
let licence_name = Path::new(licence).file_name().ok_or_else(|| format!("{licence}: not a file"))?;
|
let licence_name = Path::new(licence).file_name().ok_or_else(|| format!("{licence}: not a file"))?;
|
||||||
std::fs::copy(dir.join(licence), folder.join(licence_name)).map_err(|err| format!("{licence}: {err}"))?;
|
// The manifest is supplier data: the licence must be a file inside the checkout.
|
||||||
|
let root = std::fs::canonicalize(dir).map_err(|err| format!("{}: {err}", dir.display()))?;
|
||||||
|
let src = std::fs::canonicalize(dir.join(licence)).map_err(|err| format!("{licence}: {err}"))?;
|
||||||
|
if !src.starts_with(&root) || !src.is_file() {
|
||||||
|
return Err(format!("{licence}: not a file inside the font checkout"));
|
||||||
|
}
|
||||||
|
std::fs::copy(&src, folder.join(licence_name)).map_err(|err| format!("{licence}: {err}"))?;
|
||||||
copied += 1;
|
copied += 1;
|
||||||
}
|
}
|
||||||
Ok((copied, skipped))
|
Ok((copied, skipped))
|
||||||
@@ -110,6 +116,17 @@ mod tests {
|
|||||||
assert!(e.contains("A-Regular.ttf") && e.contains("sha256"), "{e}");
|
assert!(e.contains("A-Regular.ttf") && e.contains("sha256"), "{e}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_licence_path_that_leaves_the_checkout_is_refused() {
|
||||||
|
let (list, dir, site, _) = fixture("escape", None);
|
||||||
|
std::fs::write(dir.parent().unwrap().join("secret.txt"), b"secret").unwrap();
|
||||||
|
let m = dir.join("fonts/manifest.txt");
|
||||||
|
let text = std::fs::read_to_string(&m).unwrap().replace("fonts/a/OFL.txt", "../secret.txt");
|
||||||
|
std::fs::write(&m, text).unwrap();
|
||||||
|
let e = copy(&list, &dir, &site).unwrap_err();
|
||||||
|
assert!(e.contains("inside the font checkout"), "{e}");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn without_craft_fonts_the_hook_does_nothing() {
|
fn without_craft_fonts_the_hook_does_nothing() {
|
||||||
let (_, _, site, _) = fixture("noop", None);
|
let (_, _, site, _) = fixture("noop", None);
|
||||||
|
|||||||
Reference in New Issue
Block a user