all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 06/10] client: allow passing specific master key
Date: Fri,  5 Feb 2021 16:35:32 +0100	[thread overview]
Message-ID: <20210205153535.2578184-8-f.gruenbichler@proxmox.com> (raw)
In-Reply-To: <20210205153535.2578184-1-f.gruenbichler@proxmox.com>

it's needed for PVE's LXC integration, and might be interesting for
other more special usage scenarios as well.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/bin/proxmox-backup-client.rs     | 180 ++++++++++++++++++++++-----
 src/bin/proxmox_backup_client/key.rs |  27 ++++
 2 files changed, 178 insertions(+), 29 deletions(-)

diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 8268098e..b737d9f0 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -77,15 +77,24 @@ pub const REPO_URL_SCHEMA: Schema = StringSchema::new("Repository URL.")
     .max_length(256)
     .schema();
 
-pub const KEYFILE_SCHEMA: Schema = StringSchema::new(
-    "Path to encryption key. All data will be encrypted using this key.")
-    .schema();
+pub const KEYFILE_SCHEMA: Schema =
+    StringSchema::new("Path to encryption key. All data will be encrypted using this key.")
+        .schema();
+
+pub const KEYFD_SCHEMA: Schema =
+    IntegerSchema::new("Pass an encryption key via an already opened file descriptor.")
+        .minimum(0)
+        .schema();
 
-pub const KEYFD_SCHEMA: Schema = IntegerSchema::new(
-    "Pass an encryption key via an already opened file descriptor.")
-    .minimum(0)
+pub const MASTER_PUBKEY_FILE_SCHEMA: Schema = StringSchema::new(
+    "Path to master public key. The encryption key used for a backup will be encrypted using this key and appended to the backup.")
     .schema();
 
+pub const MASTER_PUBKEY_FD_SCHEMA: Schema =
+    IntegerSchema::new("Pass a master public key via an already opened file descriptor.")
+        .minimum(0)
+        .schema();
+
 const CHUNK_SIZE_SCHEMA: Schema = IntegerSchema::new(
     "Chunk size in KB. Must be a power of 2.")
     .minimum(64)
@@ -601,6 +610,8 @@ fn spawn_catalog_upload(
 struct CryptoParams {
     mode: CryptMode,
     enc_key: Option<Vec<u8>>,
+    // FIXME switch to openssl::rsa::rsa<openssl::pkey::Public> once that is Eq?
+    master_pubkey: Option<Vec<u8>>,
 }
 
 fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
@@ -622,6 +633,24 @@ fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
         None => None,
     };
 
+    let master_pubkey_file = match param.get("master-pubkey-file") {
+        Some(Value::String(keyfile)) => Some(keyfile),
+        Some(_) => bail!("bad --master-pubkey-file parameter type"),
+        None => None,
+    };
+
+    let master_pubkey_fd = match param.get("master-pubkey-fd") {
+        Some(Value::Number(key_fd)) => Some(
+            RawFd::try_from(key_fd
+                .as_i64()
+                .ok_or_else(|| format_err!("bad master public key fd: {:?}", key_fd))?
+            )
+            .map_err(|err| format_err!("bad public master key fd: {:?}: {}", key_fd, err))?
+        ),
+        Some(_) => bail!("bad --master-pubkey-fd parameter type"),
+        None => None,
+    };
+
     let mode: Option<CryptMode> = match param.get("crypt-mode") {
         Some(mode) => Some(serde_json::from_value(mode.clone())?),
         None => None,
@@ -646,39 +675,105 @@ fn crypto_parameters(param: &Value) -> Result<CryptoParams, Error> {
         }
     };
 
-    Ok(match (keydata, mode) {
+    let master_pubkey_data = match (master_pubkey_file, master_pubkey_fd) {
+        (None, None) => None,
+        (Some(_), Some(_)) => bail!("--keyfile and --keyfd are mutually exclusive"),
+        (Some(keyfile), None) => {
+            eprintln!("Using master key from file: {}", keyfile);
+            Some(file_get_contents(keyfile)?)
+        },
+        (None, Some(fd)) => {
+            let input = unsafe { std::fs::File::from_raw_fd(fd) };
+            let mut data = Vec::new();
+            let _len: usize = { input }.read_to_end(&mut data)
+                .map_err(|err| {
+                    format_err!("error reading master key from fd {}: {}", fd, err)
+                })?;
+            eprintln!("Using master key from file descriptor");
+            Some(data)
+        }
+    };
+
+    Ok(match (keydata, master_pubkey_data, mode) {
         // no parameters:
-        (None, None) => match key::read_optional_default_encryption_key()? {
-            None => CryptoParams { enc_key: None, mode: CryptMode::None },
+        (None, None, None) => match key::read_optional_default_encryption_key()? {
+            None => CryptoParams { mode: CryptMode::None, enc_key: None, master_pubkey: None },
             enc_key => {
                 eprintln!("Encrypting with default encryption key!");
-                CryptoParams { enc_key, mode: CryptMode::Encrypt }
+                let master_pubkey = key::read_optional_default_master_pubkey()?;
+                CryptoParams {
+                    mode: CryptMode::Encrypt,
+                    enc_key,
+                    master_pubkey,
+                }
             },
         },
 
         // just --crypt-mode=none
-        (None, Some(CryptMode::None)) => CryptoParams { enc_key: None, mode: CryptMode::None },
+        (None, None, Some(CryptMode::None)) => CryptoParams { mode: CryptMode::None, enc_key: None, master_pubkey: None },
+
+        // --keyfile and --crypt-mode=none
+        (Some(_), _, Some(CryptMode::None)) => {
+            bail!("--keyfile/--keyfd and --crypt-mode=none are mutually exclusive");
+        },
+
+        // --master-pubkey-file and --crypt-mode=none
+        (_, Some(_), Some(CryptMode::None)) => {
+            bail!("--master-pubkey-file/--master-pubkey-fd and --crypt-mode=none are mutually exclusive");
+        },
 
-        // just --crypt-mode other than none
-        (None, Some(mode)) => match key::read_optional_default_encryption_key()? {
+        // --master-pubkey-file and nothing else
+        (None, master_pubkey, None) => {
+            match key::read_optional_default_encryption_key()? {
+                None => bail!("--master-pubkey-file/--master-pubkey-fd specified, but no key available"),
+                enc_key => {
+                    eprintln!("Encrypting with default encryption key!");
+                    CryptoParams {
+                        mode: CryptMode::Encrypt,
+                        enc_key,
+                        master_pubkey,
+                    }
+                },
+            }
+        },
+
+        // --crypt-mode other than none, without keyfile, with or without master key
+        (None, master_pubkey, Some(mode)) => match key::read_optional_default_encryption_key()? {
             None => bail!("--crypt-mode without --keyfile and no default key file available"),
             enc_key => {
                 eprintln!("Encrypting with default encryption key!");
+                let master_pubkey = match master_pubkey {
+                    None => key::read_optional_default_master_pubkey()?,
+                    master_pubkey => master_pubkey,
+                };
 
-                CryptoParams { enc_key, mode }
+                CryptoParams {
+                    mode,
+                    enc_key,
+                    master_pubkey,
+                }
             },
         }
 
         // just --keyfile
-        (enc_key, None) => CryptoParams { enc_key, mode: CryptMode::Encrypt },
+        (enc_key, master_pubkey, None) => {
+            let master_pubkey = match master_pubkey {
+                None => key::read_optional_default_master_pubkey()?,
+                master_pubkey => master_pubkey,
+            };
 
-        // --keyfile and --crypt-mode=none
-        (Some(_), Some(CryptMode::None)) => {
-            bail!("--keyfile/--keyfd and --crypt-mode=none are mutually exclusive");
-        }
+            CryptoParams { mode: CryptMode::Encrypt, enc_key, master_pubkey }
+        },
 
         // --keyfile and --crypt-mode other than none
-        (enc_key, Some(mode)) => CryptoParams { enc_key, mode },
+        (enc_key, master_pubkey, Some(mode)) => {
+            let master_pubkey = match master_pubkey {
+                None => key::read_optional_default_master_pubkey()?,
+                master_pubkey => master_pubkey,
+            };
+
+            CryptoParams { mode, enc_key, master_pubkey }
+        },
     })
 }
 
@@ -689,11 +784,31 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
     let some_key = Some(vec![1;1]);
     let default_key = Some(vec![2;1]);
 
-    let no_key_res = CryptoParams { enc_key: None, mode: CryptMode::None };
-    let some_key_res = CryptoParams { enc_key: some_key.clone(), mode: CryptMode::Encrypt };
-    let some_key_sign_res = CryptoParams { enc_key: some_key.clone(), mode: CryptMode::SignOnly };
-    let default_key_res = CryptoParams { enc_key: default_key.clone(), mode: CryptMode::Encrypt };
-    let default_key_sign_res = CryptoParams { enc_key: default_key.clone(), mode: CryptMode::SignOnly };
+    let no_key_res = CryptoParams {
+        enc_key: None,
+        master_pubkey: None,
+        mode: CryptMode::None,
+    };
+    let some_key_res = CryptoParams {
+        enc_key: some_key.clone(),
+        master_pubkey: None,
+        mode: CryptMode::Encrypt,
+    };
+    let some_key_sign_res = CryptoParams {
+        enc_key: some_key.clone(),
+        master_pubkey: None,
+        mode: CryptMode::SignOnly,
+    };
+    let default_key_res = CryptoParams {
+        enc_key: default_key.clone(),
+        master_pubkey: None,
+        mode: CryptMode::Encrypt,
+    };
+    let default_key_sign_res = CryptoParams {
+        enc_key: default_key.clone(),
+        master_pubkey: None,
+        mode: CryptMode::SignOnly,
+    };
 
     let keypath = "./tests/keyfile.test";
     replace_file(&keypath, some_key.as_ref().unwrap(), CreateOptions::default())?;
@@ -840,6 +955,14 @@ fn test_crypto_parameters_handling() -> Result<(), Error> {
                schema: KEYFD_SCHEMA,
                optional: true,
            },
+           "master-pubkey-file": {
+               schema: MASTER_PUBKEY_FILE_SCHEMA,
+               optional: true,
+           },
+           "master-pubkey-fd": {
+               schema: MASTER_PUBKEY_FD_SCHEMA,
+               optional: true,
+           },
            "crypt-mode": {
                type: CryptMode,
                optional: true,
@@ -1026,16 +1149,14 @@ async fn create_backup(
 
             let crypt_config = CryptConfig::new(key)?;
 
-            match key::find_default_master_pubkey()? {
-                Some(ref path) if path.exists() => {
-                    let pem_data = file_get_contents(path)?;
+            match crypto.master_pubkey {
+                Some(pem_data) => {
                     let rsa = openssl::rsa::Rsa::public_key_from_pem(&pem_data)?;
 
                     let mut key_config = KeyConfig::without_password(key)?;
                     key_config.created = created; // keep original value
 
                     let enc_key = rsa_encrypt_key_config(rsa, &key_config)?;
-                    println!("Master key '{:?}'", path);
 
                     (Some(Arc::new(crypt_config)), Some(enc_key))
                 }
@@ -1941,6 +2062,7 @@ fn main() {
         .completion_cb("repository", complete_repository)
         .completion_cb("backupspec", complete_backup_source)
         .completion_cb("keyfile", tools::complete_file_name)
+        .completion_cb("master-pubkey-file", tools::complete_file_name)
         .completion_cb("chunk-size", complete_chunk_size);
 
     let benchmark_cmd_def = CliCommand::new(&API_METHOD_BENCHMARK)
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index 070f2a0b..7dd28473 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -58,6 +58,13 @@ pub fn read_optional_default_encryption_key() -> Result<Option<Vec<u8>>, Error>
         .transpose()
 }
 
+#[cfg(not(test))]
+pub fn read_optional_default_master_pubkey() -> Result<Option<Vec<u8>>, Error> {
+    find_default_master_pubkey()?
+        .map(file_get_contents)
+        .transpose()
+}
+
 #[cfg(test)]
 static mut TEST_DEFAULT_ENCRYPTION_KEY: Result<Option<Vec<u8>>, Error> = Ok(None);
 
@@ -78,6 +85,26 @@ pub unsafe fn set_test_encryption_key(value: Result<Option<Vec<u8>>, Error>) {
     TEST_DEFAULT_ENCRYPTION_KEY = value;
 }
 
+#[cfg(test)]
+static mut TEST_DEFAULT_MASTER_PUBKEY: Result<Option<Vec<u8>>, Error> = Ok(None);
+
+#[cfg(test)]
+pub fn read_optional_default_master_pubkey() -> Result<Option<Vec<u8>>, Error> {
+    // not safe when multiple concurrent test cases end up here!
+    unsafe {
+        match &TEST_DEFAULT_MASTER_PUBKEY {
+            Ok(key) => Ok(key.clone()),
+            Err(_) => bail!("test error"),
+        }
+    }
+}
+
+#[cfg(test)]
+// not safe when multiple concurrent test cases end up here!
+pub unsafe fn set_test_default_master_pubkey(value: Result<Option<Vec<u8>>, Error>) {
+    TEST_DEFAULT_MASTER_PUBKEY = value;
+}
+
 pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
     // fixme: implement other input methods
 
-- 
2.20.1





  parent reply	other threads:[~2021-02-05 15:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-05 15:35 [pbs-devel] [PATCH proxmox-backup 00/11] extend master key feature Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 01/10] key: make 'default' master key explicit Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH storage] pbs: allow setting up a master key Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 02/10] key: add show-master-pubkey command Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 03/10] key: rustfmt module Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 04/10] client: add test for keyfile_parameters Fabian Grünbichler
2021-02-06  8:00   ` Dietmar Maurer
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 05/10] client: refactor keyfile_parameters Fabian Grünbichler
2021-02-05 15:35 ` Fabian Grünbichler [this message]
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 07/10] client: extend tests for master key handling Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 08/10] client: refactor crypto_parameter handling Fabian Grünbichler
2021-02-05 15:35 ` [pbs-devel] [PATCH proxmox-backup 09/10] client: track key source, print when used Fabian Grünbichler
2021-02-06  8:13 ` [pbs-devel] applied: [PATCH proxmox-backup 00/11] extend master key feature Dietmar Maurer
2021-02-08 11:02   ` Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210205153535.2578184-8-f.gruenbichler@proxmox.com \
    --to=f.gruenbichler@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal