diff --git a/rust/krb5-provision-keytab/src/lib.rs b/rust/krb5-provision-keytab/src/lib.rs index 41181b88..543c02bf 100644 --- a/rust/krb5-provision-keytab/src/lib.rs +++ b/rust/krb5-provision-keytab/src/lib.rs @@ -71,25 +71,60 @@ pub async fn provision_keytab(krb5_config_path: &Path, req: &Request) -> Result< let req_str = serde_json::to_vec(&req).context(SerializeRequestSnafu)?; let mut child = Command::new("stackable-krb5-provision-keytab") + // make sure the process is killed if we error out of this fn somewhere due to + // an error when writing to stdin or getting stdout + // Usually we'd expect the process to terminate on its own, this is a fail safe to ensure + // it gets killed in case it hangs for some reason. .kill_on_drop(true) .env("KRB5_CONFIG", krb5_config_path) // ldap3 uses the default client keytab to authenticate to the LDAP server .env("KRB5_CLIENT_KTNAME", &req.admin_keytab_path) - // avoid leaking credentials between secret volumes/secretclasses + // avoid leaking credentials between secret volumes/secretclasses by only storing the + // TGT that is obtained for the operation in the memory of the short lives process + // spawned by `Command::new` above - this way it'll be wiped from memory once this exits + // With any shared or persistent ticket cache this might stick around and potentially be + // reused by other volumes (which could cause privilege escalations and similar fun issues) .env("KRB5CCNAME", "MEMORY:") .stdin(Stdio::piped()) .stdout(Stdio::piped()) .spawn() .context(SpawnProvisionerSnafu)?; - let mut stdin = child.stdin.take().unwrap(); + + // Get a `ChildStdin` object for the spawned process and write the serialized request + // for a Principal into it in order for the child process to deserialize it and + // process the request + let mut stdin = child + .stdin + .take() + .expect("Failed to read from stdin of stackable-krb5-provision-keytab script! "); stdin.write_all(&req_str).await.context(WriteRequestSnafu)?; stdin.flush().await.context(WriteRequestSnafu)?; drop(stdin); + + // Wait for the process to finish and capture output + // This will always return Ok(...) regardless of exit code or output of the child process + // Failure here means that something went wrong with connecting to the process or obtaining + // exit code or output let output = child .wait_with_output() .await .context(WaitProvisionerSnafu)?; - serde_json::from_slice::>(&output.stdout) - .context(DeserializeResponseSnafu)? - .map_err(|msg| Error::RunProvisioner { msg }) + + // Check if the spawned command returned 0 as return code + if output.status.success() { + // Check for success of the operation by deserializing stdout of the process to a `Response` + // struct - since `Response` is an empty struct with no fields this effectively means that + // any output will fail to deserialize and cause an `Error::RunProvisioner` to be propagated + // with the output of the child process + serde_json::from_slice::>(&output.stdout) + .context(DeserializeResponseSnafu)? + .map_err(|msg| Error::RunProvisioner { msg }) + } else { + Err(Error::RunProvisioner { + msg: format!( + "Got non zero return code from stackable-krb5-provision-keytab: [{:?}]", + output.status.code() + ), + }) + } }