[pve-devel] [PATCH-SERIES proxmox{, -mail-forward, -backup}/pve-manager] add proxmox-mail-forward helper binary

[pve-devel] [PATCH-SERIES proxmox{, -mail-forward, -backup}/pve-manager] add proxmox-mail-forward helper binary

[Fiona Ebner]

written in Rust, and replacing the pvemailforward binary in PVE. Can
be used in PVE and PBS as well as in a mixed installations of the two.

To make reading the config files work, it is a setuid binary owned by
root, but it sets the effective UID to the real UID after reading the
configs, so parsing and sendmail invocation happen with lower
privileges again (well, except if the binary was called by root
directly).

The .forward file is updated during postinst as currently done in
pve-manager. proxmox-mail-forward's postinst will not do anything
when detecting a pvemailforward entry. Instead pve-manager is
responsible for the switchover in PVE.


proxmox-mail-forward needs a depenency bump for proxmox-section-config
for the functionality added by the first patch.

proxmox-backup recommends proxmox-mail-forward (I felt a Recommends
is more fitting, but feel free to change it) and pve-manager depends
on proxmox-mail-forward are part of the series.


proxmox:

Fiona Ebner (1):
  section config: parse additional properties when schema allows it

 proxmox-section-config/src/lib.rs | 79 ++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)


proxmox-mail-forward:

Fiona Ebner (3):
  initial commit
  add Debian packaging
  d/postinst: register binary in .forward


proxmox-backup:

Fiona Ebner (1):
  fix #4287: d/control: recommend proxmox-mail-forward

 debian/control | 1 +
 1 file changed, 1 insertion(+)


pve-manager:

Fiona Ebner (4):
  d/control: depend on proxmox-mail-forward
  d/postinst: replace pvemailforward with proxmox-mail-forward
  remove pvemailforward binary
  d/control: drop ${shlibs:Depends} for pve-manager

 bin/Makefile             | 11 +++-------
 bin/pvemailforward.c     | 17 ---------------
 bin/pvemailforward.pl    | 45 ----------------------------------------
 debian/control           |  2 +-
 debian/lintian-overrides |  4 ----
 debian/postinst          | 10 +++++++--
 debian/rules             |  2 +-
 7 files changed, 13 insertions(+), 78 deletions(-)
 delete mode 100644 bin/pvemailforward.c
 delete mode 100755 bin/pvemailforward.pl

-- 
2.30.2

[pve-devel] [PATCH proxmox 1/1] section config: parse additional properties when schema allows it

[Fiona Ebner]

Additional properties will be parsed according to the default string
schema.

This is relevant for use cases when the full schema is not known for
some reason or another. In particular this allows support for parsing
older/newer versions of configuration files. One example of this is
the proposed proxmox-mail-forward helper binary, which currently
doesn't have access to the PBS API types for dependency reasons and
is only interested in the email field for the root user. If it can
only use a minimal schema with additional_properties set to true, it
will be robust against changes.

Writing already works, because the ObjectSchema's verify_json()
already handles additional_properties correctly and
format_section_content() handles them like all other properties
(method doesn't depend on the schema).

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 proxmox-section-config/src/lib.rs | 79 ++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)

diff --git a/proxmox-section-config/src/lib.rs b/proxmox-section-config/src/lib.rs
index 154f56e..080f23c 100644
--- a/proxmox-section-config/src/lib.rs
+++ b/proxmox-section-config/src/lib.rs
@@ -31,6 +31,9 @@ use proxmox_lang::try_block;
 use proxmox_schema::format::{dump_properties, wrap_text, ParameterDisplayStyle};
 use proxmox_schema::*;
 
+/// Used for additional properties when the schema allows them.
+const ADDITIONAL_PROPERTY_SCHEMA: Schema = StringSchema::new("Additional property").schema();
+
 /// Associates a section type name with a `Schema`.
 pub struct SectionConfigPlugin {
     type_name: String,
@@ -446,7 +449,10 @@ impl SectionConfig {
                                         (true, items)
                                     }
                                     Some((_optional, ref prop_schema)) => (false, prop_schema),
-                                    None => bail!("unknown property '{}'", key),
+                                    None => match plugin.properties.additional_properties() {
+                                        true => (false, &&ADDITIONAL_PROPERTY_SCHEMA),
+                                        false => bail!("unknown property '{}'", key),
+                                    },
                                 };
 
                                 let value = match prop_schema.parse_simple_value(&value) {
@@ -884,6 +890,77 @@ lvmthin: local-lvm2
     assert_eq!(raw, created);
 }
 
+#[test]
+fn test_section_config_with_additional_properties() {
+    let filename = "user.cfg";
+
+    const ID_SCHEMA: Schema = StringSchema::new("default id schema.")
+        .min_length(3)
+        .schema();
+    let mut config = SectionConfig::new(&ID_SCHEMA);
+    let mut config_with_additional = SectionConfig::new(&ID_SCHEMA);
+
+    const PROPERTIES: [(&str, bool, &proxmox_schema::Schema); 2] = [
+        (
+            "email",
+            false,
+            &StringSchema::new("The e-mail of the user").schema(),
+        ),
+        (
+            "userid",
+            true,
+            &StringSchema::new("The id of the user (name@realm).")
+                .min_length(3)
+                .schema(),
+        ),
+    ];
+
+    const USER_PROPERTIES: ObjectSchema = ObjectSchema {
+        description: "user properties",
+        properties: &PROPERTIES,
+        additional_properties: false,
+        default_key: None,
+    };
+
+    const USER_PROPERTIES_WITH_ADDTIONAL: ObjectSchema = ObjectSchema {
+        description: "user properties with additional",
+        properties: &PROPERTIES,
+        additional_properties: true,
+        default_key: None,
+    };
+
+    let plugin = SectionConfigPlugin::new(
+        "user".to_string(),
+        Some("userid".to_string()),
+        &USER_PROPERTIES,
+    );
+    config.register_plugin(plugin);
+
+    let plugin = SectionConfigPlugin::new(
+        "user".to_string(),
+        Some("userid".to_string()),
+        &USER_PROPERTIES_WITH_ADDTIONAL,
+    );
+    config_with_additional.register_plugin(plugin);
+
+    let raw = r"
+
+user: root@pam
+        email root@example.com
+        shinynewoption somevalue
+";
+
+    let res = config_with_additional.parse(filename, raw);
+    println!("RES: {:?}", res);
+    let written = config_with_additional.write(filename, &res.unwrap());
+    println!("CONFIG:\n{}", written.unwrap());
+
+    assert!(config.parse(filename, raw).is_err());
+    // SectionConfigData doesn't have Clone and it would only be needed here currently.
+    let res = config_with_additional.parse(filename, raw);
+    assert!(config.write(filename, &res.unwrap()).is_err());
+}
+
 /// Generate ReST Documentaion for ``SectionConfig``
 pub fn dump_section_config(config: &SectionConfig) -> String {
     let mut res = String::new();
-- 
2.30.2

[pve-devel] [PATCH proxmox-mail-forward 1/3] initial commit

[Fiona Ebner]

It is intended to replace the current pvemailforward binary+script in
PVE and also be used in PBS. The implemenation is largely based on the
pvemailforward script to try and keep behavior mostly the same in PVE.

To read the config in PBS, the binary would need to belong to
backup:backup with setuid and setgid bits (proxmox-backup is 700 owned
by backup:backup and user.cfg is 640 owned by root:backup). To read
the configs in PVE the setgid bit for www-data would need to be set.

To avoid this issue, the helper will be a root-owned setuid binary and
set the effective UID to the real UID, after reading in the config
files.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Dependency bump for proxmox-section-config needed!

 .cargo/config |   5 ++
 .gitignore    |   2 +
 Cargo.toml    |  25 ++++++++
 rustfmt.toml  |   1 +
 src/main.rs   | 174 ++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 207 insertions(+)
 create mode 100644 .cargo/config
 create mode 100644 .gitignore
 create mode 100644 Cargo.toml
 create mode 100644 rustfmt.toml
 create mode 100644 src/main.rs

diff --git a/.cargo/config b/.cargo/config
new file mode 100644
index 0000000..3b5b6e4
--- /dev/null
+++ b/.cargo/config
@@ -0,0 +1,5 @@
+[source]
+[source.debian-packages]
+directory = "/usr/share/cargo/registry"
+[source.crates-io]
+replace-with = "debian-packages"
diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..1e7caa9
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,2 @@
+Cargo.lock
+target/
diff --git a/Cargo.toml b/Cargo.toml
new file mode 100644
index 0000000..4dd0681
--- /dev/null
+++ b/Cargo.toml
@@ -0,0 +1,25 @@
+[package]
+name = "proxmox-mail-forward"
+version = "0.1.0"
+authors = [
+    "Fiona Ebner <f.ebner@proxmox.com>",
+    "Proxmox Support Team <support@proxmox.com>",
+]
+edition = "2021"
+license = "AGPL-3"
+description = "Proxmox mail forward helper"
+homepage = "https://www.proxmox.com"
+
+exclude = [ "debian" ]
+
+[dependencies]
+anyhow = "1.0"
+log = "0.4.17"
+nix = "0.24"
+serde = { version = "1.0", features = ["derive"] }
+#serde_json = "1.0"
+syslog = "4.0"
+
+proxmox-schema = "1.3.4"
+proxmox-section-config = "1.0"
+proxmox-sys = "0.4"
diff --git a/rustfmt.toml b/rustfmt.toml
new file mode 100644
index 0000000..3a26366
--- /dev/null
+++ b/rustfmt.toml
@@ -0,0 +1 @@
+edition = "2021"
diff --git a/src/main.rs b/src/main.rs
new file mode 100644
index 0000000..e0cacb2
--- /dev/null
+++ b/src/main.rs
@@ -0,0 +1,174 @@
+use std::path::Path;
+use std::process::Command;
+
+use anyhow::{bail, format_err, Error};
+use serde::Deserialize;
+
+use proxmox_schema::{ObjectSchema, Schema, StringSchema};
+use proxmox_section_config::{SectionConfig, SectionConfigPlugin};
+use proxmox_sys::fs;
+
+const PBS_USER_CFG_FILENAME: &str = "/etc/proxmox-backup/user.cfg";
+const PBS_ROOT_USER: &str = "root@pam";
+
+// FIXME: Switch to the actual schema when possible in terms of dependency.
+// It's safe to assume that the config was written with the actual schema restrictions, so parsing
+// it with the less restrictive schema should be enough for the purpose of getting the mail address.
+const DUMMY_ID_SCHEMA: Schema = StringSchema::new("dummy ID").min_length(3).schema();
+const DUMMY_EMAIL_SCHEMA: Schema = StringSchema::new("dummy email").schema();
+const DUMMY_USER_SCHEMA: ObjectSchema = ObjectSchema {
+    description: "minimal PBS user",
+    properties: &[
+        ("userid", false, &DUMMY_ID_SCHEMA),
+        ("email", true, &DUMMY_EMAIL_SCHEMA),
+    ],
+    additional_properties: true,
+    default_key: None,
+};
+
+#[derive(Deserialize)]
+struct DummyPbsUser {
+    pub email: Option<String>,
+}
+
+const PVE_USER_CFG_FILENAME: &str = "/etc/pve/user.cfg";
+const PVE_DATACENTER_CFG_FILENAME: &str = "/etc/pve/datacenter.cfg";
+const PVE_ROOT_USER: &str = "root@pam";
+
+/// Convenience helper to get the trimmed contents of an optional &str, mapping blank ones to `None`
+/// and creating a String from it for returning.
+fn normalize_for_return(s: Option<&str>) -> Option<String> {
+    match s?.trim() {
+        "" => None,
+        s => Some(s.to_string()),
+    }
+}
+
+/// Extract the root user's email address from the PBS user config.
+fn get_pbs_mail_to(content: &str) -> Option<String> {
+    let mut config = SectionConfig::new(&DUMMY_ID_SCHEMA);
+    let user_plugin = SectionConfigPlugin::new(
+        "user".to_string(),
+        Some("userid".to_string()),
+        &DUMMY_USER_SCHEMA,
+    );
+    config.register_plugin(user_plugin);
+
+    match config.parse(PBS_USER_CFG_FILENAME, content) {
+        Ok(parsed) => {
+            parsed.sections.get(PBS_ROOT_USER)?;
+            match parsed.lookup::<DummyPbsUser>("user", PBS_ROOT_USER) {
+                Ok(user) => normalize_for_return(user.email.as_deref()),
+                Err(err) => {
+                    log::error!("unable to parse {} - {}", PBS_USER_CFG_FILENAME, err);
+                    None
+                }
+            }
+        }
+        Err(err) => {
+            log::error!("unable to parse {} - {}", PBS_USER_CFG_FILENAME, err);
+            None
+        }
+    }
+}
+
+/// Extract the root user's email address from the PVE user config.
+fn get_pve_mail_to(content: &str) -> Option<String> {
+    normalize_for_return(content.lines().find_map(|line| {
+        let fields: Vec<&str> = line.split(':').collect();
+        #[allow(clippy::get_first)] // to keep expression style consistent
+        match fields.get(0)?.trim() == "user" && fields.get(1)?.trim() == PVE_ROOT_USER {
+            true => fields.get(6).copied(),
+            false => None,
+        }
+    }))
+}
+
+/// Extract the From-address configured in the PVE datacenter config.
+fn get_pve_mail_from(content: &str) -> Option<String> {
+    normalize_for_return(
+        content
+            .lines()
+            .find_map(|line| line.strip_prefix("email_from:")),
+    )
+}
+
+/// Executes sendmail as a child process with the specified From/To-addresses, expecting the mail
+/// contents to be passed via stdin inherited from this program.
+fn forward_mail(mail_from: String, mail_to: Vec<String>) -> Result<(), Error> {
+    if mail_to.is_empty() {
+        bail!("user 'root@pam' does not have an email address");
+    }
+
+    log::info!("forward mail to <{}>", mail_to.join(","));
+
+    let mut cmd = Command::new("sendmail");
+    cmd.args([
+        "-bm", "-N", "never", // never send DSN (avoid mail loops)
+        "-f", &mail_from, "--",
+    ]);
+    cmd.args(mail_to);
+    cmd.env("PATH", "/sbin:/bin:/usr/sbin:/usr/bin");
+
+    // with status(), child inherits stdin
+    cmd.status()
+        .map_err(|err| format_err!("command {:?} failed - {}", cmd, err))?;
+
+    Ok(())
+}
+
+/// Wrapper around `proxmox_sys::fs::file_read_optional_string` which also returns `None` upon error
+/// after logging it.
+fn attempt_file_read<P: AsRef<Path>>(path: P) -> Option<String> {
+    match fs::file_read_optional_string(path) {
+        Ok(contents) => contents,
+        Err(err) => {
+            log::error!("{}", err);
+            None
+        }
+    }
+}
+
+fn main() {
+    if let Err(err) = syslog::init(
+        syslog::Facility::LOG_DAEMON,
+        log::LevelFilter::Info,
+        Some("proxmox-mail-forward"),
+    ) {
+        eprintln!("unable to inititialize syslog - {}", err);
+    }
+
+    let pbs_user_cfg_content = attempt_file_read(PBS_USER_CFG_FILENAME);
+    let pve_user_cfg_content = attempt_file_read(PVE_USER_CFG_FILENAME);
+    let pve_datacenter_cfg_content = attempt_file_read(PVE_DATACENTER_CFG_FILENAME);
+
+    let real_uid = nix::unistd::getuid();
+    if let Err(err) = nix::unistd::seteuid(real_uid) {
+        log::error!(
+            "mail forward failed: unable to set effective uid to {}: {}",
+            real_uid,
+            err
+        );
+        return;
+    }
+
+    let pbs_mail_to = pbs_user_cfg_content.and_then(|content| get_pbs_mail_to(&content));
+    let pve_mail_to = pve_user_cfg_content.and_then(|content| get_pve_mail_to(&content));
+    let pve_mail_from = pve_datacenter_cfg_content.and_then(|content| get_pve_mail_from(&content));
+
+    let mail_from = pve_mail_from.unwrap_or_else(|| "root".to_string());
+
+    let mut mail_to = vec![];
+    if let Some(pve_mail_to) = pve_mail_to {
+        mail_to.push(pve_mail_to);
+    }
+    if let Some(pbs_mail_to) = pbs_mail_to {
+        if !mail_to.contains(&pbs_mail_to) {
+            mail_to.push(pbs_mail_to);
+        }
+    }
+
+    if let Err(err) = forward_mail(mail_from, mail_to) {
+        log::error!("mail forward failed: {}", err);
+    }
+}
-- 
2.30.2

[pve-devel] [PATCH proxmox-mail-forward 2/3] add Debian packaging

[Fiona Ebner]

It is a setuid binary owned by root, since the initial step of reading
the configuration files on both PVE and PBS requires higher privileges
which can't be mapped easily otherwise.

Used parts of the packaging in proxmox-backup as a basis.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 .gitignore               |  5 +++
 Makefile                 | 73 ++++++++++++++++++++++++++++++++++++++++
 debian/changelog         |  5 +++
 debian/compat            |  1 +
 debian/control           | 30 +++++++++++++++++
 debian/copyright         | 16 +++++++++
 debian/lintian-overrides |  2 ++
 debian/rules             | 18 ++++++++++
 8 files changed, 150 insertions(+)
 create mode 100644 Makefile
 create mode 100644 debian/changelog
 create mode 100644 debian/compat
 create mode 100644 debian/control
 create mode 100644 debian/copyright
 create mode 100644 debian/lintian-overrides
 create mode 100755 debian/rules

diff --git a/.gitignore b/.gitignore
index 1e7caa9..3bee320 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,7 @@
 Cargo.lock
 target/
+proxmox-mail-forward-*/
+*proxmox-mail-forward*.buildinfo
+*proxmox-mail-forward*.tar.?z
+*proxmox-mail-forward*.changes
+*proxmox-mail-forward*.deb
diff --git a/Makefile b/Makefile
new file mode 100644
index 0000000..89e7843
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,73 @@
+include /usr/share/dpkg/pkg-info.mk
+include /usr/share/dpkg/architecture.mk
+
+PACKAGE=proxmox-mail-forward
+BUILDDIR ?= $(PACKAGE)-$(DEB_VERSION_UPSTREAM)
+BUILDDIR_TMP ?= $(BUILDDIR).tmp
+
+ifeq ($(BUILD_MODE), release)
+CARGO_BUILD_ARGS += --release
+COMPILEDIR := target/release
+else
+COMPILEDIR := target/debug
+endif
+
+CARGO ?= cargo
+
+DEB=$(PACKAGE)_$(DEB_VERSION_UPSTREAM_REVISION)_$(DEB_BUILD_ARCH).deb
+DBG_DEB=$(PACKAGE)-dbgsym_$(DEB_VERSION_UPSTREAM_REVISION)_$(DEB_BUILD_ARCH).deb
+DSC=rust-$(PACKAGE)_$(DEB_VERSION_UPSTREAM_REVISION).dsc
+
+DEBS=$(DEB) $(DBG_DEB)
+
+.PHONY: build
+build:
+	@echo "Setting Cargo.toml version to: $(DEB_VERSION_UPSTREAM)"
+	sed -i -e 's/^version =.*$$/version = "$(DEB_VERSION_UPSTREAM)"/' Cargo.toml
+	rm -rf $(BUILDDIR) $(BUILDDIR_TMP); mkdir $(BUILDDIR_TMP)
+	cp -a debian \
+	  Cargo.toml src \
+	  Makefile \
+	  $(BUILDDIR_TMP)
+	rm -f $(BUILDDIR_TMP)/Cargo.lock
+	find $(BUILDDIR_TMP)/debian -name "*.hint" -delete
+	mv $(BUILDDIR_TMP) $(BUILDDIR)
+
+.PHONY: deb
+$(DEBS): deb
+deb: build
+	cd $(BUILDDIR); dpkg-buildpackage -b -us -uc --no-pre-clean
+	lintian $(DEBS)
+
+.PHONY: dsc
+dsc: $(DSC)
+$(DSC): build
+	cd $(BUILDDIR); dpkg-buildpackage -S -us -uc -d -nc
+	lintian $(DSC)
+
+.PHONY: dinstall
+dinstall: $(DEBS)
+	dpkg -i $(DEBS)
+
+.PHONY: cargo-build
+cargo-build:
+	$(CARGO) build $(CARGO_BUILD_ARGS) \
+	    --package proxmox-mail-forward \
+	    --bin proxmox-mail-forward
+
+install: cargo-build
+	install -dm755 $(DESTDIR)/usr/bin
+	install -m4755 -o root -g root $(COMPILEDIR)/proxmox-mail-forward $(DESTDIR)/usr/bin/proxmox-mail-forward
+
+.PHONY: upload
+upload: $(DEBS)
+	tar cf - $(DEBS) | ssh -X repoman@repo.proxmox.com -- upload --product "pve,pbs" --dist bullseye --arch $(DEB_BUILD_ARCH)
+
+.PHONY: distclean
+distclean: clean
+
+.PHONY: clean
+clean:
+	cargo clean
+	rm -rf *.deb *.buildinfo *.changes *.dsc rust-$(PACKAGE)_*.tar.?z $(BUILDDIR) $(BUILDDIR_TMP)
+	find . -name '*~' -exec rm {} ';'
diff --git a/debian/changelog b/debian/changelog
new file mode 100644
index 0000000..cb9cc94
--- /dev/null
+++ b/debian/changelog
@@ -0,0 +1,5 @@
+rust-proxmox-mail-forward (0.1.0-1) stable; urgency=medium
+
+  * Initial release.
+
+ -- Proxmox Support Team <support@proxmox.com>  Thu, 20 Oct 2022 11:28:37 +0200
diff --git a/debian/compat b/debian/compat
new file mode 100644
index 0000000..48082f7
--- /dev/null
+++ b/debian/compat
@@ -0,0 +1 @@
+12
diff --git a/debian/control b/debian/control
new file mode 100644
index 0000000..72e00ff
--- /dev/null
+++ b/debian/control
@@ -0,0 +1,30 @@
+Source: rust-proxmox-mail-forward
+Section: rust
+Priority: optional
+Build-Depends: debhelper (>= 12),
+ cargo:native,
+ rustc:native,
+ libstd-rust-dev,
+ librust-anyhow-1+default-dev,
+ librust-log-0.4+default-dev (>= 0.4.17-~~),
+ librust-nix-0.24+default-dev,
+ librust-proxmox-schema-1+default-dev (>= 1.3.4-~~),
+ librust-proxmox-section-config-1+default-dev,
+ librust-proxmox-sys-0.4+default-dev,
+ librust-serde-1+default-dev,
+ librust-serde-1+derive-dev,
+ librust-syslog-4+default-dev
+Maintainer: Proxmox Support Team <support@proxmox.com>
+Standards-Version: 4.5.1
+Vcs-Git: git://git.proxmox.com/git/proxmox-mail-forward.git
+Vcs-Browser: https://git.proxmox.com/?p=proxmox-mail-forward.git
+Homepage: https://www.proxmox.com
+Rules-Requires-Root: binary-targets
+
+Package: proxmox-mail-forward
+Architecture: any
+Depends: ${misc:Depends},
+         ${shlibs:Depends},
+Description: Proxmox mail forward helper
+ This package contains the Proxmox mail forward helper. It forwards mails to the
+ address(es) of the root@pam user in Proxmox Backup Server and Proxmox VE.
diff --git a/debian/copyright b/debian/copyright
new file mode 100644
index 0000000..d2d30fc
--- /dev/null
+++ b/debian/copyright
@@ -0,0 +1,16 @@
+Copyright (C) 2022 Proxmox Server Solutions GmbH
+
+This software is written by Proxmox Server Solutions GmbH <support@proxmox.com>
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU Affero General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Affero General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
diff --git a/debian/lintian-overrides b/debian/lintian-overrides
new file mode 100644
index 0000000..bdd0599
--- /dev/null
+++ b/debian/lintian-overrides
@@ -0,0 +1,2 @@
+proxmox-mail-forward: no-manual-page usr/bin/proxmox-mail-forward
+proxmox-mail-forward: setuid-binary usr/bin/proxmox-mail-forward 4755 root/root
diff --git a/debian/rules b/debian/rules
new file mode 100755
index 0000000..0e5f66c
--- /dev/null
+++ b/debian/rules
@@ -0,0 +1,18 @@
+#!/usr/bin/make -f
+# See debhelper(7) (uncomment to enable)
+# output every command that modifies files on the build system.
+DH_VERBOSE = 1
+
+export BUILD_MODE=release
+
+CARGO=/usr/share/cargo/bin/cargo
+
+export CFLAGS CXXFLAGS CPPFLAGS LDFLAGS
+export DEB_HOST_RUST_TYPE DEB_HOST_GNU_TYPE
+export CARGO_HOME = $(CURDIR)/debian/cargo_home
+
+%:
+	dh $@
+
+override_dh_fixperms:
+	dh_fixperms --exclude proxmox-mail-forward
-- 
2.30.2

[pve-devel] [PATCH proxmox-mail-forward 3/3] d/postinst: register binary in .forward

[Fiona Ebner]

Similar to how it is done for pve-manager in PVE. If pvemailforward is
detected, nothing is done. An adapted pve-manager will cleanly handle
the switchover for PVE installations.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 debian/proxmox-mail-forward.postinst | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100755 debian/proxmox-mail-forward.postinst

diff --git a/debian/proxmox-mail-forward.postinst b/debian/proxmox-mail-forward.postinst
new file mode 100755
index 0000000..1c81eea
--- /dev/null
+++ b/debian/proxmox-mail-forward.postinst
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+set -e
+
+case "$1" in
+    configure)
+        # pve-manager is responsible for switching over, so skip when detecting pvemailforward
+        if ! test -f /root/.forward || ! grep -E -q '\|/usr/bin/(proxmox-mail-|pvemail)forward' /root/.forward; then
+            echo '|/usr/bin/proxmox-mail-forward' >>/root/.forward
+        fi
+    ;;
+
+    abort-upgrade|abort-remove|abort-deconfigure|triggered)
+    ;;
+
+    *)
+        echo "postinst called with unknown argument \`$1'" >&2
+        exit 1
+    ;;
+esac
+
+exit 0
-- 
2.30.2

[pve-devel] [PATCH proxmox-backup 1/1] fix #4287: d/control: recommend proxmox-mail-forward

[Fiona Ebner]

which registers a binary in /root/.forward and handles mail forwarding
to the mail addresss configured for root@pam in PBS. Similar to how it
is done in PVE currently.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 debian/control | 1 +
 1 file changed, 1 insertion(+)

diff --git a/debian/control b/debian/control
index 65331524..35066256 100644
--- a/debian/control
+++ b/debian/control
@@ -173,6 +173,7 @@ Depends: fonts-font-awesome,
 Recommends: zfsutils-linux,
             ifupdown2,
             proxmox-offline-mirror-helper,
+            proxmox-mail-forward,
 Description: Proxmox Backup Server daemon with tools and GUI
  This package contains the Proxmox Backup Server daemons and related
  tools. This includes a web-based graphical user interface.
-- 
2.30.2

[pve-devel] [PATCH manager 1/4] d/control: depend on proxmox-mail-forward

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 debian/control | 1 +
 1 file changed, 1 insertion(+)

diff --git a/debian/control b/debian/control
index 57d92f1e..251bdf77 100644
--- a/debian/control
+++ b/debian/control
@@ -74,6 +74,7 @@ Depends: apt-transport-https | apt (>= 1.5~),
          pciutils,
          perl (>= 5.10.0-19),
          postfix | mail-transport-agent,
+         proxmox-mail-forward,
          proxmox-mini-journalreader (>= 1.3-1),
          proxmox-widget-toolkit (>= 3.4-9),
          pve-cluster (>= 7.0-4),
-- 
2.30.2

[pve-devel] [PATCH manager 2/4] d/postinst: replace pvemailforward with proxmox-mail-forward

[Fiona Ebner]

proxmox-mail-forward is a new helper binary in Rust intended to behave
essentially the same on PVE installations. It can also handle mixed
PBS+PVE installations.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
The downside with this approach is that a downgrade of
pve-manager will re-introduce the pvemailforward entry in .forward,
which means duplicate mails (except proxmox-mail-forward is removed
again at the same time).

An alternative would be using a Breaks on pve-manager:
1. don't touch .forward in pve-manager's postinst anymore
2. have proxmox-mail-forward break older pve-manager
3. have proxmox-mail-forward's postinst replace the entry in .forward

 debian/postinst | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/debian/postinst b/debian/postinst
index 7980ef98..d8ee170d 100755
--- a/debian/postinst
+++ b/debian/postinst
@@ -126,8 +126,14 @@ case "$1" in
         pveam update || true
     fi
 
-    if ! test -f /root/.forward || ! grep -q '|/usr/bin/pvemailforward' /root/.forward; then
-        echo '|/usr/bin/pvemailforward' >>/root/.forward
+    # Always try to clean old entry, even when proxmox-mail-forward entry is already present.
+    # This ensures it will still be cleaned after an upgrade following a downgrade.
+    if test -f /root/.forward; then
+        sed -i '\!|/usr/bin/pvemailforward!d' /root/.forward
+    fi
+
+    if ! test -f /root/.forward || ! grep -q '|/usr/bin/proxmox-mail-forward' /root/.forward; then
+        echo '|/usr/bin/proxmox-mail-forward' >>/root/.forward
     fi
 
     systemctl --system daemon-reload >/dev/null || true
-- 
2.30.2

[pve-devel] [PATCH manager 3/4] remove pvemailforward binary

[Fiona Ebner]

which was replaced by proxmox-mail-forward living in its own package.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 bin/Makefile             | 11 +++-------
 bin/pvemailforward.c     | 17 ---------------
 bin/pvemailforward.pl    | 45 ----------------------------------------
 debian/lintian-overrides |  4 ----
 debian/rules             |  2 +-
 5 files changed, 4 insertions(+), 75 deletions(-)
 delete mode 100644 bin/pvemailforward.c
 delete mode 100755 bin/pvemailforward.pl

diff --git a/bin/Makefile b/bin/Makefile
index fb475be3..55e5f20d 100644
--- a/bin/Makefile
+++ b/bin/Makefile
@@ -14,7 +14,6 @@ SCRIPTS =  			\
 	${CLITOOLS}		\
 	pvebanner		\
 	pveversion		\
-	pvemailforward.pl	\
 	pveupgrade		\
 	pveupdate		\
 	pveperf			\
@@ -37,7 +36,7 @@ ZSH_COMPLETIONS =						\
 	$(addsuffix .service-zsh-completion, ${SERVICES}) 	\
 	$(addsuffix .zsh-completion, ${CLITOOLS})		\
 
-all: ${SERVICE_MANS} ${CLI_MANS} pvemailforward
+all: ${SERVICE_MANS} ${CLI_MANS}
 
 %.1: %.1.pod
 	rm -f $@
@@ -56,20 +55,16 @@ pveversion.1.pod: pveversion
 pveupgrade.1.pod: pveupgrade
 pvereport.1.pod: pvereport
 
-pvemailforward: pvemailforward.c
-	$(CC) $(CPPFLAGS) $(CFLAGS) -Wall -g -O2 $< -o $@
-
 .PHONY: check
 check: $(addsuffix .service-api-verified, ${SERVICES}) $(addsuffix .api-verified, ${CLITOOLS})
 	rm -f *.service-api-verified *.api-verified
 
 .PHONY: install
-install: ${SCRIPTS} ${CLI_MANS} ${SERVICE_MANS} pvemailforward ${BASH_COMPLETIONS} ${ZSH_COMPLETIONS}
+install: ${SCRIPTS} ${CLI_MANS} ${SERVICE_MANS} ${BASH_COMPLETIONS} ${ZSH_COMPLETIONS}
 	install -d ${BINDIR}
 	install -m 0755 ${SCRIPTS} ${BINDIR}
 	install -d ${USRSHARE}/helpers
 	install -m 0755 pve-startall-delay ${USRSHARE}/helpers
-	install -s -m 2755 -g www-data pvemailforward ${BINDIR}
 	install -d ${MAN1DIR}
 	install -m 0644 ${CLI_MANS} ${MAN1DIR}
 	install -d ${MAN8DIR}
@@ -82,7 +77,7 @@ install: ${SCRIPTS} ${CLI_MANS} ${SERVICE_MANS} pvemailforward ${BASH_COMPLETION
 .PHONY: clean
 clean:
 	make cleanup-docgen
-	rm -rf *~ *.tmp ${CLI_MANS} ${SERVICE_MANS} *.1.pod *.8.pod pvemailforward  \
+	rm -rf *~ *.tmp ${CLI_MANS} ${SERVICE_MANS} *.1.pod *.8.pod		    \
 	    *.bash-completion *.service-bash-completion				    \
 	    *.zsh-completion *.service-zsh-completion				    \
 	    *.api-verified *.service-api-verified
diff --git a/bin/pvemailforward.c b/bin/pvemailforward.c
deleted file mode 100644
index 06013a63..00000000
--- a/bin/pvemailforward.c
+++ /dev/null
@@ -1,17 +0,0 @@
-/* see 'man perlsec'
- *
- */ 
-#include <unistd.h>
-#include <stdlib.h>
-#include <stdio.h>
-
-#define REAL_PATH "/usr/bin/pvemailforward.pl"
-
-int main(int argc, char **argv)
-{
-    execv(REAL_PATH, argv);
- 
-    fprintf(stderr, "exec '%s' failed\n", REAL_PATH);
-
-    exit(-1);
-}
diff --git a/bin/pvemailforward.pl b/bin/pvemailforward.pl
deleted file mode 100755
index 18a57497..00000000
--- a/bin/pvemailforward.pl
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/usr/bin/perl -T
-
-use strict;
-use warnings;
-use PVE::Tools;
-use PVE::SafeSyslog;
-use PVE::AccessControl;
-use PVE::Cluster qw (cfs_read_file);
-use PVE::DataCenterConfig;
-
-# NOTE: we need to run this with setgid www-data
-# else we cant read /etc/pve/user.cfg
-
-$( = $); # $GID = $EGID
-
-$ENV{'PATH'} = '/sbin:/bin:/usr/sbin:/usr/bin';
-
-initlog('pvemailforward');
-
-
-PVE::Cluster::cfs_update();
-
-eval {
-    my $usercfg = cfs_read_file("user.cfg");
-    my $rootcfg = $usercfg->{users}->{'root@pam'} || {};
-    my $mailto = $rootcfg->{email};
-
-    my $dcconf = cfs_read_file('datacenter.cfg');
-    my $mailfrom = $dcconf->{email_from} || "root";
-
-    die "user 'root\@pam' does not have a email address\n" if !$mailto;
-
-    syslog("info", "forward mail to <$mailto>");
-
-    # we never send DSN (avoid mail loops)
-    open(CMD, "|sendmail -bm -N never -f $mailfrom $mailto") ||
-	die "can't exec sendmail - $!\n";
-    while (<>) { print CMD $_; }
-    close(CMD);
-};
-if (my $err = $@) {
-    syslog('err', "mail forward failed: $err");
-}
-
-exit(0);
diff --git a/debian/lintian-overrides b/debian/lintian-overrides
index fba73dcf..e30e7054 100644
--- a/debian/lintian-overrides
+++ b/debian/lintian-overrides
@@ -1,11 +1,7 @@
 pve-manager: mail-transport-agent-dependency-does-not-specify-default-mta *
 pve-manager: no-manual-page usr/bin/pvebanner
-pve-manager: no-manual-page usr/bin/pvemailforward
-pve-manager: no-manual-page usr/bin/pvemailforward.pl
 pve-manager: no-manual-page usr/bin/pveupdate
 pve-manager: non-standard-dir-perm var/log/pveproxy/ 0700 != 0755
 pve-manager: package-installs-apt-sources etc/apt/sources.list.d/pve-enterprise.list
 pve-manager: privacy-breach-generic usr/share/pve-manager/touch/sencha-touch-all-debug.js *
-pve-manager: script-with-language-extension usr/bin/pvemailforward.pl
-pve-manager: setgid-binary usr/bin/pvemailforward 2755 root/www-data
 pve-manager: systemd-service-file-refers-to-unusual-wantedby-target lib/systemd/system/pvebanner.service getty.target
diff --git a/debian/rules b/debian/rules
index ec49d52d..f28352e1 100755
--- a/debian/rules
+++ b/debian/rules
@@ -13,4 +13,4 @@ override_dh_strip_nondeterminism:
 	dh_strip_nondeterminism -X.png
 
 override_dh_fixperms:
-	dh_fixperms -Xpvemailforward -Xvar/log/pveproxy
+	dh_fixperms -Xvar/log/pveproxy
-- 
2.30.2

[pve-devel] [PATCH manager 4/4] d/control: drop ${shlibs:Depends} for pve-manager

[Fiona Ebner]

Now that the pvemailforward binary was dropped, there is a lintian
warning that it's not defined anymore.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 debian/control | 1 -
 1 file changed, 1 deletion(-)

diff --git a/debian/control b/debian/control
index 251bdf77..35c9eee3 100644
--- a/debian/control
+++ b/debian/control
@@ -92,7 +92,6 @@ Depends: apt-transport-https | apt (>= 1.5~),
          wget,
          ${misc:Depends},
          ${perl:Depends},
-         ${shlibs:Depends},
 Recommends: proxmox-offline-mirror-helper
 Suggests: libpve-network-perl (>= 0.5-1)
 Conflicts: vlan,
-- 
2.30.2

[pve-devel] applied: [PATCH proxmox 1/1] section config: parse additional properties when schema allows it

[Wolfgang Bumiller]

applied, thanks

[pve-devel] applied: [PATCH proxmox-mail-forward 1/3] initial commit

[Wolfgang Bumiller]

created repo & applied
added followups to:
- bump the section-config dependency in Cargo.toml to include the
  changes from this series
- use setresuid() to also drop the saved uid
- initial dummy version bump commit

[pve-devel] applied: [PATCH proxmox-backup 1/1] fix #4287: d/control: recommend proxmox-mail-forward

[Wolfgang Bumiller]

applied

On Fri, Oct 21, 2022 at 03:02:48PM +0200, Fiona Ebner wrote:
> which registers a binary in /root/.forward and handles mail forwarding
> to the mail addresss configured for root@pam in PBS. Similar to how it
> is done in PVE currently.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  debian/control | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/debian/control b/debian/control
> index 65331524..35066256 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -173,6 +173,7 @@ Depends: fonts-font-awesome,
>  Recommends: zfsutils-linux,
>              ifupdown2,
>              proxmox-offline-mirror-helper,
> +            proxmox-mail-forward,
>  Description: Proxmox Backup Server daemon with tools and GUI
>   This package contains the Proxmox Backup Server daemons and related
>   tools. This includes a web-based graphical user interface.
> -- 
> 2.30.2

[pve-devel] applied-series: [PATCH-SERIES proxmox{, -mail-forward, -backup}/pve-manager] add proxmox-mail-forward helper binary

[Wolfgang Bumiller]

applied remaining patches

On Fri, Oct 21, 2022 at 03:02:43PM +0200, Fiona Ebner wrote:
> written in Rust, and replacing the pvemailforward binary in PVE. Can
> be used in PVE and PBS as well as in a mixed installations of the two.
> 
> To make reading the config files work, it is a setuid binary owned by
> root, but it sets the effective UID to the real UID after reading the
> configs, so parsing and sendmail invocation happen with lower
> privileges again (well, except if the binary was called by root
> directly).
> 
> The .forward file is updated during postinst as currently done in
> pve-manager. proxmox-mail-forward's postinst will not do anything
> when detecting a pvemailforward entry. Instead pve-manager is
> responsible for the switchover in PVE.
> 
> 
> proxmox-mail-forward needs a depenency bump for proxmox-section-config
> for the functionality added by the first patch.
> 
> proxmox-backup recommends proxmox-mail-forward (I felt a Recommends
> is more fitting, but feel free to change it) and pve-manager depends
> on proxmox-mail-forward are part of the series.
> 
> 
> proxmox:
> 
> Fiona Ebner (1):
>   section config: parse additional properties when schema allows it
> 
>  proxmox-section-config/src/lib.rs | 79 ++++++++++++++++++++++++++++++-
>  1 file changed, 78 insertions(+), 1 deletion(-)
> 
> 
> proxmox-mail-forward:
> 
> Fiona Ebner (3):
>   initial commit
>   add Debian packaging
>   d/postinst: register binary in .forward
> 
> 
> proxmox-backup:
> 
> Fiona Ebner (1):
>   fix #4287: d/control: recommend proxmox-mail-forward
> 
>  debian/control | 1 +
>  1 file changed, 1 insertion(+)
> 
> 
> pve-manager:
> 
> Fiona Ebner (4):
>   d/control: depend on proxmox-mail-forward
>   d/postinst: replace pvemailforward with proxmox-mail-forward
>   remove pvemailforward binary
>   d/control: drop ${shlibs:Depends} for pve-manager
> 
>  bin/Makefile             | 11 +++-------
>  bin/pvemailforward.c     | 17 ---------------
>  bin/pvemailforward.pl    | 45 ----------------------------------------
>  debian/control           |  2 +-
>  debian/lintian-overrides |  4 ----
>  debian/postinst          | 10 +++++++--
>  debian/rules             |  2 +-
>  7 files changed, 13 insertions(+), 78 deletions(-)
>  delete mode 100644 bin/pvemailforward.c
>  delete mode 100755 bin/pvemailforward.pl
> 
> -- 
> 2.30.2

Re: [pve-devel] [PATCH manager 4/4] d/control: drop ${shlibs:Depends} for pve-manager

[Thomas Lamprecht]

Am 21/10/2022 um 15:02 schrieb Fiona Ebner:
> Now that the pvemailforward binary was dropped, there is a lintian
> warning that it's not defined anymore.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  debian/control | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/debian/control b/debian/control
> index 251bdf77..35c9eee3 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -92,7 +92,6 @@ Depends: apt-transport-https | apt (>= 1.5~),
>           wget,
>           ${misc:Depends},
>           ${perl:Depends},
> -         ${shlibs:Depends},
>  Recommends: proxmox-offline-mirror-helper
>  Suggests: libpve-network-perl (>= 0.5-1)
>  Conflicts: vlan,

we can now probably also move this from an `Architecture: any` to an `Architecture: all`
package, not high priority but it would make cross-building community projects a bit
simpler.