comment inline
On 10/18/19 11:28 AM, Fabian Grünbichler wrote:
so this got a bit longer than expected - just a high-level feedback, I
haven't actually tested anything yet since there are too many open
general design questions for that to make sense.
On October 14, 2019 1:08 pm, Wolfgang Link wrote:
This series also includes a new GIT repository on proxdev.
The path is staff/wl/pve-acme.
please send those as patches as well for the next iteration - otherwise
how are we supposed to discuss them here? some quick notes anyway:
I ask Thomas before, and he said it is ok for this new repository to go
this way.
- don't set permissions via patches (do we really need that? those files
are either unused or just sourced, not directly executed?)
No from a technical point of view, but lintian was thrown a warning
about the permission.
- use debian/patches, not manual patching via Makefile
Will do.
do we really need to package all of acme.sh? wouldn't a short stub with
the following methods be enough (note: this is for ALL plugins, if we
just want to support a subset this list would probably get smaller):
From a technical point of the problem no.
But in the future, a new plugin uses a not stub new function,
we have to search what feature is missing and stub it.
I guess it is much easier to maintain it if we have these small four
patches on top of the acme.sh repository.
Also, if we ship this package user could use it for alternative use like
appalling certs in containers.
I know this is not the main propose of this here, but it does not harm?
implemented:
_base64
_dbase64
_digest
_contains
_egrep_o
_endswith
_exists
_get
_post
_inithttp (depending on implementation of _get/_post)
_getfield
_head_n
_tail_n
_hex_dump
_hmac
_idn
_is_idn
_lower_case
_math
_normalizeJson
_sed_i
_sleep
_stat
_time
_url_encode
_utc_date
stubbed/aliased:
__green
__red
_err
_info
_log
_readaccountconf
_readaccountconf_mutable
no-ops:
_clearaccountconf
_cleardomainconf
_debug
_debug2
_debug3
_secure_debug
_secure_debug2
_secure_debug3
_saveaccountconf
_saveaccountconf_mutable
_save_conf
_savedomainconf
if we really go down the 'ship full copy of acme.sh' route, we should:
- not ship it as acme.sh IMHO (to prevent future namespace collisions,
e.g. pve-acme-sh-wrapper)
- disable/remove all the commands we don't use (to prevent users from using it)
- not ship the main script in /usr/sbin, but just in /usr/share/...
whether we ship our own wrapper with stubs, or use a patched acme.sh as
wrapper does not change anything w.r.t. the rest of the series though,
so we can leave that decision and just discuss the rest for now! it
would even be possible to switch one way or another in the future
without much hassle, since there is a single point where our code and
that wrapper meet.
The acme_sh project is used as DNS API plugin systems.
So we can reuse the already defiend plugins.
I deside to install the complett acme_sh scrips so somwone could use it
for alternative use.
but the whole reason this series exists is that we want to eliminate the
need for users to manually setup and use acme.sh instead of our
integration?
See the container case above.
I don't meant that the user should use it for Proxmox web server
certificates.
I'm not sure about where we save the information about the dns_plugin.
I deside to load it dynamicly like we do with ceph key for the storage.
Alternative we could save the information in the node config,
as I already specify in patch manger 6 Add dns_api_config.
The dns key file is not standiziert so ervery plugin expect other paramerts.
So I would say the dns key file has to be created from the user manually.
see acme.sh's _read_conf()/_save_conf() - the format is a simple
key=value, with the latter optionally and transparently base64
encoded/decoded (although none of the current dnsapi plugins seem to use
this feature).
see the long comment to pve-common #1
It is indeed a key=value format, but the keys count and the key itself
aren't the same.
If someone need a OVH key for testing please contact me.
Steps to test.
The api key file must exists on /etc/pve/priv/acme/dns_ovh.cred
1.) pvenode acme account register default<mail@example.invalid>
2.) pvenode config set --acme domains=test.linksystems.li,plugin=acme_sh
3.) pvenode cert order
These patches are only tested against the OVH API because of missing
alternative possibilities.
There are two bugs in this Series.
I send it anyway because they are not essential to the genrall functionality
and this is anyway an RFC and not the final version.
Known bugs:
Alias does not work in acme_sh.
Multiples domains will only use one domain in certivicate.
multiple domain support is pretty central to the rest of the series
though - see my comments regarding 'plugin' and 'alias' properties in
node config..
I guess the most common use case is the wildcard in the cluster what
lets-encrypt supports with DNS-Challenge.
https://letsencrypt.org/de/docs/challenge-types/
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
