git.delta.rocks / jrsonnet / refs/commits / 3e7b063c34a7

difftreelog

feat secret regeneration

Yaroslav Bolyukin2024-11-30parent: #2a6bd68.patch.diff
in: trunk

6 files changed

modifiedcmds/fleet/src/cmds/secrets/mod.rsdiffbeforeafterboth
--- a/cmds/fleet/src/cmds/secrets/mod.rs
+++ b/cmds/fleet/src/cmds/secrets/mod.rs
@@ -130,13 +130,28 @@
 	},
 }
 
+fn secret_needs_regeneration(
+	secret: &FleetSecret,
+	expected_generation_data: &serde_json::Value,
+) -> bool {
+	let data_is_expected = secret.generation_data == *expected_generation_data;
+	// TODO: Leeway?
+	let expired = secret
+		.expires_at
+		.map(|expiration| expiration < Utc::now())
+		.unwrap_or(false);
+	expired || !data_is_expected
+}
+
+#[allow(clippy::too_many_arguments)]
 #[tracing::instrument(skip(config, secret, field, prefer_identities, batch))]
-async fn update_owner_set(
+async fn maybe_regenerate_shared_secret(
 	secret_name: &str,
 	config: &Config,
 	mut secret: FleetSharedSecret,
 	field: Value,
 	expected_owners: &[String],
+	expected_generation_data: serde_json::Value,
 	prefer_identities: &[String],
 	batch: Option<NixBuildBatch>,
 ) -> Result<FleetSharedSecret> {
@@ -145,12 +160,18 @@
 	let set = original_set.iter().collect::<BTreeSet<_>>();
 	let expected_set = expected_owners.iter().collect::<BTreeSet<_>>();
 
-	if set == expected_set {
+	let regeneration_required =
+		secret_needs_regeneration(&secret.secret, &expected_generation_data);
+
+	if set == expected_set && !regeneration_required {
 		info!("no need to update owner list, it is already correct");
 		return Ok(secret);
 	}
 
-	let should_regenerate = if set.difference(&expected_set).next().is_some() {
+	let should_regenerate = if regeneration_required {
+		info!("secret has its generation data changed, regeneration is required");
+		true
+	} else if set.difference(&expected_set).next().is_some() {
 		// TODO: Remove this warning for revokable secrets.
 		warn!("host was removed from secret owners, but until this host rebuild, the secret will still be stored on it.");
 		nix_go_json!(field.regenerateOnOwnerRemoved)
@@ -161,9 +182,16 @@
 	};
 
 	if should_regenerate {
-		info!("secret is owner-dependent, will regenerate");
-		let generated =
-			generate_shared(config, secret_name, field, expected_owners.to_vec(), batch).await?;
+		info!("secret needs to be regenerated");
+		let generated = generate_shared(
+			config,
+			secret_name,
+			field,
+			expected_owners.to_vec(),
+			expected_generation_data,
+			batch,
+		)
+		.await?;
 		Ok(generated)
 	} else {
 		drop(batch);
@@ -216,7 +244,8 @@
 	_display_name: &str,
 	secret: Value,
 	default_generator: Value,
-	owners: &[String],
+	expected_owners: &[String],
+	expected_generation_data: serde_json::Value,
 	batch: Option<NixBuildBatch>,
 ) -> Result<FleetSecret> {
 	let generator = nix_go!(secret.generator);
@@ -232,7 +261,7 @@
 	let mk_secret_generators = nix_go!(on_pkgs.mkSecretGenerators);
 
 	let mut recipients = Vec::new();
-	for owner in owners {
+	for owner in expected_owners {
 		let key = config.key(owner).await?;
 		recipients.push(key);
 	}
@@ -288,15 +317,15 @@
 		created_at,
 		expires_at,
 		parts,
-		// TODO: Fill with expected
-		generation_data: serde_json::Value::Null,
+		generation_data: expected_generation_data,
 	})
 }
 async fn generate(
 	config: &Config,
 	display_name: &str,
 	secret: Value,
-	owners: &[String],
+	expected_owners: &[String],
+	expected_generation_data: serde_json::Value,
 	batch: Option<NixBuildBatch>,
 ) -> Result<FleetSecret> {
 	let generator = nix_go!(secret.generator);
@@ -335,13 +364,21 @@
 				display_name,
 				secret,
 				default_generator,
-				owners,
+				expected_owners,
+				expected_generation_data,
 				batch,
 			)
 			.await
 		}
 		GeneratorKind::Pure => {
-			generate_pure(config, display_name, secret, default_generator, owners).await
+			generate_pure(
+				config,
+				display_name,
+				secret,
+				default_generator,
+				expected_owners,
+			)
+			.await
 		}
 	}
 }
@@ -350,11 +387,20 @@
 	display_name: &str,
 	secret: Value,
 	expected_owners: Vec<String>,
+	expected_generation_data: serde_json::Value,
 	batch: Option<NixBuildBatch>,
 ) -> Result<FleetSharedSecret> {
 	// let owners: Vec<String> = nix_go_json!(secret.expectedOwners);
 	Ok(FleetSharedSecret {
-		secret: generate(config, display_name, secret, &expected_owners, batch).await?,
+		secret: generate(
+			config,
+			display_name,
+			secret,
+			&expected_owners,
+			expected_generation_data,
+			batch,
+		)
+		.await?,
 		owners: expected_owners,
 	})
 }
@@ -615,13 +661,15 @@
 
 				let config_field = &config.config_field;
 				let field = nix_go!(config_field.sharedSecrets[{ name }]);
+				let expected_generation_data = nix_go_json!(field.expectedGenerationData);
 
-				let updated = update_owner_set(
+				let updated = maybe_regenerate_shared_secret(
 					&name,
 					config,
 					secret,
 					field,
 					&target_machines,
+					expected_generation_data,
 					&prefer_identities,
 					None,
 				)
@@ -630,7 +678,9 @@
 			}
 			Secret::Regenerate { prefer_identities } => {
 				info!("checking for secrets to regenerate");
+				let stored_shared_set = config.list_shared().into_iter().collect::<HashSet<_>>();
 				{
+					// Generate missing shared
 					let shared_batch = None;
 					let _span = info_span!("shared").entered();
 					let expected_shared_set = config
@@ -638,14 +688,15 @@
 						.await?
 						.into_iter()
 						.collect::<HashSet<_>>();
-					let shared_set = config.list_shared().into_iter().collect::<HashSet<_>>();
-					for missing in expected_shared_set.difference(&shared_set) {
+					for missing in expected_shared_set.difference(&stored_shared_set) {
 						let config_field = &config.config_field;
 						let secret = nix_go!(config_field.sharedSecrets[{ missing }]);
+						let expected_generation_data: serde_json::Value =
+							nix_go_json!(secret.expectedGenerationData);
 						let expected_owners: Option<Vec<String>> =
 							nix_go_json!(secret.expectedOwners);
 						let Some(expected_owners) = expected_owners else {
-							// TODO: Might still need to regenerate
+							// Can't generate this missing secret, as it has no defined owners.
 							continue;
 						};
 						info!("generating secret: {missing}");
@@ -654,6 +705,7 @@
 							missing,
 							secret,
 							expected_owners,
+							expected_generation_data,
 							shared_batch.clone(),
 						)
 						.in_current_span()
@@ -681,11 +733,13 @@
 					for missing in expected_set.difference(&stored_set) {
 						info!("generating secret: {missing}");
 						let secret = host.secret_field(missing).in_current_span().await?;
+						let expected_generation_data = nix_go_json!(secret.expectedGenerationData);
 						let generated = match generate(
 							config,
 							missing,
 							secret,
 							&[host.name.clone()],
+							expected_generation_data,
 							hosts_batch.clone(),
 						)
 						.in_current_span()
@@ -699,9 +753,35 @@
 						};
 						config.insert_secret(&host.name, missing.to_string(), generated)
 					}
+					for name in stored_set {
+						info!("updating secret: {name}");
+						let data = config.host_secret(&host.name, &name)?;
+						let secret = host.secret_field(&name).in_current_span().await?;
+						let expected_generation_data = nix_go_json!(secret.expectedGenerationData);
+						if secret_needs_regeneration(&data, &expected_generation_data) {
+							let generated = match generate(
+								config,
+								&name,
+								secret,
+								&[host.name.clone()],
+								expected_generation_data,
+								hosts_batch.clone(),
+							)
+							.in_current_span()
+							.await
+							{
+								Ok(v) => v,
+								Err(e) => {
+									error!("{e:?}");
+									continue;
+								}
+							};
+							config.insert_secret(&host.name, name.to_string(), generated)
+						}
+					}
 				}
 				let mut to_remove = Vec::new();
-				for name in &config.list_shared() {
+				for name in &stored_shared_set {
 					info!("updating secret: {name}");
 					let data = config.shared_secret(name)?;
 					let config_field = &config.config_field;
@@ -714,14 +794,16 @@
 					}
 
 					let secret = nix_go!(config_field.sharedSecrets[{ name }]);
+					let expected_generation_data = nix_go_json!(secret.expectedGenerationData);
 					config.replace_shared(
 						name.to_owned(),
-						update_owner_set(
+						maybe_regenerate_shared_secret(
 							name,
 							config,
 							data,
 							secret,
 							&expected_owners,
+							expected_generation_data,
 							&prefer_identities,
 							None,
 						)
modifiedcrates/fleet-base/src/host.rsdiffbeforeafterboth
--- a/crates/fleet-base/src/host.rs
+++ b/crates/fleet-base/src/host.rs
@@ -1,5 +1,6 @@
 use std::{
 	cell::OnceCell,
+	collections::BTreeSet,
 	ffi::{OsStr, OsString},
 	fmt::Display,
 	io::Write,
@@ -312,6 +313,23 @@
 }
 
 impl Config {
+	pub async fn tagged_hostnames(&self, tag: &str) -> Result<Vec<String>> {
+		let config = &self.config_field;
+		let tagged: Vec<String> = nix_go_json!(config.taggedWith[{ tag }]);
+		Ok(tagged)
+	}
+	pub async fn expand_owner_set(&self, owners: Vec<String>) -> Result<BTreeSet<String>> {
+		let mut out = BTreeSet::new();
+		for owner in owners {
+			if let Some(tag) = owner.strip_prefix('@') {
+				let hosts = self.tagged_hostnames(tag).await?;
+				out.extend(hosts);
+			} else {
+				out.insert(owner);
+			}
+		}
+		Ok(out)
+	}
 	pub fn local_host(&self) -> ConfigHost {
 		ConfigHost {
 			config: self.clone(),
modifiedcrates/fleet-base/src/keys.rsdiffbeforeafterboth
--- a/crates/fleet-base/src/keys.rs
+++ b/crates/fleet-base/src/keys.rs
@@ -45,6 +45,7 @@
 	}
 
 	pub async fn recipients(&self, hosts: Vec<String>) -> Result<Vec<impl Recipient>> {
+		let hosts = self.expand_owner_set(hosts).await?;
 		futures::stream::iter(hosts.iter())
 			.then(|m| self.recipient(m.as_ref()))
 			.try_collect::<Vec<_>>()
modifiedmodules/nixos/secrets.nixdiffbeforeafterboth
--- a/modules/nixos/secrets.nix
+++ b/modules/nixos/secrets.nix
@@ -41,17 +41,6 @@
           type = str;
           description = "Secret public data (only available for plaintext)";
         };
-
-        expectedGenerationData = mkOption {
-          type = unspecified;
-          description = "Data that gets embedded into secret part";
-          default = null;
-        };
-        generationData = mkOption {
-          type = unspecified;
-          description = "Data that is embedded into secret part";
-          default = null;
-        };
       };
       config = {
         hash = hashString "sha1" config.raw;
@@ -91,6 +80,11 @@
         default = sysConfig.users.users.${config.owner}.group;
         defaultText = literalExpression "config.users.users.$${owner}.group";
       };
+      expectedGenerationData = mkOption {
+        type = unspecified;
+        description = "Data that gets embedded into secret part";
+        default = null;
+      };
     };
   });
   processPart = part: {
modifiedmodules/secrets-data.nixdiffbeforeafterboth
--- a/modules/secrets-data.nix
+++ b/modules/secrets-data.nix
@@ -6,7 +6,7 @@
 }: let
   inherit (fleetLib.options) mkDataOption;
   inherit (lib.options) mkOption;
-  inherit (lib.types) nullOr listOf str attrsOf submodule bool;
+  inherit (lib.types) nullOr listOf str attrsOf submodule bool unspecified;
   inherit (lib.attrsets) mapAttrsToList mapAttrs filterAttrs genAttrs;
   inherit (lib.lists) sort unique concatLists;
   inherit (lib.strings) toJSON;
@@ -46,6 +46,11 @@
         '';
         default = [];
       };
+      generationData = mkOption {
+        type = unspecified;
+        description = "Data that is embedded into secret part";
+        default = null;
+      };
     };
   };
 
@@ -67,6 +72,11 @@
         description = "On which date this secret will expire, someone should regenerate this secret before it expires.";
         default = false;
       };
+      generationData = mkOption {
+        type = unspecified;
+        description = "Data that is embedded into secret part";
+        default = null;
+      };
     };
   };
 in {
@@ -93,12 +103,19 @@
   });
   config = {
     assertions =
-      mapAttrsToList
-      (name: secret: {
-        assertion = secret.expectedOwners == null || sort (a: b: a < b) config.data.sharedSecrets.${name}.owners == sort (a: b: a < b) secret.expectedOwners;
-        message = "Shared secret ${name} is expected to be encrypted for ${toJSON secret.expectedOwners}, but it is encrypted for ${toJSON config.data.sharedSecrets.${name}.owners}. Run fleet secrets regenerate to fix";
-      })
-      config.sharedSecrets;
+      (mapAttrsToList
+        (name: secret: {
+          assertion = secret.expectedOwners == null || sort (a: b: a < b) config.data.sharedSecrets.${name}.owners == sort (a: b: a < b) secret.expectedOwners;
+          message = "Shared secret ${name} is expected to be encrypted for ${toJSON secret.expectedOwners}, but it is encrypted for ${toJSON config.data.sharedSecrets.${name}.owners}. Run fleet secrets regenerate to fix";
+        })
+        config.sharedSecrets)
+      ++ (mapAttrsToList
+        (name: secret: {
+          # TODO: Same aassertion should be in host secrets
+          assertion = config.data.sharedSecrets.${name}.generationData == secret.expectedGenerationData;
+          message = "Shared secret ${name} has unexpected generation data ${toJSON secret.expectedGenerationData} != ${toJSON config.data.sharedSecrets.${name}.expectedGenerationData}. Run fleet secrets regenerate to fix";
+        })
+        config.sharedSecrets);
     sharedSecrets =
       mapAttrs (_: _: {}) config.data.sharedSecrets;
   };
modifiedmodules/secrets.nixdiffbeforeafterboth
before · modules/secrets.nix
1{2  lib,3  config,4  ...5}: let6  inherit (lib.options) mkOption literalExpression;7  inherit (lib.types) unspecified nullOr listOf str bool attrsOf submodule;8  inherit (lib.strings) concatStringsSep;9  inherit (lib.attrsets) mapAttrs;1011  sharedSecret = {config, ...}: {12    options = {13      expectedOwners = mkOption {14        type = nullOr (listOf str);15        description = ''16          List of hosts to encrypt secret for. null if managed by user (= via owners field from fleet.nix)1718          Secrets would be decrypted and stored to /run/secrets/$\{name} on owners19        '';20        default = null;21      };22      # TODO: Aren't those options may be just desugared to data/expectedData?23      regenerateOnOwnerAdded = mkOption {24        type = bool;25        description = ''26          Is this secret owner-dependent, and needs to be regenerated on ownership set change, or it may be just reencrypted.2728          You want to have this option set to true, when this secret contains some reference to its owners, i.e x509 SANs.29        '';30      };31      regenerateOnOwnerRemoved = mkOption {32        default = config.regenerateOnOwnerAdded;33        defaultText = literalExpression "regenerateOnOwnerAdded";34        type = bool;35        description = ''36          Should this secret be removed on owner removal, or it may be just reencrypted3738          Most probably its value should be equal to regenerateOnOwnerAdded, override only if you know what are you doing.39          Contrary to regenerateOnOwnerAdded, you may want to set this option to false, when host permissions are revoked40          in some other way than by this secret ownership, I.e by firewall/etc.41        '';42      };43      generator = mkOption {44        type = nullOr unspecified;45        description = "Derivation to evaluate for secret generation";46        default = null;47      };48    };49  };50in {51  options = {52    sharedSecrets = mkOption {53      type = attrsOf (submodule sharedSecret);54      default = {};55      description = "Shared secrets";56    };57  };58  config = {59    hosts =60      mapAttrs (_: secretMap: {61        nixos.secrets = mapAttrs (_: s: removeAttrs s ["createdAt" "expiresAt"]) secretMap;62      })63      config.data.hostSecrets;64    nixpkgs.overlays = [65      (final: prev: {66        mkSecretGenerators = {recipients}: rec {67          # TODO: Merge both generators to one with consistent options syntax?68          # Impure generator is built on local machine, then built closure is copied to remote machine,69          # and then it is ran in inpure context, so that this generator may access HSMs and other things.70          mkImpureSecretGenerator = {71            script,72            # If set - script will be run on remote machine, otherwise it will be run with fleet project in CWD73            # (Some secrets-encryption-in-git/managed PKI solution is expected)74            impureOn ? null,75          }:76            (prev.writeShellScript "impureGenerator.sh" ''77              #!/bin/sh78              set -eu7980              export GENERATOR_HELPER_IDENTITIES="${concatStringsSep "\n" recipients}";81              export PATH=${final.fleet-generator-helper}/bin:$PATH8283              # TODO: Provide tempdir from outside, to make it securely erasurable as needed?84              tmp=$(mktemp -d)85              cd $tmp86              # cd /var/empty8788              created_at=$(date -u +"%Y-%m-%dT%H:%M:%S.%NZ")8990              ${script}9192              if ! test -d $out; then93                echo "impure generator script did not produce expected \$out output"94                exit 195              fi9697              echo -n $created_at > $out/created_at98              echo -n SUCCESS > $out/marker99            '')100            .overrideAttrs (old: {101              passthru = {102                inherit impureOn;103                generatorKind = "impure";104              };105            });106          # Pure generators are disabled for now107          mkSecretGenerator = {script}: mkImpureSecretGenerator {inherit script;};108109          # TODO: Implement consistent naming110          # Pure secret generator is supposed to be run entirely by nix, using `__impure` derivation type...111          # But for now, it is ran the same way as `impureSecretGenerator`, but on the local machine.112          # mkSecretGenerator = {script}:113          #   (prev.writeShellScript "generator.sh" ''114          #     #!/bin/sh115          #     set -eu116          #     # TODO: make nix daemon build secret, not just the script.117          #     cd /var/empty118          #119          #     created_at=$(date -u +"%Y-%m-%dT%H:%M:%S.%NZ")120          #121          #     ${script}122          #     if ! test -d $out; then123          #       echo "impure generator script did not produce expected \$out output"124          #       exit 1125          #     fi126          #127          #     echo -n $created_at > $out/created_at128          #     echo -n SUCCESS > $out/marker129          #   '')130          #   .overrideAttrs (old: {131          #     passthru = {132          #       generatorKind = "pure";133          #     };134          #     # TODO: make nix daemon build secret, not just the script.135          #     # __impure = true;136          #   });137        };138      })139    ];140  };141}
after · modules/secrets.nix
1{2  lib,3  config,4  ...5}: let6  inherit (lib.options) mkOption literalExpression;7  inherit (lib.types) unspecified nullOr listOf str bool attrsOf submodule;8  inherit (lib.strings) concatStringsSep;9  inherit (lib.attrsets) mapAttrs;1011  sharedSecret = {config, ...}: {12    options = {13      expectedOwners = mkOption {14        type = nullOr (listOf str);15        description = ''16          List of hosts to encrypt secret for. null if managed by user (= via owners field from fleet.nix)1718          Secrets would be decrypted and stored to /run/secrets/$\{name} on owners19        '';20        default = null;21      };22      # TODO: Aren't those options may be just desugared to data/expectedData?23      regenerateOnOwnerAdded = mkOption {24        type = bool;25        description = ''26          Is this secret owner-dependent, and needs to be regenerated on ownership set change, or it may be just reencrypted.2728          You want to have this option set to true, when this secret contains some reference to its owners, i.e x509 SANs.29        '';30      };31      regenerateOnOwnerRemoved = mkOption {32        default = config.regenerateOnOwnerAdded;33        defaultText = literalExpression "regenerateOnOwnerAdded";34        type = bool;35        description = ''36          Should this secret be removed on owner removal, or it may be just reencrypted3738          Most probably its value should be equal to regenerateOnOwnerAdded, override only if you know what are you doing.39          Contrary to regenerateOnOwnerAdded, you may want to set this option to false, when host permissions are revoked40          in some other way than by this secret ownership, I.e by firewall/etc.41        '';42      };43      generator = mkOption {44        type = nullOr unspecified;45        description = "Derivation to evaluate for secret generation";46        default = null;47      };48      expectedGenerationData = mkOption {49        type = unspecified;50        description = "Data that gets embedded into secret part";51        default = null;52      };53    };54  };55in {56  options = {57    sharedSecrets = mkOption {58      type = attrsOf (submodule sharedSecret);59      default = {};60      description = "Shared secrets";61    };62  };63  config = {64    hosts =65      mapAttrs (_: secretMap: {66        nixos.secrets = mapAttrs (_: s: removeAttrs s ["createdAt" "expiresAt"]) secretMap;67      })68      config.data.hostSecrets;69    nixpkgs.overlays = [70      (final: prev: {71        mkSecretGenerators = {recipients}: rec {72          # TODO: Merge both generators to one with consistent options syntax?73          # Impure generator is built on local machine, then built closure is copied to remote machine,74          # and then it is ran in inpure context, so that this generator may access HSMs and other things.75          mkImpureSecretGenerator = {76            script,77            # If set - script will be run on remote machine, otherwise it will be run with fleet project in CWD78            # (Some secrets-encryption-in-git/managed PKI solution is expected)79            impureOn ? null,80          }:81            (prev.writeShellScript "impureGenerator.sh" ''82              #!/bin/sh83              set -eu8485              export GENERATOR_HELPER_IDENTITIES="${concatStringsSep "\n" recipients}";86              export PATH=${final.fleet-generator-helper}/bin:$PATH8788              # TODO: Provide tempdir from outside, to make it securely erasurable as needed?89              tmp=$(mktemp -d)90              cd $tmp91              # cd /var/empty9293              created_at=$(date -u +"%Y-%m-%dT%H:%M:%S.%NZ")9495              ${script}9697              if ! test -d $out; then98                echo "impure generator script did not produce expected \$out output"99                exit 1100              fi101102              echo -n $created_at > $out/created_at103              echo -n SUCCESS > $out/marker104            '')105            .overrideAttrs (old: {106              passthru = {107                inherit impureOn;108                generatorKind = "impure";109              };110            });111          # Pure generators are disabled for now112          mkSecretGenerator = {script}: mkImpureSecretGenerator {inherit script;};113114          # TODO: Implement consistent naming115          # Pure secret generator is supposed to be run entirely by nix, using `__impure` derivation type...116          # But for now, it is ran the same way as `impureSecretGenerator`, but on the local machine.117          # mkSecretGenerator = {script}:118          #   (prev.writeShellScript "generator.sh" ''119          #     #!/bin/sh120          #     set -eu121          #     # TODO: make nix daemon build secret, not just the script.122          #     cd /var/empty123          #124          #     created_at=$(date -u +"%Y-%m-%dT%H:%M:%S.%NZ")125          #126          #     ${script}127          #     if ! test -d $out; then128          #       echo "impure generator script did not produce expected \$out output"129          #       exit 1130          #     fi131          #132          #     echo -n $created_at > $out/created_at133          #     echo -n SUCCESS > $out/marker134          #   '')135          #   .overrideAttrs (old: {136          #     passthru = {137          #       generatorKind = "pure";138          #     };139          #     # TODO: make nix daemon build secret, not just the script.140          #     # __impure = true;141          #   });142        };143      })144    ];145  };146}