public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* Re: [pbs-devel] [RFC backup 00/23] Implements ACME suport for PBS
@ 2021-04-20 10:53 Wolfgang Bumiller
  2021-04-21 11:56 ` Dominic Jäger
  0 siblings, 1 reply; 5+ messages in thread
From: Wolfgang Bumiller @ 2021-04-20 10:53 UTC (permalink / raw)
  To: Dominic Jäger, Proxmox Backup Server development discussion


> On 04/20/2021 12:27 PM Dominic Jäger <d.jaeger@proxmox.com> wrote:
> 
>  
> Creating the first account gives missing directory

should be an easy fix

> > TASK ERROR: failed to open "/etc/proxmox-backup/acme/accounts/test" for
> > writing: No such file or directory (os error 2)
> After manually adding it, the HTTP Challenged worked for me.
> 
> In the Window "Add: ACME DNS Plugin" choosing (or writing) something in the
> dropdown menu DNS API is not possible with only the PBS repositories
> configured.  It is necessary to install libproxmox-acme-perl from PVE
> repositories in addition.

Yeah we should turn the proxmox-acme repo into a split package and have the acme.sh
wrapper separate so we can depend/suggest that without pulling in the perl code.

> 
> Deleting a certificate shows a confirmation dialog with a truncated message:
> "Are you sure you want to remove the certificate used for"

That'll need some fixing in the widget toolkit.

> 
> In the window "Register Account" the textfield "Account Name" has the empty
> text "default".  As far as I know, we use empty texts for real default values.
> So this should be removed and get a validator (already in the GUI) instead.

GUI specifics aren't really in scope of this series as this just reuses the existing components.
So this should be handled separately.

> But the API rejects correctly: "parameter verification errors parameter 'name':
> parameter is missing and it is not optional."
> 
> Registering accounts for both staging and production works.  Ordering
> certificates with HTTP challenge generally works for both, too.  A few times
> the HTTP challenge required a manual retry. Maybe we could do something like
> increasing timeouts?

Not sure why that happens, would need to investigate more. But yeah it's possible
that setup/teardown are racing against the request, need to recheck the code.

> I couldn't set up PowerDNS yet & my domains were not fast enough, so finishing
> the DNS challenge testing remains todo.
> 
> Tested-by: Dominic Jäger <d.jaeger@proxmox.com>




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pbs-devel] [RFC backup 00/23] Implements ACME suport for PBS
  2021-04-20 10:53 [pbs-devel] [RFC backup 00/23] Implements ACME suport for PBS Wolfgang Bumiller
@ 2021-04-21 11:56 ` Dominic Jäger
  2021-04-21 12:19   ` Wolfgang Bumiller
  0 siblings, 1 reply; 5+ messages in thread
From: Dominic Jäger @ 2021-04-21 11:56 UTC (permalink / raw)
  To: Wolfgang Bumiller, Proxmox Backup Server development discussion
  Cc: Proxmox Backup Server development discussion

I am not sure how much of what I noticed today is for this series or general

On Tue, Apr 20, 2021 at 12:53:11PM +0200, Wolfgang Bumiller wrote:
> > In the window "Register Account" the textfield "Account Name" has the empty
> > text "default".  As far as I know, we use empty texts for real default values.
> > So this should be removed and get a validator (already in the GUI) instead.
> 
> GUI specifics aren't really in scope of this series as this just reuses the existing components.
> So this should be handled separately.

or would be magically solved by the packaging changes
> 
> Yeah we should turn the proxmox-acme repo into a split package and have the acme.sh
> wrapper separate so we can depend/suggest that without pulling in the perl code.

but a few things looked working to me in PVE but not PBS:

1. In the "Create: Domain" window, when attempting to create a duplicate entry:
PVE shows an error "duplicate domain" while PBS silently replaces the previous
entry

2. I installed libproxmox-acme-perl as disucssed. The dropdown list for DNS API
then did appear.  However, I haven't found a dropdown entry yet that made the
general API Data field change to the API specific fields. This includes APIs
like kappernet for which this certainly works in PVE.

3. When trying around I got some error like
> TASK ERROR: validating challenge 'https://acme-staging-v02.api.letsencrypt.org/acme/...' failed - status: Invalid
and after editing (=removing a line) the contents of the API Data field the
error message became less useful
>Placing ACME order
>Order URL: https://acme-staging-v02.api.letsencrypt.org/acme/order/....
>Getting authorization details from 'https://acme-staging-v02.api.letsencrypt.org/acme/authz-v3/...'
>The validation for dominicjaeger.com is pending
>Setting up validation plugin
>TASK ERROR: '/usr/share/proxmox-acme/proxmox-acme setup' exited with error (1)

Would it be possible to show more reasons for the error here?

4. The dropdown list is different for PBS and PVE. For example, PVE contains
"Cloudflare Managed DNS" while PBS contains just "cf"




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pbs-devel] [RFC backup 00/23] Implements ACME suport for PBS
  2021-04-21 11:56 ` Dominic Jäger
@ 2021-04-21 12:19   ` Wolfgang Bumiller
  0 siblings, 0 replies; 5+ messages in thread
From: Wolfgang Bumiller @ 2021-04-21 12:19 UTC (permalink / raw)
  To: Dominic Jäger; +Cc: Proxmox Backup Server development discussion

On Wed, Apr 21, 2021 at 01:56:18PM +0200, Dominic Jäger wrote:
> I am not sure how much of what I noticed today is for this series or general
> 
> On Tue, Apr 20, 2021 at 12:53:11PM +0200, Wolfgang Bumiller wrote:
> > > In the window "Register Account" the textfield "Account Name" has the empty
> > > text "default".  As far as I know, we use empty texts for real default values.
> > > So this should be removed and get a validator (already in the GUI) instead.
> > 
> > GUI specifics aren't really in scope of this series as this just reuses the existing components.
> > So this should be handled separately.
> 
> or would be magically solved by the packaging changes
> > 
> > Yeah we should turn the proxmox-acme repo into a split package and have the acme.sh
> > wrapper separate so we can depend/suggest that without pulling in the perl code.
> 
> but a few things looked working to me in PVE but not PBS:
> 
> 1. In the "Create: Domain" window, when attempting to create a duplicate entry:
> PVE shows an error "duplicate domain" while PBS silently replaces the previous
> entry

Sounds like a UI thing, can you also check PMG? PVE stores domains which
use the standalone http challenge differently to PMG and PBS, so the UI
modifies those in a different way which may be inconsistent.

> 2. I installed libproxmox-acme-perl as disucssed. The dropdown list for DNS API
> then did appear.  However, I haven't found a dropdown entry yet that made the
> general API Data field change to the API specific fields. This includes APIs
> like kappernet for which this certainly works in PVE.

The field schema is not available yet. This will come later.

> 3. When trying around I got some error like
> > TASK ERROR: validating challenge 'https://acme-staging-v02.api.letsencrypt.org/acme/...' failed - status: Invalid
> and after editing (=removing a line) the contents of the API Data field the
> error message became less useful
> >Placing ACME order
> >Order URL: https://acme-staging-v02.api.letsencrypt.org/acme/order/....
> >Getting authorization details from 'https://acme-staging-v02.api.letsencrypt.org/acme/authz-v3/...'
> >The validation for dominicjaeger.com is pending
> >Setting up validation plugin
> >TASK ERROR: '/usr/share/proxmox-acme/proxmox-acme setup' exited with error (1)
> 
> Would it be possible to show more reasons for the error here?

Ah, via the API the command output probably doesn't make it into the
task log. Should be able to pass that through, but what exactly you'll
get from that is up to the actual plugin you're using.

> 
> 4. The dropdown list is different for PBS and PVE. For example, PVE contains
> "Cloudflare Managed DNS" while PBS contains just "cf"

As with #2, that's just the detailed plugin schema not being available
right now.




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pbs-devel] [RFC backup 00/23] Implements ACME suport for PBS
  2021-04-16 13:34 Wolfgang Bumiller
@ 2021-04-20 10:27 ` Dominic Jäger
  0 siblings, 0 replies; 5+ messages in thread
From: Dominic Jäger @ 2021-04-20 10:27 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Wolfgang Bumiller

Creating the first account gives missing directory
> TASK ERROR: failed to open "/etc/proxmox-backup/acme/accounts/test" for
> writing: No such file or directory (os error 2)
After manually adding it, the HTTP Challenged worked for me.

In the Window "Add: ACME DNS Plugin" choosing (or writing) something in the
dropdown menu DNS API is not possible with only the PBS repositories
configured.  It is necessary to install libproxmox-acme-perl from PVE
repositories in addition.

Deleting a certificate shows a confirmation dialog with a truncated message:
"Are you sure you want to remove the certificate used for"

In the window "Register Account" the textfield "Account Name" has the empty
text "default".  As far as I know, we use empty texts for real default values.
So this should be removed and get a validator (already in the GUI) instead.
But the API rejects correctly: "parameter verification errors parameter 'name':
parameter is missing and it is not optional."

Registering accounts for both staging and production works.  Ordering
certificates with HTTP challenge generally works for both, too.  A few times
the HTTP challenge required a manual retry. Maybe we could do something like
increasing timeouts?

I couldn't set up PowerDNS yet & my domains were not fast enough, so finishing
the DNS challenge testing remains todo.

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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pbs-devel] [RFC backup 00/23] Implements ACME suport for PBS
@ 2021-04-16 13:34 Wolfgang Bumiller
  2021-04-20 10:27 ` Dominic Jäger
  0 siblings, 1 reply; 5+ messages in thread
From: Wolfgang Bumiller @ 2021-04-16 13:34 UTC (permalink / raw)
  To: pbs-devel

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 (23):
  systemd: add reload_unit
  add dns alias schema
  tools::fs::scan_subdir: use nix::Error instead of anyhow
  tools::http: generic 'fn request' and dedup agent string
  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
  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

 Cargo.toml                             |   3 +
 debian/control                         |   2 +
 src/acme/client.rs                     | 627 +++++++++++++++++++++
 src/acme/mod.rs                        |   2 +
 src/api2/config.rs                     |   2 +
 src/api2/config/acme.rs                | 719 +++++++++++++++++++++++++
 src/api2/node.rs                       |   4 +
 src/api2/node/certificates.rs          | 572 ++++++++++++++++++++
 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 | 414 ++++++++++++++
 src/bin/proxmox_backup_manager/mod.rs  |   2 +
 src/config.rs                          |  55 +-
 src/config/acl.rs                      |   2 +-
 src/config/acme/mod.rs                 | 198 +++++++
 src/config/acme/plugin.rs              | 492 +++++++++++++++++
 src/config/node.rs                     | 225 ++++++++
 src/lib.rs                             |   2 +
 src/tools.rs                           |  12 +
 src/tools/cert.rs                      |  41 +-
 src/tools/config/de.rs                 | 656 ++++++++++++++++++++++
 src/tools/config/mod.rs                |  89 +++
 src/tools/config/ser.rs                | 642 ++++++++++++++++++++++
 src/tools/fs.rs                        |   2 +-
 src/tools/http.rs                      |  10 +-
 src/tools/systemd.rs                   |  11 +
 www/Makefile                           |   1 +
 www/NavigationTree.js                  |   6 +
 www/config/CertificateView.js          |  80 +++
 32 files changed, 4972 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/de.rs
 create mode 100644 src/tools/config/mod.rs
 create mode 100644 src/tools/config/ser.rs
 create mode 100644 www/config/CertificateView.js

-- 
2.20.1





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-04-21 12:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-20 10:53 [pbs-devel] [RFC backup 00/23] Implements ACME suport for PBS Wolfgang Bumiller
2021-04-21 11:56 ` Dominic Jäger
2021-04-21 12:19   ` Wolfgang Bumiller
  -- strict thread matches above, loose matches on Subject: below --
2021-04-16 13:34 Wolfgang Bumiller
2021-04-20 10:27 ` Dominic Jäger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal