From a16e5001cd7bfd94da90ac2fd9f69332adcb87c2 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Mon, 27 Jul 2026 15:23:08 +0100 Subject: [PATCH 1/3] fix(config): keep relay owner secrets out of process arguments The NixOS module expanded relayOwnerNsecFile into --relay-owner-nsec, making the private key visible through process listings and routine service diagnostics. Removing the flag at the parser boundary prevents other deployments from recreating the same exposure. Load a protected systemd credential before the environment and persistent key file, fail closed for empty or invalid configured values, and preserve auto-generation only when no identity was provisioned. The NixOS service can now execute the binary directly and generated fallback keys are created with private permissions. --- .env.example | 8 +- CHANGELOG.md | 6 + README.md | 15 +- docs/how-to/deploy.md | 16 +- nix/example-configuration.nix | 6 +- nix/module.nix | 38 +++-- src/config.rs | 272 ++++++++++++++++++++++++++++------ src/main.rs | 8 +- 8 files changed, 288 insertions(+), 81 deletions(-) diff --git a/.env.example b/.env.example index 721b793..d6bbd24 100644 --- a/.env.example +++ b/.env.example @@ -36,9 +36,11 @@ # - NIP-42 authentication when syncing from other relays # - Future: signing events, WoT-based rate limiting of syncing relays # -# CLI: --relay-owner-nsec -# Default: Loaded from/saved to .relay-owner.nsec file in current directory -# If file doesn't exist, a new key is generated and saved automatically +# Never accepted on the command line, so it cannot appear in process listings. +# systemd: use the relay_owner_nsec credential (highest precedence) +# Default: Loaded from/saved to .relay-owner.nsec in the current directory. +# If the file doesn't exist, a new key is generated with mode 0600. +# Empty or invalid configured values stop startup instead of rotating identity. # NGIT_RELAY_OWNER_NSEC=nsec1... # Relay name shown in NIP-11 information document diff --git a/CHANGELOG.md b/CHANGELOG.md index f247a7d..19bc901 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Prevent relay-owner private keys from appearing in process arguments by + loading NixOS secret files through a systemd credential. Configured empty or + invalid keys now stop startup instead of silently generating a new identity. - Fix maintainer invitation recovery after a production restart by starting the relay and SyncManager before scanning retained deletion requests. The potentially long retention catch-up now begins immediately in the existing @@ -59,6 +62,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Relay-owner private keys are no longer accepted through + `--relay-owner-nsec`; use the `relay_owner_nsec` systemd credential, + `NGIT_RELAY_OWNER_NSEC`, or `.relay-owner.nsec`. - Addressed a production storage imbalance where roughly 50k of 60k stored events were deletion requests. Deletion requests now have a bounded lifecycle, so requests that are no longer relevant are reconciled and retired while requests that may still affect valid event handling are preserved. - Deletion-disrespector mode now explicitly applies to both NIP-09 deletion requests and NIP-62 request-to-vanish events. - Retired the hidden `repair-deletion-requests` maintenance command. diff --git a/README.md b/README.md index 826ad33..fe27847 100644 --- a/README.md +++ b/README.md @@ -434,13 +434,18 @@ This means CLI flags always take precedence over environment variables, which ta # View all options with defaults ngit-grasp --help -# Run with CLI flags (override everything else) -ngit-grasp --domain relay.example.com --relay-owner-nsec nsec1... --bind-address 0.0.0.0:7334 +# Run with CLI flags and let ngit-grasp create .relay-owner.nsec +ngit-grasp --domain relay.example.com --bind-address 0.0.0.0:7334 -# Mix CLI flags with environment variables +# Supply an existing owner key through the environment NGIT_RELAY_OWNER_NSEC=nsec1... ngit-grasp --domain relay.example.com ``` +The relay-owner nsec is deliberately not accepted as a command-line argument, +because process arguments are visible through tools such as `ps` and +`/proc//cmdline`. NixOS deployments should use `relayOwnerNsecFile`, which +passes the key through a protected systemd credential. + ### Configuration Options #### Core Settings @@ -448,7 +453,7 @@ NGIT_RELAY_OWNER_NSEC=nsec1... ngit-grasp --domain relay.example.com | Option | CLI Flag | Environment Variable | Default | | ----------------- | --------------------- | ------------------------ | -------------------------------------------- | | Domain | `--domain` | `NGIT_DOMAIN` | (required) | -| Relay owner nsec | `--relay-owner-nsec` | `NGIT_RELAY_OWNER_NSEC` | `.relay-owner.nsec` file (auto-generated) | +| Relay owner nsec | — | `NGIT_RELAY_OWNER_NSEC` | systemd credential, then `.relay-owner.nsec` | | Relay name | `--relay-name` | `NGIT_RELAY_NAME` | `${domain} grasp relay` | | Relay description | `--relay-description` | `NGIT_RELAY_DESCRIPTION` | `Git Nostr Relay - a grasp implementation` | | Git data path | `--git-data-path` | `NGIT_GIT_DATA_PATH` | `./data/git` (temp dir for memory backend) | @@ -504,7 +509,7 @@ NGIT_RELAY_OWNER_NSEC=nsec1... ngit-grasp --domain relay.example.com ### Example: Production Deployment ```bash -# Using environment variables (recommended for production) +# Using environment variables (for containers and other non-systemd deployments) export NGIT_DOMAIN=gitnostr.com export NGIT_RELAY_OWNER_NSEC=nsec1... # Or let it auto-generate from .relay-owner.nsec export NGIT_BIND_ADDRESS=0.0.0.0:7334 diff --git a/docs/how-to/deploy.md b/docs/how-to/deploy.md index 9117fe2..1be3138 100644 --- a/docs/how-to/deploy.md +++ b/docs/how-to/deploy.md @@ -79,7 +79,7 @@ Create a new file for your ngit-grasp service (e.g., `services/ngit-grasp.nix`): # Identity relayName = "My GRASP Relay"; relayDescription = "A Rust GRASP implementation with proactive sync"; - relayOwnerNsecFile = "/persistent/ngit-grasp/relay-owner.nsec"; + relayOwnerNsecFile = "/run/agenix/ngit-grasp-relay-owner-nsec"; # Sync - bootstrap from relay.ngit.dev syncBootstrapRelayUrl = "wss://relay.ngit.dev"; @@ -110,8 +110,10 @@ Create a new file for your ngit-grasp service (e.g., `services/ngit-grasp.nix`): - **port**: Local port (use reverse proxy for HTTPS) - **dataDir**: Where git repos and database are stored - **relayOwnerNsecFile**: Path to file containing relay owner's nsec - - If file doesn't exist, ngit-grasp will auto-generate one + - Passed to ngit-grasp as a protected systemd credential, not a process argument + - The runtime secret file must already exist (for example through agenix or sops-nix) - Alternative: `relayOwnerNsec = "nsec1..."` (less secure, in nix store) + - If neither option is set, ngit-grasp loads or creates `.relay-owner.nsec` in `dataDir` - **syncBootstrapRelayUrl**: Bootstrap relay to sync from on startup See [nix/example-configuration.nix](../../nix/example-configuration.nix) for more examples. @@ -238,7 +240,7 @@ git ls-remote https://ngit.example.com//.git ### Identity - `relayName` - Relay name for NIP-11 (default: "{domain} grasp relay") - `relayDescription` - Relay description -- `relayOwnerNsecFile` - Path to file with relay owner nsec (recommended) +- `relayOwnerNsecFile` - Runtime secret file loaded as a systemd credential (recommended) - `relayOwnerNsec` - Inline nsec (less secure) ### Sync @@ -420,12 +422,16 @@ The NixOS module includes systemd hardening: Additional recommendations: -1. **Use nsec file instead of inline:** +1. **Use a runtime secret file instead of an inline key:** ```nix - relayOwnerNsecFile = "/persistent/ngit-grasp/relay-owner.nsec"; + relayOwnerNsecFile = "/run/agenix/ngit-grasp-relay-owner-nsec"; # NOT: relayOwnerNsec = "nsec1..."; # Ends up in nix store! ``` + The module exposes the file to ngit-grasp as the `relay_owner_nsec` + systemd credential. The key does not appear in `ExecStart` or the process + command line. + 2. **Restrict data directory permissions:** ```bash chmod 750 /persistent/ngit-grasp diff --git a/nix/example-configuration.nix b/nix/example-configuration.nix index 34615be..4019c66 100644 --- a/nix/example-configuration.nix +++ b/nix/example-configuration.nix @@ -34,9 +34,9 @@ relayDescription = "personal instance of ngit-grasp, a Rust GRASP implementation with proactive sync"; - # Option 1: Use nsec file (recommended - more secure) - relayOwnerNsecFile = - "/persistent/ngit-danconwaydev-com-ngit-grasp/relay-owner.nsec"; + # Option 1: Use a runtime secret file (recommended). The module loads it + # as a systemd credential, keeping the nsec out of the process command line. + relayOwnerNsecFile = "/run/agenix/ngit-grasp-relay-owner-nsec"; # Option 2: Inline nsec (less secure, ends up in nix store) # relayOwnerNsec = "nsec1..."; diff --git a/nix/module.nix b/nix/module.nix index ee3d385..f5fdab1 100644 --- a/nix/module.nix +++ b/nix/module.nix @@ -20,6 +20,8 @@ let doCheck = false; }; + relayOwnerNsecCredential = "relay_owner_nsec"; + # Per-instance options instanceOptions = { name, ... }: { options = { @@ -67,11 +69,16 @@ let relayOwnerNsecFile = mkOption { type = types.nullOr types.path; default = null; - example = "/persistent/ngit-grasp/relay-owner.nsec"; + example = "/run/agenix/ngit-grasp-relay-owner-nsec"; description = '' - Path to file containing relay owner's nsec (private key). - If file doesn't exist, ngit-grasp will auto-generate a random nsec and save it. - Takes precedence over relayOwnerNsec if both are set. + Runtime secret file containing the relay owner's nsec (private key). + The service receives it as a systemd credential named + `${relayOwnerNsecCredential}`, so the secret is never placed in the + process command line. + + Leave this null to use relayOwnerNsec or to load/generate + .relay-owner.nsec in dataDir. If set, point it at a runtime secret + file (agenix, sops-nix, etc.), not a Nix-store path. ''; }; @@ -416,7 +423,11 @@ let }; # Create systemd service config for an instance - mkService = name: cfg: { + mkService = name: cfg: + let + loadCredentials = optional (cfg.relayOwnerNsecFile != null) + "${relayOwnerNsecCredential}:${toString cfg.relayOwnerNsecFile}"; + in { description = "ngit-grasp GRASP relay (${name})"; after = [ "network.target" "ngit-grasp-${name}-setup.service" ]; requires = [ "ngit-grasp-${name}-setup.service" ]; @@ -498,17 +509,14 @@ let Environment = "PATH=${pkgs.git}/bin:${pkgs.openssh}/bin:${pkgs.coreutils}/bin"; - # Command to run - ExecStart = if cfg.relayOwnerNsecFile != null then - # Use nsec from file - need to use shell to read the file - "${pkgs.bash}/bin/bash -c '${ngit-grasp}/bin/ngit-grasp --relay-owner-nsec \"$(${pkgs.coreutils}/bin/cat ${cfg.relayOwnerNsecFile})\"'" - else - # Let ngit-grasp auto-generate nsec in .relay-owner.nsec file in dataDir - "${ngit-grasp}/bin/ngit-grasp"; + # The binary reads systemd credentials and environment configuration + # itself, keeping relay-owner secrets out of process arguments. + ExecStart = "${ngit-grasp}/bin/ngit-grasp"; # Restart policy Restart = "always"; RestartSec = "10s"; + UMask = "0077"; # Hardening NoNewPrivileges = true; @@ -517,10 +525,6 @@ let ProtectHome = true; ReadWritePaths = [ cfg.dataDir ]; - # If using nsecFile, grant read access - ReadOnlyPaths = - optionals (cfg.relayOwnerNsecFile != null) [ cfg.relayOwnerNsecFile ]; - # Additional hardening ProtectKernelTunables = true; ProtectKernelModules = true; @@ -539,6 +543,8 @@ let # System call filtering SystemCallFilter = [ "@system-service" "~@privileged" "~@resources" ]; SystemCallErrorNumber = "EPERM"; + } // optionalAttrs (loadCredentials != [ ]) { + LoadCredential = loadCredentials; }; # Directory creation handled by both ExecStartPre (above) and tmpfiles (below) diff --git a/src/config.rs b/src/config.rs index 270ca2a..32594e6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -3,9 +3,12 @@ use clap::{Parser, ValueEnum}; use nostr_sdk::prelude::*; use serde::{Deserialize, Serialize}; use std::fs; -use std::path::PathBuf; +use std::io::Write; +use std::path::{Path, PathBuf}; use std::time::Duration; +const RELAY_OWNER_NSEC_ENV: &str = "NGIT_RELAY_OWNER_NSEC"; +const RELAY_OWNER_NSEC_CREDENTIAL: &str = "relay_owner_nsec"; const DEFAULT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS: u64 = 30 * 24 * 60 * 60; const DEFAULT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS: u64 = 180 * 24 * 60 * 60; @@ -317,17 +320,16 @@ pub struct Config { #[arg(long, env = "NGIT_DOMAIN")] pub domain: String, - /// Relay operator's nsec (private key) for signing and authentication + /// Relay operator's nsec (private key) for signing and authentication. /// /// Used for: /// - NIP-11 relay information document (pubkey field derived from this nsec) /// - NIP-42 authentication when syncing from other relays /// - Future: signing events, WoT-based rate limiting of syncing relays /// - /// If not provided via CLI/env, will be loaded from/saved to `.relay-owner.nsec` file - /// in the current directory. If the file doesn't exist, a new key will be generated - /// and saved automatically. - #[arg(long, env = "NGIT_RELAY_OWNER_NSEC")] + /// Loaded from the `relay_owner_nsec` systemd credential, + /// `NGIT_RELAY_OWNER_NSEC`, or `.relay-owner.nsec`; never accepted on argv. + #[arg(skip)] pub relay_owner_nsec: Option, /// Relay name for NIP-11 information document (defaults to "${domain} grasp relay") @@ -593,18 +595,102 @@ impl Config { // Parse CLI args (clap automatically handles env var fallback) let mut config = Self::parse(); - // If relay_owner_nsec not provided, load from file or generate - if config.relay_owner_nsec.is_none() { - config.relay_owner_nsec = Some(Self::load_or_generate_relay_owner_key()?); - } else { - // If provided via CLI/env, trim any whitespace (newlines, spaces, etc.) - // This handles cases where the value is read from a file with trailing newline - config.relay_owner_nsec = config.relay_owner_nsec.map(|s| s.trim().to_string()); - } + config.relay_owner_nsec = Some(Self::load_relay_owner_key()?); Ok(config) } + /// Load the relay owner key without ever accepting the secret on argv. + /// + /// A systemd credential is the most deliberate provision, followed by the + /// environment (including `.env` loaded by [`Self::load`]), then the + /// persistent `.relay-owner.nsec` file. + pub fn load_relay_owner_key() -> Result { + let credentials_dir = std::env::var_os("CREDENTIALS_DIRECTORY").map(PathBuf::from); + let env_value = std::env::var(RELAY_OWNER_NSEC_ENV).ok(); + Self::load_relay_owner_key_from_sources( + credentials_dir.as_deref(), + env_value, + Self::load_or_generate_relay_owner_key, + ) + } + + fn load_relay_owner_key_from_sources( + credentials_dir: Option<&Path>, + env_value: Option, + fallback: F, + ) -> Result + where + F: FnOnce() -> Result, + { + if let Some(dir) = credentials_dir { + if let Some(nsec) = Self::load_relay_owner_key_credential_from_dir(dir)? { + Self::validate_relay_owner_nsec(&nsec, "relay_owner_nsec credential")?; + tracing::info!( + credential = RELAY_OWNER_NSEC_CREDENTIAL, + "Loaded relay owner key" + ); + return Ok(nsec); + } + } + + if let Some(nsec) = Self::relay_owner_key_from_env_value(env_value)? { + Self::validate_relay_owner_nsec(&nsec, RELAY_OWNER_NSEC_ENV)?; + tracing::info!(env = RELAY_OWNER_NSEC_ENV, "Loaded relay owner key"); + return Ok(nsec); + } + + fallback() + } + + fn load_relay_owner_key_credential_from_dir(dir: &Path) -> Result> { + let key_path = dir.join(RELAY_OWNER_NSEC_CREDENTIAL); + if !key_path.exists() { + return Ok(None); + } + if !key_path + .metadata() + .with_context(|| format!("stat relay owner credential '{}'", key_path.display()))? + .is_file() + { + return Err(anyhow!( + "Relay owner credential '{}' is not a regular file", + key_path.display() + )); + } + let nsec = fs::read_to_string(&key_path) + .with_context(|| format!("read relay owner credential '{}'", key_path.display()))? + .trim() + .to_string(); + if nsec.is_empty() { + return Err(anyhow!( + "Relay owner credential '{}' is empty", + key_path.display() + )); + } + Ok(Some(nsec)) + } + + fn relay_owner_key_from_env_value(value: Option) -> Result> { + let Some(raw) = value else { + return Ok(None); + }; + let nsec = raw.trim().to_string(); + if nsec.is_empty() { + return Err(anyhow!( + "{RELAY_OWNER_NSEC_ENV} is set but empty; unset it to load/generate \ + {key_file}, or provide a valid nsec", + key_file = Self::RELAY_OWNER_KEY_FILE + )); + } + Ok(Some(nsec)) + } + + fn validate_relay_owner_nsec(nsec: &str, source: &str) -> Result<()> { + Keys::parse(nsec).with_context(|| format!("Invalid relay owner nsec in {source}"))?; + Ok(()) + } + /// Load relay owner key from file, or generate and save a new one pub fn load_or_generate_relay_owner_key() -> Result { let key_path = PathBuf::from(Self::RELAY_OWNER_KEY_FILE); @@ -616,8 +702,10 @@ impl Config { .trim() .to_string(); - // Validate it's a valid nsec - Keys::parse(&nsec).context("Invalid nsec in relay owner key file")?; + Self::validate_relay_owner_nsec( + &nsec, + &format!("relay owner key file {}", key_path.display()), + )?; tracing::info!("Loaded relay owner key from {}", key_path.display()); return Ok(nsec); @@ -627,8 +715,7 @@ impl Config { let keys = Keys::generate(); let nsec = keys.secret_key().to_bech32()?; - // Save to file - fs::write(&key_path, &nsec).context("Failed to write relay owner key file")?; + write_secret_file(&key_path, &nsec).context("Failed to write relay owner key file")?; tracing::info!( "Generated new relay owner key and saved to {}", @@ -964,6 +1051,20 @@ impl Config { } } +fn write_secret_file(path: &Path, contents: &str) -> Result<()> { + let mut options = fs::OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + + let mut file = options.open(path)?; + file.write_all(contents.as_bytes())?; + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -1188,35 +1289,122 @@ mod tests { } #[test] - fn test_relay_owner_nsec_trims_whitespace() { - // Test that Config::load() trims whitespace from provided nsec - // This simulates what happens when nsec is read from a file with trailing newline - let nsec_clean = "nsec1rt5f3gfnktvd77fdarg00ff94l8j833y778ym3xlzkx89g9v5zvq4y2qee"; - let nsec_with_newline = format!("{}\n", nsec_clean); - let nsec_with_spaces = format!(" {} ", nsec_clean); + fn relay_owner_nsec_is_not_accepted_on_argv() { + let nsec = Keys::generate().secret_key().to_bech32().unwrap(); + assert!(Config::try_parse_from([ + "ngit-grasp", + "--domain", + "example.com", + "--relay-owner-nsec", + nsec.as_str(), + ]) + .is_err()); + } - // Test that trimming happens by directly creating config with whitespace - let mut config1 = Config::for_testing(); - config1.relay_owner_nsec = Some(nsec_with_newline.clone()); - // Simulate what Config::load() does - config1.relay_owner_nsec = config1.relay_owner_nsec.map(|s| s.trim().to_string()); + #[test] + fn relay_owner_nsec_credential_is_loaded_and_trimmed() { + let tempdir = tempfile::tempdir().unwrap(); + let nsec = Keys::generate().secret_key().to_bech32().unwrap(); + fs::write( + tempdir.path().join(RELAY_OWNER_NSEC_CREDENTIAL), + format!(" {nsec}\n"), + ) + .unwrap(); - let mut config2 = Config::for_testing(); - config2.relay_owner_nsec = Some(nsec_with_spaces.clone()); - config2.relay_owner_nsec = config2.relay_owner_nsec.map(|s| s.trim().to_string()); + let loaded = Config::load_relay_owner_key_credential_from_dir(tempdir.path()) + .unwrap() + .unwrap(); + assert_eq!(loaded, nsec); + } - // Both should parse successfully after trimming - assert!(config1.relay_owner_keys().is_ok()); - assert!(config2.relay_owner_keys().is_ok()); + #[test] + fn relay_owner_nsec_credential_rejects_empty_and_invalid_files() { + let tempdir = tempfile::tempdir().unwrap(); + let credential = tempdir.path().join(RELAY_OWNER_NSEC_CREDENTIAL); + fs::write(&credential, "\n").unwrap(); + assert!(Config::load_relay_owner_key_credential_from_dir(tempdir.path()).is_err()); - // Both should produce the same public key - let keys1 = config1.relay_owner_keys().unwrap(); - let keys2 = config2.relay_owner_keys().unwrap(); - assert_eq!(keys1.public_key(), keys2.public_key()); + fs::write(&credential, "nsec1invalid").unwrap(); + let error = Config::load_relay_owner_key_from_sources( + Some(tempdir.path()), + None, + || -> Result { panic!("invalid credential must not fall through") }, + ) + .unwrap_err(); + assert!(error.to_string().contains("relay_owner_nsec credential")); + } - // Verify trimmed nsec equals clean nsec - assert_eq!(config1.relay_owner_nsec.unwrap(), nsec_clean); - assert_eq!(config2.relay_owner_nsec.unwrap(), nsec_clean); + #[test] + fn relay_owner_nsec_credential_takes_precedence_over_environment() { + let tempdir = tempfile::tempdir().unwrap(); + let credential_nsec = Keys::generate().secret_key().to_bech32().unwrap(); + fs::write( + tempdir.path().join(RELAY_OWNER_NSEC_CREDENTIAL), + &credential_nsec, + ) + .unwrap(); + + let loaded = Config::load_relay_owner_key_from_sources( + Some(tempdir.path()), + Some("nsec1invalid".to_string()), + || -> Result { panic!("credential must prevent fallback") }, + ) + .unwrap(); + assert_eq!(loaded, credential_nsec); + } + + #[test] + fn relay_owner_nsec_environment_is_trimmed_and_precedes_fallback() { + let nsec = Keys::generate().secret_key().to_bech32().unwrap(); + let loaded = Config::load_relay_owner_key_from_sources( + None, + Some(format!(" {nsec}\n")), + || -> Result { panic!("environment must prevent fallback") }, + ) + .unwrap(); + assert_eq!(loaded, nsec); + } + + #[test] + fn relay_owner_nsec_environment_rejects_empty_and_invalid_values() { + for raw in ["", " ", "\n"] { + assert!( + Config::relay_owner_key_from_env_value(Some(raw.to_string())).is_err(), + "{raw:?} should be rejected" + ); + } + + let error = Config::load_relay_owner_key_from_sources( + None, + Some("nsec1invalid".to_string()), + || -> Result { panic!("invalid environment must not fall through") }, + ) + .unwrap_err(); + assert!(error.to_string().contains(RELAY_OWNER_NSEC_ENV)); + } + + #[test] + fn relay_owner_nsec_missing_sources_use_persistent_fallback() { + let fallback_nsec = Keys::generate().secret_key().to_bech32().unwrap(); + let loaded = + Config::load_relay_owner_key_from_sources(None, None, || Ok(fallback_nsec.clone())) + .unwrap(); + assert_eq!(loaded, fallback_nsec); + } + + #[cfg(unix)] + #[test] + fn generated_relay_owner_key_file_is_private() { + use std::os::unix::fs::PermissionsExt; + + let tempdir = tempfile::tempdir().unwrap(); + let key_path = tempdir.path().join(".relay-owner.nsec"); + write_secret_file(&key_path, "secret").unwrap(); + + assert_eq!( + fs::metadata(key_path).unwrap().permissions().mode() & 0o777, + 0o600 + ); } #[test] diff --git a/src/main.rs b/src/main.rs index ba8134c..2d56d96 100644 --- a/src/main.rs +++ b/src/main.rs @@ -53,13 +53,7 @@ async fn main() -> Result<()> { Cli::HoldingEject(eject_args) => nostr::lifecycle::run_holding_eject(eject_args).await, Cli::Serve(config) => { let mut config = *config; - // Finish initialising the Config (load relay owner key if not provided). - if config.relay_owner_nsec.is_none() { - config.relay_owner_nsec = Some(Config::load_or_generate_relay_owner_key()?); - } else { - config.relay_owner_nsec = - config.relay_owner_nsec.take().map(|s| s.trim().to_string()); - } + config.relay_owner_nsec = Some(Config::load_relay_owner_key()?); run_relay(config).await } } From 002e6ab9f556b28c53b11cc5095a708613dc709b Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Mon, 27 Jul 2026 15:23:21 +0100 Subject: [PATCH 2/3] fix(config): secure persistent relay owner keys on load Older ngit-grasp releases created .relay-owner.nsec with the process umask, leaving existing deployments at mode 0644 even after new key generation was fixed. Moving the secret out of argv would not repair that persistent local exposure. Restrict an existing fallback key to mode 0600 before reading it and fail startup with the affected path when the permissions cannot be secured. Systemd credential source files remain untouched because their ownership and mode belong to the operator or secret manager. --- CHANGELOG.md | 4 ++- docs/how-to/deploy.md | 4 ++- nix/module.nix | 2 ++ src/config.rs | 59 +++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19bc901..8c1923c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Prevent relay-owner private keys from appearing in process arguments by loading NixOS secret files through a systemd credential. Configured empty or - invalid keys now stop startup instead of silently generating a new identity. + invalid keys now stop startup instead of silently generating a new identity, + and existing persistent fallback key files are restricted to mode `0600` + before being read. - Fix maintainer invitation recovery after a production restart by starting the relay and SyncManager before scanning retained deletion requests. The potentially long retention catch-up now begins immediately in the existing diff --git a/docs/how-to/deploy.md b/docs/how-to/deploy.md index 1be3138..b53facc 100644 --- a/docs/how-to/deploy.md +++ b/docs/how-to/deploy.md @@ -112,6 +112,7 @@ Create a new file for your ngit-grasp service (e.g., `services/ngit-grasp.nix`): - **relayOwnerNsecFile**: Path to file containing relay owner's nsec - Passed to ngit-grasp as a protected systemd credential, not a process argument - The runtime secret file must already exist (for example through agenix or sops-nix) + - Permissions on that external source file remain the operator or secret manager's responsibility - Alternative: `relayOwnerNsec = "nsec1..."` (less secure, in nix store) - If neither option is set, ngit-grasp loads or creates `.relay-owner.nsec` in `dataDir` - **syncBootstrapRelayUrl**: Bootstrap relay to sync from on startup @@ -430,7 +431,8 @@ Additional recommendations: The module exposes the file to ngit-grasp as the `relay_owner_nsec` systemd credential. The key does not appear in `ExecStart` or the process - command line. + command line. ngit-grasp does not modify the external source file; keep its + ownership and permissions restricted through your secret manager. 2. **Restrict data directory permissions:** ```bash diff --git a/nix/module.nix b/nix/module.nix index f5fdab1..b2eebdf 100644 --- a/nix/module.nix +++ b/nix/module.nix @@ -79,6 +79,8 @@ let Leave this null to use relayOwnerNsec or to load/generate .relay-owner.nsec in dataDir. If set, point it at a runtime secret file (agenix, sops-nix, etc.), not a Nix-store path. + ngit-grasp does not modify this external source file; its ownership + and permissions remain the operator or secret manager's responsibility. ''; }; diff --git a/src/config.rs b/src/config.rs index 32594e6..e53c384 100644 --- a/src/config.rs +++ b/src/config.rs @@ -694,10 +694,20 @@ impl Config { /// Load relay owner key from file, or generate and save a new one pub fn load_or_generate_relay_owner_key() -> Result { let key_path = PathBuf::from(Self::RELAY_OWNER_KEY_FILE); + Self::load_or_generate_relay_owner_key_at(&key_path) + } + fn load_or_generate_relay_owner_key_at(key_path: &Path) -> Result { // Try to load existing key if key_path.exists() { - let nsec = fs::read_to_string(&key_path) + secure_secret_file_permissions(key_path).with_context(|| { + format!( + "Failed to secure relay owner key file {}", + key_path.display() + ) + })?; + + let nsec = fs::read_to_string(key_path) .context("Failed to read relay owner key file")? .trim() .to_string(); @@ -715,7 +725,7 @@ impl Config { let keys = Keys::generate(); let nsec = keys.secret_key().to_bech32()?; - write_secret_file(&key_path, &nsec).context("Failed to write relay owner key file")?; + write_secret_file(key_path, &nsec).context("Failed to write relay owner key file")?; tracing::info!( "Generated new relay owner key and saved to {}", @@ -1065,6 +1075,31 @@ fn write_secret_file(path: &Path, contents: &str) -> Result<()> { Ok(()) } +fn secure_secret_file_permissions(path: &Path) -> Result<()> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + let metadata = fs::metadata(path) + .with_context(|| format!("Failed to inspect secret file {}", path.display()))?; + let mut permissions = metadata.permissions(); + if permissions.mode() & 0o777 != 0o600 { + permissions.set_mode(0o600); + fs::set_permissions(path, permissions).with_context(|| { + format!( + "Failed to restrict secret file {} to mode 0600", + path.display() + ) + })?; + } + } + + #[cfg(not(unix))] + let _ = path; + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -1407,6 +1442,26 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn existing_relay_owner_key_file_permissions_are_repaired_before_loading() { + use std::os::unix::fs::PermissionsExt; + + let tempdir = tempfile::tempdir().unwrap(); + let key_path = tempdir.path().join(".relay-owner.nsec"); + let nsec = Keys::generate().secret_key().to_bech32().unwrap(); + fs::write(&key_path, &nsec).unwrap(); + fs::set_permissions(&key_path, fs::Permissions::from_mode(0o644)).unwrap(); + + let loaded = Config::load_or_generate_relay_owner_key_at(&key_path).unwrap(); + + assert_eq!(loaded, nsec); + assert_eq!( + fs::metadata(key_path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + } + #[test] fn test_metrics_config_defaults() { let config = Config::for_testing(); From 60af7cbf6f538f46b301809a49338fbbb5954576 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Mon, 27 Jul 2026 15:23:36 +0100 Subject: [PATCH 3/3] docs(config): retire stale owner npub guidance NGIT_OWNER_NPUB was replaced by the persistent relay-owner nsec before v1.0.0, but the configuration reference and its examples still described the pre-release npub-only mode. That made current security hardening look like a new compatibility removal and documented validation the server no longer performed. Describe the current identity sources, NIP-11 derivation, and NIP-42 use in one place, and update every obsolete npub example and error independently from the implementation changes. --- docs/reference/configuration.md | 42 +++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 4266d5d..90cbc29 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -84,30 +84,36 @@ NGIT_DOMAIN=localhost:7334 # Development only ### Nostr Relay Configuration -#### `NGIT_OWNER_NPUB` +#### Relay owner identity -**Description:** Nostr public key (npub format) of the relay operator -**Type:** String (npub1... format) -**Default:** None -**Required:** Yes +**Description:** Nostr secret key used for the relay operator identity +**Type:** String (`nsec1...` format) +**Default:** Load or create `.relay-owner.nsec` in the working directory +**Required:** No **Examples:** ```bash -NGIT_OWNER_NPUB=npub1alice... +NGIT_RELAY_OWNER_NSEC=nsec1... ``` **Used for:** -- NIP-11 relay information document -- Contact information -- Administrative operations (future) +- Deriving the operator pubkey in the NIP-11 relay information document +- NIP-42 authentication when synchronizing from other relays **Notes:** -- Must be valid npub format (starts with `npub1`) -- Can be generated with Nostr tools -- Publicly visible in relay metadata +- The key is never accepted on the command line, where it would be exposed by + process listings. +- Loading precedence is the systemd credential named `relay_owner_nsec`, + `NGIT_RELAY_OWNER_NSEC` (including `.env`), then `.relay-owner.nsec`. +- NixOS operators should set `relayOwnerNsecFile`; the module supplies that + file as a protected systemd credential. +- A configured credential or environment value that is empty or invalid stops + startup. It never falls through to generating a replacement identity. +- If no source is configured and `.relay-owner.nsec` does not exist, + ngit-grasp generates it with mode `0600`. --- @@ -1404,7 +1410,7 @@ For development, create a `.env` file in the project root: ```bash # .env file example NGIT_DOMAIN=localhost:7334 -NGIT_OWNER_NPUB=npub1alice... +NGIT_RELAY_OWNER_NSEC=nsec1... NGIT_RELAY_NAME="Development Relay" NGIT_RELAY_DESCRIPTION="Local development instance" NGIT_GIT_DATA_PATH=./data/git @@ -1429,7 +1435,7 @@ Configuration is validated at startup: // Example validation errors: Error: Invalid configuration - NGIT_DOMAIN is required - - NGIT_OWNER_NPUB must start with 'npub1' + - Invalid relay owner nsec in NGIT_RELAY_OWNER_NSEC - NGIT_GIT_DATA_PATH is not writable ``` @@ -1439,7 +1445,7 @@ Error: Invalid configuration - Values have correct format - Paths are accessible and writable - Ports are available -- npub keys are valid +- Relay owner nsec is valid --- @@ -1448,7 +1454,7 @@ Error: Invalid configuration ```bash # Production .env NGIT_DOMAIN=gitnostr.com -NGIT_OWNER_NPUB=npub1alice... +NGIT_RELAY_OWNER_NSEC=nsec1... NGIT_RELAY_NAME="GitNostr Public Relay" NGIT_RELAY_DESCRIPTION="Public GRASP relay for open source projects" NGIT_GIT_DATA_PATH=/var/lib/ngit-grasp/git @@ -1473,7 +1479,7 @@ RUST_LOG=info,ngit_grasp=debug ```bash # Development .env NGIT_DOMAIN=localhost:7334 -NGIT_OWNER_NPUB=npub1test... +NGIT_RELAY_OWNER_NSEC=nsec1... NGIT_RELAY_NAME="Dev Relay" NGIT_RELAY_DESCRIPTION="Local development" NGIT_GIT_DATA_PATH=./data/git @@ -1489,7 +1495,7 @@ RUST_LOG=debug ```bash # Testing .env NGIT_DOMAIN=localhost:9999 -NGIT_OWNER_NPUB=npub1test... +NGIT_RELAY_OWNER_NSEC=nsec1... NGIT_RELAY_NAME="Test Relay" NGIT_RELAY_DESCRIPTION="Automated testing" NGIT_GIT_DATA_PATH=/tmp/ngit-test/git