[pbs-devel] [PATCH proxmox-backup v2 1/2] support more ENV vars to get secret values

[pbs-devel] [PATCH proxmox-backup v2 1/2] support more ENV vars to get secret values

[Dietmar Maurer]

---

Changes in v2:

- always use the first line
- only read the first line (not whole files)
- improve error handling
- improve docs

 pbs-client/src/tools/key_source.rs |  9 +--
 pbs-client/src/tools/mod.rs        | 90 +++++++++++++++++++++++++++---
 2 files changed, 84 insertions(+), 15 deletions(-)

diff --git a/pbs-client/src/tools/key_source.rs b/pbs-client/src/tools/key_source.rs
index 6dade75e..a3a2bf1a 100644
--- a/pbs-client/src/tools/key_source.rs
+++ b/pbs-client/src/tools/key_source.rs
@@ -343,13 +343,8 @@ pub(crate) unsafe fn set_test_default_master_pubkey(value: Result<Option<Vec<u8>
 pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
     // fixme: implement other input methods
 
-    use std::env::VarError::*;
-    match std::env::var("PBS_ENCRYPTION_PASSWORD") {
-        Ok(p) => return Ok(p.as_bytes().to_vec()),
-        Err(NotUnicode(_)) => bail!("PBS_ENCRYPTION_PASSWORD contains bad characters"),
-        Err(NotPresent) => {
-            // Try another method
-        }
+    if let Some(password) = super::get_secret_from_env("PBS_ENCRYPTION_PASSWORD")? {
+        return Ok(password.as_bytes().to_vec());
     }
 
     // If we're on a TTY, query the user for a password
diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 7b932b63..2c1c6e6c 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -1,5 +1,10 @@
 //! Shared tools useful for common CLI clients.
 use std::collections::HashMap;
+use std::fs::File;
+use std::os::unix::io::FromRawFd;
+use std::env::VarError::{NotUnicode, NotPresent};
+use std::io::{BufReader, BufRead};
+use std::process::Command;
 
 use anyhow::{bail, format_err, Context, Error};
 use serde_json::{json, Value};
@@ -7,6 +12,7 @@ use xdg::BaseDirectories;
 
 use proxmox::{
     api::schema::*,
+    api::cli::shellword_split,
     tools::fs::file_get_json,
 };
 
@@ -32,6 +38,80 @@ pub const CHUNK_SIZE_SCHEMA: Schema = IntegerSchema::new("Chunk size in KB. Must
     .default(4096)
     .schema();
 
+/// Helper to read a secret through a environment variable (ENV).
+///
+/// Tries the following variable names in order and returns the value
+/// it will resolve for the first defined one:
+///
+/// BASE_NAME => use value from ENV(BASE_NAME) directly as secret
+/// BASE_NAME_FD => read the secret from the specified file descriptor
+/// BASE_NAME_FILE => read the secret from the specified file name
+/// BASE_NAME_CMD => read the secret from specified command first line of output on stdout
+///
+/// Only return the first line of data (without CRLF).
+pub fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
+
+    let firstline = |data: String| -> String {
+        match data.lines().next() {
+            Some(line) => line.to_string(),
+            None => String::new(),
+        }
+    };
+
+    let firstline_file = |file: &mut File| -> Result<String, Error> {
+        let reader = BufReader::new(file);
+        match reader.lines().next() {
+            Some(Ok(line)) => Ok(line),
+            Some(Err(err)) => Err(err.into()),
+            None => Ok(String::new()),
+        }
+    };
+
+    match std::env::var(base_name) {
+        Ok(p) => return Ok(Some(firstline(p))),
+        Err(NotUnicode(_)) => bail!(format!("{} contains bad characters", base_name)),
+        Err(NotPresent) => {},
+    };
+
+    let env_name = format!("{}_FD", base_name);
+    match std::env::var(&env_name) {
+        Ok(fd_str) => {
+            let fd: i32 = fd_str.parse()
+                .map_err(|err| format_err!("unable to parse file descriptor in ENV({}): {}", env_name, err))?;
+            let mut file = unsafe { File::from_raw_fd(fd) };
+            return Ok(Some(firstline_file(&mut file)?));
+        }
+        Err(NotUnicode(_)) => bail!(format!("{} contains bad characters", env_name)),
+        Err(NotPresent) => {},
+    }
+
+    let env_name = format!("{}_FILE", base_name);
+    match std::env::var(&env_name) {
+        Ok(filename) => {
+            let mut file = std::fs::File::open(filename)
+                .map_err(|err| format_err!("unable to open file in ENV({}): {}", env_name, err))?;
+            return Ok(Some(firstline_file(&mut file)?));
+        }
+        Err(NotUnicode(_)) => bail!(format!("{} contains bad characters", env_name)),
+        Err(NotPresent) => {},
+    }
+
+    let env_name = format!("{}_CMD", base_name);
+    match std::env::var(&env_name) {
+        Ok(ref command) => {
+            let args = shellword_split(command)?;
+            let mut command = Command::new(&args[0]);
+            command.args(&args[1..]);
+            let output = pbs_tools::run_command(command, None)?;
+            return Ok(Some(firstline(output)));
+        }
+        Err(NotUnicode(_)) => bail!(format!("{} contains bad characters", env_name)),
+        Err(NotPresent) => {},
+    }
+
+    Ok(None)
+}
+
 pub fn get_default_repository() -> Option<String> {
     std::env::var("PBS_REPOSITORY").ok()
 }
@@ -64,13 +144,7 @@ pub fn connect(repo: &BackupRepository) -> Result<HttpClient, Error> {
 fn connect_do(server: &str, port: u16, auth_id: &Authid) -> Result<HttpClient, Error> {
     let fingerprint = std::env::var(ENV_VAR_PBS_FINGERPRINT).ok();
 
-    use std::env::VarError::*;
-    let password = match std::env::var(ENV_VAR_PBS_PASSWORD) {
-        Ok(p) => Some(p),
-        Err(NotUnicode(_)) => bail!(format!("{} contains bad characters", ENV_VAR_PBS_PASSWORD)),
-        Err(NotPresent) => None,
-    };
-
+    let password = get_secret_from_env(ENV_VAR_PBS_PASSWORD)?;
     let options = HttpClientOptions::new_interactive(password, fingerprint);
 
     HttpClient::new(server, port, auth_id, options)
@@ -80,7 +154,7 @@ fn connect_do(server: &str, port: u16, auth_id: &Authid) -> Result<HttpClient, E
 pub async fn try_get(repo: &BackupRepository, url: &str) -> Value {
 
     let fingerprint = std::env::var(ENV_VAR_PBS_FINGERPRINT).ok();
-    let password = std::env::var(ENV_VAR_PBS_PASSWORD).ok();
+    let password = get_secret_from_env(ENV_VAR_PBS_PASSWORD).unwrap_or(None);
 
     // ticket cache, but no questions asked
     let options = HttpClientOptions::new_interactive(password, fingerprint)
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 2/2] doc: Document new environment vars to specify secret values

[Dietmar Maurer]

---
 docs/backup-client.rst | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/docs/backup-client.rst b/docs/backup-client.rst
index bd0fdfed..df98d540 100644
--- a/docs/backup-client.rst
+++ b/docs/backup-client.rst
@@ -49,15 +49,48 @@ Environment Variables
   When set, this value is used for the password required for the backup server.
   You can also set this to a API token secret.
 
+  .. Note:: if not set, we also query ``PBS_PASSWORD_FD``,
+     ``PBS_PASSWORD_FILE`` and ``PBS_PASSWORD_CMD`` (in that order)
+     and return the first value set.
+
+``PBS_PASSWORD_FD``
+  Like ``PBS_PASSWORD``, but read data from Unix file descriptor.
+
+``PBS_PASSWORD_FILE``
+  Like ``PBS_PASSWORD``, but read data from specified file name.
+
+``PBS_PASSWORD_CMD``
+  Like ``PBS_PASSWORD``, but read data from specified command (stdout).
+
 ``PBS_ENCRYPTION_PASSWORD``
   When set, this value is used to access the secret encryption key (if
   protected by password).
 
+  .. Note:: if not set, we also query ``PBS_ENCRYPTION_PASSWORD_FD``,
+     ``PBS_ENCRYPTION_PASSWORD_FILE`` and
+     ``PBS_ENCRYPTION_PASSWORD_CMD`` (in that order) and return the
+     first value set.
+
+``PBS_ENCRYPTION_PASSWORD_FD``
+  Like ``PBS_ENCRYPTION_PASSWORD``, but read data from Unix file descriptor.
+
+``PBS_ENCRYPTION_PASSWORD_FILE``
+  Like ``PBS_ENCRYPTION_PASSWORD``, but read data from specified file name.
+
+``PBS_ENCRYPTION_PASSWORD_CMD``
+  Like ``PBS_ENCRYPTION_PASSWORD``, but read data from specified command (stdout).
+
 ``PBS_FINGERPRINT`` When set, this value is used to verify the server
   certificate (only used if the system CA certificates cannot validate the
   certificate).
 
 
+.. Note:: Passwords must be valid UTF8 an may not contain
+   newlines. For your convienience, we just use the first line as
+   password, so you can add arbitrary comments after the
+   first newline.
+
+
 Output Format
 -------------
 
-- 
2.30.2

[pbs-devel] applied: [PATCH proxmox-backup v2 1/2] support more ENV vars to get secret values

[Thomas Lamprecht]

On 21.07.21 11:59, Dietmar Maurer wrote:
> ---
> 
> Changes in v2:
> 
> - always use the first line
> - only read the first line (not whole files)
> - improve error handling
> - improve docs
> 
>  pbs-client/src/tools/key_source.rs |  9 +--
>  pbs-client/src/tools/mod.rs        | 90 +++++++++++++++++++++++++++---
>  2 files changed, 84 insertions(+), 15 deletions(-)
> 
>

applied, thanks!

[pbs-devel] applied: [PATCH proxmox-backup v2 2/2] doc: Document new environment vars to specify secret values

[Thomas Lamprecht]

On 21.07.21 11:59, Dietmar Maurer wrote:
> ---
>  docs/backup-client.rst | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
>

applied, thanks! I moved the alternative ones to a single point in a followup though,
as they are all for the same thing, just different mechanisms, and to avoid crowding
the list with to much points.

grammar/language I and you used can surely still be improved though (@dylan) :)