[pbs-devel] [PATCH v2 backup 00/27] Implements ACME support for PBS

[pbs-devel] [PATCH v2 backup 00/27] Implements ACME support for PBS

[Wolfgang Bumiller]

Version 2 of this addresses a few raised issues:

NOTE: The widget-toolkit patch from v1 is still required. I just did not
re-send it now.

* The config file format parser does not use serde anymore as we
  a) don't need it as we have a lot of ready-to-go parsing code in the
     proxmox crate that is now being reused.
  b) is harder to read and the benefits are mostly performance related,
     while more useful things such as using structs for property strings
     in the format really would instead need more formal support on the
     schema side...

* Rebased the acme client to use the new `SimpleHttp` client.
  * and ported the changes to add the user agent string to the new api

* Fixes a few issues found by Dominic:
  * create the acme related directories if they do not exist yet
  * pipe dns plugin command output to the task log
  * made the account name optional in the register api call (since
  * pve/pmg do it too)

* Fixed a warning about a missing semicolon in the ui code.

The original patch 4 (tools/http helper) was dropped and is replaced by
patches 15 & 16. I added the main changes outlined above as separate
patches and only merged minor cleanup/style fixups into the existing
patches.

-- Original cover letter:

Reusing the ACME UI elements from the widget toolkit and therefore
providing a compatible API and pretty much the same config file layout.

Contains the async version of the acme client directly in the tree here,
though it may also be an option to move it to proxmox-acme-rs w/ a
feature-gate. (The code is also very similar to the sync version so
there's a possibility that the implementation could be wrapped in a
macro...)

The series starts out with some helpers & refactoring, followed by a
serde-driven config file format read/writer (meant to be (or become)
compatible to what we have in perl via PVE::JSONSchema::parse_config,
but without the json::Value intermediate step), followed by the config,
client & api call implementation.

(Wildcard support like stoiko just added to PMG still needs to be added,
though...)

Wolfgang Bumiller (27):
  systemd: add reload_unit
  add dns alias schema
  tools::fs::scan_subdir: use nix::Error instead of anyhow
  config: factor out certificate writing
  CertInfo: add not_{after,before}_unix
  CertInfo: add is_expired_after_epoch
  tools: add ControlFlow type
  catalog shell: replace LoopState with ControlFlow
  Cargo.toml: depend on proxmox-acme-rs
  bump d/control
  config::acl: make /system/certificates a valid path
  add 'config file format' to tools::config
  add node config
  add acme config
  tools/http: dedup user agent string
  tools/http: add request_with_agent helper
  add async acme client implementation
  add config/acme api path
  add node/{node}/certificates api call
  add node/{node}/config api path
  add acme commands to proxmox-backup-manager
  implement standalone acme validation
  ui: add certificate & acme view
  daily-update: check acme certificates
  acme: create directories as needed
  acme: pipe plugin output to task log
  api: acme: make account name optional in register call

 Cargo.toml                             |   3 +
 debian/control                         |   2 +
 src/acme/client.rs                     | 672 +++++++++++++++++++++++
 src/acme/mod.rs                        |   2 +
 src/api2/config.rs                     |   2 +
 src/api2/config/acme.rs                | 725 +++++++++++++++++++++++++
 src/api2/node.rs                       |   4 +
 src/api2/node/certificates.rs          | 577 ++++++++++++++++++++
 src/api2/node/config.rs                |  81 +++
 src/api2/types/mod.rs                  |  10 +
 src/backup/catalog_shell.rs            |  18 +-
 src/bin/proxmox-backup-manager.rs      |   1 +
 src/bin/proxmox-daily-update.rs        |  30 +-
 src/bin/proxmox_backup_manager/acme.rs | 415 ++++++++++++++
 src/bin/proxmox_backup_manager/mod.rs  |   2 +
 src/config.rs                          |  55 +-
 src/config/acl.rs                      |   2 +-
 src/config/acme/mod.rs                 | 237 ++++++++
 src/config/acme/plugin.rs              | 532 ++++++++++++++++++
 src/config/node.rs                     | 225 ++++++++
 src/lib.rs                             |   2 +
 src/tools.rs                           |  12 +
 src/tools/cert.rs                      |  41 +-
 src/tools/config.rs                    | 171 ++++++
 src/tools/fs.rs                        |   2 +-
 src/tools/http.rs                      |  15 +-
 src/tools/systemd.rs                   |  11 +
 www/Makefile                           |   1 +
 www/NavigationTree.js                  |   6 +
 www/config/CertificateView.js          |  80 +++
 30 files changed, 3897 insertions(+), 39 deletions(-)
 create mode 100644 src/acme/client.rs
 create mode 100644 src/acme/mod.rs
 create mode 100644 src/api2/config/acme.rs
 create mode 100644 src/api2/node/certificates.rs
 create mode 100644 src/api2/node/config.rs
 create mode 100644 src/bin/proxmox_backup_manager/acme.rs
 create mode 100644 src/config/acme/mod.rs
 create mode 100644 src/config/acme/plugin.rs
 create mode 100644 src/config/node.rs
 create mode 100644 src/tools/config.rs
 create mode 100644 www/config/CertificateView.js

-- 
2.20.1

[pbs-devel] [PATCH v2 backup 01/27] systemd: add reload_unit

[Wolfgang Bumiller]

via try-reload-or-restart

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/tools/systemd.rs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/tools/systemd.rs b/src/tools/systemd.rs
index f8ae789c..c1760781 100644
--- a/src/tools/systemd.rs
+++ b/src/tools/systemd.rs
@@ -131,6 +131,17 @@ pub fn stop_unit(unit: &str) -> Result<(), Error> {
     Ok(())
 }
 
+pub fn reload_unit(unit: &str) -> Result<(), Error> {
+
+    let mut command = std::process::Command::new("systemctl");
+    command.arg("try-reload-or-restart");
+    command.arg(unit);
+
+    crate::tools::run_command(command, None)?;
+
+    Ok(())
+}
+
 #[test]
 fn test_escape_unit() -> Result<(), Error> {
 
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 backup 01/27] systemd: add reload_unit

[Dietmar Maurer]

applied

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> via try-reload-or-restart
>
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/tools/systemd.rs | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/src/tools/systemd.rs b/src/tools/systemd.rs
> index f8ae789c..c1760781 100644
> --- a/src/tools/systemd.rs
> +++ b/src/tools/systemd.rs
> @@ -131,6 +131,17 @@ pub fn stop_unit(unit: &str) -> Result<(), Error> {
>       Ok(())
>   }
>   
> +pub fn reload_unit(unit: &str) -> Result<(), Error> {
> +
> +    let mut command = std::process::Command::new("systemctl");
> +    command.arg("try-reload-or-restart");
> +    command.arg(unit);
> +
> +    crate::tools::run_command(command, None)?;
> +
> +    Ok(())
> +}
> +
>   #[test]
>   fn test_escape_unit() -> Result<(), Error> {
>   

[pbs-devel] [PATCH v2 backup 02/27] add dns alias schema

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/api2/types/mod.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 9d1bd301..eee91dfd 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -51,6 +51,11 @@ pub const FILENAME_FORMAT: ApiStringFormat = ApiStringFormat::VerifyFn(|name| {
 macro_rules! DNS_LABEL { () => (r"(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?)") }
 macro_rules! DNS_NAME { () => (concat!(r"(?:(?:", DNS_LABEL!() , r"\.)*", DNS_LABEL!(), ")")) }
 
+macro_rules! DNS_ALIAS_LABEL { () => (r"(?:[a-zA-Z0-9_](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?)") }
+macro_rules! DNS_ALIAS_NAME {
+    () => (concat!(r"(?:(?:", DNS_ALIAS_LABEL!() , r"\.)*", DNS_ALIAS_LABEL!(), ")"))
+}
+
 macro_rules! CIDR_V4_REGEX_STR { () => (concat!(r"(?:", IPV4RE!(), r"/\d{1,2})$")) }
 macro_rules! CIDR_V6_REGEX_STR { () => (concat!(r"(?:", IPV6RE!(), r"/\d{1,3})$")) }
 
@@ -87,6 +92,8 @@ const_regex!{
 
     pub DNS_NAME_REGEX =  concat!(r"^", DNS_NAME!(), r"$");
 
+    pub DNS_ALIAS_REGEX =  concat!(r"^", DNS_ALIAS_NAME!(), r"$");
+
     pub DNS_NAME_OR_IP_REGEX = concat!(r"^(?:", DNS_NAME!(), "|",  IPRE!(), r")$");
 
     pub BACKUP_REPO_URL_REGEX = concat!(r"^^(?:(?:(", USER_ID_REGEX_STR!(), "|", APITOKEN_ID_REGEX_STR!(), ")@)?(", DNS_NAME!(), "|",  IPRE_BRACKET!() ,"):)?(?:([0-9]{1,5}):)?(", PROXMOX_SAFE_ID_REGEX_STR!(), r")$");
@@ -142,6 +149,9 @@ pub const HOSTNAME_FORMAT: ApiStringFormat =
 pub const DNS_NAME_FORMAT: ApiStringFormat =
     ApiStringFormat::Pattern(&DNS_NAME_REGEX);
 
+pub const DNS_ALIAS_FORMAT: ApiStringFormat =
+    ApiStringFormat::Pattern(&DNS_ALIAS_REGEX);
+
 pub const DNS_NAME_OR_IP_FORMAT: ApiStringFormat =
     ApiStringFormat::Pattern(&DNS_NAME_OR_IP_REGEX);
 
-- 
2.20.1

Re: [pbs-devel] [PATCH v2 backup 02/27] add dns alias schema

[Dietmar Maurer]

Sorry, I don't get this. Why is DNS_LABEL and DNS_ALIAS_LABEL different?


On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/api2/types/mod.rs | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
> index 9d1bd301..eee91dfd 100644
> --- a/src/api2/types/mod.rs
> +++ b/src/api2/types/mod.rs
> @@ -51,6 +51,11 @@ pub const FILENAME_FORMAT: ApiStringFormat = ApiStringFormat::VerifyFn(|name| {
>   macro_rules! DNS_LABEL { () => (r"(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?)") }
>   macro_rules! DNS_NAME { () => (concat!(r"(?:(?:", DNS_LABEL!() , r"\.)*", DNS_LABEL!(), ")")) }
>   
> +macro_rules! DNS_ALIAS_LABEL { () => (r"(?:[a-zA-Z0-9_](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?)") }
> +macro_rules! DNS_ALIAS_NAME {
> +    () => (concat!(r"(?:(?:", DNS_ALIAS_LABEL!() , r"\.)*", DNS_ALIAS_LABEL!(), ")"))
> +}
> +
>   macro_rules! CIDR_V4_REGEX_STR { () => (concat!(r"(?:", IPV4RE!(), r"/\d{1,2})$")) }
>   macro_rules! CIDR_V6_REGEX_STR { () => (concat!(r"(?:", IPV6RE!(), r"/\d{1,3})$")) }
>   
> @@ -87,6 +92,8 @@ const_regex!{
>   
>       pub DNS_NAME_REGEX =  concat!(r"^", DNS_NAME!(), r"$");
>   
> +    pub DNS_ALIAS_REGEX =  concat!(r"^", DNS_ALIAS_NAME!(), r"$");
> +
>       pub DNS_NAME_OR_IP_REGEX = concat!(r"^(?:", DNS_NAME!(), "|",  IPRE!(), r")$");
>   
>       pub BACKUP_REPO_URL_REGEX = concat!(r"^^(?:(?:(", USER_ID_REGEX_STR!(), "|", APITOKEN_ID_REGEX_STR!(), ")@)?(", DNS_NAME!(), "|",  IPRE_BRACKET!() ,"):)?(?:([0-9]{1,5}):)?(", PROXMOX_SAFE_ID_REGEX_STR!(), r")$");
> @@ -142,6 +149,9 @@ pub const HOSTNAME_FORMAT: ApiStringFormat =
>   pub const DNS_NAME_FORMAT: ApiStringFormat =
>       ApiStringFormat::Pattern(&DNS_NAME_REGEX);
>   
> +pub const DNS_ALIAS_FORMAT: ApiStringFormat =
> +    ApiStringFormat::Pattern(&DNS_ALIAS_REGEX);
> +
>   pub const DNS_NAME_OR_IP_FORMAT: ApiStringFormat =
>       ApiStringFormat::Pattern(&DNS_NAME_OR_IP_REGEX);
>   

Re: [pbs-devel] [PATCH v2 backup 02/27] add dns alias schema

[Wolfgang Bumiller]

On Wed, Apr 28, 2021 at 12:26:11PM +0200, Dietmar Maurer wrote:
> Sorry, I don't get this. Why is DNS_LABEL and DNS_ALIAS_LABEL different?

One allows underscores at the beginning, the other doesn't, as for
"regular" domains that's not allowed, and the acme challenge domain uses
`_acme_challenge` as a prefix and it makes sense to allow users to use
this in their aliases as well, as that's just the domain where the
challenge data ultimately ends up at.

> 
> 
> On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> > Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> > ---
> >   src/api2/types/mod.rs | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> > 
> > diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
> > index 9d1bd301..eee91dfd 100644
> > --- a/src/api2/types/mod.rs
> > +++ b/src/api2/types/mod.rs
> > @@ -51,6 +51,11 @@ pub const FILENAME_FORMAT: ApiStringFormat = ApiStringFormat::VerifyFn(|name| {
> >   macro_rules! DNS_LABEL { () => (r"(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?)") }
> >   macro_rules! DNS_NAME { () => (concat!(r"(?:(?:", DNS_LABEL!() , r"\.)*", DNS_LABEL!(), ")")) }
> > +macro_rules! DNS_ALIAS_LABEL { () => (r"(?:[a-zA-Z0-9_](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?)") }
> > +macro_rules! DNS_ALIAS_NAME {
> > +    () => (concat!(r"(?:(?:", DNS_ALIAS_LABEL!() , r"\.)*", DNS_ALIAS_LABEL!(), ")"))
> > +}
> > +
> >   macro_rules! CIDR_V4_REGEX_STR { () => (concat!(r"(?:", IPV4RE!(), r"/\d{1,2})$")) }
> >   macro_rules! CIDR_V6_REGEX_STR { () => (concat!(r"(?:", IPV6RE!(), r"/\d{1,3})$")) }
> > @@ -87,6 +92,8 @@ const_regex!{
> >       pub DNS_NAME_REGEX =  concat!(r"^", DNS_NAME!(), r"$");
> > +    pub DNS_ALIAS_REGEX =  concat!(r"^", DNS_ALIAS_NAME!(), r"$");
> > +
> >       pub DNS_NAME_OR_IP_REGEX = concat!(r"^(?:", DNS_NAME!(), "|",  IPRE!(), r")$");
> >       pub BACKUP_REPO_URL_REGEX = concat!(r"^^(?:(?:(", USER_ID_REGEX_STR!(), "|", APITOKEN_ID_REGEX_STR!(), ")@)?(", DNS_NAME!(), "|",  IPRE_BRACKET!() ,"):)?(?:([0-9]{1,5}):)?(", PROXMOX_SAFE_ID_REGEX_STR!(), r")$");
> > @@ -142,6 +149,9 @@ pub const HOSTNAME_FORMAT: ApiStringFormat =
> >   pub const DNS_NAME_FORMAT: ApiStringFormat =
> >       ApiStringFormat::Pattern(&DNS_NAME_REGEX);
> > +pub const DNS_ALIAS_FORMAT: ApiStringFormat =
> > +    ApiStringFormat::Pattern(&DNS_ALIAS_REGEX);
> > +
> >   pub const DNS_NAME_OR_IP_FORMAT: ApiStringFormat =
> >       ApiStringFormat::Pattern(&DNS_NAME_OR_IP_REGEX);

[pbs-devel] applied: [PATCH v2 backup 02/27] add dns alias schema

[Dietmar Maurer]

applied

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/api2/types/mod.rs | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
> index 9d1bd301..eee91dfd 100644
> --- a/src/api2/types/mod.rs
> +++ b/src/api2/types/mod.rs
> @@ -51,6 +51,11 @@ pub const FILENAME_FORMAT: ApiStringFormat = ApiStringFormat::VerifyFn(|name| {
>   macro_rules! DNS_LABEL { () => (r"(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?)") }
>   macro_rules! DNS_NAME { () => (concat!(r"(?:(?:", DNS_LABEL!() , r"\.)*", DNS_LABEL!(), ")")) }
>   
> +macro_rules! DNS_ALIAS_LABEL { () => (r"(?:[a-zA-Z0-9_](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?)") }
> +macro_rules! DNS_ALIAS_NAME {
> +    () => (concat!(r"(?:(?:", DNS_ALIAS_LABEL!() , r"\.)*", DNS_ALIAS_LABEL!(), ")"))
> +}
> +
>   macro_rules! CIDR_V4_REGEX_STR { () => (concat!(r"(?:", IPV4RE!(), r"/\d{1,2})$")) }
>   macro_rules! CIDR_V6_REGEX_STR { () => (concat!(r"(?:", IPV6RE!(), r"/\d{1,3})$")) }
>   
> @@ -87,6 +92,8 @@ const_regex!{
>   
>       pub DNS_NAME_REGEX =  concat!(r"^", DNS_NAME!(), r"$");
>   
> +    pub DNS_ALIAS_REGEX =  concat!(r"^", DNS_ALIAS_NAME!(), r"$");
> +
>       pub DNS_NAME_OR_IP_REGEX = concat!(r"^(?:", DNS_NAME!(), "|",  IPRE!(), r")$");
>   
>       pub BACKUP_REPO_URL_REGEX = concat!(r"^^(?:(?:(", USER_ID_REGEX_STR!(), "|", APITOKEN_ID_REGEX_STR!(), ")@)?(", DNS_NAME!(), "|",  IPRE_BRACKET!() ,"):)?(?:([0-9]{1,5}):)?(", PROXMOX_SAFE_ID_REGEX_STR!(), r")$");
> @@ -142,6 +149,9 @@ pub const HOSTNAME_FORMAT: ApiStringFormat =
>   pub const DNS_NAME_FORMAT: ApiStringFormat =
>       ApiStringFormat::Pattern(&DNS_NAME_REGEX);
>   
> +pub const DNS_ALIAS_FORMAT: ApiStringFormat =
> +    ApiStringFormat::Pattern(&DNS_ALIAS_REGEX);
> +
>   pub const DNS_NAME_OR_IP_FORMAT: ApiStringFormat =
>       ApiStringFormat::Pattern(&DNS_NAME_OR_IP_REGEX);
>   

[pbs-devel] [PATCH v2 backup 03/27] tools::fs::scan_subdir: use nix::Error instead of anyhow

[Wolfgang Bumiller]

allows using SysError trait on it

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/tools/fs.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/fs.rs b/src/tools/fs.rs
index 72a530d8..6e0b1271 100644
--- a/src/tools/fs.rs
+++ b/src/tools/fs.rs
@@ -117,7 +117,7 @@ pub fn scan_subdir<'a, P: ?Sized + nix::NixPath>(
     dirfd: RawFd,
     path: &P,
     regex: &'a regex::Regex,
-) -> Result<impl Iterator<Item = Result<ReadDirEntry, Error>> + 'a, Error> {
+) -> Result<impl Iterator<Item = Result<ReadDirEntry, Error>> + 'a, nix::Error> {
     Ok(read_subdir(dirfd, path)?.filter_file_name_regex(regex))
 }
 
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 backup 03/27] tools::fs::scan_subdir: use nix::Error instead of anyhow

[Dietmar Maurer]

applied

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> allows using SysError trait on it
>
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/tools/fs.rs | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/tools/fs.rs b/src/tools/fs.rs
> index 72a530d8..6e0b1271 100644
> --- a/src/tools/fs.rs
> +++ b/src/tools/fs.rs
> @@ -117,7 +117,7 @@ pub fn scan_subdir<'a, P: ?Sized + nix::NixPath>(
>       dirfd: RawFd,
>       path: &P,
>       regex: &'a regex::Regex,
> -) -> Result<impl Iterator<Item = Result<ReadDirEntry, Error>> + 'a, Error> {
> +) -> Result<impl Iterator<Item = Result<ReadDirEntry, Error>> + 'a, nix::Error> {
>       Ok(read_subdir(dirfd, path)?.filter_file_name_regex(regex))
>   }
>   

[pbs-devel] [PATCH v2 backup 04/27] config: factor out certificate writing

[Wolfgang Bumiller]

for reuse in the certificate api

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/config.rs | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/src/config.rs b/src/config.rs
index 1557e20a..37df2fd2 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -98,10 +98,6 @@ pub fn create_configdir() -> Result<(), Error> {
 /// Update self signed node certificate.
 pub fn update_self_signed_cert(force: bool) -> Result<(), Error> {
 
-    let backup_user = crate::backup::backup_user()?;
-
-    create_configdir()?;
-
     let key_path = PathBuf::from(configdir!("/proxy.key"));
     let cert_path = PathBuf::from(configdir!("/proxy.pem"));
 
@@ -111,15 +107,6 @@ pub fn update_self_signed_cert(force: bool) -> Result<(), Error> {
 
     let priv_pem = rsa.private_key_to_pem()?;
 
-    replace_file(
-        &key_path,
-        &priv_pem,
-        CreateOptions::new()
-            .perm(Mode::from_bits_truncate(0o0640))
-            .owner(nix::unistd::ROOT)
-            .group(backup_user.gid),
-    )?;
-
     let mut x509 = X509Builder::new()?;
 
     x509.set_version(2)?;
@@ -198,14 +185,24 @@ pub fn update_self_signed_cert(force: bool) -> Result<(), Error> {
     let x509 = x509.build();
     let cert_pem = x509.to_pem()?;
 
-    replace_file(
-        &cert_path,
-        &cert_pem,
-        CreateOptions::new()
-            .perm(Mode::from_bits_truncate(0o0640))
-            .owner(nix::unistd::ROOT)
-            .group(backup_user.gid),
-    )?;
+    set_proxy_certificate(&cert_pem, &priv_pem)?;
 
     Ok(())
 }
+
+pub(crate) fn set_proxy_certificate(cert_pem: &[u8], key_pem: &[u8]) -> Result<(), Error> {
+    let backup_user = crate::backup::backup_user()?;
+    let options = CreateOptions::new()
+        .perm(Mode::from_bits_truncate(0o0640))
+        .owner(nix::unistd::ROOT)
+        .group(backup_user.gid);
+    let key_path = PathBuf::from(configdir!("/proxy.key"));
+    let cert_path = PathBuf::from(configdir!("/proxy.pem"));
+
+    create_configdir()?;
+    replace_file(&key_path, &key_pem, options.clone())
+        .map_err(|err| format_err!("error writing certificate private key - {}", err))?;
+    replace_file(&cert_path, &cert_pem, options)
+        .map_err(|err| format_err!("error writing certificate file - {}", err))?;
+    Ok(())
+}
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 backup 04/27] config: factor out certificate writing

[Dietmar Maurer]

applied

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> for reuse in the certificate api
>
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/config.rs | 39 ++++++++++++++++++---------------------
>   1 file changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/src/config.rs b/src/config.rs
> index 1557e20a..37df2fd2 100644
> --- a/src/config.rs
> +++ b/src/config.rs
> @@ -98,10 +98,6 @@ pub fn create_configdir() -> Result<(), Error> {
>   /// Update self signed node certificate.
>   pub fn update_self_signed_cert(force: bool) -> Result<(), Error> {
>   
> -    let backup_user = crate::backup::backup_user()?;
> -
> -    create_configdir()?;
> -
>       let key_path = PathBuf::from(configdir!("/proxy.key"));
>       let cert_path = PathBuf::from(configdir!("/proxy.pem"));
>   
> @@ -111,15 +107,6 @@ pub fn update_self_signed_cert(force: bool) -> Result<(), Error> {
>   
>       let priv_pem = rsa.private_key_to_pem()?;
>   
> -    replace_file(
> -        &key_path,
> -        &priv_pem,
> -        CreateOptions::new()
> -            .perm(Mode::from_bits_truncate(0o0640))
> -            .owner(nix::unistd::ROOT)
> -            .group(backup_user.gid),
> -    )?;
> -
>       let mut x509 = X509Builder::new()?;
>   
>       x509.set_version(2)?;
> @@ -198,14 +185,24 @@ pub fn update_self_signed_cert(force: bool) -> Result<(), Error> {
>       let x509 = x509.build();
>       let cert_pem = x509.to_pem()?;
>   
> -    replace_file(
> -        &cert_path,
> -        &cert_pem,
> -        CreateOptions::new()
> -            .perm(Mode::from_bits_truncate(0o0640))
> -            .owner(nix::unistd::ROOT)
> -            .group(backup_user.gid),
> -    )?;
> +    set_proxy_certificate(&cert_pem, &priv_pem)?;
>   
>       Ok(())
>   }
> +
> +pub(crate) fn set_proxy_certificate(cert_pem: &[u8], key_pem: &[u8]) -> Result<(), Error> {
> +    let backup_user = crate::backup::backup_user()?;
> +    let options = CreateOptions::new()
> +        .perm(Mode::from_bits_truncate(0o0640))
> +        .owner(nix::unistd::ROOT)
> +        .group(backup_user.gid);
> +    let key_path = PathBuf::from(configdir!("/proxy.key"));
> +    let cert_path = PathBuf::from(configdir!("/proxy.pem"));
> +
> +    create_configdir()?;
> +    replace_file(&key_path, &key_pem, options.clone())
> +        .map_err(|err| format_err!("error writing certificate private key - {}", err))?;
> +    replace_file(&cert_path, &cert_pem, options)
> +        .map_err(|err| format_err!("error writing certificate file - {}", err))?;
> +    Ok(())
> +}

[pbs-devel] [PATCH v2 backup 05/27] CertInfo: add not_{after, before}_unix

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 Cargo.toml        |  1 +
 src/tools/cert.rs | 36 ++++++++++++++++++++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 3739d3af..74ccf361 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -32,6 +32,7 @@ endian_trait = { version = "0.6", features = ["arrays"] }
 env_logger = "0.7"
 flate2 = "1.0"
 anyhow = "1.0"
+foreign-types = "0.3"
 thiserror = "1.0"
 futures = "0.3"
 h2 = { version = "0.3", features = [ "stream" ] }
diff --git a/src/tools/cert.rs b/src/tools/cert.rs
index 0c7e9e5d..d3c221a4 100644
--- a/src/tools/cert.rs
+++ b/src/tools/cert.rs
@@ -1,12 +1,32 @@
 use std::path::PathBuf;
+use std::mem::MaybeUninit;
 
-use anyhow::Error;
+use anyhow::{bail, format_err, Error};
+use foreign_types::ForeignTypeRef;
 use openssl::x509::{X509, GeneralName};
 use openssl::stack::Stack;
 use openssl::pkey::{Public, PKey};
 
 use crate::configdir;
 
+// C type:
+#[allow(non_camel_case_types)]
+type ASN1_TIME = <openssl::asn1::Asn1TimeRef as ForeignTypeRef>::CType;
+
+extern "C" {
+    fn ASN1_TIME_to_tm(s: *const ASN1_TIME, tm: *mut libc::tm) -> libc::c_int;
+}
+
+fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64, Error> {
+    let mut c_tm = MaybeUninit::<libc::tm>::uninit();
+    let rc = unsafe { ASN1_TIME_to_tm(time.as_ptr(), c_tm.as_mut_ptr()) };
+    if rc != 1 {
+        bail!("failed to parse ASN1 time");
+    }
+    let mut c_tm = unsafe { c_tm.assume_init() };
+    proxmox::tools::time::timegm(&mut c_tm)
+}
+
 pub struct CertInfo {
     x509: X509,
 }
@@ -25,7 +45,11 @@ impl CertInfo {
     }
 
     pub fn from_path(path: PathBuf) -> Result<Self, Error> {
-        let cert_pem = proxmox::tools::fs::file_get_contents(&path)?;
+        Self::from_pem(&proxmox::tools::fs::file_get_contents(&path)?)
+            .map_err(|err| format_err!("failed to load certificate from {:?} - {}", path, err))
+    }
+
+    pub fn from_pem(cert_pem: &[u8]) -> Result<Self, Error> {
         let x509 = openssl::x509::X509::from_pem(&cert_pem)?;
         Ok(Self{
             x509
@@ -64,4 +88,12 @@ impl CertInfo {
     pub fn not_after(&self) -> &openssl::asn1::Asn1TimeRef {
         self.x509.not_after()
     }
+
+    pub fn not_before_unix(&self) -> Result<i64, Error> {
+        asn1_time_to_unix(&self.not_before())
+    }
+
+    pub fn not_after_unix(&self) -> Result<i64, Error> {
+        asn1_time_to_unix(&self.not_after())
+    }
 }
-- 
2.20.1

Re: [pbs-devel] [PATCH v2 backup 05/27] CertInfo: add not_{after, before}_unix

[Dietmar Maurer]

is it really worth to add another dependency?

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   Cargo.toml        |  1 +
>   src/tools/cert.rs | 36 ++++++++++++++++++++++++++++++++++--
>   2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/Cargo.toml b/Cargo.toml
> index 3739d3af..74ccf361 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -32,6 +32,7 @@ endian_trait = { version = "0.6", features = ["arrays"] }
>   env_logger = "0.7"
>   flate2 = "1.0"
>   anyhow = "1.0"
> +foreign-types = "0.3"
>   thiserror = "1.0"
>   futures = "0.3"
>   h2 = { version = "0.3", features = [ "stream" ] }
> diff --git a/src/tools/cert.rs b/src/tools/cert.rs
> index 0c7e9e5d..d3c221a4 100644
> --- a/src/tools/cert.rs
> +++ b/src/tools/cert.rs
> @@ -1,12 +1,32 @@
>   use std::path::PathBuf;
> +use std::mem::MaybeUninit;
>   
> -use anyhow::Error;
> +use anyhow::{bail, format_err, Error};
> +use foreign_types::ForeignTypeRef;
>   use openssl::x509::{X509, GeneralName};
>   use openssl::stack::Stack;
>   use openssl::pkey::{Public, PKey};
>   
>   use crate::configdir;
>   
> +// C type:
> +#[allow(non_camel_case_types)]
> +type ASN1_TIME = <openssl::asn1::Asn1TimeRef as ForeignTypeRef>::CType;
> +
> +extern "C" {
> +    fn ASN1_TIME_to_tm(s: *const ASN1_TIME, tm: *mut libc::tm) -> libc::c_int;
> +}
> +
> +fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64, Error> {
> +    let mut c_tm = MaybeUninit::<libc::tm>::uninit();
> +    let rc = unsafe { ASN1_TIME_to_tm(time.as_ptr(), c_tm.as_mut_ptr()) };
> +    if rc != 1 {
> +        bail!("failed to parse ASN1 time");
> +    }
> +    let mut c_tm = unsafe { c_tm.assume_init() };
> +    proxmox::tools::time::timegm(&mut c_tm)
> +}
> +
>   pub struct CertInfo {
>       x509: X509,
>   }
> @@ -25,7 +45,11 @@ impl CertInfo {
>       }
>   
>       pub fn from_path(path: PathBuf) -> Result<Self, Error> {
> -        let cert_pem = proxmox::tools::fs::file_get_contents(&path)?;
> +        Self::from_pem(&proxmox::tools::fs::file_get_contents(&path)?)
> +            .map_err(|err| format_err!("failed to load certificate from {:?} - {}", path, err))
> +    }
> +
> +    pub fn from_pem(cert_pem: &[u8]) -> Result<Self, Error> {
>           let x509 = openssl::x509::X509::from_pem(&cert_pem)?;
>           Ok(Self{
>               x509
> @@ -64,4 +88,12 @@ impl CertInfo {
>       pub fn not_after(&self) -> &openssl::asn1::Asn1TimeRef {
>           self.x509.not_after()
>       }
> +
> +    pub fn not_before_unix(&self) -> Result<i64, Error> {
> +        asn1_time_to_unix(&self.not_before())
> +    }
> +
> +    pub fn not_after_unix(&self) -> Result<i64, Error> {
> +        asn1_time_to_unix(&self.not_after())
> +    }
>   }

Re: [pbs-devel] [PATCH v2 backup 05/27] CertInfo: add not_{after, before}_unix

[Wolfgang Bumiller]

On Wed, Apr 28, 2021 at 01:05:33PM +0200, Dietmar Maurer wrote:
> is it really worth to add another dependency?

openssl uses `foreign-types` to provide access to the underlying data,
it is described as a "framework for rust wrappers over C APIs", so if
more crates use it, their raw data becomes available this way as well.

The alternative would be to explicitly depend on openssl-sys.
I don't mind either way. The function isn't otherwise exposed by the
crate currently.

> 
> On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> > Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> > ---
> >   Cargo.toml        |  1 +
> >   src/tools/cert.rs | 36 ++++++++++++++++++++++++++++++++++--
> >   2 files changed, 35 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Cargo.toml b/Cargo.toml
> > index 3739d3af..74ccf361 100644
> > --- a/Cargo.toml
> > +++ b/Cargo.toml
> > @@ -32,6 +32,7 @@ endian_trait = { version = "0.6", features = ["arrays"] }
> >   env_logger = "0.7"
> >   flate2 = "1.0"
> >   anyhow = "1.0"
> > +foreign-types = "0.3"
> >   thiserror = "1.0"
> >   futures = "0.3"
> >   h2 = { version = "0.3", features = [ "stream" ] }
> > diff --git a/src/tools/cert.rs b/src/tools/cert.rs
> > index 0c7e9e5d..d3c221a4 100644
> > --- a/src/tools/cert.rs
> > +++ b/src/tools/cert.rs
> > @@ -1,12 +1,32 @@
> >   use std::path::PathBuf;
> > +use std::mem::MaybeUninit;
> > -use anyhow::Error;
> > +use anyhow::{bail, format_err, Error};
> > +use foreign_types::ForeignTypeRef;
> >   use openssl::x509::{X509, GeneralName};
> >   use openssl::stack::Stack;
> >   use openssl::pkey::{Public, PKey};
> >   use crate::configdir;
> > +// C type:
> > +#[allow(non_camel_case_types)]
> > +type ASN1_TIME = <openssl::asn1::Asn1TimeRef as ForeignTypeRef>::CType;
> > +
> > +extern "C" {
> > +    fn ASN1_TIME_to_tm(s: *const ASN1_TIME, tm: *mut libc::tm) -> libc::c_int;
> > +}
> > +
> > +fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64, Error> {
> > +    let mut c_tm = MaybeUninit::<libc::tm>::uninit();
> > +    let rc = unsafe { ASN1_TIME_to_tm(time.as_ptr(), c_tm.as_mut_ptr()) };
> > +    if rc != 1 {
> > +        bail!("failed to parse ASN1 time");
> > +    }
> > +    let mut c_tm = unsafe { c_tm.assume_init() };
> > +    proxmox::tools::time::timegm(&mut c_tm)
> > +}
> > +
> >   pub struct CertInfo {
> >       x509: X509,
> >   }
> > @@ -25,7 +45,11 @@ impl CertInfo {
> >       }
> >       pub fn from_path(path: PathBuf) -> Result<Self, Error> {
> > -        let cert_pem = proxmox::tools::fs::file_get_contents(&path)?;
> > +        Self::from_pem(&proxmox::tools::fs::file_get_contents(&path)?)
> > +            .map_err(|err| format_err!("failed to load certificate from {:?} - {}", path, err))
> > +    }
> > +
> > +    pub fn from_pem(cert_pem: &[u8]) -> Result<Self, Error> {
> >           let x509 = openssl::x509::X509::from_pem(&cert_pem)?;
> >           Ok(Self{
> >               x509
> > @@ -64,4 +88,12 @@ impl CertInfo {
> >       pub fn not_after(&self) -> &openssl::asn1::Asn1TimeRef {
> >           self.x509.not_after()
> >       }
> > +
> > +    pub fn not_before_unix(&self) -> Result<i64, Error> {
> > +        asn1_time_to_unix(&self.not_before())
> > +    }
> > +
> > +    pub fn not_after_unix(&self) -> Result<i64, Error> {
> > +        asn1_time_to_unix(&self.not_after())
> > +    }
> >   }

Re: [pbs-devel] [PATCH v2 backup 05/27] CertInfo: add not_{after, before}_unix

[Dietmar Maurer]

Seems I can do it without foreign-types:

fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64, 
Error> {
     let epoch0 = openssl::asn1::Asn1Time::from_unix(0)?;
     let diff = epoch0.diff(time)?;
     let seconds = (diff.days as i64) * 24*60*60 + (diff.secs as i64);
     Ok(seconds)
}

Any objections?


On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   Cargo.toml        |  1 +
>   src/tools/cert.rs | 36 ++++++++++++++++++++++++++++++++++--
>   2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/Cargo.toml b/Cargo.toml
> index 3739d3af..74ccf361 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -32,6 +32,7 @@ endian_trait = { version = "0.6", features = ["arrays"] }
>   env_logger = "0.7"
>   flate2 = "1.0"
>   anyhow = "1.0"
> +foreign-types = "0.3"
>   thiserror = "1.0"
>   futures = "0.3"
>   h2 = { version = "0.3", features = [ "stream" ] }
> diff --git a/src/tools/cert.rs b/src/tools/cert.rs
> index 0c7e9e5d..d3c221a4 100644
> --- a/src/tools/cert.rs
> +++ b/src/tools/cert.rs
> @@ -1,12 +1,32 @@
>   use std::path::PathBuf;
> +use std::mem::MaybeUninit;
>   
> -use anyhow::Error;
> +use anyhow::{bail, format_err, Error};
> +use foreign_types::ForeignTypeRef;
>   use openssl::x509::{X509, GeneralName};
>   use openssl::stack::Stack;
>   use openssl::pkey::{Public, PKey};
>   
>   use crate::configdir;
>   
> +// C type:
> +#[allow(non_camel_case_types)]
> +type ASN1_TIME = <openssl::asn1::Asn1TimeRef as ForeignTypeRef>::CType;
> +
> +extern "C" {
> +    fn ASN1_TIME_to_tm(s: *const ASN1_TIME, tm: *mut libc::tm) -> libc::c_int;
> +}
> +
> +fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64, Error> {
> +    let mut c_tm = MaybeUninit::<libc::tm>::uninit();
> +    let rc = unsafe { ASN1_TIME_to_tm(time.as_ptr(), c_tm.as_mut_ptr()) };
> +    if rc != 1 {
> +        bail!("failed to parse ASN1 time");
> +    }
> +    let mut c_tm = unsafe { c_tm.assume_init() };
> +    proxmox::tools::time::timegm(&mut c_tm)
> +}
> +
>   pub struct CertInfo {
>       x509: X509,
>   }
> @@ -25,7 +45,11 @@ impl CertInfo {
>       }
>   
>       pub fn from_path(path: PathBuf) -> Result<Self, Error> {
> -        let cert_pem = proxmox::tools::fs::file_get_contents(&path)?;
> +        Self::from_pem(&proxmox::tools::fs::file_get_contents(&path)?)
> +            .map_err(|err| format_err!("failed to load certificate from {:?} - {}", path, err))
> +    }
> +
> +    pub fn from_pem(cert_pem: &[u8]) -> Result<Self, Error> {
>           let x509 = openssl::x509::X509::from_pem(&cert_pem)?;
>           Ok(Self{
>               x509
> @@ -64,4 +88,12 @@ impl CertInfo {
>       pub fn not_after(&self) -> &openssl::asn1::Asn1TimeRef {
>           self.x509.not_after()
>       }
> +
> +    pub fn not_before_unix(&self) -> Result<i64, Error> {
> +        asn1_time_to_unix(&self.not_before())
> +    }
> +
> +    pub fn not_after_unix(&self) -> Result<i64, Error> {
> +        asn1_time_to_unix(&self.not_after())
> +    }
>   }

Re: [pbs-devel] [PATCH v2 backup 05/27] CertInfo: add not_{after, before}_unix

[Wolfgang Bumiller]

On Thu, Apr 29, 2021 at 08:13:19AM +0200, Dietmar Maurer wrote:
> Seems I can do it without foreign-types:
> 
> fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64,
> Error> {
>     let epoch0 = openssl::asn1::Asn1Time::from_unix(0)?;
>     let diff = epoch0.diff(time)?;
>     let seconds = (diff.days as i64) * 24*60*60 + (diff.secs as i64);
>     Ok(seconds)
> }
> 
> Any objections?

Yes, for 2 reasons:
* openssl does provide the functionality and the dependency is already
  in our tree because openssl pulls it in
* 1100 days in already covers 3 leap seconds and I don't want to worry
  about whether `diff.days` takes that into account, the best time math
  is no time math at all

Re: [pbs-devel] [PATCH v2 backup 05/27] CertInfo: add not_{after, before}_unix

[Dietmar Maurer]

On 4/29/21 9:01 AM, Wolfgang Bumiller wrote:
> On Thu, Apr 29, 2021 at 08:13:19AM +0200, Dietmar Maurer wrote:
>> Seems I can do it without foreign-types:
>>
>> fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64,
>> Error> {
>>      let epoch0 = openssl::asn1::Asn1Time::from_unix(0)?;
>>      let diff = epoch0.diff(time)?;
>>      let seconds = (diff.days as i64) * 24*60*60 + (diff.secs as i64);
>>      Ok(seconds)
>> }
>>
>> Any objections?
> Yes, for 2 reasons:
> * openssl does provide the functionality and the dependency is already
>    in our tree because openssl pulls it in
> * 1100 days in already covers 3 leap seconds and I don't want to worry
>    about whether `diff.days` takes that into account, the best time math
>    is no time math at all

Agreed, but your code is unsafe and hard to read. IMHO that whole 
foreign_type thing is hard to understand. And Unix Epoch does not care 
about leap seconds, so why should we do?

Re: [pbs-devel] [PATCH v2 backup 05/27] CertInfo: add not_{after, before}_unix

[Wolfgang Bumiller]

On Thu, Apr 29, 2021 at 09:08:03AM +0200, Dietmar Maurer wrote:
> 
> On 4/29/21 9:01 AM, Wolfgang Bumiller wrote:
> > On Thu, Apr 29, 2021 at 08:13:19AM +0200, Dietmar Maurer wrote:
> > > Seems I can do it without foreign-types:
> > > 
> > > fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64,
> > > Error> {
> > >      let epoch0 = openssl::asn1::Asn1Time::from_unix(0)?;
> > >      let diff = epoch0.diff(time)?;
> > >      let seconds = (diff.days as i64) * 24*60*60 + (diff.secs as i64);
> > >      Ok(seconds)
> > > }
> > > 
> > > Any objections?
> > Yes, for 2 reasons:
> > * openssl does provide the functionality and the dependency is already
> >    in our tree because openssl pulls it in
> > * 1100 days in already covers 3 leap seconds and I don't want to worry
> >    about whether `diff.days` takes that into account, the best time math
> >    is no time math at all
> 
> Agreed, but your code is unsafe and hard to read. IMHO that whole
> foreign_type thing is hard to understand. And Unix Epoch does not care about
> leap seconds, so why should we do?

Because the diff method doesn't give you a unix epoch, it gives you a
number of days without context which originally come from calendar
dates, and this way days aren't well-enough defined for my taste.

Re: [pbs-devel] [PATCH v2 backup 05/27] CertInfo: add not_{after, before}_unix

[Dietmar Maurer]

On 4/29/21 9:14 AM, Wolfgang Bumiller wrote:
> On Thu, Apr 29, 2021 at 09:08:03AM +0200, Dietmar Maurer wrote:
>> On 4/29/21 9:01 AM, Wolfgang Bumiller wrote:
>>> On Thu, Apr 29, 2021 at 08:13:19AM +0200, Dietmar Maurer wrote:
>>>> Seems I can do it without foreign-types:
>>>>
>>>> fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64,
>>>> Error> {
>>>>       let epoch0 = openssl::asn1::Asn1Time::from_unix(0)?;
>>>>       let diff = epoch0.diff(time)?;
>>>>       let seconds = (diff.days as i64) * 24*60*60 + (diff.secs as i64);
>>>>       Ok(seconds)
>>>> }
>>>>
>>>> Any objections?
>>> Yes, for 2 reasons:
>>> * openssl does provide the functionality and the dependency is already
>>>     in our tree because openssl pulls it in
>>> * 1100 days in already covers 3 leap seconds and I don't want to worry
>>>     about whether `diff.days` takes that into account, the best time math
>>>     is no time math at all
>> Agreed, but your code is unsafe and hard to read. IMHO that whole
>> foreign_type thing is hard to understand. And Unix Epoch does not care about
>> leap seconds, so why should we do?
> Because the diff method doesn't give you a unix epoch, it gives you a
> number of days without context which originally come from calendar
> dates, and this way days aren't well-enough defined for my taste.

Beside, it seems we do not need those methods at all if we return the 
tlme as String in the API.

Returning time as String is better anyways, because it shows whats 
encoded inside the cert.

For example. on my host:

# openssl x509 -in /etc/proxmox-backup/proxy.pem -noout -text|grep After
             Not After : Sep  2 13:45:56 3019 GMT

If it convert to epoch and print that I get:

3019-09-02T15:45:56+02:00

We loose the original time zone info, so this is not optimal.

Re: [pbs-devel] [PATCH v2 backup 05/27] CertInfo: add not_{after, before}_unix

[Wolfgang Bumiller]

On Thu, Apr 29, 2021 at 10:33:06AM +0200, Dietmar Maurer wrote:
> 
> On 4/29/21 9:14 AM, Wolfgang Bumiller wrote:
> > On Thu, Apr 29, 2021 at 09:08:03AM +0200, Dietmar Maurer wrote:
> > > On 4/29/21 9:01 AM, Wolfgang Bumiller wrote:
> > > > On Thu, Apr 29, 2021 at 08:13:19AM +0200, Dietmar Maurer wrote:
> > > > > Seems I can do it without foreign-types:
> > > > > 
> > > > > fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64,
> > > > > Error> {
> > > > >       let epoch0 = openssl::asn1::Asn1Time::from_unix(0)?;
> > > > >       let diff = epoch0.diff(time)?;
> > > > >       let seconds = (diff.days as i64) * 24*60*60 + (diff.secs as i64);
> > > > >       Ok(seconds)
> > > > > }
> > > > > 
> > > > > Any objections?
> > > > Yes, for 2 reasons:
> > > > * openssl does provide the functionality and the dependency is already
> > > >     in our tree because openssl pulls it in
> > > > * 1100 days in already covers 3 leap seconds and I don't want to worry
> > > >     about whether `diff.days` takes that into account, the best time math
> > > >     is no time math at all
> > > Agreed, but your code is unsafe and hard to read. IMHO that whole
> > > foreign_type thing is hard to understand. And Unix Epoch does not care about
> > > leap seconds, so why should we do?
> > Because the diff method doesn't give you a unix epoch, it gives you a
> > number of days without context which originally come from calendar
> > dates, and this way days aren't well-enough defined for my taste.
> 
> Beside, it seems we do not need those methods at all if we return the tlme
> as String in the API.

I disagree.

On the one hand, the API is basically the same as PVE & PMG and is what
the GUI expects (since we use the same components).

On the other hand, IMO it makes more sense if a client can just take a
timestamp and display it in *their* local time zone. Having to first
parse the string output is weird, APIs are supposed to be simple, the
interfacing-with-a-human part is the UI's job.

[pbs-devel] applied: [PATCH v2 backup 05/27] CertInfo: add not_{after, before}_unix

[Dietmar Maurer]

applied (because I found no better way to do it)

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   Cargo.toml        |  1 +
>   src/tools/cert.rs | 36 ++++++++++++++++++++++++++++++++++--
>   2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/Cargo.toml b/Cargo.toml
> index 3739d3af..74ccf361 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -32,6 +32,7 @@ endian_trait = { version = "0.6", features = ["arrays"] }
>   env_logger = "0.7"
>   flate2 = "1.0"
>   anyhow = "1.0"
> +foreign-types = "0.3"
>   thiserror = "1.0"
>   futures = "0.3"
>   h2 = { version = "0.3", features = [ "stream" ] }
> diff --git a/src/tools/cert.rs b/src/tools/cert.rs
> index 0c7e9e5d..d3c221a4 100644
> --- a/src/tools/cert.rs
> +++ b/src/tools/cert.rs
> @@ -1,12 +1,32 @@
>   use std::path::PathBuf;
> +use std::mem::MaybeUninit;
>   
> -use anyhow::Error;
> +use anyhow::{bail, format_err, Error};
> +use foreign_types::ForeignTypeRef;
>   use openssl::x509::{X509, GeneralName};
>   use openssl::stack::Stack;
>   use openssl::pkey::{Public, PKey};
>   
>   use crate::configdir;
>   
> +// C type:
> +#[allow(non_camel_case_types)]
> +type ASN1_TIME = <openssl::asn1::Asn1TimeRef as ForeignTypeRef>::CType;
> +
> +extern "C" {
> +    fn ASN1_TIME_to_tm(s: *const ASN1_TIME, tm: *mut libc::tm) -> libc::c_int;
> +}
> +
> +fn asn1_time_to_unix(time: &openssl::asn1::Asn1TimeRef) -> Result<i64, Error> {
> +    let mut c_tm = MaybeUninit::<libc::tm>::uninit();
> +    let rc = unsafe { ASN1_TIME_to_tm(time.as_ptr(), c_tm.as_mut_ptr()) };
> +    if rc != 1 {
> +        bail!("failed to parse ASN1 time");
> +    }
> +    let mut c_tm = unsafe { c_tm.assume_init() };
> +    proxmox::tools::time::timegm(&mut c_tm)
> +}
> +
>   pub struct CertInfo {
>       x509: X509,
>   }
> @@ -25,7 +45,11 @@ impl CertInfo {
>       }
>   
>       pub fn from_path(path: PathBuf) -> Result<Self, Error> {
> -        let cert_pem = proxmox::tools::fs::file_get_contents(&path)?;
> +        Self::from_pem(&proxmox::tools::fs::file_get_contents(&path)?)
> +            .map_err(|err| format_err!("failed to load certificate from {:?} - {}", path, err))
> +    }
> +
> +    pub fn from_pem(cert_pem: &[u8]) -> Result<Self, Error> {
>           let x509 = openssl::x509::X509::from_pem(&cert_pem)?;
>           Ok(Self{
>               x509
> @@ -64,4 +88,12 @@ impl CertInfo {
>       pub fn not_after(&self) -> &openssl::asn1::Asn1TimeRef {
>           self.x509.not_after()
>       }
> +
> +    pub fn not_before_unix(&self) -> Result<i64, Error> {
> +        asn1_time_to_unix(&self.not_before())
> +    }
> +
> +    pub fn not_after_unix(&self) -> Result<i64, Error> {
> +        asn1_time_to_unix(&self.not_after())
> +    }
>   }

[pbs-devel] [PATCH v2 backup 06/27] CertInfo: add is_expired_after_epoch

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/tools/cert.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/tools/cert.rs b/src/tools/cert.rs
index d3c221a4..bcc3e69b 100644
--- a/src/tools/cert.rs
+++ b/src/tools/cert.rs
@@ -96,4 +96,9 @@ impl CertInfo {
     pub fn not_after_unix(&self) -> Result<i64, Error> {
         asn1_time_to_unix(&self.not_after())
     }
+
+    /// Check if the certificate is expired at or after a specific unix epoch.
+    pub fn is_expired_after_epoch(&self, epoch: i64) -> Result<bool, Error> {
+        Ok(self.not_after_unix()? < epoch)
+    }
 }
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 backup 06/27] CertInfo: add is_expired_after_epoch

[Dietmar Maurer]

applied

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/tools/cert.rs | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/src/tools/cert.rs b/src/tools/cert.rs
> index d3c221a4..bcc3e69b 100644
> --- a/src/tools/cert.rs
> +++ b/src/tools/cert.rs
> @@ -96,4 +96,9 @@ impl CertInfo {
>       pub fn not_after_unix(&self) -> Result<i64, Error> {
>           asn1_time_to_unix(&self.not_after())
>       }
> +
> +    /// Check if the certificate is expired at or after a specific unix epoch.
> +    pub fn is_expired_after_epoch(&self, epoch: i64) -> Result<bool, Error> {
> +        Ok(self.not_after_unix()? < epoch)
> +    }
>   }

[pbs-devel] [PATCH v2 backup 07/27] tools: add ControlFlow type

[Wolfgang Bumiller]

modeled after std::ops::ControlFlow

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/tools.rs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/tools.rs b/src/tools.rs
index 08af55e5..890db826 100644
--- a/src/tools.rs
+++ b/src/tools.rs
@@ -571,3 +571,14 @@ pub fn create_run_dir() -> Result<(), Error> {
     let _: bool = proxmox::tools::fs::create_path(PROXMOX_BACKUP_RUN_DIR_M!(), None, None)?;
     Ok(())
 }
+
+/// Modeled after the nightly `std::ops::ControlFlow`.
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum ControlFlow<B, C = ()> {
+    Continue(C),
+    Break(B),
+}
+
+impl<B> ControlFlow<B> {
+    pub const CONTINUE: ControlFlow<B, ()> = ControlFlow::Continue(());
+}
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 backup 07/27] tools: add ControlFlow type

[Dietmar Maurer]

applied.

I guess we want to replace that with std::ops::ControlFlow once that is 
stable?


On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> modeled after std::ops::ControlFlow
>
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/tools.rs | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/src/tools.rs b/src/tools.rs
> index 08af55e5..890db826 100644
> --- a/src/tools.rs
> +++ b/src/tools.rs
> @@ -571,3 +571,14 @@ pub fn create_run_dir() -> Result<(), Error> {
>       let _: bool = proxmox::tools::fs::create_path(PROXMOX_BACKUP_RUN_DIR_M!(), None, None)?;
>       Ok(())
>   }
> +
> +/// Modeled after the nightly `std::ops::ControlFlow`.
> +#[derive(Clone, Copy, Debug, PartialEq)]
> +pub enum ControlFlow<B, C = ()> {
> +    Continue(C),
> +    Break(B),
> +}
> +
> +impl<B> ControlFlow<B> {
> +    pub const CONTINUE: ControlFlow<B, ()> = ControlFlow::Continue(());
> +}

Re: [pbs-devel] applied: [PATCH v2 backup 07/27] tools: add ControlFlow type

[Wolfgang Bumiller]

On Thu, Apr 29, 2021 at 11:17:06AM +0200, Dietmar Maurer wrote:
> applied.
> 
> I guess we want to replace that with std::ops::ControlFlow once that is
> stable?

Yeah that's the idea.
Just something to be on the lookout in the future where we may be able
to simplify code.
(Particularly I expect a bunch of `try_fold` calls to become something
simpler like a `try_collect` which IIRC is one of the things they want
to add later when `ControlFlow` and the new try-trait variant are
stable, though I have no idea how long the latter will take...)

[pbs-devel] [PATCH v2 backup 08/27] catalog shell: replace LoopState with ControlFlow

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/backup/catalog_shell.rs | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/src/backup/catalog_shell.rs b/src/backup/catalog_shell.rs
index 00b5b580..11866c30 100644
--- a/src/backup/catalog_shell.rs
+++ b/src/backup/catalog_shell.rs
@@ -19,9 +19,10 @@ use proxmox::tools::fs::{create_path, CreateOptions};
 use pxar::{EntryKind, Metadata};
 
 use crate::backup::catalog::{self, DirEntryAttribute};
-use crate::pxar::Flags;
 use crate::pxar::fuse::{Accessor, FileEntry};
+use crate::pxar::Flags;
 use crate::tools::runtime::block_in_place;
+use crate::tools::ControlFlow;
 
 type CatalogReader = crate::backup::CatalogReader<std::fs::File>;
 
@@ -998,11 +999,6 @@ impl Shell {
     }
 }
 
-enum LoopState {
-    Break,
-    Continue,
-}
-
 struct ExtractorState<'a> {
     path: Vec<u8>,
     path_len: usize,
@@ -1060,8 +1056,8 @@ impl<'a> ExtractorState<'a> {
             let entry = match self.read_dir.next() {
                 Some(entry) => entry,
                 None => match self.handle_end_of_directory()? {
-                    LoopState::Break => break, // done with root directory
-                    LoopState::Continue => continue,
+                    ControlFlow::Break(()) => break, // done with root directory
+                    ControlFlow::Continue(()) => continue,
                 },
             };
 
@@ -1079,11 +1075,11 @@ impl<'a> ExtractorState<'a> {
         Ok(())
     }
 
-    fn handle_end_of_directory(&mut self) -> Result<LoopState, Error> {
+    fn handle_end_of_directory(&mut self) -> Result<ControlFlow<()>, Error> {
         // go up a directory:
         self.read_dir = match self.read_dir_stack.pop() {
             Some(r) => r,
-            None => return Ok(LoopState::Break), // out of root directory
+            None => return Ok(ControlFlow::Break(())), // out of root directory
         };
 
         self.matches = self
@@ -1102,7 +1098,7 @@ impl<'a> ExtractorState<'a> {
 
         self.extractor.leave_directory()?;
 
-        Ok(LoopState::Continue)
+        Ok(ControlFlow::CONTINUE)
     }
 
     async fn handle_new_directory(
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 backup 08/27] catalog shell: replace LoopState with ControlFlow

[Dietmar Maurer]

applied

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/backup/catalog_shell.rs | 18 +++++++-----------
>   1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/src/backup/catalog_shell.rs b/src/backup/catalog_shell.rs
> index 00b5b580..11866c30 100644
> --- a/src/backup/catalog_shell.rs
> +++ b/src/backup/catalog_shell.rs
> @@ -19,9 +19,10 @@ use proxmox::tools::fs::{create_path, CreateOptions};
>   use pxar::{EntryKind, Metadata};
>   
>   use crate::backup::catalog::{self, DirEntryAttribute};
> -use crate::pxar::Flags;
>   use crate::pxar::fuse::{Accessor, FileEntry};
> +use crate::pxar::Flags;
>   use crate::tools::runtime::block_in_place;
> +use crate::tools::ControlFlow;
>   
>   type CatalogReader = crate::backup::CatalogReader<std::fs::File>;
>   
> @@ -998,11 +999,6 @@ impl Shell {
>       }
>   }
>   
> -enum LoopState {
> -    Break,
> -    Continue,
> -}
> -
>   struct ExtractorState<'a> {
>       path: Vec<u8>,
>       path_len: usize,
> @@ -1060,8 +1056,8 @@ impl<'a> ExtractorState<'a> {
>               let entry = match self.read_dir.next() {
>                   Some(entry) => entry,
>                   None => match self.handle_end_of_directory()? {
> -                    LoopState::Break => break, // done with root directory
> -                    LoopState::Continue => continue,
> +                    ControlFlow::Break(()) => break, // done with root directory
> +                    ControlFlow::Continue(()) => continue,
>                   },
>               };
>   
> @@ -1079,11 +1075,11 @@ impl<'a> ExtractorState<'a> {
>           Ok(())
>       }
>   
> -    fn handle_end_of_directory(&mut self) -> Result<LoopState, Error> {
> +    fn handle_end_of_directory(&mut self) -> Result<ControlFlow<()>, Error> {
>           // go up a directory:
>           self.read_dir = match self.read_dir_stack.pop() {
>               Some(r) => r,
> -            None => return Ok(LoopState::Break), // out of root directory
> +            None => return Ok(ControlFlow::Break(())), // out of root directory
>           };
>   
>           self.matches = self
> @@ -1102,7 +1098,7 @@ impl<'a> ExtractorState<'a> {
>   
>           self.extractor.leave_directory()?;
>   
> -        Ok(LoopState::Continue)
> +        Ok(ControlFlow::CONTINUE)
>       }
>   
>       async fn handle_new_directory(

[pbs-devel] [PATCH v2 backup 09/27] Cargo.toml: depend on proxmox-acme-rs

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 Cargo.toml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Cargo.toml b/Cargo.toml
index 74ccf361..322e6ab9 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -79,6 +79,8 @@ zstd = { version = "0.4", features = [ "bindgen" ] }
 nom = "5.1"
 crossbeam-channel = "0.5"
 
+proxmox-acme-rs = "0.2.1"
+
 [features]
 default = []
 #valgrind = ["valgrind_request"]
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 backup 09/27] Cargo.toml: depend on proxmox-acme-rs

[Dietmar Maurer]

applied

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   Cargo.toml | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/Cargo.toml b/Cargo.toml
> index 74ccf361..322e6ab9 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -79,6 +79,8 @@ zstd = { version = "0.4", features = [ "bindgen" ] }
>   nom = "5.1"
>   crossbeam-channel = "0.5"
>   
> +proxmox-acme-rs = "0.2.1"
> +
>   [features]
>   default = []
>   #valgrind = ["valgrind_request"]

[pbs-devel] [PATCH v2 backup 10/27] bump d/control

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 debian/control | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/debian/control b/debian/control
index d50ac970..05a3ccd3 100644
--- a/debian/control
+++ b/debian/control
@@ -17,6 +17,7 @@ Build-Depends: debhelper (>= 11),
  librust-endian-trait-0.6+default-dev,
  librust-env-logger-0.7+default-dev,
  librust-flate2-1+default-dev,
+ librust-foreign-types-0.3+default-dev,
  librust-futures-0.3+default-dev,
  librust-h2-0.3+default-dev,
  librust-h2-0.3+stream-dev,
@@ -42,6 +43,7 @@ Build-Depends: debhelper (>= 11),
  librust-proxmox-0.11+default-dev (>= 0.11.1-~~),
  librust-proxmox-0.11+sortable-macro-dev (>= 0.11.1-~~),
  librust-proxmox-0.11+websocket-dev (>= 0.11.1-~~),
+ librust-proxmox-acme-rs-0.2+default-dev (>= 0.2.1-~~),
  librust-proxmox-fuse-0.1+default-dev (>= 0.1.1-~~),
  librust-pxar-0.10+default-dev (>= 0.10.1-~~),
  librust-pxar-0.10+tokio-io-dev (>= 0.10.1-~~),
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 backup 10/27] bump d/control

[Dietmar Maurer]

applied

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   debian/control | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/debian/control b/debian/control
> index d50ac970..05a3ccd3 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -17,6 +17,7 @@ Build-Depends: debhelper (>= 11),
>    librust-endian-trait-0.6+default-dev,
>    librust-env-logger-0.7+default-dev,
>    librust-flate2-1+default-dev,
> + librust-foreign-types-0.3+default-dev,
>    librust-futures-0.3+default-dev,
>    librust-h2-0.3+default-dev,
>    librust-h2-0.3+stream-dev,
> @@ -42,6 +43,7 @@ Build-Depends: debhelper (>= 11),
>    librust-proxmox-0.11+default-dev (>= 0.11.1-~~),
>    librust-proxmox-0.11+sortable-macro-dev (>= 0.11.1-~~),
>    librust-proxmox-0.11+websocket-dev (>= 0.11.1-~~),
> + librust-proxmox-acme-rs-0.2+default-dev (>= 0.2.1-~~),
>    librust-proxmox-fuse-0.1+default-dev (>= 0.1.1-~~),
>    librust-pxar-0.10+default-dev (>= 0.10.1-~~),
>    librust-pxar-0.10+tokio-io-dev (>= 0.10.1-~~),

[pbs-devel] [PATCH v2 backup 11/27] config::acl: make /system/certificates a valid path

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/config/acl.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/config/acl.rs b/src/config/acl.rs
index 61e507ec..04d42854 100644
--- a/src/config/acl.rs
+++ b/src/config/acl.rs
@@ -308,7 +308,7 @@ pub fn check_acl_path(path: &str) -> Result<(), Error> {
                 return Ok(());
             }
             match components[1] {
-                "disks" | "log" | "status" | "tasks" | "time" => {
+                "certificates" | "disks" | "log" | "status" | "tasks" | "time" => {
                     if components_len == 2 {
                         return Ok(());
                     }
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 backup 11/27] config::acl: make /system/certificates a valid path

[Dietmar Maurer]

applied

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/config/acl.rs | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/config/acl.rs b/src/config/acl.rs
> index 61e507ec..04d42854 100644
> --- a/src/config/acl.rs
> +++ b/src/config/acl.rs
> @@ -308,7 +308,7 @@ pub fn check_acl_path(path: &str) -> Result<(), Error> {
>                   return Ok(());
>               }
>               match components[1] {
> -                "disks" | "log" | "status" | "tasks" | "time" => {
> +                "certificates" | "disks" | "log" | "status" | "tasks" | "time" => {
>                       if components_len == 2 {
>                           return Ok(());
>                       }

[pbs-devel] [PATCH v2 backup 12/27] add 'config file format' to tools::config

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
* Replaces the serde-based parser from v1. Outside API stays similar (with
  `from_str`, `from_property_string`, `to_bytes` ...
* Added a very simple testcase.

 src/tools.rs        |   1 +
 src/tools/config.rs | 171 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 172 insertions(+)
 create mode 100644 src/tools/config.rs

diff --git a/src/tools.rs b/src/tools.rs
index 890db826..25323881 100644
--- a/src/tools.rs
+++ b/src/tools.rs
@@ -23,6 +23,7 @@ pub mod async_io;
 pub mod borrow;
 pub mod cert;
 pub mod compression;
+pub mod config;
 pub mod cpio;
 pub mod daemon;
 pub mod disks;
diff --git a/src/tools/config.rs b/src/tools/config.rs
new file mode 100644
index 00000000..499bd187
--- /dev/null
+++ b/src/tools/config.rs
@@ -0,0 +1,171 @@
+//! Our 'key: value' config format.
+
+use std::io::Write;
+
+use anyhow::{bail, format_err, Error};
+use serde::{Deserialize, Serialize};
+use serde_json::Value;
+
+use proxmox::api::schema::{
+    parse_property_string, parse_simple_value, verify_json_object, ObjectSchemaType, Schema,
+};
+
+type Object = serde_json::Map<String, Value>;
+
+fn object_schema(schema: &'static Schema) -> Result<&'static dyn ObjectSchemaType, Error> {
+    Ok(match schema {
+        Schema::Object(schema) => schema,
+        Schema::AllOf(schema) => schema,
+        _ => bail!("invalid schema for config, must be an object schema"),
+    })
+}
+
+/// Parse a full string representing a config file.
+pub fn from_str<T: for<'de> Deserialize<'de>>(
+    input: &str,
+    schema: &'static Schema,
+) -> Result<T, Error> {
+    Ok(serde_json::from_value(value_from_str(input, schema)?)?)
+}
+
+/// Parse a full string representing a config file.
+pub fn value_from_str(input: &str, schema: &'static Schema) -> Result<Value, Error> {
+    let schema = object_schema(schema)?;
+
+    let mut config = Object::new();
+
+    for (lineno, line) in input.lines().enumerate() {
+        let line = line.trim();
+        if line.starts_with('#') || line.is_empty() {
+            continue;
+        }
+
+        parse_line(&mut config, line, schema)
+            .map_err(|err| format_err!("line {}: {}", lineno, err))?;
+    }
+
+    Ok(Value::Object(config))
+}
+
+/// Parse a single `key: value` line from a config file.
+fn parse_line(
+    config: &mut Object,
+    line: &str,
+    schema: &'static dyn ObjectSchemaType,
+) -> Result<(), Error> {
+    if line.starts_with('#') || line.is_empty() {
+        return Ok(());
+    }
+
+    let colon = line
+        .find(':')
+        .ok_or_else(|| format_err!("missing colon to separate key from value"))?;
+    if colon == 0 {
+        bail!("empty key not allowed");
+    }
+
+    let key = &line[..colon];
+    let value = line[(colon + 1)..].trim_start();
+
+    parse_key_value(config, key, value, schema)
+}
+
+/// Lookup the key in the schema, parse the value and insert it into the config object.
+fn parse_key_value(
+    config: &mut Object,
+    key: &str,
+    value: &str,
+    schema: &'static dyn ObjectSchemaType,
+) -> Result<(), Error> {
+    let schema = match schema.lookup(key) {
+        Some((_optional, schema)) => Some(schema),
+        None if schema.additional_properties() => None,
+        None => bail!(
+            "invalid key '{}' and schema does not allow additional properties",
+            key
+        ),
+    };
+
+    let value = parse_value(value, schema)?;
+    config.insert(key.to_owned(), value);
+    Ok(())
+}
+
+/// For this we can just reuse the schema's "parse_simple_value".
+///
+/// "Additional" properties (`None` schema) will simply become strings.
+///
+/// Note that this does not handle Object or Array types at all, so if we want to support them
+/// natively without going over a `String` type, we can add this here.
+fn parse_value(value: &str, schema: Option<&'static Schema>) -> Result<Value, Error> {
+    match schema {
+        None => Ok(Value::String(value.to_owned())),
+        Some(schema) => parse_simple_value(value, schema),
+    }
+}
+
+/// Parse a string as a property string into a deserializable type. This is just a short wrapper
+/// around deserializing the s
+pub fn from_property_string<T>(input: &str, schema: &'static Schema) -> Result<T, Error>
+where
+    T: for<'de> Deserialize<'de>,
+{
+    Ok(serde_json::from_value(parse_property_string(
+        input, schema,
+    )?)?)
+}
+
+/// Serialize a data structure using a 'key: value' config file format.
+pub fn to_bytes<T: Serialize>(value: &T, schema: &'static Schema) -> Result<Vec<u8>, Error> {
+    value_to_bytes(&serde_json::to_value(value)?, schema)
+}
+
+/// Serialize a json value using a 'key: value' config file format.
+pub fn value_to_bytes(value: &Value, schema: &'static Schema) -> Result<Vec<u8>, Error> {
+    let schema = object_schema(schema)?;
+
+    verify_json_object(value, schema)?;
+
+    let object = value
+        .as_object()
+        .ok_or_else(|| format_err!("value must be an object"))?;
+
+    let mut out = Vec::new();
+    object_to_writer(&mut out, object)?;
+    Ok(out)
+}
+
+/// Note: the object must have already been verified at this point.
+fn object_to_writer(output: &mut dyn Write, object: &Object) -> Result<(), Error> {
+    for (key, value) in object.iter() {
+        match value {
+            Value::Null => continue, // delete this entry
+            Value::Bool(v) => writeln!(output, "{}: {}", key, v)?,
+            Value::String(v) => writeln!(output, "{}: {}", key, v)?,
+            Value::Number(v) => writeln!(output, "{}: {}", key, v)?,
+            Value::Array(_) => bail!("arrays are not supported in config files"),
+            Value::Object(_) => bail!("complex objects are not supported in config files"),
+        }
+    }
+    Ok(())
+}
+
+#[test]
+fn test() {
+    // let's just reuse some schema we actually have available:
+    use crate::config::node::NodeConfig;
+
+    const NODE_CONFIG: &str = "\
+        acme: account=pebble\n\
+        acmedomain0: test1.invalid.local,plugin=power\n\
+        acmedomain1: test2.invalid.local\n\
+    ";
+
+    let data: NodeConfig = from_str(NODE_CONFIG, &NodeConfig::API_SCHEMA)
+        .expect("failed to parse simple node config");
+
+    let config = to_bytes(&data, &NodeConfig::API_SCHEMA)
+        .expect("failed to serialize node config");
+
+    assert_eq!(config, NODE_CONFIG.as_bytes());
+}
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 backup 12/27] add 'config file format' to tools::config

[Dietmar Maurer]

applied

On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
> * Replaces the serde-based parser from v1. Outside API stays similar (with
>    `from_str`, `from_property_string`, `to_bytes` ...
> * Added a very simple testcase.
>
>   src/tools.rs        |   1 +
>   src/tools/config.rs | 171 ++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 172 insertions(+)
>   create mode 100644 src/tools/config.rs
>
> diff --git a/src/tools.rs b/src/tools.rs
> index 890db826..25323881 100644
> --- a/src/tools.rs
> +++ b/src/tools.rs
> @@ -23,6 +23,7 @@ pub mod async_io;
>   pub mod borrow;
>   pub mod cert;
>   pub mod compression;
> +pub mod config;
>   pub mod cpio;
>   pub mod daemon;
>   pub mod disks;
> diff --git a/src/tools/config.rs b/src/tools/config.rs
> new file mode 100644
> index 00000000..499bd187
> --- /dev/null
> +++ b/src/tools/config.rs
> @@ -0,0 +1,171 @@
> +//! Our 'key: value' config format.
> +
> +use std::io::Write;
> +
> +use anyhow::{bail, format_err, Error};
> +use serde::{Deserialize, Serialize};
> +use serde_json::Value;
> +
> +use proxmox::api::schema::{
> +    parse_property_string, parse_simple_value, verify_json_object, ObjectSchemaType, Schema,
> +};
> +
> +type Object = serde_json::Map<String, Value>;
> +
> +fn object_schema(schema: &'static Schema) -> Result<&'static dyn ObjectSchemaType, Error> {
> +    Ok(match schema {
> +        Schema::Object(schema) => schema,
> +        Schema::AllOf(schema) => schema,
> +        _ => bail!("invalid schema for config, must be an object schema"),
> +    })
> +}
> +
> +/// Parse a full string representing a config file.
> +pub fn from_str<T: for<'de> Deserialize<'de>>(
> +    input: &str,
> +    schema: &'static Schema,
> +) -> Result<T, Error> {
> +    Ok(serde_json::from_value(value_from_str(input, schema)?)?)
> +}
> +
> +/// Parse a full string representing a config file.
> +pub fn value_from_str(input: &str, schema: &'static Schema) -> Result<Value, Error> {
> +    let schema = object_schema(schema)?;
> +
> +    let mut config = Object::new();
> +
> +    for (lineno, line) in input.lines().enumerate() {
> +        let line = line.trim();
> +        if line.starts_with('#') || line.is_empty() {
> +            continue;
> +        }
> +
> +        parse_line(&mut config, line, schema)
> +            .map_err(|err| format_err!("line {}: {}", lineno, err))?;
> +    }
> +
> +    Ok(Value::Object(config))
> +}
> +
> +/// Parse a single `key: value` line from a config file.
> +fn parse_line(
> +    config: &mut Object,
> +    line: &str,
> +    schema: &'static dyn ObjectSchemaType,
> +) -> Result<(), Error> {
> +    if line.starts_with('#') || line.is_empty() {
> +        return Ok(());
> +    }
> +
> +    let colon = line
> +        .find(':')
> +        .ok_or_else(|| format_err!("missing colon to separate key from value"))?;
> +    if colon == 0 {
> +        bail!("empty key not allowed");
> +    }
> +
> +    let key = &line[..colon];
> +    let value = line[(colon + 1)..].trim_start();
> +
> +    parse_key_value(config, key, value, schema)
> +}
> +
> +/// Lookup the key in the schema, parse the value and insert it into the config object.
> +fn parse_key_value(
> +    config: &mut Object,
> +    key: &str,
> +    value: &str,
> +    schema: &'static dyn ObjectSchemaType,
> +) -> Result<(), Error> {
> +    let schema = match schema.lookup(key) {
> +        Some((_optional, schema)) => Some(schema),
> +        None if schema.additional_properties() => None,
> +        None => bail!(
> +            "invalid key '{}' and schema does not allow additional properties",
> +            key
> +        ),
> +    };
> +
> +    let value = parse_value(value, schema)?;
> +    config.insert(key.to_owned(), value);
> +    Ok(())
> +}
> +
> +/// For this we can just reuse the schema's "parse_simple_value".
> +///
> +/// "Additional" properties (`None` schema) will simply become strings.
> +///
> +/// Note that this does not handle Object or Array types at all, so if we want to support them
> +/// natively without going over a `String` type, we can add this here.
> +fn parse_value(value: &str, schema: Option<&'static Schema>) -> Result<Value, Error> {
> +    match schema {
> +        None => Ok(Value::String(value.to_owned())),
> +        Some(schema) => parse_simple_value(value, schema),
> +    }
> +}
> +
> +/// Parse a string as a property string into a deserializable type. This is just a short wrapper
> +/// around deserializing the s
> +pub fn from_property_string<T>(input: &str, schema: &'static Schema) -> Result<T, Error>
> +where
> +    T: for<'de> Deserialize<'de>,
> +{
> +    Ok(serde_json::from_value(parse_property_string(
> +        input, schema,
> +    )?)?)
> +}
> +
> +/// Serialize a data structure using a 'key: value' config file format.
> +pub fn to_bytes<T: Serialize>(value: &T, schema: &'static Schema) -> Result<Vec<u8>, Error> {
> +    value_to_bytes(&serde_json::to_value(value)?, schema)
> +}
> +
> +/// Serialize a json value using a 'key: value' config file format.
> +pub fn value_to_bytes(value: &Value, schema: &'static Schema) -> Result<Vec<u8>, Error> {
> +    let schema = object_schema(schema)?;
> +
> +    verify_json_object(value, schema)?;
> +
> +    let object = value
> +        .as_object()
> +        .ok_or_else(|| format_err!("value must be an object"))?;
> +
> +    let mut out = Vec::new();
> +    object_to_writer(&mut out, object)?;
> +    Ok(out)
> +}
> +
> +/// Note: the object must have already been verified at this point.
> +fn object_to_writer(output: &mut dyn Write, object: &Object) -> Result<(), Error> {
> +    for (key, value) in object.iter() {
> +        match value {
> +            Value::Null => continue, // delete this entry
> +            Value::Bool(v) => writeln!(output, "{}: {}", key, v)?,
> +            Value::String(v) => writeln!(output, "{}: {}", key, v)?,
> +            Value::Number(v) => writeln!(output, "{}: {}", key, v)?,
> +            Value::Array(_) => bail!("arrays are not supported in config files"),
> +            Value::Object(_) => bail!("complex objects are not supported in config files"),
> +        }
> +    }
> +    Ok(())
> +}
> +
> +#[test]
> +fn test() {
> +    // let's just reuse some schema we actually have available:
> +    use crate::config::node::NodeConfig;
> +
> +    const NODE_CONFIG: &str = "\
> +        acme: account=pebble\n\
> +        acmedomain0: test1.invalid.local,plugin=power\n\
> +        acmedomain1: test2.invalid.local\n\
> +    ";
> +
> +    let data: NodeConfig = from_str(NODE_CONFIG, &NodeConfig::API_SCHEMA)
> +        .expect("failed to parse simple node config");
> +
> +    let config = to_bytes(&data, &NodeConfig::API_SCHEMA)
> +        .expect("failed to serialize node config");
> +
> +    assert_eq!(config, NODE_CONFIG.as_bytes());
> +}

[pbs-devel] [PATCH v2 backup 13/27] add node config

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/config.rs      |   1 +
 src/config/node.rs | 225 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 226 insertions(+)
 create mode 100644 src/config/node.rs

diff --git a/src/config.rs b/src/config.rs
index 37df2fd2..717829e2 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -19,6 +19,7 @@ pub mod acl;
 pub mod cached_user_info;
 pub mod datastore;
 pub mod network;
+pub mod node;
 pub mod remote;
 pub mod sync;
 pub mod tfa;
diff --git a/src/config/node.rs b/src/config/node.rs
new file mode 100644
index 00000000..7bfa95d6
--- /dev/null
+++ b/src/config/node.rs
@@ -0,0 +1,225 @@
+use std::fs::File;
+use std::time::Duration;
+
+use anyhow::{format_err, Error};
+use nix::sys::stat::Mode;
+use serde::{Deserialize, Serialize};
+
+use proxmox::api::api;
+use proxmox::api::schema::{self, Updater};
+use proxmox::tools::fs::{replace_file, CreateOptions};
+
+use crate::acme::AcmeClient;
+use crate::api2::types::{DNS_ALIAS_FORMAT, DNS_NAME_FORMAT, PROXMOX_SAFE_ID_FORMAT};
+use crate::config::acme::AccountName;
+
+const CONF_FILE: &str = configdir!("/node.cfg");
+const LOCK_FILE: &str = configdir!("/.node.cfg.lock");
+const LOCK_TIMEOUT: Duration = Duration::from_secs(5);
+
+pub fn read_lock() -> Result<File, Error> {
+    proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, false)
+}
+
+pub fn write_lock() -> Result<File, Error> {
+    proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)
+}
+
+/// Read the Node Config.
+pub fn config() -> Result<(NodeConfig, [u8; 32]), Error> {
+    let content =
+        proxmox::tools::fs::file_read_optional_string(CONF_FILE)?.unwrap_or_else(|| "".to_string());
+
+    let digest = openssl::sha::sha256(content.as_bytes());
+    let data: NodeConfig = crate::tools::config::from_str(&content, &NodeConfig::API_SCHEMA)?;
+
+    Ok((data, digest))
+}
+
+/// Write the Node Config, requires the write lock to be held.
+pub fn save_config(config: &NodeConfig) -> Result<(), Error> {
+    let raw = crate::tools::config::to_bytes(config, &NodeConfig::API_SCHEMA)?;
+
+    let backup_user = crate::backup::backup_user()?;
+    let options = CreateOptions::new()
+        .perm(Mode::from_bits_truncate(0o0640))
+        .owner(nix::unistd::ROOT)
+        .group(backup_user.gid);
+
+    replace_file(CONF_FILE, &raw, options)
+}
+
+#[api(
+    properties: {
+        "domain": { format: &DNS_NAME_FORMAT },
+        "alias": {
+            optional: true,
+            format: &DNS_ALIAS_FORMAT,
+        },
+        "plugin": {
+            optional: true,
+            format: &PROXMOX_SAFE_ID_FORMAT,
+        },
+    },
+    default_key: "domain",
+)]
+#[derive(Deserialize, Serialize)]
+/// A domain entry for an ACME certificate.
+pub struct AcmeDomain {
+    /// The domain to certify for.
+    pub domain: String,
+
+    /// The domain to use for challenges instead of the default acme challenge domain.
+    ///
+    /// This is useful if you use CNAME entries to redirect `_acme-challenge.*` domains to a
+    /// different DNS server.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub alias: Option<String>,
+
+    /// The plugin to use to validate this domain.
+    ///
+    /// Empty means standalone HTTP validation is used.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub plugin: Option<String>,
+}
+
+#[api(
+    properties: {
+        account: { type: AccountName },
+    }
+)]
+#[derive(Deserialize, Serialize)]
+/// The ACME configuration.
+///
+/// Currently only contains the name of the account use.
+pub struct AcmeConfig {
+    /// Account to use to acquire ACME certificates.
+    account: AccountName,
+}
+
+#[api(
+    properties: {
+        acme: {
+            optional: true,
+            type: String,
+            format: &schema::ApiStringFormat::PropertyString(&AcmeConfig::API_SCHEMA),
+        },
+        acmedomain0: {
+            type: String,
+            optional: true,
+            format: &schema::ApiStringFormat::PropertyString(&AcmeDomain::API_SCHEMA),
+        },
+        acmedomain1: {
+            type: String,
+            optional: true,
+            format: &schema::ApiStringFormat::PropertyString(&AcmeDomain::API_SCHEMA),
+        },
+        acmedomain2: {
+            type: String,
+            optional: true,
+            format: &schema::ApiStringFormat::PropertyString(&AcmeDomain::API_SCHEMA),
+        },
+        acmedomain3: {
+            type: String,
+            optional: true,
+            format: &schema::ApiStringFormat::PropertyString(&AcmeDomain::API_SCHEMA),
+        },
+        acmedomain4: {
+            type: String,
+            optional: true,
+            format: &schema::ApiStringFormat::PropertyString(&AcmeDomain::API_SCHEMA),
+        },
+    },
+)]
+#[derive(Deserialize, Serialize, Updater)]
+/// Node specific configuration.
+pub struct NodeConfig {
+    /// The acme account to use on this node.
+    #[serde(skip_serializing_if = "Updater::is_empty")]
+    acme: Option<String>,
+
+    /// ACME domain to get a certificate for for this node.
+    #[serde(skip_serializing_if = "Updater::is_empty")]
+    acmedomain0: Option<String>,
+
+    /// ACME domain to get a certificate for for this node.
+    #[serde(skip_serializing_if = "Updater::is_empty")]
+    acmedomain1: Option<String>,
+
+    /// ACME domain to get a certificate for for this node.
+    #[serde(skip_serializing_if = "Updater::is_empty")]
+    acmedomain2: Option<String>,
+
+    /// ACME domain to get a certificate for for this node.
+    #[serde(skip_serializing_if = "Updater::is_empty")]
+    acmedomain3: Option<String>,
+
+    /// ACME domain to get a certificate for for this node.
+    #[serde(skip_serializing_if = "Updater::is_empty")]
+    acmedomain4: Option<String>,
+}
+
+impl NodeConfig {
+    pub fn acme_config(&self) -> Option<Result<AcmeConfig, Error>> {
+        self.acme.as_deref().map(|config| -> Result<_, Error> {
+            Ok(crate::tools::config::from_property_string(
+                config,
+                &AcmeConfig::API_SCHEMA,
+            )?)
+        })
+    }
+
+    pub async fn acme_client(&self) -> Result<AcmeClient, Error> {
+        AcmeClient::load(
+            &self
+                .acme_config()
+                .ok_or_else(|| format_err!("no acme client configured"))??
+                .account,
+        )
+        .await
+    }
+
+    pub fn acme_domains(&self) -> AcmeDomainIter {
+        AcmeDomainIter::new(self)
+    }
+}
+
+pub struct AcmeDomainIter<'a> {
+    config: &'a NodeConfig,
+    index: usize,
+}
+
+impl<'a> AcmeDomainIter<'a> {
+    fn new(config: &'a NodeConfig) -> Self {
+        Self { config, index: 0 }
+    }
+}
+
+impl<'a> Iterator for AcmeDomainIter<'a> {
+    type Item = Result<AcmeDomain, Error>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let domain = loop {
+            let index = self.index;
+            self.index += 1;
+
+            let domain = match index {
+                0 => self.config.acmedomain0.as_deref(),
+                1 => self.config.acmedomain1.as_deref(),
+                2 => self.config.acmedomain2.as_deref(),
+                3 => self.config.acmedomain3.as_deref(),
+                4 => self.config.acmedomain4.as_deref(),
+                _ => return None,
+            };
+
+            if let Some(domain) = domain {
+                break domain;
+            }
+        };
+
+        Some(crate::tools::config::from_property_string(
+            domain,
+            &AcmeDomain::API_SCHEMA,
+        ))
+    }
+}
-- 
2.20.1

Re: [pbs-devel] [PATCH v2 backup 13/27] add node config

[Dietmar Maurer]

Using different naming styles for lock files is really bad. Instead, we 
should use
the same style consistently. I suggest:

const CONF_FILE: &str = configdir!("/node.cfg");
const LOCK_FILE: &str = configdir!("/.node.lck");

Any objections?

You already use this style for ACME plugins:

pub const ACME_PLUGIN_CFG_FILENAME: &str = 
"/etc/proxmox-backup/acme/plugins.cfg";
pub const ACME_PLUGIN_CFG_LOCKFILE: &str = 
"/etc/proxmox-backup/acme/.plugins.lck";


> +
> +const CONF_FILE: &str = configdir!("/node.cfg");
> +const LOCK_FILE: &str = configdir!("/.node.cfg.lock");
> +const LOCK_TIMEOUT: Duration = Duration::from_secs(5);

Re: [pbs-devel] [PATCH v2 backup 13/27] add node config

[Dietmar Maurer]

We use replace_file() to atomically replace the config file, so
why do we need a read_lock()?

A) We don't need it => remove it for your patch series

B) We need it for some reason => We need to add that for other config 
files too?


On 4/22/21 4:01 PM, Wolfgang Bumiller wrote:
> +
> +const CONF_FILE: &str = configdir!("/node.cfg");
> +const LOCK_FILE: &str = configdir!("/.node.cfg.lock");
> +const LOCK_TIMEOUT: Duration = Duration::from_secs(5);
> +
> +pub fn read_lock() -> Result<File, Error> {
> +    proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, false)
> +}
> +
> +pub fn write_lock() -> Result<File, Error> {
> +    proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)
> +}
> +
> +/// Read the Node Config.
> +pub fn config() -> Result<(NodeConfig, [u8; 32]), Error> {
> +    let content =
> +        proxmox::tools::fs::file_read_optional_string(CONF_FILE)?.unwrap_or_else(|| "".to_string());
> +
> +    let digest = openssl::sha::sha256(content.as_bytes());
> +    let data: NodeConfig = crate::tools::config::from_str(&content, &NodeConfig::API_SCHEMA)?;
> +
> +    Ok((data, digest))
> +}
> +
> +/// Write the Node Config, requires the write lock to be held.
> +pub fn save_config(config: &NodeConfig) -> Result<(), Error> {
> +    let raw = crate::tools::config::to_bytes(config, &NodeConfig::API_SCHEMA)?;
> +
> +    let backup_user = crate::backup::backup_user()?;
> +    let options = CreateOptions::new()
> +        .perm(Mode::from_bits_truncate(0o0640))
> +        .owner(nix::unistd::ROOT)
> +        .group(backup_user.gid);
> +
> +    replace_file(CONF_FILE, &raw, options)
> +}
> +

Re: [pbs-devel] [PATCH v2 backup 13/27] add node config

[Wolfgang Bumiller]

On Thu, Apr 29, 2021 at 02:40:39PM +0200, Dietmar Maurer wrote:
> We use replace_file() to atomically replace the config file, so
> why do we need a read_lock()?
> 
> A) We don't need it => remove it for your patch series
> 
> B) We need it for some reason => We need to add that for other config files
> too?

Sorry, saw this too late. Yeah, the read lock helper can be dropped.

[pbs-devel] [PATCH v2 backup 14/27] add acme config

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/config.rs             |   1 +
 src/config/acme/mod.rs    | 198 ++++++++++++++++++++
 src/config/acme/plugin.rs | 380 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 579 insertions(+)
 create mode 100644 src/config/acme/mod.rs
 create mode 100644 src/config/acme/plugin.rs

diff --git a/src/config.rs b/src/config.rs
index 717829e2..94b7fb6c 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -16,6 +16,7 @@ use proxmox::try_block;
 use crate::buildcfg;
 
 pub mod acl;
+pub mod acme;
 pub mod cached_user_info;
 pub mod datastore;
 pub mod network;
diff --git a/src/config/acme/mod.rs b/src/config/acme/mod.rs
new file mode 100644
index 00000000..ac409c20
--- /dev/null
+++ b/src/config/acme/mod.rs
@@ -0,0 +1,198 @@
+use std::collections::HashMap;
+use std::fmt;
+use std::path::Path;
+
+use anyhow::{bail, format_err, Error};
+use serde::{Deserialize, Serialize};
+
+use proxmox::api::api;
+use proxmox::sys::error::SysError;
+
+use crate::api2::types::{PROXMOX_SAFE_ID_FORMAT, PROXMOX_SAFE_ID_REGEX};
+use crate::tools::ControlFlow;
+
+pub(crate) const ACME_ACCOUNT_DIR: &str = configdir!("/acme/accounts");
+
+pub mod plugin;
+
+#[api(
+    properties: {
+        name: { type: String },
+        url: { type: String },
+    },
+)]
+/// An ACME directory endpoint with a name and URL.
+#[derive(Serialize)]
+pub struct KnownAcmeDirectory {
+    /// The ACME directory's name.
+    pub name: &'static str,
+
+    /// The ACME directory's endpoint URL.
+    pub url: &'static str,
+}
+
+pub const KNOWN_ACME_DIRECTORIES: &[KnownAcmeDirectory] = &[
+    KnownAcmeDirectory {
+        name: "Let's Encrypt V2",
+        url: "https://acme-v02.api.letsencrypt.org/directory",
+    },
+    KnownAcmeDirectory {
+        name: "Let's Encrypt V2 Staging",
+        url: "https://acme-staging-v02.api.letsencrypt.org/directory",
+    },
+];
+
+pub const DEFAULT_ACME_DIRECTORY_ENTRY: &KnownAcmeDirectory = &KNOWN_ACME_DIRECTORIES[0];
+
+pub fn account_path(name: &str) -> String {
+    format!("{}/{}", ACME_ACCOUNT_DIR, name)
+}
+
+#[api(format: &PROXMOX_SAFE_ID_FORMAT)]
+/// ACME account name.
+#[derive(Clone, Eq, PartialEq, Hash, Deserialize, Serialize)]
+#[serde(transparent)]
+pub struct AccountName(String);
+
+impl AccountName {
+    pub fn into_string(self) -> String {
+        self.0
+    }
+}
+
+impl std::ops::Deref for AccountName {
+    type Target = str;
+
+    #[inline]
+    fn deref(&self) -> &str {
+        &self.0
+    }
+}
+
+impl std::ops::DerefMut for AccountName {
+    #[inline]
+    fn deref_mut(&mut self) -> &mut str {
+        &mut self.0
+    }
+}
+
+impl AsRef<str> for AccountName {
+    #[inline]
+    fn as_ref(&self) -> &str {
+        self.0.as_ref()
+    }
+}
+
+impl fmt::Debug for AccountName {
+    #[inline]
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(&self.0, f)
+    }
+}
+
+impl fmt::Display for AccountName {
+    #[inline]
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Display::fmt(&self.0, f)
+    }
+}
+
+pub fn foreach_acme_account<F>(mut func: F) -> Result<(), Error>
+where
+    F: FnMut(AccountName) -> ControlFlow<Result<(), Error>>,
+{
+    match crate::tools::fs::scan_subdir(-1, ACME_ACCOUNT_DIR, &PROXMOX_SAFE_ID_REGEX) {
+        Ok(files) => {
+            for file in files {
+                let file = file?;
+                let file_name = unsafe { file.file_name_utf8_unchecked() };
+
+                if file_name.starts_with('_') {
+                    continue;
+                }
+
+                let account_name = AccountName(file_name.to_owned());
+
+                if let ControlFlow::Break(result) = func(account_name) {
+                    return result;
+                }
+            }
+            Ok(())
+        }
+        Err(err) if err.not_found() => Ok(()),
+        Err(err) => Err(err.into()),
+    }
+}
+
+/// Run a function for each DNS plugin ID.
+pub fn foreach_dns_plugin<F>(mut func: F) -> Result<(), Error>
+where
+    F: FnMut(&str) -> ControlFlow<Result<(), Error>>,
+{
+    match crate::tools::fs::read_subdir(-1, "/usr/share/proxmox-acme/dnsapi") {
+        Ok(files) => {
+            for file in files.filter_map(Result::ok) {
+                if let Some(id) = file
+                    .file_name()
+                    .to_str()
+                    .ok()
+                    .and_then(|name| name.strip_prefix("dns_"))
+                    .and_then(|name| name.strip_suffix(".sh"))
+                {
+                    if let ControlFlow::Break(result) = func(id) {
+                        return result;
+                    }
+                }
+            }
+
+            Ok(())
+        }
+        Err(err) if err.not_found() => Ok(()),
+        Err(err) => Err(err.into()),
+    }
+}
+
+pub fn mark_account_deactivated(name: &str) -> Result<(), Error> {
+    let from = account_path(name);
+    for i in 0..100 {
+        let to = account_path(&format!("_deactivated_{}_{}", name, i));
+        if !Path::new(&to).exists() {
+            return std::fs::rename(&from, &to).map_err(|err| {
+                format_err!(
+                    "failed to move account path {:?} to {:?} - {}",
+                    from,
+                    to,
+                    err
+                )
+            });
+        }
+    }
+    bail!(
+        "No free slot to rename deactivated account {:?}, please cleanup {:?}",
+        from,
+        ACME_ACCOUNT_DIR
+    );
+}
+
+pub fn complete_acme_account(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    let mut out = Vec::new();
+    let _ = foreach_acme_account(|name| {
+        out.push(name.into_string());
+        ControlFlow::CONTINUE
+    });
+    out
+}
+
+pub fn complete_acme_plugin(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    match plugin::config() {
+        Ok((config, _digest)) => config
+            .iter()
+            .map(|(id, (_type, _cfg))| id.clone())
+            .collect(),
+        Err(_) => Vec::new(),
+    }
+}
+
+pub fn complete_acme_plugin_type(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    vec!["dns".to_string(), "http".to_string()]
+}
diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
new file mode 100644
index 00000000..b0c655c0
--- /dev/null
+++ b/src/config/acme/plugin.rs
@@ -0,0 +1,380 @@
+use std::future::Future;
+use std::pin::Pin;
+use std::process::Stdio;
+
+use anyhow::{bail, format_err, Error};
+use lazy_static::lazy_static;
+use serde::{Deserialize, Serialize};
+use serde_json::Value;
+use tokio::io::AsyncWriteExt;
+use tokio::process::Command;
+
+use proxmox::api::{
+    api,
+    schema::*,
+    section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin},
+};
+
+use proxmox::tools::{fs::replace_file, fs::CreateOptions};
+
+use proxmox_acme_rs::{Authorization, Challenge};
+
+use crate::acme::AcmeClient;
+use crate::api2::types::PROXMOX_SAFE_ID_FORMAT;
+use crate::config::node::AcmeDomain;
+
+const ACME_PATH: &str = "/usr/share/proxmox-acme/proxmox-acme";
+
+pub const PLUGIN_ID_SCHEMA: Schema = StringSchema::new("ACME Challenge Plugin ID.")
+    .format(&PROXMOX_SAFE_ID_FORMAT)
+    .schema();
+
+lazy_static! {
+    pub static ref CONFIG: SectionConfig = init();
+}
+
+#[api(
+    properties: {
+        id: { schema: PLUGIN_ID_SCHEMA },
+    },
+)]
+#[derive(Deserialize, Serialize)]
+/// Standalone ACME Plugin for the http-1 challenge.
+pub struct StandalonePlugin {
+    /// Plugin ID.
+    id: String,
+}
+
+impl Default for StandalonePlugin {
+    fn default() -> Self {
+        Self {
+            id: "standalone".to_string(),
+        }
+    }
+}
+
+/// In PVE/PMG we store the plugin's "data" member as base64url encoded string. The UI sends
+/// regular base64 encoded data. We need to "fix" this up.
+
+#[api(
+    properties: {
+        id: { schema: PLUGIN_ID_SCHEMA },
+        disable: {
+            optional: true,
+            default: false,
+        },
+        "validation-delay": {
+            default: 30,
+            optional: true,
+            minimum: 0,
+            maximum: 2 * 24 * 60 * 60,
+        },
+    },
+)]
+/// DNS ACME Challenge Plugin core data.
+#[derive(Deserialize, Serialize, Updater)]
+#[serde(rename_all = "kebab-case")]
+pub struct DnsPluginCore {
+    /// Plugin ID.
+    pub(crate) id: String,
+
+    /// DNS API Plugin Id.
+    api: String,
+
+    /// Extra delay in seconds to wait before requesting validation.
+    ///
+    /// Allows to cope with long TTL of DNS records.
+    #[serde(skip_serializing_if = "Option::is_none", default)]
+    validation_delay: Option<u32>,
+
+    /// Flag to disable the config.
+    #[serde(skip_serializing_if = "Option::is_none", default)]
+    disable: Option<bool>,
+}
+
+#[api(
+    properties: {
+        core: { type: DnsPluginCore },
+    },
+)]
+/// DNS ACME Challenge Plugin.
+#[derive(Deserialize, Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct DnsPlugin {
+    #[serde(flatten)]
+    pub(crate) core: DnsPluginCore,
+
+    // FIXME: The `Updater` should allow:
+    //   * having different descriptions for this and the Updater version
+    //   * having different `#[serde]` attributes for the Updater
+    //   * or, well, leaving fields out completely in teh Updater but this means we may need to
+    //     separate Updater and Builder deriving.
+    // We handle this property separately in the API calls.
+    /// DNS plugin data (base64url encoded without padding).
+    #[serde(with = "proxmox::tools::serde::string_as_base64url_nopad")]
+    pub(crate) data: String,
+}
+
+impl DnsPlugin {
+    pub fn decode_data(&self, output: &mut Vec<u8>) -> Result<(), Error> {
+        Ok(base64::decode_config_buf(&self.data, base64::URL_SAFE_NO_PAD, output)?)
+    }
+}
+
+//impl DnsPluginUpdater {
+//    // The UI passes regular base64 data, we need base64url data. In PVE/PMG this happens magically
+//    // since perl parses both on decode...
+//    pub fn api_fixup(&mut self) -> Result<(), Error> {
+//        if let Some(data) = self.data.as_mut() {
+//            let new = base64::encode_config(&base64::decode(&data)?, base64::URL_SAFE_NO_PAD);
+//            *data = new;
+//        }
+//        Ok(())
+//    }
+//}
+
+fn init() -> SectionConfig {
+    let mut config = SectionConfig::new(&PLUGIN_ID_SCHEMA);
+
+    let standalone_schema = match &StandalonePlugin::API_SCHEMA {
+        Schema::Object(schema) => schema,
+        _ => unreachable!(),
+    };
+    let standalone_plugin = SectionConfigPlugin::new(
+        "standalone".to_string(),
+        Some("id".to_string()),
+        standalone_schema,
+    );
+    config.register_plugin(standalone_plugin);
+
+    let dns_challenge_schema = match DnsPlugin::API_SCHEMA {
+        Schema::AllOf(ref schema) => schema,
+        _ => unreachable!(),
+    };
+    let dns_challenge_plugin = SectionConfigPlugin::new(
+        "dns".to_string(),
+        Some("id".to_string()),
+        dns_challenge_schema,
+    );
+    config.register_plugin(dns_challenge_plugin);
+
+    config
+}
+
+pub const ACME_PLUGIN_CFG_FILENAME: &str = "/etc/proxmox-backup/acme/plugins.cfg";
+pub const ACME_PLUGIN_CFG_LOCKFILE: &str = "/etc/proxmox-backup/acme/.plugins.lck";
+const LOCK_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5);
+
+pub fn read_lock() -> Result<std::fs::File, Error> {
+    proxmox::tools::fs::open_file_locked(ACME_PLUGIN_CFG_LOCKFILE, LOCK_TIMEOUT, false)
+}
+
+pub fn write_lock() -> Result<std::fs::File, Error> {
+    proxmox::tools::fs::open_file_locked(ACME_PLUGIN_CFG_LOCKFILE, LOCK_TIMEOUT, true)
+}
+
+pub fn config() -> Result<(PluginData, [u8; 32]), Error> {
+    let content = proxmox::tools::fs::file_read_optional_string(ACME_PLUGIN_CFG_FILENAME)?
+        .unwrap_or_else(|| "".to_string());
+
+    let digest = openssl::sha::sha256(content.as_bytes());
+    let mut data = CONFIG.parse(ACME_PLUGIN_CFG_FILENAME, &content)?;
+
+    if data.sections.get("standalone").is_none() {
+        let standalone = StandalonePlugin::default();
+        data.set_data("standalone", "standalone", &standalone)
+            .unwrap();
+    }
+
+    Ok((PluginData { data }, digest))
+}
+
+pub fn save_config(config: &PluginData) -> Result<(), Error> {
+    let raw = CONFIG.write(ACME_PLUGIN_CFG_FILENAME, &config.data)?;
+
+    let backup_user = crate::backup::backup_user()?;
+    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
+    // set the correct owner/group/permissions while saving file
+    // owner(rw) = root, group(r)= backup
+    let options = CreateOptions::new()
+        .perm(mode)
+        .owner(nix::unistd::ROOT)
+        .group(backup_user.gid);
+
+    replace_file(ACME_PLUGIN_CFG_FILENAME, raw.as_bytes(), options)?;
+
+    Ok(())
+}
+
+pub struct PluginData {
+    data: SectionConfigData,
+}
+
+impl PluginData {
+    #[inline]
+    pub fn remove(&mut self, name: &str) -> Option<(String, Value)> {
+        self.data.sections.remove(name)
+    }
+
+    #[inline]
+    pub fn contains_key(&mut self, name: &str) -> bool {
+        self.data.sections.contains_key(name)
+    }
+
+    #[inline]
+    pub fn get(&self, name: &str) -> Option<&(String, Value)> {
+        self.data.sections.get(name)
+    }
+
+    #[inline]
+    pub fn get_mut(&mut self, name: &str) -> Option<&mut (String, Value)> {
+        self.data.sections.get_mut(name)
+    }
+
+    // FIXME: Verify the plugin type *exists* and check its config schema...
+    pub fn insert(&mut self, id: String, ty: String, plugin: Value) {
+        self.data.sections.insert(id, (ty, plugin));
+    }
+
+    pub fn get_plugin(
+        &self,
+        name: &str,
+    ) -> Result<Option<Box<dyn AcmePlugin + Send + Sync + 'static>>, Error> {
+        let (ty, data) = match self.get(name) {
+            Some(plugin) => plugin,
+            None => return Ok(None),
+        };
+
+        Ok(Some(match ty.as_str() {
+            "dns" => {
+                let plugin: DnsPlugin = serde_json::from_value(data.clone())?;
+                Box::new(plugin)
+            }
+            // "standalone" => todo!("standalone plugin"),
+            other => bail!("missing implementation for plugin type '{}'", other),
+        }))
+    }
+
+    pub fn iter(&self) -> impl Iterator<Item = (&String, &(String, Value))> + Send {
+        self.data.sections.iter()
+    }
+}
+
+pub trait AcmePlugin {
+    /// Setup everything required to trigger the validation and return the corresponding validation
+    /// URL.
+    fn setup<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
+        &'a self,
+        client: &'b mut AcmeClient,
+        authorization: &'c Authorization,
+        domain: &'d AcmeDomain,
+    ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>>;
+
+    fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
+        &'a self,
+        client: &'b mut AcmeClient,
+        authorization: &'c Authorization,
+        domain: &'d AcmeDomain,
+    ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>>;
+}
+
+impl DnsPlugin {
+    fn extract_challenge(authorization: &Authorization) -> Result<&Challenge, Error> {
+        authorization
+            .challenges
+            .iter()
+            .find(|ch| ch.ty == "dns-01")
+            .ok_or_else(|| format_err!("no supported challenge type (dns-01) found"))
+    }
+
+    async fn action<'a>(
+        &self,
+        client: &mut AcmeClient,
+        authorization: &'a Authorization,
+        domain: &AcmeDomain,
+        action: &str,
+    ) -> Result<&'a str, Error> {
+        let challenge = Self::extract_challenge(authorization)?;
+        let mut stdin_data = client
+            .dns_01_txt_value(
+                challenge
+                    .token()
+                    .ok_or_else(|| format_err!("missing token in challenge"))?,
+            )?
+            .into_bytes();
+        stdin_data.push(b'\n');
+        stdin_data.extend(self.data.as_bytes());
+        if stdin_data.last() != Some(&b'\n') {
+            stdin_data.push(b'\n');
+        }
+
+        let mut command = Command::new("/usr/bin/setpriv");
+
+        #[rustfmt::skip]
+        command.args(&[
+            "--reuid", "nobody",
+            "--regid", "nogroup",
+            "--clear-groups",
+            "--reset-env",
+            "--",
+            "/bin/bash",
+                ACME_PATH,
+                action,
+                &self.core.api,
+                domain.alias.as_deref().unwrap_or(&domain.domain),
+        ]);
+
+        let mut child = command.stdin(Stdio::piped()).spawn()?;
+
+        let mut stdin = child.stdin.take().expect("Stdio::piped()");
+        match async move {
+            stdin.write_all(&stdin_data).await?;
+            stdin.flush().await?;
+            Ok::<_, std::io::Error>(())
+        }.await {
+            Ok(()) => (),
+            Err(err) => {
+                if let Err(err) = child.kill().await {
+                    eprintln!("failed to kill '{} {}' command: {}", ACME_PATH, action, err);
+                }
+                bail!("'{}' failed: {}", ACME_PATH, err);
+            }
+        }
+
+        let status = child.wait().await?;
+        if !status.success() {
+            bail!(
+                "'{} {}' exited with error ({})",
+                ACME_PATH,
+                action,
+                status.code().unwrap_or(-1)
+            );
+        }
+
+        Ok(&challenge.url)
+    }
+}
+
+impl AcmePlugin for DnsPlugin {
+    fn setup<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
+        &'a self,
+        client: &'b mut AcmeClient,
+        authorization: &'c Authorization,
+        domain: &'d AcmeDomain,
+    ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>> {
+        Box::pin(self.action(client, authorization, domain, "setup"))
+    }
+
+    fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
+        &'a self,
+        client: &'b mut AcmeClient,
+        authorization: &'c Authorization,
+        domain: &'d AcmeDomain,
+    ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>> {
+        Box::pin(async move {
+            self.action(client, authorization, domain, "teardown")
+                .await
+                .map(drop)
+        })
+    }
+}
-- 
2.20.1

Re: [pbs-devel] [PATCH v2 backup 14/27] add acme config

[Dietmar Maurer]

What is the purpose of this AccountName wrapper type?

I would prefer to simply use String...

On 4/22/21 4:02 PM, Wolfgang Bumiller wrote:
> +#[api(format: &PROXMOX_SAFE_ID_FORMAT)]
> +/// ACME account name.
> +#[derive(Clone, Eq, PartialEq, Hash, Deserialize, Serialize)]
> +#[serde(transparent)]
> +pub struct AccountName(String);
> +
> +impl AccountName {
> +    pub fn into_string(self) -> String {
> +        self.0
> +    }
> +}
> +
> +impl std::ops::Deref for AccountName {
> +    type Target = str;
> +
> +    #[inline]
> +    fn deref(&self) -> &str {
> +        &self.0
> +    }
> +}
> +
> +impl std::ops::DerefMut for AccountName {
> +    #[inline]
> +    fn deref_mut(&mut self) -> &mut str {
> +        &mut self.0
> +    }
> +}
> +
> +impl AsRef<str> for AccountName {
> +    #[inline]
> +    fn as_ref(&self) -> &str {
> +        self.0.as_ref()
> +    }
> +}
> +
> +impl fmt::Debug for AccountName {
> +    #[inline]
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        fmt::Debug::fmt(&self.0, f)
> +    }
> +}
> +
> +impl fmt::Display for AccountName {
> +    #[inline]
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        fmt::Display::fmt(&self.0, f)
> +    }
> +}
> +

Re: [pbs-devel] [PATCH v2 backup 14/27] add acme config

[Wolfgang Bumiller]

On Thu, Apr 29, 2021 at 12:48:52PM +0200, Dietmar Maurer wrote:
> What is the purpose of this AccountName wrapper type?
> 
> I would prefer to simply use String...

If you want to... On the other hand I was thinking about adding a macro
for the boilerplate string stuff.

I mean, I write it once, then use `{ type: AccountName }` in the api
code and it's easy to see what it means and more difficult to get
wrong...

Re: [pbs-devel] [PATCH v2 backup 14/27] add acme config

[Dietmar Maurer]

I get a compile error:

make
cargo build --release
    Compiling proxmox-backup v1.1.5 (/home/dietmar/rust/proxmox-backup)
error[E0432]: unresolved import `crate::acme`
   --> src/config/acme/plugin.rs:22:12
    |
22 | use crate::acme::AcmeClient;
    |            ^^^^
    |            |
    |            unresolved import
    |            help: a similar path exists: `crate::config::acme`

error[E0432]: unresolved import `crate::acme`
   --> src/config/node.rs:12:12
    |
12 | use crate::acme::AcmeClient;
    |            ^^^^
    |            |
    |            unresolved import
    |            help: a similar path exists: `crate::config::acme`

error: aborting due to 2 previous errors


On 4/22/21 4:02 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/config.rs             |   1 +
>   src/config/acme/mod.rs    | 198 ++++++++++++++++++++
>   src/config/acme/plugin.rs | 380 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 579 insertions(+)
>   create mode 100644 src/config/acme/mod.rs
>   create mode 100644 src/config/acme/plugin.rs
>
> diff --git a/src/config.rs b/src/config.rs
> index 717829e2..94b7fb6c 100644
> --- a/src/config.rs
> +++ b/src/config.rs
> @@ -16,6 +16,7 @@ use proxmox::try_block;
>   use crate::buildcfg;
>   
>   pub mod acl;
> +pub mod acme;
>   pub mod cached_user_info;
>   pub mod datastore;
>   pub mod network;
> diff --git a/src/config/acme/mod.rs b/src/config/acme/mod.rs
> new file mode 100644
> index 00000000..ac409c20
> --- /dev/null
> +++ b/src/config/acme/mod.rs
> @@ -0,0 +1,198 @@
> +use std::collections::HashMap;
> +use std::fmt;
> +use std::path::Path;
> +
> +use anyhow::{bail, format_err, Error};
> +use serde::{Deserialize, Serialize};
> +
> +use proxmox::api::api;
> +use proxmox::sys::error::SysError;
> +
> +use crate::api2::types::{PROXMOX_SAFE_ID_FORMAT, PROXMOX_SAFE_ID_REGEX};
> +use crate::tools::ControlFlow;
> +
> +pub(crate) const ACME_ACCOUNT_DIR: &str = configdir!("/acme/accounts");
> +
> +pub mod plugin;
> +
> +#[api(
> +    properties: {
> +        name: { type: String },
> +        url: { type: String },
> +    },
> +)]
> +/// An ACME directory endpoint with a name and URL.
> +#[derive(Serialize)]
> +pub struct KnownAcmeDirectory {
> +    /// The ACME directory's name.
> +    pub name: &'static str,
> +
> +    /// The ACME directory's endpoint URL.
> +    pub url: &'static str,
> +}
> +
> +pub const KNOWN_ACME_DIRECTORIES: &[KnownAcmeDirectory] = &[
> +    KnownAcmeDirectory {
> +        name: "Let's Encrypt V2",
> +        url: "https://acme-v02.api.letsencrypt.org/directory",
> +    },
> +    KnownAcmeDirectory {
> +        name: "Let's Encrypt V2 Staging",
> +        url: "https://acme-staging-v02.api.letsencrypt.org/directory",
> +    },
> +];
> +
> +pub const DEFAULT_ACME_DIRECTORY_ENTRY: &KnownAcmeDirectory = &KNOWN_ACME_DIRECTORIES[0];
> +
> +pub fn account_path(name: &str) -> String {
> +    format!("{}/{}", ACME_ACCOUNT_DIR, name)
> +}
> +
> +#[api(format: &PROXMOX_SAFE_ID_FORMAT)]
> +/// ACME account name.
> +#[derive(Clone, Eq, PartialEq, Hash, Deserialize, Serialize)]
> +#[serde(transparent)]
> +pub struct AccountName(String);
> +
> +impl AccountName {
> +    pub fn into_string(self) -> String {
> +        self.0
> +    }
> +}
> +
> +impl std::ops::Deref for AccountName {
> +    type Target = str;
> +
> +    #[inline]
> +    fn deref(&self) -> &str {
> +        &self.0
> +    }
> +}
> +
> +impl std::ops::DerefMut for AccountName {
> +    #[inline]
> +    fn deref_mut(&mut self) -> &mut str {
> +        &mut self.0
> +    }
> +}
> +
> +impl AsRef<str> for AccountName {
> +    #[inline]
> +    fn as_ref(&self) -> &str {
> +        self.0.as_ref()
> +    }
> +}
> +
> +impl fmt::Debug for AccountName {
> +    #[inline]
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        fmt::Debug::fmt(&self.0, f)
> +    }
> +}
> +
> +impl fmt::Display for AccountName {
> +    #[inline]
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        fmt::Display::fmt(&self.0, f)
> +    }
> +}
> +
> +pub fn foreach_acme_account<F>(mut func: F) -> Result<(), Error>
> +where
> +    F: FnMut(AccountName) -> ControlFlow<Result<(), Error>>,
> +{
> +    match crate::tools::fs::scan_subdir(-1, ACME_ACCOUNT_DIR, &PROXMOX_SAFE_ID_REGEX) {
> +        Ok(files) => {
> +            for file in files {
> +                let file = file?;
> +                let file_name = unsafe { file.file_name_utf8_unchecked() };
> +
> +                if file_name.starts_with('_') {
> +                    continue;
> +                }
> +
> +                let account_name = AccountName(file_name.to_owned());
> +
> +                if let ControlFlow::Break(result) = func(account_name) {
> +                    return result;
> +                }
> +            }
> +            Ok(())
> +        }
> +        Err(err) if err.not_found() => Ok(()),
> +        Err(err) => Err(err.into()),
> +    }
> +}
> +
> +/// Run a function for each DNS plugin ID.
> +pub fn foreach_dns_plugin<F>(mut func: F) -> Result<(), Error>
> +where
> +    F: FnMut(&str) -> ControlFlow<Result<(), Error>>,
> +{
> +    match crate::tools::fs::read_subdir(-1, "/usr/share/proxmox-acme/dnsapi") {
> +        Ok(files) => {
> +            for file in files.filter_map(Result::ok) {
> +                if let Some(id) = file
> +                    .file_name()
> +                    .to_str()
> +                    .ok()
> +                    .and_then(|name| name.strip_prefix("dns_"))
> +                    .and_then(|name| name.strip_suffix(".sh"))
> +                {
> +                    if let ControlFlow::Break(result) = func(id) {
> +                        return result;
> +                    }
> +                }
> +            }
> +
> +            Ok(())
> +        }
> +        Err(err) if err.not_found() => Ok(()),
> +        Err(err) => Err(err.into()),
> +    }
> +}
> +
> +pub fn mark_account_deactivated(name: &str) -> Result<(), Error> {
> +    let from = account_path(name);
> +    for i in 0..100 {
> +        let to = account_path(&format!("_deactivated_{}_{}", name, i));
> +        if !Path::new(&to).exists() {
> +            return std::fs::rename(&from, &to).map_err(|err| {
> +                format_err!(
> +                    "failed to move account path {:?} to {:?} - {}",
> +                    from,
> +                    to,
> +                    err
> +                )
> +            });
> +        }
> +    }
> +    bail!(
> +        "No free slot to rename deactivated account {:?}, please cleanup {:?}",
> +        from,
> +        ACME_ACCOUNT_DIR
> +    );
> +}
> +
> +pub fn complete_acme_account(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
> +    let mut out = Vec::new();
> +    let _ = foreach_acme_account(|name| {
> +        out.push(name.into_string());
> +        ControlFlow::CONTINUE
> +    });
> +    out
> +}
> +
> +pub fn complete_acme_plugin(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
> +    match plugin::config() {
> +        Ok((config, _digest)) => config
> +            .iter()
> +            .map(|(id, (_type, _cfg))| id.clone())
> +            .collect(),
> +        Err(_) => Vec::new(),
> +    }
> +}
> +
> +pub fn complete_acme_plugin_type(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
> +    vec!["dns".to_string(), "http".to_string()]
> +}
> diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
> new file mode 100644
> index 00000000..b0c655c0
> --- /dev/null
> +++ b/src/config/acme/plugin.rs
> @@ -0,0 +1,380 @@
> +use std::future::Future;
> +use std::pin::Pin;
> +use std::process::Stdio;
> +
> +use anyhow::{bail, format_err, Error};
> +use lazy_static::lazy_static;
> +use serde::{Deserialize, Serialize};
> +use serde_json::Value;
> +use tokio::io::AsyncWriteExt;
> +use tokio::process::Command;
> +
> +use proxmox::api::{
> +    api,
> +    schema::*,
> +    section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin},
> +};
> +
> +use proxmox::tools::{fs::replace_file, fs::CreateOptions};
> +
> +use proxmox_acme_rs::{Authorization, Challenge};
> +
> +use crate::acme::AcmeClient;
> +use crate::api2::types::PROXMOX_SAFE_ID_FORMAT;
> +use crate::config::node::AcmeDomain;
> +
> +const ACME_PATH: &str = "/usr/share/proxmox-acme/proxmox-acme";
> +
> +pub const PLUGIN_ID_SCHEMA: Schema = StringSchema::new("ACME Challenge Plugin ID.")
> +    .format(&PROXMOX_SAFE_ID_FORMAT)
> +    .schema();
> +
> +lazy_static! {
> +    pub static ref CONFIG: SectionConfig = init();
> +}
> +
> +#[api(
> +    properties: {
> +        id: { schema: PLUGIN_ID_SCHEMA },
> +    },
> +)]
> +#[derive(Deserialize, Serialize)]
> +/// Standalone ACME Plugin for the http-1 challenge.
> +pub struct StandalonePlugin {
> +    /// Plugin ID.
> +    id: String,
> +}
> +
> +impl Default for StandalonePlugin {
> +    fn default() -> Self {
> +        Self {
> +            id: "standalone".to_string(),
> +        }
> +    }
> +}
> +
> +/// In PVE/PMG we store the plugin's "data" member as base64url encoded string. The UI sends
> +/// regular base64 encoded data. We need to "fix" this up.
> +
> +#[api(
> +    properties: {
> +        id: { schema: PLUGIN_ID_SCHEMA },
> +        disable: {
> +            optional: true,
> +            default: false,
> +        },
> +        "validation-delay": {
> +            default: 30,
> +            optional: true,
> +            minimum: 0,
> +            maximum: 2 * 24 * 60 * 60,
> +        },
> +    },
> +)]
> +/// DNS ACME Challenge Plugin core data.
> +#[derive(Deserialize, Serialize, Updater)]
> +#[serde(rename_all = "kebab-case")]
> +pub struct DnsPluginCore {
> +    /// Plugin ID.
> +    pub(crate) id: String,
> +
> +    /// DNS API Plugin Id.
> +    api: String,
> +
> +    /// Extra delay in seconds to wait before requesting validation.
> +    ///
> +    /// Allows to cope with long TTL of DNS records.
> +    #[serde(skip_serializing_if = "Option::is_none", default)]
> +    validation_delay: Option<u32>,
> +
> +    /// Flag to disable the config.
> +    #[serde(skip_serializing_if = "Option::is_none", default)]
> +    disable: Option<bool>,
> +}
> +
> +#[api(
> +    properties: {
> +        core: { type: DnsPluginCore },
> +    },
> +)]
> +/// DNS ACME Challenge Plugin.
> +#[derive(Deserialize, Serialize)]
> +#[serde(rename_all = "kebab-case")]
> +pub struct DnsPlugin {
> +    #[serde(flatten)]
> +    pub(crate) core: DnsPluginCore,
> +
> +    // FIXME: The `Updater` should allow:
> +    //   * having different descriptions for this and the Updater version
> +    //   * having different `#[serde]` attributes for the Updater
> +    //   * or, well, leaving fields out completely in teh Updater but this means we may need to
> +    //     separate Updater and Builder deriving.
> +    // We handle this property separately in the API calls.
> +    /// DNS plugin data (base64url encoded without padding).
> +    #[serde(with = "proxmox::tools::serde::string_as_base64url_nopad")]
> +    pub(crate) data: String,
> +}
> +
> +impl DnsPlugin {
> +    pub fn decode_data(&self, output: &mut Vec<u8>) -> Result<(), Error> {
> +        Ok(base64::decode_config_buf(&self.data, base64::URL_SAFE_NO_PAD, output)?)
> +    }
> +}
> +
> +//impl DnsPluginUpdater {
> +//    // The UI passes regular base64 data, we need base64url data. In PVE/PMG this happens magically
> +//    // since perl parses both on decode...
> +//    pub fn api_fixup(&mut self) -> Result<(), Error> {
> +//        if let Some(data) = self.data.as_mut() {
> +//            let new = base64::encode_config(&base64::decode(&data)?, base64::URL_SAFE_NO_PAD);
> +//            *data = new;
> +//        }
> +//        Ok(())
> +//    }
> +//}
> +
> +fn init() -> SectionConfig {
> +    let mut config = SectionConfig::new(&PLUGIN_ID_SCHEMA);
> +
> +    let standalone_schema = match &StandalonePlugin::API_SCHEMA {
> +        Schema::Object(schema) => schema,
> +        _ => unreachable!(),
> +    };
> +    let standalone_plugin = SectionConfigPlugin::new(
> +        "standalone".to_string(),
> +        Some("id".to_string()),
> +        standalone_schema,
> +    );
> +    config.register_plugin(standalone_plugin);
> +
> +    let dns_challenge_schema = match DnsPlugin::API_SCHEMA {
> +        Schema::AllOf(ref schema) => schema,
> +        _ => unreachable!(),
> +    };
> +    let dns_challenge_plugin = SectionConfigPlugin::new(
> +        "dns".to_string(),
> +        Some("id".to_string()),
> +        dns_challenge_schema,
> +    );
> +    config.register_plugin(dns_challenge_plugin);
> +
> +    config
> +}
> +
> +pub const ACME_PLUGIN_CFG_FILENAME: &str = "/etc/proxmox-backup/acme/plugins.cfg";
> +pub const ACME_PLUGIN_CFG_LOCKFILE: &str = "/etc/proxmox-backup/acme/.plugins.lck";
> +const LOCK_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5);
> +
> +pub fn read_lock() -> Result<std::fs::File, Error> {
> +    proxmox::tools::fs::open_file_locked(ACME_PLUGIN_CFG_LOCKFILE, LOCK_TIMEOUT, false)
> +}
> +
> +pub fn write_lock() -> Result<std::fs::File, Error> {
> +    proxmox::tools::fs::open_file_locked(ACME_PLUGIN_CFG_LOCKFILE, LOCK_TIMEOUT, true)
> +}
> +
> +pub fn config() -> Result<(PluginData, [u8; 32]), Error> {
> +    let content = proxmox::tools::fs::file_read_optional_string(ACME_PLUGIN_CFG_FILENAME)?
> +        .unwrap_or_else(|| "".to_string());
> +
> +    let digest = openssl::sha::sha256(content.as_bytes());
> +    let mut data = CONFIG.parse(ACME_PLUGIN_CFG_FILENAME, &content)?;
> +
> +    if data.sections.get("standalone").is_none() {
> +        let standalone = StandalonePlugin::default();
> +        data.set_data("standalone", "standalone", &standalone)
> +            .unwrap();
> +    }
> +
> +    Ok((PluginData { data }, digest))
> +}
> +
> +pub fn save_config(config: &PluginData) -> Result<(), Error> {
> +    let raw = CONFIG.write(ACME_PLUGIN_CFG_FILENAME, &config.data)?;
> +
> +    let backup_user = crate::backup::backup_user()?;
> +    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0640);
> +    // set the correct owner/group/permissions while saving file
> +    // owner(rw) = root, group(r)= backup
> +    let options = CreateOptions::new()
> +        .perm(mode)
> +        .owner(nix::unistd::ROOT)
> +        .group(backup_user.gid);
> +
> +    replace_file(ACME_PLUGIN_CFG_FILENAME, raw.as_bytes(), options)?;
> +
> +    Ok(())
> +}
> +
> +pub struct PluginData {
> +    data: SectionConfigData,
> +}
> +
> +impl PluginData {
> +    #[inline]
> +    pub fn remove(&mut self, name: &str) -> Option<(String, Value)> {
> +        self.data.sections.remove(name)
> +    }
> +
> +    #[inline]
> +    pub fn contains_key(&mut self, name: &str) -> bool {
> +        self.data.sections.contains_key(name)
> +    }
> +
> +    #[inline]
> +    pub fn get(&self, name: &str) -> Option<&(String, Value)> {
> +        self.data.sections.get(name)
> +    }
> +
> +    #[inline]
> +    pub fn get_mut(&mut self, name: &str) -> Option<&mut (String, Value)> {
> +        self.data.sections.get_mut(name)
> +    }
> +
> +    // FIXME: Verify the plugin type *exists* and check its config schema...
> +    pub fn insert(&mut self, id: String, ty: String, plugin: Value) {
> +        self.data.sections.insert(id, (ty, plugin));
> +    }
> +
> +    pub fn get_plugin(
> +        &self,
> +        name: &str,
> +    ) -> Result<Option<Box<dyn AcmePlugin + Send + Sync + 'static>>, Error> {
> +        let (ty, data) = match self.get(name) {
> +            Some(plugin) => plugin,
> +            None => return Ok(None),
> +        };
> +
> +        Ok(Some(match ty.as_str() {
> +            "dns" => {
> +                let plugin: DnsPlugin = serde_json::from_value(data.clone())?;
> +                Box::new(plugin)
> +            }
> +            // "standalone" => todo!("standalone plugin"),
> +            other => bail!("missing implementation for plugin type '{}'", other),
> +        }))
> +    }
> +
> +    pub fn iter(&self) -> impl Iterator<Item = (&String, &(String, Value))> + Send {
> +        self.data.sections.iter()
> +    }
> +}
> +
> +pub trait AcmePlugin {
> +    /// Setup everything required to trigger the validation and return the corresponding validation
> +    /// URL.
> +    fn setup<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
> +        &'a self,
> +        client: &'b mut AcmeClient,
> +        authorization: &'c Authorization,
> +        domain: &'d AcmeDomain,
> +    ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>>;
> +
> +    fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
> +        &'a self,
> +        client: &'b mut AcmeClient,
> +        authorization: &'c Authorization,
> +        domain: &'d AcmeDomain,
> +    ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>>;
> +}
> +
> +impl DnsPlugin {
> +    fn extract_challenge(authorization: &Authorization) -> Result<&Challenge, Error> {
> +        authorization
> +            .challenges
> +            .iter()
> +            .find(|ch| ch.ty == "dns-01")
> +            .ok_or_else(|| format_err!("no supported challenge type (dns-01) found"))
> +    }
> +
> +    async fn action<'a>(
> +        &self,
> +        client: &mut AcmeClient,
> +        authorization: &'a Authorization,
> +        domain: &AcmeDomain,
> +        action: &str,
> +    ) -> Result<&'a str, Error> {
> +        let challenge = Self::extract_challenge(authorization)?;
> +        let mut stdin_data = client
> +            .dns_01_txt_value(
> +                challenge
> +                    .token()
> +                    .ok_or_else(|| format_err!("missing token in challenge"))?,
> +            )?
> +            .into_bytes();
> +        stdin_data.push(b'\n');
> +        stdin_data.extend(self.data.as_bytes());
> +        if stdin_data.last() != Some(&b'\n') {
> +            stdin_data.push(b'\n');
> +        }
> +
> +        let mut command = Command::new("/usr/bin/setpriv");
> +
> +        #[rustfmt::skip]
> +        command.args(&[
> +            "--reuid", "nobody",
> +            "--regid", "nogroup",
> +            "--clear-groups",
> +            "--reset-env",
> +            "--",
> +            "/bin/bash",
> +                ACME_PATH,
> +                action,
> +                &self.core.api,
> +                domain.alias.as_deref().unwrap_or(&domain.domain),
> +        ]);
> +
> +        let mut child = command.stdin(Stdio::piped()).spawn()?;
> +
> +        let mut stdin = child.stdin.take().expect("Stdio::piped()");
> +        match async move {
> +            stdin.write_all(&stdin_data).await?;
> +            stdin.flush().await?;
> +            Ok::<_, std::io::Error>(())
> +        }.await {
> +            Ok(()) => (),
> +            Err(err) => {
> +                if let Err(err) = child.kill().await {
> +                    eprintln!("failed to kill '{} {}' command: {}", ACME_PATH, action, err);
> +                }
> +                bail!("'{}' failed: {}", ACME_PATH, err);
> +            }
> +        }
> +
> +        let status = child.wait().await?;
> +        if !status.success() {
> +            bail!(
> +                "'{} {}' exited with error ({})",
> +                ACME_PATH,
> +                action,
> +                status.code().unwrap_or(-1)
> +            );
> +        }
> +
> +        Ok(&challenge.url)
> +    }
> +}
> +
> +impl AcmePlugin for DnsPlugin {
> +    fn setup<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
> +        &'a self,
> +        client: &'b mut AcmeClient,
> +        authorization: &'c Authorization,
> +        domain: &'d AcmeDomain,
> +    ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>> {
> +        Box::pin(self.action(client, authorization, domain, "setup"))
> +    }
> +
> +    fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
> +        &'a self,
> +        client: &'b mut AcmeClient,
> +        authorization: &'c Authorization,
> +        domain: &'d AcmeDomain,
> +    ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>> {
> +        Box::pin(async move {
> +            self.action(client, authorization, domain, "teardown")
> +                .await
> +                .map(drop)
> +        })
> +    }
> +}

Re: [pbs-devel] [PATCH v2 backup 14/27] add acme config

[Wolfgang Bumiller]

On Thu, Apr 29, 2021 at 12:53:01PM +0200, Dietmar Maurer wrote:
> I get a compile error:
> 
> make
> cargo build --release
>    Compiling proxmox-backup v1.1.5 (/home/dietmar/rust/proxmox-backup)
> error[E0432]: unresolved import `crate::acme`
>   --> src/config/acme/plugin.rs:22:12
>    |
> 22 | use crate::acme::AcmeClient;
>    |            ^^^^
>    |            |
>    |            unresolved import
>    |            help: a similar path exists: `crate::config::acme`
> 
> error[E0432]: unresolved import `crate::acme`
>   --> src/config/node.rs:12:12
>    |
> 12 | use crate::acme::AcmeClient;
>    |            ^^^^
>    |            |
>    |            unresolved import
>    |            help: a similar path exists: `crate::config::acme`
> 
> error: aborting due to 2 previous errors

looks like I mis-ordered the patches, this will be in 17/27

[pbs-devel] [PATCH v2 backup 15/27] tools/http: dedup user agent string

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/tools/http.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/tools/http.rs b/src/tools/http.rs
index 8ed2fdb7..31dea33a 100644
--- a/src/tools/http.rs
+++ b/src/tools/http.rs
@@ -28,6 +28,8 @@ use crate::tools::{
     },
 };
 
+const USER_AGENT_STRING: &str = "proxmox-backup-client/1.0";
+
 /// HTTP Proxy Configuration
 #[derive(Clone)]
 pub struct ProxyConfig {
@@ -81,7 +83,7 @@ impl SimpleHttp {
         let request = Request::builder()
             .method("POST")
             .uri(uri)
-            .header("User-Agent", "proxmox-backup-client/1.0")
+            .header("User-Agent", USER_AGENT_STRING)
             .header(hyper::header::CONTENT_TYPE, content_type)
             .body(body)?;
 
@@ -99,7 +101,7 @@ impl SimpleHttp {
         let mut request = Request::builder()
             .method("GET")
             .uri(uri)
-            .header("User-Agent", "proxmox-backup-client/1.0");
+            .header("User-Agent", USER_AGENT_STRING);
 
         if let Some(hs) = extra_headers {
             for (h, v) in hs.iter() {
-- 
2.20.1

Re: [pbs-devel] [PATCH v2 backup 15/27] tools/http: dedup user agent string

[Dietmar Maurer]

replaced by my own cleanup patch

On 4/22/21 4:02 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/tools/http.rs | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/tools/http.rs b/src/tools/http.rs
> index 8ed2fdb7..31dea33a 100644
> --- a/src/tools/http.rs
> +++ b/src/tools/http.rs
> @@ -28,6 +28,8 @@ use crate::tools::{
>       },
>   };
>   
> +const USER_AGENT_STRING: &str = "proxmox-backup-client/1.0";
> +
>   /// HTTP Proxy Configuration
>   #[derive(Clone)]
>   pub struct ProxyConfig {
> @@ -81,7 +83,7 @@ impl SimpleHttp {
>           let request = Request::builder()
>               .method("POST")
>               .uri(uri)
> -            .header("User-Agent", "proxmox-backup-client/1.0")
> +            .header("User-Agent", USER_AGENT_STRING)
>               .header(hyper::header::CONTENT_TYPE, content_type)
>               .body(body)?;
>   
> @@ -99,7 +101,7 @@ impl SimpleHttp {
>           let mut request = Request::builder()
>               .method("GET")
>               .uri(uri)
> -            .header("User-Agent", "proxmox-backup-client/1.0");
> +            .header("User-Agent", USER_AGENT_STRING);
>   
>           if let Some(hs) = extra_headers {
>               for (h, v) in hs.iter() {

[pbs-devel] [PATCH v2 backup 16/27] tools/http: add request_with_agent helper

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/tools/http.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/tools/http.rs b/src/tools/http.rs
index 31dea33a..55c8e41e 100644
--- a/src/tools/http.rs
+++ b/src/tools/http.rs
@@ -60,6 +60,15 @@ impl SimpleHttp {
         Self { client }
     }
 
+    /// Helper to finish a request with our user agent string and perform the request:
+    pub async fn request_with_agent(
+        &mut self,
+        request: http::request::Builder,
+        body: Body,
+    ) -> Result<Response<Body>, Error> {
+        self.request(request.header("User-Agent", USER_AGENT_STRING).body(body)?).await
+    }
+
     pub async fn request(&self, request: Request<Body>) -> Result<Response<Body>, Error> {
         self.client.request(request)
             .map_err(Error::from)
-- 
2.20.1

Re: [pbs-devel] [PATCH v2 backup 16/27] tools/http: add request_with_agent helper

[Dietmar Maurer]

replaced by my own cleanup patch

On 4/22/21 4:02 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>   src/tools/http.rs | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/src/tools/http.rs b/src/tools/http.rs
> index 31dea33a..55c8e41e 100644
> --- a/src/tools/http.rs
> +++ b/src/tools/http.rs
> @@ -60,6 +60,15 @@ impl SimpleHttp {
>           Self { client }
>       }
>   
> +    /// Helper to finish a request with our user agent string and perform the request:
> +    pub async fn request_with_agent(
> +        &mut self,
> +        request: http::request::Builder,
> +        body: Body,
> +    ) -> Result<Response<Body>, Error> {
> +        self.request(request.header("User-Agent", USER_AGENT_STRING).body(body)?).await
> +    }
> +
>       pub async fn request(&self, request: Request<Body>) -> Result<Response<Body>, Error> {
>           self.client.request(request)
>               .map_err(Error::from)

[pbs-devel] [PATCH v2 backup 17/27] add async acme client implementation

[Wolfgang Bumiller]

This is the highlevel part using proxmox-acme-rs to create
requests and our hyper code to issue them to the acme
server.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/acme/client.rs | 670 +++++++++++++++++++++++++++++++++++++++++++++
 src/acme/mod.rs    |   2 +
 src/lib.rs         |   2 +
 3 files changed, 674 insertions(+)
 create mode 100644 src/acme/client.rs
 create mode 100644 src/acme/mod.rs

diff --git a/src/acme/client.rs b/src/acme/client.rs
new file mode 100644
index 00000000..dea2e3b0
--- /dev/null
+++ b/src/acme/client.rs
@@ -0,0 +1,670 @@
+//! HTTP Client for the ACME protocol.
+
+use std::fs::OpenOptions;
+use std::io;
+use std::os::unix::fs::OpenOptionsExt;
+
+use anyhow::format_err;
+use bytes::Bytes;
+use hyper::{Body, Request};
+use nix::sys::stat::Mode;
+use serde::{Deserialize, Serialize};
+
+use proxmox::tools::fs::{replace_file, CreateOptions};
+use proxmox_acme_rs::account::AccountCreator;
+use proxmox_acme_rs::account::AccountData as AcmeAccountData;
+use proxmox_acme_rs::order::{Order, OrderData};
+use proxmox_acme_rs::Request as AcmeRequest;
+use proxmox_acme_rs::{Account, Authorization, Challenge, Directory, Error, ErrorResponse};
+
+use crate::config::acme::{account_path, AccountName};
+use crate::tools::http::SimpleHttp;
+
+/// Our on-disk format inherited from PVE's proxmox-acme code.
+#[derive(Deserialize, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct AccountData {
+    /// The account's location URL.
+    location: String,
+
+    /// The account data.
+    account: AcmeAccountData,
+
+    /// The private key as PEM formatted string.
+    key: String,
+
+    /// ToS URL the user agreed to.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    tos: Option<String>,
+
+    #[serde(skip_serializing_if = "is_false", default)]
+    debug: bool,
+
+    /// The directory's URL.
+    directory_url: String,
+}
+
+#[inline]
+fn is_false(b: &bool) -> bool {
+    !*b
+}
+
+pub struct AcmeClient {
+    directory_url: String,
+    debug: bool,
+    account_path: Option<String>,
+    tos: Option<String>,
+    account: Option<Account>,
+    directory: Option<Directory>,
+    nonce: Option<String>,
+    http_client: Option<SimpleHttp>,
+}
+
+impl AcmeClient {
+    /// Create a new ACME client for a given ACME directory URL.
+    pub fn new(directory_url: String) -> Self {
+        Self {
+            directory_url,
+            debug: false,
+            account_path: None,
+            tos: None,
+            account: None,
+            directory: None,
+            nonce: None,
+            http_client: None,
+        }
+    }
+
+    /// Load an existing ACME account by name.
+    pub async fn load(account_name: &AccountName) -> Result<Self, anyhow::Error> {
+        Self::load_path(account_path(account_name.as_ref())).await
+    }
+
+    /// Load an existing ACME account by path.
+    async fn load_path(account_path: String) -> Result<Self, anyhow::Error> {
+        let data = tokio::fs::read(&account_path).await?;
+        let data: AccountData = serde_json::from_slice(&data)?;
+
+        let account = Account::from_parts(data.location, data.key, data.account);
+
+        Ok(Self {
+            directory_url: data.directory_url,
+            debug: data.debug,
+            account_path: Some(account_path),
+            tos: data.tos,
+            account: Some(account),
+            directory: None,
+            nonce: None,
+            http_client: None,
+        })
+    }
+
+    pub async fn new_account<'a>(
+        &'a mut self,
+        account_name: &AccountName,
+        tos_agreed: bool,
+        contact: Vec<String>,
+        rsa_bits: Option<u32>,
+    ) -> Result<&'a Account, anyhow::Error> {
+        self.tos = if tos_agreed {
+            self.terms_of_service_url().await?.map(str::to_owned)
+        } else {
+            None
+        };
+
+        let account = Account::creator()
+            .set_contacts(contact)
+            .agree_to_tos(tos_agreed);
+
+        let account = if let Some(bits) = rsa_bits {
+            account.generate_rsa_key(bits)?
+        } else {
+            account.generate_ec_key()?
+        };
+
+        let _ = self.register_account(account).await?;
+
+        let account_path = account_path(account_name.as_ref());
+        let file = OpenOptions::new()
+            .write(true)
+            .create(true)
+            .mode(0o600)
+            .open(&account_path)
+            .map_err(|err| format_err!("failed to open {:?} for writing: {}", account_path, err))?;
+        self.write_to(file).map_err(|err| {
+            format_err!(
+                "failed to write acme account to {:?}: {}",
+                account_path,
+                err
+            )
+        })?;
+        self.account_path = Some(account_path);
+
+        // unwrap: Setting `self.account` is literally this function's job, we just can't keep
+        // the borrow from from `self.register_account()` active due to clashes.
+        Ok(self.account.as_ref().unwrap())
+    }
+
+    fn save(&self) -> Result<(), anyhow::Error> {
+        let mut data = Vec::<u8>::new();
+        self.write_to(&mut data)?;
+        let account_path = self.account_path.as_ref().ok_or_else(|| {
+            format_err!("no account path set, cannot save upated account information")
+        })?;
+        replace_file(
+            account_path,
+            &data,
+            CreateOptions::new()
+                .perm(Mode::from_bits_truncate(0o600))
+                .owner(nix::unistd::ROOT)
+                .group(nix::unistd::Gid::from_raw(0)),
+        )
+    }
+
+    /// Shortcut to `account().ok_or_else(...).key_authorization()`.
+    pub fn key_authorization(&self, token: &str) -> Result<String, anyhow::Error> {
+        Ok(Self::need_account(&self.account)?.key_authorization(token)?)
+    }
+
+    /// Shortcut to `account().ok_or_else(...).dns_01_txt_value()`.
+    /// the key authorization value.
+    pub fn dns_01_txt_value(&self, token: &str) -> Result<String, anyhow::Error> {
+        Ok(Self::need_account(&self.account)?.dns_01_txt_value(token)?)
+    }
+
+    async fn register_account(
+        &mut self,
+        account: AccountCreator,
+    ) -> Result<&Account, anyhow::Error> {
+        let mut retry = retry();
+        let mut response = loop {
+            retry.tick()?;
+
+            let (directory, nonce) = Self::get_dir_nonce(
+                &mut self.http_client,
+                &self.directory_url,
+                &mut self.directory,
+                &mut self.nonce,
+            )
+            .await?;
+            let request = account.request(directory, nonce)?;
+            match self.run_request(request).await {
+                Ok(response) => break response,
+                Err(err) if err.is_bad_nonce() => continue,
+                Err(err) => return Err(err.into()),
+            }
+        };
+
+        let account = account.response(response.location_required()?, &response.body)?;
+
+        self.account = Some(account);
+        Ok(self.account.as_ref().unwrap())
+    }
+
+    pub async fn update_account<T: Serialize>(
+        &mut self,
+        data: &T,
+    ) -> Result<&Account, anyhow::Error> {
+        let account = Self::need_account(&self.account)?;
+
+        let mut retry = retry();
+        let response = loop {
+            retry.tick()?;
+
+            let (_directory, nonce) = Self::get_dir_nonce(
+                &mut self.http_client,
+                &self.directory_url,
+                &mut self.directory,
+                &mut self.nonce,
+            )
+            .await?;
+
+            let request = account.post_request(&account.location, &nonce, data)?;
+            match Self::execute(&mut self.http_client, request, &mut self.nonce).await {
+                Ok(response) => break response,
+                Err(err) if err.is_bad_nonce() => continue,
+                Err(err) => return Err(err.into()),
+            }
+        };
+
+        // unwrap: we've been keeping an immutable reference to it from the top of the method
+        let _ = account;
+        self.account.as_mut().unwrap().data = response.json()?;
+        self.save()?;
+        Ok(self.account.as_ref().unwrap())
+    }
+
+    pub async fn new_order<I>(&mut self, domains: I) -> Result<Order, anyhow::Error>
+    where
+        I: IntoIterator<Item = String>,
+    {
+        let account = Self::need_account(&self.account)?;
+
+        let order = domains
+            .into_iter()
+            .fold(OrderData::new(), |order, domain| order.domain(domain));
+
+        let mut retry = retry();
+        loop {
+            retry.tick()?;
+
+            let (directory, nonce) = Self::get_dir_nonce(
+                &mut self.http_client,
+                &self.directory_url,
+                &mut self.directory,
+                &mut self.nonce,
+            )
+            .await?;
+
+            let mut new_order = account.new_order(&order, directory, nonce)?;
+            let mut response = match Self::execute(
+                &mut self.http_client,
+                new_order.request.take().unwrap(),
+                &mut self.nonce,
+            )
+            .await
+            {
+                Ok(response) => response,
+                Err(err) if err.is_bad_nonce() => continue,
+                Err(err) => return Err(err.into()),
+            };
+
+            return Ok(
+                new_order.response(response.location_required()?, response.bytes().as_ref())?
+            );
+        }
+    }
+
+    /// Low level "POST-as-GET" request.
+    async fn post_as_get(&mut self, url: &str) -> Result<AcmeResponse, anyhow::Error> {
+        let account = Self::need_account(&self.account)?;
+
+        let mut retry = retry();
+        loop {
+            retry.tick()?;
+
+            let (_directory, nonce) = Self::get_dir_nonce(
+                &mut self.http_client,
+                &self.directory_url,
+                &mut self.directory,
+                &mut self.nonce,
+            )
+            .await?;
+
+            let request = account.get_request(url, nonce)?;
+            match Self::execute(&mut self.http_client, request, &mut self.nonce).await {
+                Ok(response) => return Ok(response),
+                Err(err) if err.is_bad_nonce() => continue,
+                Err(err) => return Err(err.into()),
+            }
+        }
+    }
+
+    /// Low level POST request.
+    async fn post<T: Serialize>(
+        &mut self,
+        url: &str,
+        data: &T,
+    ) -> Result<AcmeResponse, anyhow::Error> {
+        let account = Self::need_account(&self.account)?;
+
+        let mut retry = retry();
+        loop {
+            retry.tick()?;
+
+            let (_directory, nonce) = Self::get_dir_nonce(
+                &mut self.http_client,
+                &self.directory_url,
+                &mut self.directory,
+                &mut self.nonce,
+            )
+            .await?;
+
+            let request = account.post_request(url, nonce, data)?;
+            match Self::execute(&mut self.http_client, request, &mut self.nonce).await {
+                Ok(response) => return Ok(response),
+                Err(err) if err.is_bad_nonce() => continue,
+                Err(err) => return Err(err.into()),
+            }
+        }
+    }
+
+    /// Request challenge validation. Afterwards, the challenge should be polled.
+    pub async fn request_challenge_validation(
+        &mut self,
+        url: &str,
+    ) -> Result<Challenge, anyhow::Error> {
+        Ok(self
+            .post(url, &serde_json::Value::Object(Default::default()))
+            .await?
+            .json()?)
+    }
+
+    /// Assuming the provided URL is an 'Authorization' URL, get and deserialize it.
+    pub async fn get_authorization(&mut self, url: &str) -> Result<Authorization, anyhow::Error> {
+        Ok(self.post_as_get(url).await?.json()?)
+    }
+
+    /// Assuming the provided URL is an 'Order' URL, get and deserialize it.
+    pub async fn get_order(&mut self, url: &str) -> Result<OrderData, anyhow::Error> {
+        Ok(self.post_as_get(url).await?.json()?)
+    }
+
+    /// Finalize an Order via its `finalize` URL property and the DER encoded CSR.
+    pub async fn finalize(&mut self, url: &str, csr: &[u8]) -> Result<(), anyhow::Error> {
+        let csr = base64::encode_config(csr, base64::URL_SAFE_NO_PAD);
+        let data = serde_json::json!({ "csr": csr });
+        self.post(url, &data).await?;
+        Ok(())
+    }
+
+    /// Download a certificate via its 'certificate' URL property.
+    ///
+    /// The certificate will be a PEM certificate chain.
+    pub async fn get_certificate(&mut self, url: &str) -> Result<Bytes, anyhow::Error> {
+        Ok(self.post_as_get(url).await?.body)
+    }
+
+    /// Revoke an existing certificate (PEM or DER formatted).
+    pub async fn revoke_certificate(
+        &mut self,
+        certificate: &[u8],
+        reason: Option<u32>,
+    ) -> Result<(), anyhow::Error> {
+        // TODO: This can also work without an account.
+        let account = Self::need_account(&self.account)?;
+
+        let revocation = account.revoke_certificate(certificate, reason)?;
+
+        let mut retry = retry();
+        loop {
+            retry.tick()?;
+
+            let (directory, nonce) = Self::get_dir_nonce(
+                &mut self.http_client,
+                &self.directory_url,
+                &mut self.directory,
+                &mut self.nonce,
+            )
+            .await?;
+
+            let request = revocation.request(&directory, nonce)?;
+            match Self::execute(&mut self.http_client, request, &mut self.nonce).await {
+                Ok(_response) => return Ok(()),
+                Err(err) if err.is_bad_nonce() => continue,
+                Err(err) => return Err(err.into()),
+            }
+        }
+    }
+
+    fn need_account(account: &Option<Account>) -> Result<&Account, anyhow::Error> {
+        account
+            .as_ref()
+            .ok_or_else(|| format_err!("cannot use client without an account"))
+    }
+
+    pub(crate) fn account(&self) -> Result<&Account, anyhow::Error> {
+        Self::need_account(&self.account)
+    }
+
+    pub fn tos(&self) -> Option<&str> {
+        self.tos.as_deref()
+    }
+
+    pub fn directory_url(&self) -> &str {
+        &self.directory_url
+    }
+
+    fn to_account_data(&self) -> Result<AccountData, anyhow::Error> {
+        let account = self.account()?;
+
+        Ok(AccountData {
+            location: account.location.clone(),
+            key: account.private_key.clone(),
+            account: AcmeAccountData {
+                only_return_existing: false, // don't actually write this out in case it's set
+                ..account.data.clone()
+            },
+            tos: self.tos.clone(),
+            debug: self.debug,
+            directory_url: self.directory_url.clone(),
+        })
+    }
+
+    fn write_to<T: io::Write>(&self, out: T) -> Result<(), anyhow::Error> {
+        let data = self.to_account_data()?;
+
+        Ok(serde_json::to_writer_pretty(out, &data)?)
+    }
+}
+
+struct AcmeResponse {
+    body: Bytes,
+    location: Option<String>,
+    got_nonce: bool,
+}
+
+impl AcmeResponse {
+    /// Convenience helper to assert that a location header was part of the response.
+    fn location_required(&mut self) -> Result<String, anyhow::Error> {
+        self.location
+            .take()
+            .ok_or_else(|| format_err!("missing Location header"))
+    }
+
+    /// Convenience shortcut to perform json deserialization of the returned body.
+    fn json<T: for<'a> Deserialize<'a>>(&self) -> Result<T, Error> {
+        Ok(serde_json::from_slice(&self.body)?)
+    }
+
+    /// Convenience shortcut to get the body as bytes.
+    fn bytes(&self) -> &[u8] {
+        &self.body
+    }
+}
+
+impl AcmeClient {
+    /// Non-self-borrowing run_request version for borrow workarounds.
+    async fn execute(
+        http_client: &mut Option<SimpleHttp>,
+        request: AcmeRequest,
+        nonce: &mut Option<String>,
+    ) -> Result<AcmeResponse, Error> {
+        let mut req_builder = Request::builder().method(request.method).uri(&request.url);
+
+        let body: Body = if !request.content_type.is_empty() {
+            req_builder = req_builder
+                .header("Content-Type", request.content_type)
+                .header("Content-Length", request.body.len());
+            request.body.into()
+        } else {
+            Body::empty()
+        };
+
+        let response = http_client
+            .get_or_insert_with(|| SimpleHttp::new(None))
+            .request_with_agent(req_builder, body)
+            .await
+            .map_err(|err| Error::Custom(err.to_string()))?;
+        let (parts, body) = response.into_parts();
+
+        let status = parts.status.as_u16();
+        let body = hyper::body::to_bytes(body)
+            .await
+            .map_err(|err| Error::Custom(format!("failed to retrieve response body: {}", err)))?;
+
+        let got_nonce = if let Some(new_nonce) = parts.headers.get(proxmox_acme_rs::REPLAY_NONCE) {
+            let new_nonce = new_nonce.to_str().map_err(|err| {
+                Error::Client(format!(
+                    "received invalid replay-nonce header from ACME server: {}",
+                    err
+                ))
+            })?;
+            *nonce = Some(new_nonce.to_owned());
+            true
+        } else {
+            false
+        };
+
+        if parts.status.is_success() {
+            if status != request.expected {
+                return Err(Error::InvalidApi(format!(
+                    "ACME server responded with unexpected status code: {:?}",
+                    parts.status
+                )));
+            }
+
+            let location = parts
+                .headers
+                .get("Location")
+                .map(|header| {
+                    header.to_str().map(str::to_owned).map_err(|err| {
+                        Error::Client(format!(
+                            "received invalid location header from ACME server: {}",
+                            err
+                        ))
+                    })
+                })
+                .transpose()?;
+
+            return Ok(AcmeResponse {
+                body,
+                location,
+                got_nonce,
+            });
+        }
+
+        let error: ErrorResponse = serde_json::from_slice(&body).map_err(|err| {
+            Error::Client(format!(
+                "error status with improper error ACME response: {}",
+                err
+            ))
+        })?;
+
+        if error.ty == proxmox_acme_rs::error::BAD_NONCE {
+            if !got_nonce {
+                return Err(Error::InvalidApi(
+                    "badNonce without a new Replay-Nonce header".to_string(),
+                ));
+            }
+            return Err(Error::BadNonce);
+        }
+
+        Err(Error::Api(error))
+    }
+
+    /// Low-level API to run an n API request. This automatically updates the current nonce!
+    async fn run_request(&mut self, request: AcmeRequest) -> Result<AcmeResponse, Error> {
+        Self::execute(&mut self.http_client, request, &mut self.nonce).await
+    }
+
+    async fn directory(&mut self) -> Result<&Directory, Error> {
+        Ok(Self::get_directory(
+            &mut self.http_client,
+            &self.directory_url,
+            &mut self.directory,
+            &mut self.nonce,
+        )
+        .await?
+        .0)
+    }
+
+    async fn get_directory<'a, 'b>(
+        http_client: &mut Option<SimpleHttp>,
+        directory_url: &str,
+        directory: &'a mut Option<Directory>,
+        nonce: &'b mut Option<String>,
+    ) -> Result<(&'a Directory, Option<&'b str>), Error> {
+        if let Some(d) = directory {
+            return Ok((d, nonce.as_deref()));
+        }
+
+        let response = Self::execute(
+            http_client,
+            AcmeRequest {
+                url: directory_url.to_string(),
+                method: "GET",
+                content_type: "",
+                body: String::new(),
+                expected: 200,
+            },
+            nonce,
+        )
+        .await?;
+
+        *directory = Some(Directory::from_parts(
+            directory_url.to_string(),
+            response.json()?,
+        ));
+
+        Ok((directory.as_ref().unwrap(), nonce.as_deref()))
+    }
+
+    /// Like `get_directory`, but if the directory provides no nonce, also performs a `HEAD`
+    /// request on the new nonce URL.
+    async fn get_dir_nonce<'a, 'b>(
+        http_client: &mut Option<SimpleHttp>,
+        directory_url: &str,
+        directory: &'a mut Option<Directory>,
+        nonce: &'b mut Option<String>,
+    ) -> Result<(&'a Directory, &'b str), Error> {
+        // this let construct is a lifetime workaround:
+        let _ = Self::get_directory(http_client, directory_url, directory, nonce).await?;
+        let dir = directory.as_ref().unwrap(); // the above fails if it couldn't fill this option
+        if nonce.is_none() {
+            // this is also a lifetime issue...
+            let _ = Self::get_nonce(http_client, nonce, dir.new_nonce_url()).await?;
+        };
+        Ok((dir, nonce.as_deref().unwrap()))
+    }
+
+    pub async fn terms_of_service_url(&mut self) -> Result<Option<&str>, Error> {
+        Ok(self.directory().await?.terms_of_service_url())
+    }
+
+    async fn get_nonce<'a>(
+        http_client: &mut Option<SimpleHttp>,
+        nonce: &'a mut Option<String>,
+        new_nonce_url: &str,
+    ) -> Result<&'a str, Error> {
+        let response = Self::execute(
+            http_client,
+            AcmeRequest {
+                url: new_nonce_url.to_owned(),
+                method: "HEAD",
+                content_type: "",
+                body: String::new(),
+                expected: 200,
+            },
+            nonce,
+        )
+        .await?;
+
+        if !response.got_nonce {
+            return Err(Error::InvalidApi(
+                "no new nonce received from new nonce URL".to_string(),
+            ));
+        }
+
+        nonce
+            .as_deref()
+            .ok_or_else(|| Error::Client("failed to update nonce".to_string()))
+    }
+}
+
+/// bad nonce retry count helper
+struct Retry(usize);
+
+const fn retry() -> Retry {
+    Retry(0)
+}
+
+impl Retry {
+    fn tick(&mut self) -> Result<(), Error> {
+        if self.0 >= 3 {
+            Error::Client(format!("kept getting a badNonce error!"));
+        }
+        self.0 += 1;
+        Ok(())
+    }
+}
diff --git a/src/acme/mod.rs b/src/acme/mod.rs
new file mode 100644
index 00000000..5923f8da
--- /dev/null
+++ b/src/acme/mod.rs
@@ -0,0 +1,2 @@
+pub mod client;
+pub use client::AcmeClient;
diff --git a/src/lib.rs b/src/lib.rs
index 200cf496..1b1de527 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -32,3 +32,5 @@ pub mod auth;
 pub mod rrd;
 
 pub mod tape;
+
+pub mod acme;
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 18/27] add config/acme api path

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/api2/config.rs      |   2 +
 src/api2/config/acme.rs | 719 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 721 insertions(+)
 create mode 100644 src/api2/config/acme.rs

diff --git a/src/api2/config.rs b/src/api2/config.rs
index 996ec268..9befa0e5 100644
--- a/src/api2/config.rs
+++ b/src/api2/config.rs
@@ -4,6 +4,7 @@ use proxmox::api::router::{Router, SubdirMap};
 use proxmox::list_subdirs_api_method;
 
 pub mod access;
+pub mod acme;
 pub mod datastore;
 pub mod remote;
 pub mod sync;
@@ -16,6 +17,7 @@ pub mod tape_backup_job;
 
 const SUBDIRS: SubdirMap = &[
     ("access", &access::ROUTER),
+    ("acme", &acme::ROUTER),
     ("changer", &changer::ROUTER),
     ("datastore", &datastore::ROUTER),
     ("drive", &drive::ROUTER),
diff --git a/src/api2/config/acme.rs b/src/api2/config/acme.rs
new file mode 100644
index 00000000..4f72a94e
--- /dev/null
+++ b/src/api2/config/acme.rs
@@ -0,0 +1,719 @@
+use std::path::Path;
+
+use anyhow::{bail, format_err, Error};
+use serde::{Deserialize, Serialize};
+use serde_json::{json, Value};
+
+use proxmox::api::router::SubdirMap;
+use proxmox::api::schema::Updatable;
+use proxmox::api::{api, Permission, Router, RpcEnvironment};
+use proxmox::http_bail;
+use proxmox::list_subdirs_api_method;
+
+use proxmox_acme_rs::account::AccountData as AcmeAccountData;
+use proxmox_acme_rs::Account;
+
+use crate::acme::AcmeClient;
+use crate::api2::types::Authid;
+use crate::config::acl::PRIV_SYS_MODIFY;
+use crate::config::acme::plugin::{
+    DnsPlugin, DnsPluginCore, DnsPluginCoreUpdater, PLUGIN_ID_SCHEMA,
+};
+use crate::config::acme::{AccountName, KnownAcmeDirectory};
+use crate::server::WorkerTask;
+use crate::tools::ControlFlow;
+
+pub(crate) const ROUTER: Router = Router::new()
+    .get(&list_subdirs_api_method!(SUBDIRS))
+    .subdirs(SUBDIRS);
+
+const SUBDIRS: SubdirMap = &[
+    (
+        "account",
+        &Router::new()
+            .get(&API_METHOD_LIST_ACCOUNTS)
+            .post(&API_METHOD_REGISTER_ACCOUNT)
+            .match_all("name", &ACCOUNT_ITEM_ROUTER),
+    ),
+    (
+        "challenge-schema",
+        &Router::new().get(&API_METHOD_GET_CHALLENGE_SCHEMA),
+    ),
+    (
+        "directories",
+        &Router::new().get(&API_METHOD_GET_DIRECTORIES),
+    ),
+    (
+        "plugins",
+        &Router::new()
+            .get(&API_METHOD_LIST_PLUGINS)
+            .post(&API_METHOD_ADD_PLUGIN)
+            .match_all("id", &PLUGIN_ITEM_ROUTER),
+    ),
+    ("tos", &Router::new().get(&API_METHOD_GET_TOS)),
+];
+
+const ACCOUNT_ITEM_ROUTER: Router = Router::new()
+    .get(&API_METHOD_GET_ACCOUNT)
+    .put(&API_METHOD_UPDATE_ACCOUNT)
+    .delete(&API_METHOD_DEACTIVATE_ACCOUNT);
+
+const PLUGIN_ITEM_ROUTER: Router = Router::new()
+    .get(&API_METHOD_GET_PLUGIN)
+    .put(&API_METHOD_UPDATE_PLUGIN)
+    .delete(&API_METHOD_DELETE_PLUGIN);
+
+#[api(
+    properties: {
+        name: { type: AccountName },
+    },
+)]
+/// An ACME Account entry.
+///
+/// Currently only contains a 'name' property.
+#[derive(Serialize)]
+pub struct AccountEntry {
+    name: AccountName,
+}
+
+#[api(
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    returns: {
+        type: Array,
+        items: { type: AccountEntry },
+        description: "List of ACME accounts.",
+    },
+    protected: true,
+)]
+/// List ACME accounts.
+pub fn list_accounts() -> Result<Vec<AccountEntry>, Error> {
+    let mut entries = Vec::new();
+    crate::config::acme::foreach_acme_account(|name| {
+        entries.push(AccountEntry { name });
+        ControlFlow::Continue(())
+    })?;
+    Ok(entries)
+}
+
+#[api(
+    properties: {
+        account: { type: Object, properties: {}, additional_properties: true },
+        tos: {
+            type: String,
+            optional: true,
+        },
+    },
+)]
+/// ACME Account information.
+///
+/// This is what we return via the API.
+#[derive(Serialize)]
+pub struct AccountInfo {
+    /// Raw account data.
+    account: AcmeAccountData,
+
+    /// The ACME directory URL the account was created at.
+    directory: String,
+
+    /// The account's own URL within the ACME directory.
+    location: String,
+
+    /// The ToS URL, if the user agreed to one.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    tos: Option<String>,
+}
+
+#[api(
+    input: {
+        properties: {
+            name: { type: AccountName },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    returns: { type: AccountInfo },
+    protected: true,
+)]
+/// Return existing ACME account information.
+pub async fn get_account(name: AccountName) -> Result<AccountInfo, Error> {
+    let client = AcmeClient::load(&name).await?;
+    let account = client.account()?;
+    Ok(AccountInfo {
+        location: account.location.clone(),
+        tos: client.tos().map(str::to_owned),
+        directory: client.directory_url().to_owned(),
+        account: AcmeAccountData {
+            only_return_existing: false, // don't actually write this out in case it's set
+            ..account.data.clone()
+        },
+    })
+}
+
+fn account_contact_from_string(s: &str) -> Vec<String> {
+    s.split(&[' ', ';', ',', '\0'][..])
+        .map(|s| format!("mailto:{}", s))
+        .collect()
+}
+
+#[api(
+    input: {
+        properties: {
+            name: { type: AccountName },
+            contact: {
+                description: "List of email addresses.",
+            },
+            tos_url: {
+                description: "URL of CA TermsOfService - setting this indicates agreement.",
+                optional: true,
+            },
+            directory: {
+                type: String,
+                description: "The ACME Directory.",
+                optional: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Register an ACME account.
+fn register_account(
+    name: AccountName,
+    // Todo: email & email-list schema
+    contact: String,
+    tos_url: Option<String>,
+    directory: Option<String>,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+
+    if Path::new(&crate::config::acme::account_path(&name)).exists() {
+        http_bail!(BAD_REQUEST, "account {:?} already exists", name);
+    }
+
+    let directory = directory.unwrap_or_else(|| {
+        crate::config::acme::DEFAULT_ACME_DIRECTORY_ENTRY
+            .url
+            .to_owned()
+    });
+
+    WorkerTask::spawn(
+        "acme-register",
+        None,
+        auth_id,
+        true,
+        move |worker| async move {
+            let mut client = AcmeClient::new(directory);
+
+            worker.log("Registering ACME account...");
+
+            let account =
+                do_register_account(&mut client, &name, tos_url.is_some(), contact, None).await?;
+
+            worker.log(format!(
+                "Registration successful, account URL: {}",
+                account.location
+            ));
+
+            Ok(())
+        },
+    )
+}
+
+pub async fn do_register_account<'a>(
+    client: &'a mut AcmeClient,
+    name: &AccountName,
+    agree_to_tos: bool,
+    contact: String,
+    rsa_bits: Option<u32>,
+) -> Result<&'a Account, Error> {
+    let contact = account_contact_from_string(&contact);
+    Ok(client
+        .new_account(name, agree_to_tos, contact, rsa_bits)
+        .await?)
+}
+
+#[api(
+    input: {
+        properties: {
+            name: { type: AccountName },
+            contact: {
+                description: "List of email addresses.",
+                optional: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Update an ACME account.
+pub fn update_account(
+    name: AccountName,
+    // Todo: email & email-list schema
+    contact: Option<String>,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+
+    WorkerTask::spawn(
+        "acme-update",
+        None,
+        auth_id,
+        true,
+        move |_worker| async move {
+            let data = match contact {
+                Some(data) => json!({
+                    "contact": account_contact_from_string(&data),
+                }),
+                None => json!({}),
+            };
+
+            AcmeClient::load(&name).await?.update_account(&data).await?;
+
+            Ok(())
+        },
+    )
+}
+
+#[api(
+    input: {
+        properties: {
+            name: { type: AccountName },
+            force: {
+                description:
+                    "Delete account data even if the server refuses to deactivate the account.",
+                optional: true,
+                default: false,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Deactivate an ACME account.
+pub fn deactivate_account(
+    name: AccountName,
+    force: bool,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+
+    WorkerTask::spawn(
+        "acme-deactivate",
+        None,
+        auth_id,
+        true,
+        move |worker| async move {
+            match AcmeClient::load(&name)
+                .await?
+                .update_account(&json!({"status": "deactivated"}))
+                .await
+            {
+                Ok(_account) => (),
+                Err(err) if !force => return Err(err),
+                Err(err) => {
+                    worker.warn(format!(
+                        "error deactivating account {:?}, proceedeing anyway - {}",
+                        name, err,
+                    ));
+                }
+            }
+            crate::config::acme::mark_account_deactivated(&name)?;
+            Ok(())
+        },
+    )
+}
+
+#[api(
+    input: {
+        properties: {
+            directory: {
+                type: String,
+                description: "The ACME Directory.",
+                optional: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Anybody,
+    },
+    returns: {
+        type: String,
+        optional: true,
+        description: "The ACME Directory's ToS URL, if any.",
+    },
+)]
+/// Get the Terms of Service URL for an ACME directory.
+async fn get_tos(directory: Option<String>) -> Result<Option<String>, Error> {
+    let directory = directory.unwrap_or_else(|| {
+        crate::config::acme::DEFAULT_ACME_DIRECTORY_ENTRY
+            .url
+            .to_owned()
+    });
+    Ok(AcmeClient::new(directory)
+        .terms_of_service_url()
+        .await?
+        .map(str::to_owned))
+}
+
+#[api(
+    access: {
+        permission: &Permission::Anybody,
+    },
+    returns: {
+        description: "List of known ACME directories.",
+        type: Array,
+        items: { type: KnownAcmeDirectory },
+    },
+)]
+/// Get named known ACME directory endpoints.
+fn get_directories() -> Result<&'static [KnownAcmeDirectory], Error> {
+    Ok(crate::config::acme::KNOWN_ACME_DIRECTORIES)
+}
+
+#[api(
+    properties: {
+        schema: {
+            type: Object,
+            additional_properties: true,
+            properties: {},
+        },
+        type: {
+            type: String,
+        },
+    },
+)]
+#[derive(Serialize)]
+/// Schema for an ACME challenge plugin.
+pub struct ChallengeSchema {
+    /// Plugin ID.
+    id: String,
+
+    /// Human readable name, falls back to id.
+    name: String,
+
+    /// Plugin Type.
+    #[serde(rename = "type")]
+    ty: &'static str,
+
+    /// The plugin's parameter schema.
+    schema: Value,
+}
+
+#[api(
+    access: {
+        permission: &Permission::Anybody,
+    },
+    returns: {
+        description: "ACME Challenge Plugin Shema.",
+        type: Array,
+        items: { type: ChallengeSchema },
+    },
+)]
+/// Get named known ACME directory endpoints.
+fn get_challenge_schema() -> Result<Vec<ChallengeSchema>, Error> {
+    let mut out = Vec::new();
+    crate::config::acme::foreach_dns_plugin(|id| {
+        out.push(ChallengeSchema {
+            id: id.to_owned(),
+            name: id.to_owned(),
+            ty: "dns",
+            schema: Value::Object(Default::default()),
+        });
+        ControlFlow::Continue(())
+    })?;
+    Ok(out)
+}
+
+#[api]
+#[derive(Default, Deserialize, Serialize)]
+#[serde(rename_all = "kebab-case")]
+/// The API's format is inherited from PVE/PMG:
+pub struct PluginConfig {
+    /// Plugin ID.
+    plugin: String,
+
+    /// Plugin type.
+    #[serde(rename = "type")]
+    ty: String,
+
+    /// DNS Api name.
+    api: Option<String>,
+
+    /// Plugin configuration data.
+    data: Option<String>,
+
+    /// Extra delay in seconds to wait before requesting validation.
+    ///
+    /// Allows to cope with long TTL of DNS records.
+    #[serde(skip_serializing_if = "Option::is_none", default)]
+    validation_delay: Option<u32>,
+
+    /// Flag to disable the config.
+    #[serde(skip_serializing_if = "Option::is_none", default)]
+    disable: Option<bool>,
+}
+
+// See PMG/PVE's $modify_cfg_for_api sub
+fn modify_cfg_for_api(id: &str, ty: &str, data: &Value) -> PluginConfig {
+    let mut entry = data.clone();
+
+    let obj = entry.as_object_mut().unwrap();
+    obj.remove("id");
+    obj.insert("plugin".to_string(), Value::String(id.to_owned()));
+    obj.insert("type".to_string(), Value::String(ty.to_owned()));
+
+    // FIXME: This needs to go once the `Updater` is fixed.
+    // None of these should be able to fail unless the user changed the files by hand, in which
+    // case we leave the unmodified string in the Value for now. This will be handled with an error
+    // later.
+    if let Some(Value::String(ref mut data)) = obj.get_mut("data") {
+        if let Ok(new) = base64::decode_config(&data, base64::URL_SAFE_NO_PAD) {
+            if let Ok(utf8) = String::from_utf8(new) {
+                *data = utf8;
+            }
+        }
+    }
+
+    // PVE/PMG do this explicitly for ACME plugins...
+    // obj.insert("digest".to_string(), Value::String(digest.clone()));
+
+    serde_json::from_value(entry).unwrap_or_else(|_| PluginConfig {
+        plugin: "*Error*".to_string(),
+        ty: "*Error*".to_string(),
+        ..Default::default()
+    })
+}
+
+#[api(
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+    returns: {
+        type: Array,
+        description: "List of ACME plugin configurations.",
+        items: { type: PluginConfig },
+    },
+)]
+/// List ACME challenge plugins.
+pub fn list_plugins(mut rpcenv: &mut dyn RpcEnvironment) -> Result<Vec<PluginConfig>, Error> {
+    use crate::config::acme::plugin;
+
+    let (plugins, digest) = plugin::config()?;
+    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
+    Ok(plugins
+        .iter()
+        .map(|(id, (ty, data))| modify_cfg_for_api(&id, &ty, data))
+        .collect())
+}
+
+#[api(
+    input: {
+        properties: {
+            id: { schema: PLUGIN_ID_SCHEMA },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+    returns: { type: PluginConfig },
+)]
+/// List ACME challenge plugins.
+pub fn get_plugin(id: String, mut rpcenv: &mut dyn RpcEnvironment) -> Result<PluginConfig, Error> {
+    use crate::config::acme::plugin;
+
+    let (plugins, digest) = plugin::config()?;
+    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
+
+    match plugins.get(&id) {
+        Some((ty, data)) => Ok(modify_cfg_for_api(&id, &ty, &data)),
+        None => http_bail!(NOT_FOUND, "no such plugin"),
+    }
+}
+
+// Currently we only have "the" standalone plugin and DNS plugins so we can just flatten a
+// DnsPluginUpdater:
+//
+// FIXME: The 'id' parameter should not be "optional" in the schema.
+#[api(
+    input: {
+        properties: {
+            type: {
+                type: String,
+                description: "The ACME challenge plugin type.",
+            },
+            core: {
+                type: DnsPluginCoreUpdater,
+                flatten: true,
+            },
+            data: {
+                type: String,
+                // This is different in the API!
+                description: "DNS plugin data (base64 encoded with padding).",
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Add ACME plugin configuration.
+pub fn add_plugin(r#type: String, core: DnsPluginCoreUpdater, data: String) -> Result<(), Error> {
+    use crate::config::acme::plugin;
+
+    // Currently we only support DNS plugins and the standalone plugin is "fixed":
+    if r#type != "dns" {
+        bail!("invalid ACME plugin type: {:?}", r#type);
+    }
+
+    let data = String::from_utf8(base64::decode(&data)?)
+        .map_err(|_| format_err!("data must be valid UTF-8"))?;
+    //core.api_fixup()?;
+
+    // FIXME: Solve the Updater with non-optional fields thing...
+    let id = core
+        .id
+        .clone()
+        .ok_or_else(|| format_err!("missing required 'id' parameter"))?;
+
+    let _lock = plugin::write_lock()?;
+
+    let (mut plugins, _digest) = plugin::config()?;
+    if plugins.contains_key(&id) {
+        bail!("ACME plugin ID {:?} already exists", id);
+    }
+
+    let plugin = serde_json::to_value(DnsPlugin {
+        core: DnsPluginCore::try_build_from(core)?,
+        data,
+    })?;
+
+    plugins.insert(id, r#type, plugin);
+
+    plugin::save_config(&plugins)?;
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            id: { schema: PLUGIN_ID_SCHEMA },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Delete an ACME plugin configuration.
+pub fn delete_plugin(id: String) -> Result<(), Error> {
+    use crate::config::acme::plugin;
+
+    let _lock = plugin::write_lock()?;
+
+    let (mut plugins, _digest) = plugin::config()?;
+    if plugins.remove(&id).is_none() {
+        http_bail!(NOT_FOUND, "no such plugin");
+    }
+    plugin::save_config(&plugins)?;
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            core_update: {
+                type: DnsPluginCoreUpdater,
+                flatten: true,
+            },
+            data: {
+                type: String,
+                optional: true,
+                // This is different in the API!
+                description: "DNS plugin data (base64 encoded with padding).",
+            },
+            digest: {
+                description: "Digest to protect against concurrent updates",
+                optional: true,
+            },
+            delete: {
+                description: "Options to remove from the configuration",
+                optional: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Update an ACME plugin configuration.
+pub fn update_plugin(
+    core_update: DnsPluginCoreUpdater,
+    data: Option<String>,
+    delete: Option<String>,
+    digest: Option<String>,
+) -> Result<(), Error> {
+    use crate::config::acme::plugin;
+
+    let data = data
+        .as_deref()
+        .map(base64::decode)
+        .transpose()?
+        .map(String::from_utf8)
+        .transpose()
+        .map_err(|_| format_err!("data must be valid UTF-8"))?;
+    //core_update.api_fixup()?;
+
+    // unwrap: the id is matched by this method's API path
+    let id = core_update.id.clone().unwrap();
+
+    let delete: Vec<&str> = delete
+        .as_deref()
+        .unwrap_or("")
+        .split(&[' ', ',', ';', '\0'][..])
+        .collect();
+
+    let _lock = plugin::write_lock()?;
+
+    let (mut plugins, expected_digest) = plugin::config()?;
+
+    if let Some(digest) = digest {
+        let digest = proxmox::tools::hex_to_digest(&digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    match plugins.get_mut(&id) {
+        Some((ty, ref mut entry)) => {
+            if ty != "dns" {
+                bail!("cannot update plugin of type {:?}", ty);
+            }
+
+            let mut plugin: DnsPlugin = serde_json::from_value(entry.clone())?;
+            plugin.core.update_from(core_update, &delete)?;
+            if let Some(data) = data {
+                plugin.data = data;
+            }
+            *entry = serde_json::to_value(plugin)?;
+        }
+        None => http_bail!(NOT_FOUND, "no such plugin"),
+    }
+
+    plugin::save_config(&plugins)?;
+
+    Ok(())
+}
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 19/27] add node/{node}/certificates api call

[Wolfgang Bumiller]

API like in PVE:

GET    .../info             => current cert information
POST   .../custom           => upload custom certificate
DELETE .../custom           => delete custom certificate
POST   .../acme/certificate => order acme certificate
PUT    .../acme/certificate => renew expiring acme cert

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/api2/node.rs              |   2 +
 src/api2/node/certificates.rs | 572 ++++++++++++++++++++++++++++++++++
 src/config.rs                 |  18 +-
 3 files changed, 590 insertions(+), 2 deletions(-)
 create mode 100644 src/api2/node/certificates.rs

diff --git a/src/api2/node.rs b/src/api2/node.rs
index 1f3e46a9..ebb51aaf 100644
--- a/src/api2/node.rs
+++ b/src/api2/node.rs
@@ -27,6 +27,7 @@ use crate::tools;
 use crate::tools::ticket::{self, Empty, Ticket};
 
 pub mod apt;
+pub mod certificates;
 pub mod disks;
 pub mod dns;
 pub mod network;
@@ -314,6 +315,7 @@ fn upgrade_to_websocket(
 
 pub const SUBDIRS: SubdirMap = &[
     ("apt", &apt::ROUTER),
+    ("certificates", &certificates::ROUTER),
     ("disks", &disks::ROUTER),
     ("dns", &dns::ROUTER),
     ("journal", &journal::ROUTER),
diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
new file mode 100644
index 00000000..4dd75027
--- /dev/null
+++ b/src/api2/node/certificates.rs
@@ -0,0 +1,572 @@
+use std::convert::TryFrom;
+use std::sync::Arc;
+use std::time::Duration;
+
+use anyhow::{bail, format_err, Error};
+use openssl::pkey::PKey;
+use openssl::x509::X509;
+use serde::{Deserialize, Serialize};
+
+use proxmox::api::router::SubdirMap;
+use proxmox::api::{api, Permission, Router, RpcEnvironment};
+use proxmox::list_subdirs_api_method;
+
+use crate::acme::AcmeClient;
+use crate::api2::types::Authid;
+use crate::api2::types::NODE_SCHEMA;
+use crate::config::acl::PRIV_SYS_MODIFY;
+use crate::config::node::{AcmeDomain, NodeConfig};
+use crate::server::WorkerTask;
+use crate::tools::cert;
+
+pub const ROUTER: Router = Router::new()
+    .get(&list_subdirs_api_method!(SUBDIRS))
+    .subdirs(SUBDIRS);
+
+const SUBDIRS: SubdirMap = &[
+    ("acme", &ACME_ROUTER),
+    (
+        "custom",
+        &Router::new()
+            .post(&API_METHOD_UPLOAD_CUSTOM_CERTIFICATE)
+            .delete(&API_METHOD_DELETE_CUSTOM_CERTIFICATE),
+    ),
+    ("info", &Router::new().get(&API_METHOD_GET_INFO)),
+];
+
+const ACME_ROUTER: Router = Router::new()
+    .get(&list_subdirs_api_method!(ACME_SUBDIRS))
+    .subdirs(ACME_SUBDIRS);
+
+const ACME_SUBDIRS: SubdirMap = &[(
+    "certificate",
+    &Router::new()
+        .post(&API_METHOD_NEW_ACME_CERT)
+        .put(&API_METHOD_RENEW_ACME_CERT),
+)];
+
+#[api(
+    properties: {
+        san: {
+            type: Array,
+            items: {
+                description: "A SubjectAlternateName entry.",
+                type: String,
+            },
+        },
+    },
+)]
+/// Certificate information.
+#[derive(Deserialize, Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct CertificateInfo {
+    /// Certificate file name.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    filename: Option<String>,
+
+    /// Certificate subject name.
+    subject: String,
+
+    /// List of certificate's SubjectAlternativeName entries.
+    san: Vec<String>,
+
+    /// Certificate issuer name.
+    issuer: String,
+
+    /// Certificate's notBefore timestamp (UNIX epoch).
+    #[serde(skip_serializing_if = "Option::is_none")]
+    notbefore: Option<i64>,
+
+    /// Certificate's notAfter timestamp (UNIX epoch).
+    #[serde(skip_serializing_if = "Option::is_none")]
+    notafter: Option<i64>,
+
+    /// Certificate in PEM format.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pem: Option<String>,
+
+    /// Certificate's public key algorithm.
+    public_key_type: String,
+
+    /// Certificate's public key size if available.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    public_key_bits: Option<u32>,
+
+    /// The SSL Fingerprint.
+    fingerprint: Option<String>,
+}
+
+impl TryFrom<&cert::CertInfo> for CertificateInfo {
+    type Error = Error;
+
+    fn try_from(info: &cert::CertInfo) -> Result<Self, Self::Error> {
+        let pubkey = info.public_key()?;
+
+        Ok(Self {
+            filename: None,
+            subject: info.subject_name()?,
+            san: info
+                .subject_alt_names()
+                .map(|san| {
+                    san.into_iter()
+                        // FIXME: Support `.ipaddress()`?
+                        .filter_map(|name| name.dnsname().map(str::to_owned))
+                        .collect()
+                })
+                .unwrap_or_default(),
+            issuer: info.issuer_name()?,
+            notbefore: info.not_before_unix().ok(),
+            notafter: info.not_after_unix().ok(),
+            pem: None,
+            public_key_type: openssl::nid::Nid::from_raw(pubkey.id().as_raw())
+                .long_name()
+                .unwrap_or("<unsupported key type>")
+                .to_owned(),
+            public_key_bits: Some(pubkey.bits()),
+            fingerprint: Some(info.fingerprint()?),
+        })
+    }
+}
+
+fn get_certificate_pem() -> Result<String, Error> {
+    let cert_path = configdir!("/proxy.pem");
+    let cert_pem = proxmox::tools::fs::file_get_contents(&cert_path)?;
+    String::from_utf8(cert_pem)
+        .map_err(|_| format_err!("certificate in {:?} is not a valid PEM file", cert_path))
+}
+
+// to deduplicate error messages
+fn pem_to_cert_info(pem: &[u8]) -> Result<cert::CertInfo, Error> {
+    cert::CertInfo::from_pem(pem)
+        .map_err(|err| format_err!("error loading proxy certificate: {}", err))
+}
+
+#[api(
+    input: {
+        properties: {
+            node: { schema: NODE_SCHEMA },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    returns: {
+        type: Array,
+        items: { type: CertificateInfo },
+        description: "List of certificate infos.",
+    },
+)]
+/// Get certificate info.
+pub fn get_info() -> Result<Vec<CertificateInfo>, Error> {
+    let cert_pem = get_certificate_pem()?;
+    let cert = pem_to_cert_info(cert_pem.as_bytes())?;
+
+    Ok(vec![CertificateInfo {
+        filename: Some("proxy.pem".to_string()), // we only have the one
+        pem: Some(cert_pem),
+        ..CertificateInfo::try_from(&cert)?
+    }])
+}
+
+#[api(
+    input: {
+        properties: {
+            node: { schema: NODE_SCHEMA },
+            certificates: { description: "PEM encoded certificate (chain)." },
+            key: { description: "PEM encoded private key." },
+            restart: {
+                description: "Restart proxmox-backup-proxy",
+                optional: true,
+                default: false,
+            },
+            // FIXME: widget-toolkit should have an option to disable using this parameter...
+            force: {
+                description: "Force replacement of existing files.",
+                type: Boolean,
+                optional: true,
+                default: false,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    returns: {
+        type: Array,
+        items: { type: CertificateInfo },
+        description: "List of certificate infos.",
+    },
+    protected: true,
+)]
+/// Upload a custom certificate.
+pub fn upload_custom_certificate(
+    certificates: String,
+    key: String,
+    restart: bool,
+) -> Result<Vec<CertificateInfo>, Error> {
+    let certificates = X509::stack_from_pem(certificates.as_bytes())
+        .map_err(|err| format_err!("failed to decode certificate chain: {}", err))?;
+    let key = PKey::private_key_from_pem(key.as_bytes())
+        .map_err(|err| format_err!("failed to parse private key: {}", err))?;
+
+    let certificates = certificates
+        .into_iter()
+        .try_fold(Vec::<u8>::new(), |mut stack, cert| -> Result<_, Error> {
+            if !stack.is_empty() {
+                stack.push(b'\n');
+            }
+            stack.extend(cert.to_pem()?);
+            Ok(stack)
+        })
+        .map_err(|err| format_err!("error formatting certificate chain as PEM: {}", err))?;
+
+    let key = key.private_key_to_pem_pkcs8()?;
+
+    crate::config::set_proxy_certificate(&certificates, &key, restart)?;
+
+    get_info()
+}
+
+#[api(
+    input: {
+        properties: {
+            node: { schema: NODE_SCHEMA },
+            restart: {
+                description: "Restart proxmox-backup-proxy",
+                optional: true,
+                default: false,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Delete the current certificate and regenerate a self signed one.
+pub fn delete_custom_certificate(restart: bool) -> Result<(), Error> {
+    let cert_path = configdir!("/proxy.pem");
+    // Here we fail since if this fails nothing else breaks anyway
+    std::fs::remove_file(&cert_path)
+        .map_err(|err| format_err!("failed to unlink {:?} - {}", cert_path, err))?;
+
+    let key_path = configdir!("/proxy.key");
+    if let Err(err) = std::fs::remove_file(&key_path) {
+        // Here we just log since the certificate is already gone and we'd rather try to generate
+        // the self-signed certificate even if this fails:
+        log::error!(
+            "failed to remove certificate private key {:?} - {}",
+            key_path,
+            err
+        );
+    }
+
+    crate::config::update_self_signed_cert(true)?;
+
+    if restart {
+        crate::config::reload_proxy()?;
+    }
+
+    Ok(())
+}
+
+struct OrderedCertificate {
+    certificate: hyper::body::Bytes,
+    private_key_pem: Vec<u8>,
+}
+
+async fn order_certificate(
+    worker: Arc<WorkerTask>,
+    node_config: &NodeConfig,
+) -> Result<Option<OrderedCertificate>, Error> {
+    use proxmox_acme_rs::authorization::Status;
+    use proxmox_acme_rs::order::Identifier;
+
+    let domains = node_config.acme_domains().try_fold(
+        Vec::<AcmeDomain>::new(),
+        |mut acc, domain| -> Result<_, Error> {
+            let mut domain = domain?;
+            domain.domain.make_ascii_lowercase();
+            if let Some(alias) = &mut domain.alias {
+                alias.make_ascii_lowercase();
+            }
+            acc.push(domain);
+            Ok(acc)
+        },
+    )?;
+
+    let get_domain_config = |domain: &str| {
+        domains
+            .iter()
+            .find(|d| d.domain == domain)
+            .ok_or_else(|| format_err!("no config for domain '{}'", domain))
+    };
+
+    if domains.is_empty() {
+        worker.log("No domains configured to be ordered from an ACME server.");
+        return Ok(None);
+    }
+
+    let (plugins, _) = crate::config::acme::plugin::config()?;
+
+    let mut acme = node_config.acme_client().await?;
+
+    worker.log("Placing ACME order");
+    let order = acme
+        .new_order(domains.iter().map(|d| d.domain.to_ascii_lowercase()))
+        .await?;
+    worker.log(format!("Order URL: {}", order.location));
+
+    let identifiers: Vec<String> = order
+        .data
+        .identifiers
+        .iter()
+        .map(|identifier| match identifier {
+            Identifier::Dns(domain) => domain.clone(),
+        })
+        .collect();
+
+    for auth_url in &order.data.authorizations {
+        worker.log(format!("Getting authorization details from '{}'", auth_url));
+        let mut auth = acme.get_authorization(&auth_url).await?;
+
+        let domain = match &mut auth.identifier {
+            Identifier::Dns(domain) => domain.to_ascii_lowercase(),
+        };
+
+        if auth.status == Status::Valid {
+            worker.log(format!("{} is already validated!", domain));
+            continue;
+        }
+
+        worker.log(format!("The validation for {} is pending", domain));
+        let domain_config: &AcmeDomain = get_domain_config(&domain)?;
+        let plugin_id = domain_config.plugin.as_deref().unwrap_or("standalone");
+        let plugin_cfg = plugins.get_plugin(plugin_id)?.ok_or_else(|| {
+            format_err!("plugin '{}' for domain '{}' not found!", plugin_id, domain)
+        })?;
+
+        worker.log("Setting up validation plugin");
+        let validation_url = plugin_cfg.setup(&mut acme, &auth, domain_config).await?;
+
+        let result = request_validation(&worker, &mut acme, auth_url, validation_url).await;
+
+        if let Err(err) = plugin_cfg.teardown(&mut acme, &auth, domain_config).await {
+            worker.warn(format!(
+                "Failed to teardown plugin '{}' for domain '{}' - {}",
+                plugin_id, domain, err
+            ));
+        }
+
+        let _: () = result?;
+    }
+
+    worker.log("All domains validated");
+    worker.log("Creating CSR");
+
+    let csr = proxmox_acme_rs::util::Csr::generate(&identifiers, &Default::default())?;
+    let mut finalize_error_cnt = 0u8;
+    let order_url = &order.location;
+    let mut order;
+    loop {
+        use proxmox_acme_rs::order::Status;
+
+        order = acme.get_order(order_url).await?;
+
+        match order.status {
+            Status::Pending => {
+                worker.log("still pending, trying to finalize anyway");
+                let finalize = order
+                    .finalize
+                    .as_deref()
+                    .ok_or_else(|| format_err!("missing 'finalize' URL in order"))?;
+                if let Err(err) = acme.finalize(finalize, &csr.data).await {
+                    if finalize_error_cnt >= 5 {
+                        return Err(err.into());
+                    }
+
+                    finalize_error_cnt += 1;
+                }
+                tokio::time::sleep(Duration::from_secs(5)).await;
+            }
+            Status::Ready => {
+                worker.log("order is ready, finalizing");
+                let finalize = order
+                    .finalize
+                    .as_deref()
+                    .ok_or_else(|| format_err!("missing 'finalize' URL in order"))?;
+                acme.finalize(finalize, &csr.data).await?;
+                tokio::time::sleep(Duration::from_secs(5)).await;
+            }
+            Status::Processing => {
+                worker.log("still processing, trying again in 30 seconds");
+                tokio::time::sleep(Duration::from_secs(30)).await;
+            }
+            Status::Valid => {
+                worker.log("valid");
+                break;
+            }
+            other => bail!("order status: {:?}", other),
+        }
+    }
+
+    worker.log("Downloading certificate");
+    let certificate = acme
+        .get_certificate(
+            order
+                .certificate
+                .as_deref()
+                .ok_or_else(|| format_err!("missing certificate url in finalized order"))?,
+        )
+        .await?;
+
+    Ok(Some(OrderedCertificate {
+        certificate,
+        private_key_pem: csr.private_key_pem,
+    }))
+}
+
+async fn request_validation(
+    worker: &WorkerTask,
+    acme: &mut AcmeClient,
+    auth_url: &str,
+    validation_url: &str,
+) -> Result<(), Error> {
+    worker.log("Triggering validation");
+    acme.request_challenge_validation(&validation_url).await?;
+
+    worker.log("Sleeping for 5 seconds");
+    tokio::time::sleep(Duration::from_secs(5)).await;
+
+    loop {
+        use proxmox_acme_rs::authorization::Status;
+
+        let auth = acme.get_authorization(&auth_url).await?;
+        match auth.status {
+            Status::Pending => {
+                worker.log("Status is still 'pending', trying again in 10 seconds");
+                tokio::time::sleep(Duration::from_secs(10)).await;
+            }
+            Status::Valid => return Ok(()),
+            other => bail!(
+                "validating challenge '{}' failed - status: {:?}",
+                validation_url,
+                other
+            ),
+        }
+    }
+}
+
+#[api(
+    input: {
+        properties: {
+            node: { schema: NODE_SCHEMA },
+            force: {
+                description: "Force replacement of existing files.",
+                type: Boolean,
+                optional: true,
+                default: false,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Order a new ACME certificate.
+pub fn new_acme_cert(force: bool, rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error> {
+    spawn_certificate_worker("acme-new-cert", force, rpcenv)
+}
+
+#[api(
+    input: {
+        properties: {
+            node: { schema: NODE_SCHEMA },
+            force: {
+                description: "Force replacement of existing files.",
+                type: Boolean,
+                optional: true,
+                default: false,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Renew the current ACME certificate if it expires within 30 days (or always if the `force`
+/// parameter is set).
+pub fn renew_acme_cert(force: bool, rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error> {
+    if !cert_expires_soon()? && !force {
+        bail!("Certificate does not expire within the next 30 days and 'force' is not set.")
+    }
+
+    spawn_certificate_worker("acme-renew-cert", force, rpcenv)
+}
+
+/// Check whether the current certificate expires within the next 30 days.
+pub fn cert_expires_soon() -> Result<bool, Error> {
+    let cert = pem_to_cert_info(get_certificate_pem()?.as_bytes())?;
+    cert.is_expired_after_epoch(proxmox::tools::time::epoch_i64() + 30 * 24 * 60 * 60)
+        .map_err(|err| format_err!("Failed to check certificate expiration date: {}", err))
+}
+
+fn spawn_certificate_worker(
+    name: &'static str,
+    force: bool,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+    // We only have 1 certificate path in PBS which makes figuring out whether or not it is a
+    // custom one too hard... We keep the parameter because the widget-toolkit may be using it...
+    let _ = force;
+
+    let (node_config, _digest) = crate::config::node::config()?;
+
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+
+    WorkerTask::spawn(name, None, auth_id, true, move |worker| async move {
+        if let Some(cert) = order_certificate(worker, &node_config).await? {
+            crate::config::set_proxy_certificate(&cert.certificate, &cert.private_key_pem, true)?;
+        }
+        Ok(())
+    })
+}
+
+#[api(
+    input: {
+        properties: {
+            node: { schema: NODE_SCHEMA },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Renew the current ACME certificate if it expires within 30 days (or always if the `force`
+/// parameter is set).
+pub fn revoke_acme_cert(rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error> {
+    let (node_config, _digest) = crate::config::node::config()?;
+
+    let cert_pem = get_certificate_pem()?;
+
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+
+    WorkerTask::spawn(
+        "acme-revoke-cert",
+        None,
+        auth_id,
+        true,
+        move |worker| async move {
+            worker.log("Loading ACME account");
+            let mut acme = node_config.acme_client().await?;
+            worker.log("Revoking old certificate");
+            acme.revoke_certificate(cert_pem.as_bytes(), None).await?;
+            worker.log("Deleting certificate and regenerating a self-signed one");
+            delete_custom_certificate(true)?;
+            Ok(())
+        },
+    )
+}
diff --git a/src/config.rs b/src/config.rs
index 94b7fb6c..22c293c9 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -187,12 +187,16 @@ pub fn update_self_signed_cert(force: bool) -> Result<(), Error> {
     let x509 = x509.build();
     let cert_pem = x509.to_pem()?;
 
-    set_proxy_certificate(&cert_pem, &priv_pem)?;
+    set_proxy_certificate(&cert_pem, &priv_pem, false)?;
 
     Ok(())
 }
 
-pub(crate) fn set_proxy_certificate(cert_pem: &[u8], key_pem: &[u8]) -> Result<(), Error> {
+pub(crate) fn set_proxy_certificate(
+    cert_pem: &[u8],
+    key_pem: &[u8],
+    reload: bool,
+) -> Result<(), Error> {
     let backup_user = crate::backup::backup_user()?;
     let options = CreateOptions::new()
         .perm(Mode::from_bits_truncate(0o0640))
@@ -206,5 +210,15 @@ pub(crate) fn set_proxy_certificate(cert_pem: &[u8], key_pem: &[u8]) -> Result<(
         .map_err(|err| format_err!("error writing certificate private key - {}", err))?;
     replace_file(&cert_path, &cert_pem, options)
         .map_err(|err| format_err!("error writing certificate file - {}", err))?;
+
+    if reload {
+        reload_proxy()?;
+    }
+
     Ok(())
 }
+
+pub(crate) fn reload_proxy() -> Result<(), Error> {
+    crate::tools::systemd::reload_unit("proxmox-backup-proxy")
+        .map_err(|err| format_err!("error signaling reload to pbs proxy: {}", err))
+}
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 20/27] add node/{node}/config api path

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/api2/node.rs        |  2 +
 src/api2/node/config.rs | 81 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)
 create mode 100644 src/api2/node/config.rs

diff --git a/src/api2/node.rs b/src/api2/node.rs
index ebb51aaf..75271cd5 100644
--- a/src/api2/node.rs
+++ b/src/api2/node.rs
@@ -28,6 +28,7 @@ use crate::tools::ticket::{self, Empty, Ticket};
 
 pub mod apt;
 pub mod certificates;
+pub mod config;
 pub mod disks;
 pub mod dns;
 pub mod network;
@@ -316,6 +317,7 @@ fn upgrade_to_websocket(
 pub const SUBDIRS: SubdirMap = &[
     ("apt", &apt::ROUTER),
     ("certificates", &certificates::ROUTER),
+    ("config", &config::ROUTER),
     ("disks", &disks::ROUTER),
     ("dns", &dns::ROUTER),
     ("journal", &journal::ROUTER),
diff --git a/src/api2/node/config.rs b/src/api2/node/config.rs
new file mode 100644
index 00000000..2e7fd670
--- /dev/null
+++ b/src/api2/node/config.rs
@@ -0,0 +1,81 @@
+use anyhow::Error;
+use serde_json::Value;
+
+use proxmox::api::schema::Updatable;
+use proxmox::api::{api, Permission, Router, RpcEnvironment};
+
+use crate::api2::types::NODE_SCHEMA;
+use crate::config::acl::PRIV_SYS_MODIFY;
+use crate::config::node::NodeConfigUpdater;
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_GET_NODE_CONFIG)
+    .put(&API_METHOD_UPDATE_NODE_CONFIG);
+
+#[api(
+    input: {
+        properties: {
+            node: { schema: NODE_SCHEMA },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system"], PRIV_SYS_MODIFY, false),
+    },
+)]
+/// Create a new changer device.
+pub fn get_node_config(mut rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    let _lock = crate::config::node::read_lock()?;
+    let (config, digest) = crate::config::node::config()?;
+    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
+    Ok(serde_json::to_value(config)?)
+}
+
+#[api(
+    input: {
+        properties: {
+            node: { schema: NODE_SCHEMA },
+            digest: {
+                description: "Digest to protect against concurrent updates",
+                optional: true,
+            },
+            updater: {
+                type: NodeConfigUpdater,
+                flatten: true,
+            },
+            delete: {
+                description: "Options to remove from the configuration",
+                optional: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["system"], PRIV_SYS_MODIFY, false),
+    },
+    protected: true,
+)]
+/// Create a new changer device.
+pub fn update_node_config(
+    updater: NodeConfigUpdater,
+    delete: Option<String>,
+    digest: Option<String>,
+) -> Result<(), Error> {
+    let _lock = crate::config::node::write_lock()?;
+    let (mut config, expected_digest) = crate::config::node::config()?;
+    if let Some(digest) = digest {
+        // FIXME: GUI doesn't handle our non-inlined digest part here properly...
+        if !digest.is_empty() {
+            let digest = proxmox::tools::hex_to_digest(&digest)?;
+            crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+        }
+    }
+
+    let delete: Vec<&str> = delete
+        .as_deref()
+        .unwrap_or("")
+        .split(&[' ', ',', ';', '\0'][..])
+        .collect();
+
+    config.update_from(updater, &delete)?;
+
+    crate::config::node::save_config(&config)
+}
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 21/27] add acme commands to proxmox-backup-manager

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/bin/proxmox-backup-manager.rs      |   1 +
 src/bin/proxmox_backup_manager/acme.rs | 415 +++++++++++++++++++++++++
 src/bin/proxmox_backup_manager/mod.rs  |   2 +
 3 files changed, 418 insertions(+)
 create mode 100644 src/bin/proxmox_backup_manager/acme.rs

diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index 105a11f8..522c800e 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -355,6 +355,7 @@ fn main() {
         .insert("user", user_commands())
         .insert("remote", remote_commands())
         .insert("garbage-collection", garbage_collection_commands())
+        .insert("acme", acme_mgmt_cli())
         .insert("cert", cert_mgmt_cli())
         .insert("subscription", subscription_commands())
         .insert("sync-job", sync_job_commands())
diff --git a/src/bin/proxmox_backup_manager/acme.rs b/src/bin/proxmox_backup_manager/acme.rs
new file mode 100644
index 00000000..317473cb
--- /dev/null
+++ b/src/bin/proxmox_backup_manager/acme.rs
@@ -0,0 +1,415 @@
+use std::io::Write;
+
+use anyhow::{bail, Error};
+use serde_json::Value;
+
+use proxmox::api::{api, cli::*, ApiHandler, RpcEnvironment};
+use proxmox::tools::fs::file_get_contents;
+
+use proxmox_backup::acme::AcmeClient;
+use proxmox_backup::api2;
+use proxmox_backup::config::acme::plugin::DnsPluginCoreUpdater;
+use proxmox_backup::config::acme::{AccountName, KNOWN_ACME_DIRECTORIES};
+
+pub fn acme_mgmt_cli() -> CommandLineInterface {
+    let cmd_def = CliCommandMap::new()
+        .insert("account", account_cli())
+        .insert("cert", cert_cli())
+        .insert("plugin", plugin_cli());
+
+    cmd_def.into()
+}
+
+#[api(
+    input: {
+        properties: {
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// List acme accounts.
+fn list_accounts(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+    let output_format = get_output_format(&param);
+
+    let info = &api2::config::acme::API_METHOD_LIST_ACCOUNTS;
+    let mut data = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    let options = default_table_format_options();
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            name: { type: AccountName },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// Show acme account information.
+async fn get_account(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+    let output_format = get_output_format(&param);
+
+    let info = &api2::config::acme::API_METHOD_GET_ACCOUNT;
+    let mut data = match info.handler {
+        ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await?,
+        _ => unreachable!(),
+    };
+
+    let options = default_table_format_options()
+        .column(
+            ColumnConfig::new("account")
+                .renderer(|value, _record| Ok(serde_json::to_string_pretty(value)?)),
+        )
+        .column(ColumnConfig::new("directory"))
+        .column(ColumnConfig::new("location"))
+        .column(ColumnConfig::new("tos"));
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            name: { type: AccountName },
+            contact: {
+                description: "List of email addresses.",
+            },
+            directory: {
+                type: String,
+                description: "The ACME Directory.",
+                optional: true,
+            },
+        }
+    }
+)]
+/// Register an ACME account.
+async fn register_account(
+    name: AccountName,
+    contact: String,
+    directory: Option<String>,
+) -> Result<(), Error> {
+    let directory = match directory {
+        Some(directory) => directory,
+        None => {
+            println!("Directory endpoints:");
+            for (i, dir) in KNOWN_ACME_DIRECTORIES.iter().enumerate() {
+                println!("{}) {}", i, dir.url);
+            }
+
+            println!("{}) Custom", KNOWN_ACME_DIRECTORIES.len());
+            let mut attempt = 0;
+            loop {
+                print!("Enter selection: ");
+                std::io::stdout().flush()?;
+
+                let mut input = String::new();
+                std::io::stdin().read_line(&mut input)?;
+
+                match input.trim().parse::<usize>() {
+                    Ok(n) if n < KNOWN_ACME_DIRECTORIES.len() => {
+                        break KNOWN_ACME_DIRECTORIES[n].url.to_owned();
+                    }
+                    Ok(n) if n == KNOWN_ACME_DIRECTORIES.len() => {
+                        input.clear();
+                        std::io::stdin().read_line(&mut input)?;
+                        break input.trim().to_owned();
+                    }
+                    _ => eprintln!("Invalid selection."),
+                }
+
+                attempt += 1;
+                if attempt >= 3 {
+                    bail!("Aborting.");
+                }
+            }
+        }
+    };
+
+    println!("Attempting to fetch Terms of Service from {:?}", directory);
+    let mut client = AcmeClient::new(directory.clone());
+    let tos_agreed = if let Some(tos_url) = client.terms_of_service_url().await? {
+        println!("Terms of Service: {}", tos_url);
+        print!("Do you agree to the above terms? [y|N]: ");
+        std::io::stdout().flush()?;
+        let mut input = String::new();
+        std::io::stdin().read_line(&mut input)?;
+        if input.trim().eq_ignore_ascii_case("y") {
+            true
+        } else {
+            false
+        }
+    } else {
+        false
+    };
+
+    println!("Attempting to register account with {:?}...", directory);
+
+    let account =
+        api2::config::acme::do_register_account(&mut client, &name, tos_agreed, contact, None)
+            .await?;
+
+    println!("Registration successful, account URL: {}", account.location);
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            name: { type: AccountName },
+            contact: {
+                description: "List of email addresses.",
+                type: String,
+                optional: true,
+            },
+        }
+    }
+)]
+/// Update an ACME account.
+async fn update_account(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+    let info = &api2::config::acme::API_METHOD_UPDATE_ACCOUNT;
+    let result = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    crate::wait_for_local_worker(result.as_str().unwrap()).await?;
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            name: { type: AccountName },
+            force: {
+                description:
+                    "Delete account data even if the server refuses to deactivate the account.",
+                type: Boolean,
+                optional: true,
+                default: false,
+            },
+        }
+    }
+)]
+/// Deactivate an ACME account.
+async fn deactivate_account(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+    let info = &api2::config::acme::API_METHOD_DEACTIVATE_ACCOUNT;
+    let result = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    crate::wait_for_local_worker(result.as_str().unwrap()).await?;
+
+    Ok(())
+}
+
+pub fn account_cli() -> CommandLineInterface {
+    let cmd_def = CliCommandMap::new()
+        .insert("list", CliCommand::new(&API_METHOD_LIST_ACCOUNTS))
+        .insert(
+            "register",
+            CliCommand::new(&API_METHOD_REGISTER_ACCOUNT).arg_param(&["name", "contact"]),
+        )
+        .insert(
+            "deactivate",
+            CliCommand::new(&API_METHOD_DEACTIVATE_ACCOUNT)
+                .arg_param(&["name"])
+                .completion_cb("name", crate::config::acme::complete_acme_account),
+        )
+        .insert(
+            "info",
+            CliCommand::new(&API_METHOD_GET_ACCOUNT)
+                .arg_param(&["name"])
+                .completion_cb("name", crate::config::acme::complete_acme_account),
+        )
+        .insert(
+            "update",
+            CliCommand::new(&API_METHOD_UPDATE_ACCOUNT)
+                .arg_param(&["name"])
+                .completion_cb("name", crate::config::acme::complete_acme_account),
+        );
+
+    cmd_def.into()
+}
+
+#[api(
+    input: {
+        properties: {
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// List acme plugins.
+fn list_plugins(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+    let output_format = get_output_format(&param);
+
+    let info = &api2::config::acme::API_METHOD_LIST_PLUGINS;
+    let mut data = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    let options = default_table_format_options();
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            id: {
+                type: String,
+                description: "Plugin ID",
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// Show acme account information.
+fn get_plugin(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+    let output_format = get_output_format(&param);
+
+    let info = &api2::config::acme::API_METHOD_GET_PLUGIN;
+    let mut data = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    let options = default_table_format_options();
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            type: {
+                type: String,
+                description: "The ACME challenge plugin type.",
+            },
+            core: {
+                type: DnsPluginCoreUpdater,
+                flatten: true,
+            },
+            data: {
+                type: String,
+                description: "File containing the plugin data.",
+            },
+        }
+    }
+)]
+/// Show acme account information.
+fn add_plugin(r#type: String, core: DnsPluginCoreUpdater, data: String) -> Result<(), Error> {
+    let data = base64::encode(&file_get_contents(&data)?);
+    api2::config::acme::add_plugin(r#type, core, data)?;
+    Ok(())
+}
+
+pub fn plugin_cli() -> CommandLineInterface {
+    use proxmox_backup::api2::config::acme;
+    let cmd_def = CliCommandMap::new()
+        .insert("list", CliCommand::new(&API_METHOD_LIST_PLUGINS))
+        .insert(
+            "config", // name comes from pve/pmg
+            CliCommand::new(&API_METHOD_GET_PLUGIN)
+                .arg_param(&["id"])
+                .completion_cb("id", crate::config::acme::complete_acme_plugin),
+        )
+        .insert(
+            "add",
+            CliCommand::new(&API_METHOD_ADD_PLUGIN)
+                .arg_param(&["type", "id"])
+                .completion_cb("id", crate::config::acme::complete_acme_plugin)
+                .completion_cb("type", crate::config::acme::complete_acme_plugin_type),
+        )
+        .insert(
+            "remove",
+            CliCommand::new(&acme::API_METHOD_DELETE_PLUGIN)
+                .arg_param(&["id"])
+                .completion_cb("id", crate::config::acme::complete_acme_plugin),
+        )
+        .insert(
+            "set",
+            CliCommand::new(&acme::API_METHOD_UPDATE_PLUGIN)
+                .arg_param(&["id"])
+                .completion_cb("id", crate::config::acme::complete_acme_plugin),
+        );
+
+    cmd_def.into()
+}
+
+#[api(
+    input: {
+        properties: {
+            force: {
+                description: "Force renewal even if the certificate does not expire soon.",
+                type: Boolean,
+                optional: true,
+                default: false,
+            },
+        },
+    },
+)]
+/// Order a new ACME certificate.
+async fn order_acme_cert(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+    if !param["force"].as_bool().unwrap_or(false) && !api2::node::certificates::cert_expires_soon()?
+    {
+        println!("Certificate does not expire within the next 30 days, not renewing.");
+        return Ok(());
+    }
+
+    let info = &api2::node::certificates::API_METHOD_RENEW_ACME_CERT;
+    let result = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    crate::wait_for_local_worker(result.as_str().unwrap()).await?;
+
+    Ok(())
+}
+
+#[api]
+/// Order a new ACME certificate.
+async fn revoke_acme_cert(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+    let info = &api2::node::certificates::API_METHOD_REVOKE_ACME_CERT;
+    let result = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    crate::wait_for_local_worker(result.as_str().unwrap()).await?;
+
+    Ok(())
+}
+
+pub fn cert_cli() -> CommandLineInterface {
+    let cmd_def = CliCommandMap::new()
+        .insert("order", CliCommand::new(&API_METHOD_ORDER_ACME_CERT))
+        .insert("revoke", CliCommand::new(&API_METHOD_REVOKE_ACME_CERT));
+
+    cmd_def.into()
+}
diff --git a/src/bin/proxmox_backup_manager/mod.rs b/src/bin/proxmox_backup_manager/mod.rs
index 900144aa..e574e4d4 100644
--- a/src/bin/proxmox_backup_manager/mod.rs
+++ b/src/bin/proxmox_backup_manager/mod.rs
@@ -1,5 +1,7 @@
 mod acl;
 pub use acl::*;
+mod acme;
+pub use acme::*;
 mod cert;
 pub use cert::*;
 mod datastore;
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 22/27] implement standalone acme validation

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/api2/node/certificates.rs |   2 +-
 src/config/acme/plugin.rs     | 144 ++++++++++++++++++++++++++++++----
 2 files changed, 129 insertions(+), 17 deletions(-)

diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
index 4dd75027..f6da31ec 100644
--- a/src/api2/node/certificates.rs
+++ b/src/api2/node/certificates.rs
@@ -342,7 +342,7 @@ async fn order_certificate(
         worker.log(format!("The validation for {} is pending", domain));
         let domain_config: &AcmeDomain = get_domain_config(&domain)?;
         let plugin_id = domain_config.plugin.as_deref().unwrap_or("standalone");
-        let plugin_cfg = plugins.get_plugin(plugin_id)?.ok_or_else(|| {
+        let mut plugin_cfg = plugins.get_plugin(plugin_id)?.ok_or_else(|| {
             format_err!("plugin '{}' for domain '{}' not found!", plugin_id, domain)
         })?;
 
diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
index b0c655c0..a2fa0490 100644
--- a/src/config/acme/plugin.rs
+++ b/src/config/acme/plugin.rs
@@ -1,8 +1,10 @@
 use std::future::Future;
 use std::pin::Pin;
 use std::process::Stdio;
+use std::sync::Arc;
 
 use anyhow::{bail, format_err, Error};
+use hyper::{Body, Request, Response};
 use lazy_static::lazy_static;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
@@ -117,7 +119,11 @@ pub struct DnsPlugin {
 
 impl DnsPlugin {
     pub fn decode_data(&self, output: &mut Vec<u8>) -> Result<(), Error> {
-        Ok(base64::decode_config_buf(&self.data, base64::URL_SAFE_NO_PAD, output)?)
+        Ok(base64::decode_config_buf(
+            &self.data,
+            base64::URL_SAFE_NO_PAD,
+            output,
+        )?)
     }
 }
 
@@ -250,7 +256,10 @@ impl PluginData {
                 let plugin: DnsPlugin = serde_json::from_value(data.clone())?;
                 Box::new(plugin)
             }
-            // "standalone" => todo!("standalone plugin"),
+            "standalone" => {
+                // this one has no config
+                Box::new(StandaloneServer::default())
+            }
             other => bail!("missing implementation for plugin type '{}'", other),
         }))
     }
@@ -264,29 +273,32 @@ pub trait AcmePlugin {
     /// Setup everything required to trigger the validation and return the corresponding validation
     /// URL.
     fn setup<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
-        &'a self,
+        &'a mut self,
         client: &'b mut AcmeClient,
         authorization: &'c Authorization,
         domain: &'d AcmeDomain,
     ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>>;
 
     fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
-        &'a self,
+        &'a mut self,
         client: &'b mut AcmeClient,
         authorization: &'c Authorization,
         domain: &'d AcmeDomain,
     ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>>;
 }
 
-impl DnsPlugin {
-    fn extract_challenge(authorization: &Authorization) -> Result<&Challenge, Error> {
-        authorization
-            .challenges
-            .iter()
-            .find(|ch| ch.ty == "dns-01")
-            .ok_or_else(|| format_err!("no supported challenge type (dns-01) found"))
-    }
+fn extract_challenge<'a>(
+    authorization: &'a Authorization,
+    ty: &str,
+) -> Result<&'a Challenge, Error> {
+    authorization
+        .challenges
+        .iter()
+        .find(|ch| ch.ty == ty)
+        .ok_or_else(|| format_err!("no supported challenge type (dns-01) found"))
+}
 
+impl DnsPlugin {
     async fn action<'a>(
         &self,
         client: &mut AcmeClient,
@@ -294,7 +306,7 @@ impl DnsPlugin {
         domain: &AcmeDomain,
         action: &str,
     ) -> Result<&'a str, Error> {
-        let challenge = Self::extract_challenge(authorization)?;
+        let challenge = extract_challenge(authorization, "dns-01")?;
         let mut stdin_data = client
             .dns_01_txt_value(
                 challenge
@@ -331,7 +343,9 @@ impl DnsPlugin {
             stdin.write_all(&stdin_data).await?;
             stdin.flush().await?;
             Ok::<_, std::io::Error>(())
-        }.await {
+        }
+        .await
+        {
             Ok(()) => (),
             Err(err) => {
                 if let Err(err) = child.kill().await {
@@ -357,7 +371,7 @@ impl DnsPlugin {
 
 impl AcmePlugin for DnsPlugin {
     fn setup<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
-        &'a self,
+        &'a mut self,
         client: &'b mut AcmeClient,
         authorization: &'c Authorization,
         domain: &'d AcmeDomain,
@@ -366,7 +380,7 @@ impl AcmePlugin for DnsPlugin {
     }
 
     fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
-        &'a self,
+        &'a mut self,
         client: &'b mut AcmeClient,
         authorization: &'c Authorization,
         domain: &'d AcmeDomain,
@@ -378,3 +392,101 @@ impl AcmePlugin for DnsPlugin {
         })
     }
 }
+
+#[derive(Default)]
+struct StandaloneServer {
+    abort_handle: Option<futures::future::AbortHandle>,
+}
+
+// In case the "order_certificates" future gets dropped between setup & teardown, let's also cancel
+// the HTTP listener on Drop:
+impl Drop for StandaloneServer {
+    fn drop(&mut self) {
+        self.stop();
+    }
+}
+
+impl StandaloneServer {
+    fn stop(&mut self) {
+        if let Some(abort) = self.abort_handle.take() {
+            abort.abort();
+        }
+    }
+}
+
+async fn standalone_respond(
+    req: Request<Body>,
+    path: Arc<String>,
+    key_auth: Arc<String>,
+) -> Result<Response<Body>, hyper::Error> {
+    if req.method() == hyper::Method::GET && req.uri().path() == path.as_str() {
+        Ok(Response::builder()
+            .status(http::StatusCode::OK)
+            .body(key_auth.as_bytes().to_vec().into())
+            .unwrap())
+    } else {
+        Ok(Response::builder()
+            .status(http::StatusCode::NOT_FOUND)
+            .body("Not found.".into())
+            .unwrap())
+    }
+}
+
+impl AcmePlugin for StandaloneServer {
+    fn setup<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
+        &'a mut self,
+        client: &'b mut AcmeClient,
+        authorization: &'c Authorization,
+        _domain: &'d AcmeDomain,
+    ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>> {
+        use hyper::server::conn::AddrIncoming;
+        use hyper::service::{make_service_fn, service_fn};
+
+        Box::pin(async move {
+            self.stop();
+
+            let challenge = extract_challenge(authorization, "http-01")?;
+            let token = challenge
+                .token()
+                .ok_or_else(|| format_err!("missing token in challenge"))?;
+            let key_auth = Arc::new(client.key_authorization(&token)?);
+            let path = Arc::new(format!("/.well-known/acme-challenge/{}", token));
+
+            let service = make_service_fn(move |_| {
+                let path = Arc::clone(&path);
+                let key_auth = Arc::clone(&key_auth);
+                async move {
+                    Ok::<_, hyper::Error>(service_fn(move |request| {
+                        standalone_respond(request, Arc::clone(&path), Arc::clone(&key_auth))
+                    }))
+                }
+            });
+
+            // `[::]:80` first, then `*:80`
+            let incoming = AddrIncoming::bind(&(([0u16; 8], 80).into()))
+                .or_else(|_| AddrIncoming::bind(&(([0u8; 4], 80).into())))?;
+
+            let server = hyper::Server::builder(incoming).serve(service);
+
+            let (future, abort) = futures::future::abortable(server);
+            self.abort_handle = Some(abort);
+            tokio::spawn(future);
+
+            Ok(challenge.url.as_str())
+        })
+    }
+
+    fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
+        &'a mut self,
+        _client: &'b mut AcmeClient,
+        _authorization: &'c Authorization,
+        _domain: &'d AcmeDomain,
+    ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>> {
+        Box::pin(async move {
+            if let Some(abort) = self.abort_handle.take() {
+                abort.abort();
+            }
+            Ok(())
+        })
+    }
+}
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 23/27] ui: add certificate & acme view

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
Changes to v1: Added a missing semicolon.

 www/Makefile                  |  1 +
 www/NavigationTree.js         |  6 +++
 www/config/CertificateView.js | 80 +++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100644 www/config/CertificateView.js

diff --git a/www/Makefile b/www/Makefile
index 2b847e74..f0b795ca 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -53,6 +53,7 @@ JSSRC=							\
 	config/SyncView.js				\
 	config/VerifyView.js				\
 	config/WebauthnView.js				\
+	config/CertificateView.js			\
 	window/ACLEdit.js				\
 	window/AddTfaRecovery.js			\
 	window/AddTotp.js				\
diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index 8b1b96d9..6035526c 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -50,6 +50,12 @@ Ext.define('PBS.store.NavigationStore', {
 			path: 'pbsRemoteView',
 			leaf: true,
 		    },
+		    {
+			text: gettext('Certificates'),
+			iconCls: 'fa fa-certificate',
+			path: 'pbsCertificateConfiguration',
+			leaf: true,
+		    },
 		    {
 			text: gettext('Subscription'),
 			iconCls: 'fa fa-support',
diff --git a/www/config/CertificateView.js b/www/config/CertificateView.js
new file mode 100644
index 00000000..d1e26632
--- /dev/null
+++ b/www/config/CertificateView.js
@@ -0,0 +1,80 @@
+Ext.define('PBS.config.CertificateConfiguration', {
+    extend: 'Ext.tab.Panel',
+    alias: 'widget.pbsCertificateConfiguration',
+
+    title: gettext('Certificates'),
+
+    border: false,
+    defaults: { border: false },
+
+    items: [
+       {
+           itemId: 'certificates',
+           xtype: 'pbsCertificatesView',
+       },
+       {
+           itemId: 'acme',
+           xtype: 'pbsACMEConfigView',
+       },
+    ],
+});
+
+Ext.define('PBS.config.CertificatesView', {
+    extend: 'Ext.panel.Panel',
+    alias: 'widget.pbsCertificatesView',
+
+    title: gettext('Certificates'),
+    border: false,
+    defaults: {
+	border: false,
+    },
+
+    items: [
+	{
+	    xtype: 'pmxCertificates',
+	    nodename: 'localhost',
+	    infoUrl: '/nodes/localhost/certificates/info',
+	    uploadButtons: [
+		{
+		    id: 'proxy.pem',
+		    url: '/nodes/localhost/certificates/custom',
+		    deletable: true,
+		    reloadUi: true,
+		},
+	    ],
+	},
+	{
+	    xtype: 'pmxACMEDomains',
+	    border: 0,
+	    url: `/nodes/localhost/config`,
+	    nodename: 'localhost',
+	    acmeUrl: '/config/acme',
+	    orderUrl: `/nodes/localhost/certificates/acme/certificate`,
+	    separateDomainEntries: true,
+	},
+    ],
+});
+
+Ext.define('PBS.ACMEConfigView', {
+    extend: 'Ext.panel.Panel',
+    alias: 'widget.pbsACMEConfigView',
+
+    title: gettext('ACME Accounts'),
+
+    //onlineHelp: 'sysadmin_certificate_management',
+
+    items: [
+       {
+           region: 'north',
+           border: false,
+           xtype: 'pmxACMEAccounts',
+           acmeUrl: '/config/acme',
+       },
+       {
+           region: 'center',
+           border: false,
+           xtype: 'pmxACMEPluginView',
+           acmeUrl: '/config/acme',
+       },
+    ],
+});
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 24/27] daily-update: check acme certificates

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/bin/proxmox-daily-update.rs | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs
index 83c6b80c..be3bfe44 100644
--- a/src/bin/proxmox-daily-update.rs
+++ b/src/bin/proxmox-daily-update.rs
@@ -50,13 +50,41 @@ async fn do_update(
     };
     wait_for_local_worker(upid.as_str().unwrap()).await?;
 
-    // TODO: certificate checks/renewal/... ?
+    match check_acme_certificates(rpcenv).await {
+        Ok(()) => (),
+        Err(err) => {
+            eprintln!("error checking certificates: {}", err);
+        }
+    }
 
     // TODO: cleanup tasks like in PVE?
 
     Ok(Value::Null)
 }
 
+async fn check_acme_certificates(rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+    let (config, _) = proxmox_backup::config::node::config()?;
+
+    // do we even have any acme domains configures?
+    if config.acme_domains().next().is_none() {
+        return Ok(());
+    }
+
+    if !api2::node::certificates::cert_expires_soon()? {
+        println!("Certificate does not expire within the next 30 days, not renewing.");
+        return Ok(());
+    }
+
+    let info = &api2::node::certificates::API_METHOD_RENEW_ACME_CERT;
+    let result = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(json!({}), info, rpcenv)?,
+        _ => unreachable!(),
+    };
+    wait_for_local_worker(result.as_str().unwrap()).await?;
+
+    Ok(())
+}
+
 fn main() {
     proxmox_backup::tools::setup_safe_path_env();
 
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 25/27] acme: create directories as needed

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/acme/client.rs        |  2 ++
 src/config/acme/mod.rs    | 27 +++++++++++++++++++++++++++
 src/config/acme/plugin.rs |  7 +++++--
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/src/acme/client.rs b/src/acme/client.rs
index dea2e3b0..83bfdf9e 100644
--- a/src/acme/client.rs
+++ b/src/acme/client.rs
@@ -124,6 +124,7 @@ impl AcmeClient {
 
         let _ = self.register_account(account).await?;
 
+        crate::config::acme::make_acme_account_dir()?;
         let account_path = account_path(account_name.as_ref());
         let file = OpenOptions::new()
             .write(true)
@@ -151,6 +152,7 @@ impl AcmeClient {
         let account_path = self.account_path.as_ref().ok_or_else(|| {
             format_err!("no account path set, cannot save upated account information")
         })?;
+        crate::config::acme::make_acme_account_dir()?;
         replace_file(
             account_path,
             &data,
diff --git a/src/config/acme/mod.rs b/src/config/acme/mod.rs
index ac409c20..0ed4974d 100644
--- a/src/config/acme/mod.rs
+++ b/src/config/acme/mod.rs
@@ -7,14 +7,41 @@ use serde::{Deserialize, Serialize};
 
 use proxmox::api::api;
 use proxmox::sys::error::SysError;
+use proxmox::tools::fs::CreateOptions;
 
 use crate::api2::types::{PROXMOX_SAFE_ID_FORMAT, PROXMOX_SAFE_ID_REGEX};
 use crate::tools::ControlFlow;
 
+pub(crate) const ACME_DIR: &str = configdir!("/acme/accounts");
 pub(crate) const ACME_ACCOUNT_DIR: &str = configdir!("/acme/accounts");
 
 pub mod plugin;
 
+// `const fn`ify this once it is supported in `proxmox`
+fn root_only() -> CreateOptions {
+    CreateOptions::new()
+        .owner(nix::unistd::ROOT)
+        .group(nix::unistd::Gid::from_raw(0))
+        .perm(nix::sys::stat::Mode::from_bits_truncate(0o700))
+}
+
+fn create_acme_subdir(dir: &str) -> nix::Result<()> {
+    match proxmox::tools::fs::create_dir(dir, root_only()) {
+        Ok(()) => Ok(()),
+        Err(err) if err.already_exists() => Ok(()),
+        Err(err) => Err(err),
+    }
+}
+
+pub(crate) fn make_acme_dir() -> nix::Result<()> {
+    create_acme_subdir(ACME_DIR)
+}
+
+pub(crate) fn make_acme_account_dir() -> nix::Result<()> {
+    make_acme_dir()?;
+    create_acme_subdir(ACME_ACCOUNT_DIR)
+}
+
 #[api(
     properties: {
         name: { type: String },
diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
index a2fa0490..f3cbc7fa 100644
--- a/src/config/acme/plugin.rs
+++ b/src/config/acme/plugin.rs
@@ -167,15 +167,17 @@ fn init() -> SectionConfig {
     config
 }
 
-pub const ACME_PLUGIN_CFG_FILENAME: &str = "/etc/proxmox-backup/acme/plugins.cfg";
-pub const ACME_PLUGIN_CFG_LOCKFILE: &str = "/etc/proxmox-backup/acme/.plugins.lck";
+pub const ACME_PLUGIN_CFG_FILENAME: &str = configdir!("/acme/plugins.cfg");
+pub const ACME_PLUGIN_CFG_LOCKFILE: &str = configdir!("/acme/.plugins.lck");
 const LOCK_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5);
 
 pub fn read_lock() -> Result<std::fs::File, Error> {
+    super::make_acme_dir()?;
     proxmox::tools::fs::open_file_locked(ACME_PLUGIN_CFG_LOCKFILE, LOCK_TIMEOUT, false)
 }
 
 pub fn write_lock() -> Result<std::fs::File, Error> {
+    super::make_acme_dir()?;
     proxmox::tools::fs::open_file_locked(ACME_PLUGIN_CFG_LOCKFILE, LOCK_TIMEOUT, true)
 }
 
@@ -196,6 +198,7 @@ pub fn config() -> Result<(PluginData, [u8; 32]), Error> {
 }
 
 pub fn save_config(config: &PluginData) -> Result<(), Error> {
+    super::make_acme_dir()?;
     let raw = CONFIG.write(ACME_PLUGIN_CFG_FILENAME, &config.data)?;
 
     let backup_user = crate::backup::backup_user()?;
-- 
2.20.1

Re: [pbs-devel] [PATCH v2 backup 25/27] acme: create directories as needed

[Wolfgang Bumiller]

Ah damn, I failed to include a fixup in this patch:

On Thu, Apr 22, 2021 at 04:02:11PM +0200, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>  src/acme/client.rs        |  2 ++
>  src/config/acme/mod.rs    | 27 +++++++++++++++++++++++++++
>  src/config/acme/plugin.rs |  7 +++++--
>  3 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/src/acme/client.rs b/src/acme/client.rs
> index dea2e3b0..83bfdf9e 100644
> --- a/src/acme/client.rs
> +++ b/src/acme/client.rs
> @@ -124,6 +124,7 @@ impl AcmeClient {
>  
>          let _ = self.register_account(account).await?;
>  
> +        crate::config::acme::make_acme_account_dir()?;
>          let account_path = account_path(account_name.as_ref());
>          let file = OpenOptions::new()
>              .write(true)
> @@ -151,6 +152,7 @@ impl AcmeClient {
>          let account_path = self.account_path.as_ref().ok_or_else(|| {
>              format_err!("no account path set, cannot save upated account information")
>          })?;
> +        crate::config::acme::make_acme_account_dir()?;
>          replace_file(
>              account_path,
>              &data,
> diff --git a/src/config/acme/mod.rs b/src/config/acme/mod.rs
> index ac409c20..0ed4974d 100644
> --- a/src/config/acme/mod.rs
> +++ b/src/config/acme/mod.rs
> @@ -7,14 +7,41 @@ use serde::{Deserialize, Serialize};
>  
>  use proxmox::api::api;
>  use proxmox::sys::error::SysError;
> +use proxmox::tools::fs::CreateOptions;
>  
>  use crate::api2::types::{PROXMOX_SAFE_ID_FORMAT, PROXMOX_SAFE_ID_REGEX};
>  use crate::tools::ControlFlow;
>  
> +pub(crate) const ACME_DIR: &str = configdir!("/acme/accounts");

^ This should obviously not include the `/accounts` part...

>  pub(crate) const ACME_ACCOUNT_DIR: &str = configdir!("/acme/accounts");

[pbs-devel] [PATCH v2 backup 26/27] acme: pipe plugin output to task log

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/api2/node/certificates.rs |  9 ++++--
 src/config/acme/plugin.rs     | 57 +++++++++++++++++++++++++++++------
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/src/api2/node/certificates.rs b/src/api2/node/certificates.rs
index f6da31ec..37a1f44a 100644
--- a/src/api2/node/certificates.rs
+++ b/src/api2/node/certificates.rs
@@ -347,11 +347,16 @@ async fn order_certificate(
         })?;
 
         worker.log("Setting up validation plugin");
-        let validation_url = plugin_cfg.setup(&mut acme, &auth, domain_config).await?;
+        let validation_url = plugin_cfg
+            .setup(&mut acme, &auth, domain_config, Arc::clone(&worker))
+            .await?;
 
         let result = request_validation(&worker, &mut acme, auth_url, validation_url).await;
 
-        if let Err(err) = plugin_cfg.teardown(&mut acme, &auth, domain_config).await {
+        if let Err(err) = plugin_cfg
+            .teardown(&mut acme, &auth, domain_config, Arc::clone(&worker))
+            .await
+        {
             worker.warn(format!(
                 "Failed to teardown plugin '{}' for domain '{}' - {}",
                 plugin_id, domain, err
diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs
index f3cbc7fa..da47b6e6 100644
--- a/src/config/acme/plugin.rs
+++ b/src/config/acme/plugin.rs
@@ -8,7 +8,7 @@ use hyper::{Body, Request, Response};
 use lazy_static::lazy_static;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
-use tokio::io::AsyncWriteExt;
+use tokio::io::{AsyncBufReadExt, AsyncRead, AsyncWriteExt, BufReader};
 use tokio::process::Command;
 
 use proxmox::api::{
@@ -24,6 +24,7 @@ use proxmox_acme_rs::{Authorization, Challenge};
 use crate::acme::AcmeClient;
 use crate::api2::types::PROXMOX_SAFE_ID_FORMAT;
 use crate::config::node::AcmeDomain;
+use crate::server::WorkerTask;
 
 const ACME_PATH: &str = "/usr/share/proxmox-acme/proxmox-acme";
 
@@ -280,6 +281,7 @@ pub trait AcmePlugin {
         client: &'b mut AcmeClient,
         authorization: &'c Authorization,
         domain: &'d AcmeDomain,
+        task: Arc<WorkerTask>,
     ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>>;
 
     fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
@@ -287,6 +289,7 @@ pub trait AcmePlugin {
         client: &'b mut AcmeClient,
         authorization: &'c Authorization,
         domain: &'d AcmeDomain,
+        task: Arc<WorkerTask>,
     ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>>;
 }
 
@@ -301,12 +304,29 @@ fn extract_challenge<'a>(
         .ok_or_else(|| format_err!("no supported challenge type (dns-01) found"))
 }
 
+async fn pipe_to_tasklog<T: AsyncRead + Unpin>(
+    pipe: T,
+    task: Arc<WorkerTask>,
+) -> Result<(), std::io::Error> {
+    let mut pipe = BufReader::new(pipe);
+    let mut line = String::new();
+    loop {
+        line.clear();
+        match pipe.read_line(&mut line).await {
+            Ok(0) => return Ok(()),
+            Ok(_) => task.log(line.as_str()),
+            Err(err) => return Err(err),
+        }
+    }
+}
+
 impl DnsPlugin {
     async fn action<'a>(
         &self,
         client: &mut AcmeClient,
         authorization: &'a Authorization,
         domain: &AcmeDomain,
+        task: Arc<WorkerTask>,
         action: &str,
     ) -> Result<&'a str, Error> {
         let challenge = extract_challenge(authorization, "dns-01")?;
@@ -339,20 +359,33 @@ impl DnsPlugin {
                 domain.alias.as_deref().unwrap_or(&domain.domain),
         ]);
 
-        let mut child = command.stdin(Stdio::piped()).spawn()?;
+        // We could use 1 socketpair, but tokio wraps them all in `File` internally causing `close`
+        // to be called separately on all of them without exception, so we need 3 pipes :-(
+
+        let mut child = command
+            .stdin(Stdio::piped())
+            .stdout(Stdio::piped())
+            .stderr(Stdio::piped())
+            .spawn()?;
 
         let mut stdin = child.stdin.take().expect("Stdio::piped()");
-        match async move {
+        let stdout = child.stdout.take().expect("Stdio::piped() failed?");
+        let stdout = pipe_to_tasklog(stdout, Arc::clone(&task));
+        let stderr = child.stderr.take().expect("Stdio::piped() failed?");
+        let stderr = pipe_to_tasklog(stderr, Arc::clone(&task));
+        let stdin = async move {
             stdin.write_all(&stdin_data).await?;
             stdin.flush().await?;
             Ok::<_, std::io::Error>(())
-        }
-        .await
-        {
-            Ok(()) => (),
+        };
+        match futures::try_join!(stdin, stdout, stderr) {
+            Ok(((), (), ())) => (),
             Err(err) => {
                 if let Err(err) = child.kill().await {
-                    eprintln!("failed to kill '{} {}' command: {}", ACME_PATH, action, err);
+                    task.log(format!(
+                        "failed to kill '{} {}' command: {}",
+                        ACME_PATH, action, err
+                    ));
                 }
                 bail!("'{}' failed: {}", ACME_PATH, err);
             }
@@ -378,8 +411,9 @@ impl AcmePlugin for DnsPlugin {
         client: &'b mut AcmeClient,
         authorization: &'c Authorization,
         domain: &'d AcmeDomain,
+        task: Arc<WorkerTask>,
     ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>> {
-        Box::pin(self.action(client, authorization, domain, "setup"))
+        Box::pin(self.action(client, authorization, domain, task, "setup"))
     }
 
     fn teardown<'fut, 'a: 'fut, 'b: 'fut, 'c: 'fut, 'd: 'fut>(
@@ -387,9 +421,10 @@ impl AcmePlugin for DnsPlugin {
         client: &'b mut AcmeClient,
         authorization: &'c Authorization,
         domain: &'d AcmeDomain,
+        task: Arc<WorkerTask>,
     ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>> {
         Box::pin(async move {
-            self.action(client, authorization, domain, "teardown")
+            self.action(client, authorization, domain, task, "teardown")
                 .await
                 .map(drop)
         })
@@ -441,6 +476,7 @@ impl AcmePlugin for StandaloneServer {
         client: &'b mut AcmeClient,
         authorization: &'c Authorization,
         _domain: &'d AcmeDomain,
+        _task: Arc<WorkerTask>,
     ) -> Pin<Box<dyn Future<Output = Result<&'c str, Error>> + Send + 'fut>> {
         use hyper::server::conn::AddrIncoming;
         use hyper::service::{make_service_fn, service_fn};
@@ -484,6 +520,7 @@ impl AcmePlugin for StandaloneServer {
         _client: &'b mut AcmeClient,
         _authorization: &'c Authorization,
         _domain: &'d AcmeDomain,
+        _task: Arc<WorkerTask>,
     ) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send + 'fut>> {
         Box::pin(async move {
             if let Some(abort) = self.abort_handle.take() {
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 27/27] api: acme: make account name optional in register call

[Wolfgang Bumiller]

we do this in PVE and PMG and default to "default", so let's
do it here too, this is mostly for UI compatibility

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/api2/config/acme.rs | 10 ++++++++--
 src/config/acme/mod.rs  | 14 +++++++++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/api2/config/acme.rs b/src/api2/config/acme.rs
index 4f72a94e..25a050e8 100644
--- a/src/api2/config/acme.rs
+++ b/src/api2/config/acme.rs
@@ -161,7 +161,10 @@ fn account_contact_from_string(s: &str) -> Vec<String> {
 #[api(
     input: {
         properties: {
-            name: { type: AccountName },
+            name: {
+                type: AccountName,
+                optional: true,
+            },
             contact: {
                 description: "List of email addresses.",
             },
@@ -183,7 +186,7 @@ fn account_contact_from_string(s: &str) -> Vec<String> {
 )]
 /// Register an ACME account.
 fn register_account(
-    name: AccountName,
+    name: Option<AccountName>,
     // Todo: email & email-list schema
     contact: String,
     tos_url: Option<String>,
@@ -192,6 +195,9 @@ fn register_account(
 ) -> Result<String, Error> {
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
 
+    let name = name
+        .unwrap_or_else(|| unsafe { AccountName::from_string_unchecked("default".to_string()) });
+
     if Path::new(&crate::config::acme::account_path(&name)).exists() {
         http_bail!(BAD_REQUEST, "account {:?} already exists", name);
     }
diff --git a/src/config/acme/mod.rs b/src/config/acme/mod.rs
index 0ed4974d..b4f14bff 100644
--- a/src/config/acme/mod.rs
+++ b/src/config/acme/mod.rs
@@ -5,7 +5,7 @@ use std::path::Path;
 use anyhow::{bail, format_err, Error};
 use serde::{Deserialize, Serialize};
 
-use proxmox::api::api;
+use proxmox::api::{api, schema::Schema};
 use proxmox::sys::error::SysError;
 use proxmox::tools::fs::CreateOptions;
 
@@ -85,6 +85,18 @@ impl AccountName {
     pub fn into_string(self) -> String {
         self.0
     }
+
+    pub fn from_string(name: String) -> Result<Self, Error> {
+        match &Self::API_SCHEMA {
+            Schema::String(s) => s.check_constraints(&name)?,
+            _ => unreachable!(),
+        }
+        Ok(Self(name))
+    }
+
+    pub unsafe fn from_string_unchecked(name: String) -> Self {
+        Self(name)
+    }
 }
 
 impl std::ops::Deref for AccountName {
-- 
2.20.1

Re: [pbs-devel] [PATCH v2 backup 00/27] Implements ACME support for PBS

[Dominic Jäger]

Creating, editing & removing accounts and plugins and ordering with HTTP
challenge with Let's Encrypt staging & production worked again for me (all in
the GUI).

On Thu, Apr 22, 2021 at 04:01:46PM +0200, Wolfgang Bumiller wrote:
> * Fixes a few issues found by Dominic:
>   * create the acme related directories if they do not exist yet
>   * pipe dns plugin command output to the task log
>   * made the account name optional in the register api call (since
>   * pve/pmg do it too)
All 3 worked for me now.

Tested-by: Dominic Jäger <d.jaeger@proxmox.com>

Re: [pbs-devel] [PATCH v2 backup 14/27] add acme config

[Dietmar Maurer]

> > 12 | use crate::acme::AcmeClient;
> >    |            ^^^^
> >    |            |
> >    |            unresolved import
> >    |            help: a similar path exists: `crate::config::acme`
> > 
> > error: aborting due to 2 previous errors
> 
> looks like I mis-ordered the patches, this will be in 17/27

I think there are also cyclic dependencies. Please can you resend the
patches in correct order, so that each step compiles?

Re: [pbs-devel] [PATCH v2 backup 14/27] add acme config

[Wolfgang Bumiller]

On Thu, Apr 29, 2021 at 02:34:28PM +0200, Dietmar Maurer wrote:
> > > 12 | use crate::acme::AcmeClient;
> > >    |            ^^^^
> > >    |            |
> > >    |            unresolved import
> > >    |            help: a similar path exists: `crate::config::acme`
> > > 
> > > error: aborting due to 2 previous errors
> > 
> > looks like I mis-ordered the patches, this will be in 17/27
> 
> I think there are also cyclic dependencies. Please can you resend the
> patches in correct order, so that each step compiles?

Done

Re: [pbs-devel] [PATCH v2 backup 14/27] add acme config

[Dietmar Maurer]

> On 04/29/2021 1:36 PM Wolfgang Bumiller <w.bumiller@proxmox.com> wrote:
> 
>  
> On Thu, Apr 29, 2021 at 12:48:52PM +0200, Dietmar Maurer wrote:
> > What is the purpose of this AccountName wrapper type?
> > 
> > I would prefer to simply use String...
> 
> If you want to... On the other hand I was thinking about adding a macro
> for the boilerplate string stuff.

I simply do not see any advantage in this case.