Skip to content

Marketplace PKI: client key mode is only fixed when contents change, and is world-readable while written #375

Description

@v0l

Found in review of #370.

lnvps_api_common/src/host/marketplace_pki.rs:91-104:

fn copy_private_if_changed(from: &Path, to: &Path) -> Result<bool> {
    let contents = fs::read(from)?;
    if !write_if_changed(to, &contents)? { return Ok(false); }   // early return skips the chmod
    fs::set_permissions(to, Permissions::from_mode(0o600))?;

Two problems with the key that drives every marketplace node's libvirtd:

  1. The mode is only ever applied when the contents change. A clientkey.pem that already exists with identical bytes but a looser mode — written by an earlier build without the chmod, restored from a backup or volume, or left behind by a crash between the write and the set_permissions — is never repaired. materialise runs on every poll and short-circuits every time.
  2. The write is world-readable first. write_if_changed uses fs::write, which creates with 0666 & ~umask (typically 0644), so there is a window where any process on the API host can read the key.

Fix both with OpenOptions::new().mode(0o600).create(true) and applying set_permissions unconditionally rather than only on change.

lnvps_node/src/libvirt.rs:612-620 (write_private_if_changed) has the identical shape for the node's own server key and needs the same treatment.

marketplace_pki/tests.rs:66-80 only covers the fresh-write case, which is why neither gap was caught — a test that pre-creates the file 0644 with matching contents and asserts the mode afterwards would fail today.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions