[pve-devel] [PATCH proxmox-acme 0/3] update acme.sh and fix #3536 and #3546

[pve-devel] [PATCH proxmox-acme 0/3] update acme.sh and fix #3536 and #3546

[Stoiko Ivanov]

This patchset started out as attempt to add comfortable proxy-handling to
our acme client(s) and to address #3536 and #3546, but in it's current form
only fixes the two issues.

patch 1/3 is independent of the others and enables users to interact with an
ACME provider via proxy on PVE on the commandline (by exporting the
https_proxy environment variable).

the remaining patches simply update the acme.sh submodule, add the 2 new
dns-plugins to our schema.json file and and port over retrying GET and POST
requests from acme.sh.

Tested on my PVE-node with a domain of mine and the powerdns api:
* setting https_proxy (and having a squid configured on a guest) does not
  cause the `pvenode acme cert renew -force` to abort due to taint-checks
* the content type of the PATCH requests is application/json insted of
  application/x-www-form-urlencoded

Stoiko Ivanov (3):
  acme client: fix #3536 untaint data returned from acme server
  update to acme.sh dns plugins to 3.0.0
  plugin-caller: pull in changes from upstream 3.0.0

 src/Makefile                  |  2 ++
 src/PVE/ACME.pm               | 12 +++++--
 src/acme.sh                   |  2 +-
 src/dns-challenge-schema.json |  2 ++
 src/proxmox-acme              | 62 +++++++++++++++++++++++++++++++++--
 5 files changed, 75 insertions(+), 5 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH proxmox-acme 1/3] acme client: fix #3536 untaint data returned from acme server

[Stoiko Ivanov]

The data returned from the acme server (e.g. boulder) should be
considered tainted.

To see which places might need untainted I checked where $self->{ua}
was used (in $self->do) and a quick scan identified __get_result as
the consumer of the tainted data.
In all but one uses the data is decoded from json (which would die if the
result is not valid json).

The remaining use-case yields a certificate in PEM format (and is
handled at the caller of __get_result).

The issue is currently only visible if a proxy is set, because AFAICT
somewhere in SSLeay (or IO::Socket::SSL, which uses SSLeay) a taint
flag is not set on the return value.

A reproducer for the issue:
```

use strict;
use warnings;

use HTTP::Request;
use LWP::UserAgent;

$ENV{PATH} = "/usr/bin:/bin";

my $ua = LWP::UserAgent->new(env_proxy => 0);
my $request = HTTP::Request->new('GET', 'https://google.com/');
my $resp = $ua->request($request);
my $text = substr($resp->decoded_content, 0, 5);;
system("echo \"$text\""); # does work
$request = HTTP::Request->new('GET', 'http://neverssl.com/');
$resp = $ua->request($request);
$text = substr($resp->decoded_content, 0, 5);;
system("echo \"$text\""); # does not work
```

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PVE/ACME.pm | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
index c5b8d66..265482d 100644
--- a/src/PVE/ACME.pm
+++ b/src/PVE/ACME.pm
@@ -69,7 +69,9 @@ sub tojs($;%) { # shortcut for to_json with utf8=>1
 }
 
 sub fromjs($) {
-    return from_json($_[0]);
+    my ($data) = @_;
+    ($data) = ($data =~ /^(.*)$/s); # untaint from_json croaks on error anyways.
+    return from_json($data);
 }
 
 sub fatal($$;$$) {
@@ -449,7 +451,13 @@ sub get_certificate {
        if !$order->{certificate};
 
     my $r = $self->do(POST => $order->{certificate}, '');
-    my $return = eval { __get_result($r, 200, 1); };
+    my $return = eval {
+	my $res = __get_result($r, 200, 1);
+	if ($res =~ /^(-----BEGIN CERTIFICATE-----)(.+)(-----END CERTIFICATE-----)$/s) { # untaint
+	    return $1 . $2 . $3;
+	}
+	die "Server reply does not look like a PEM encoded certificate\n";
+    };
     $self->fatal("POST of '$order->{certificate}' failed - $@", $r) if $@;
     return $return;
 }
-- 
2.30.2

[pve-devel] [PATCH proxmox-acme 2/3] update to acme.sh dns plugins to 3.0.0

[Stoiko Ivanov]

fixes #3546

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/Makefile                  | 2 ++
 src/acme.sh                   | 2 +-
 src/dns-challenge-schema.json | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/Makefile b/src/Makefile
index 16166fe..2503683 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -14,6 +14,7 @@ ACME_SOURCES = \
 	dnsapi/dns_aurora.sh \
 	dnsapi/dns_autodns.sh \
 	dnsapi/dns_aws.sh \
+	dnsapi/dns_azion.sh \
 	dnsapi/dns_azure.sh \
 	dnsapi/dns_cf.sh \
 	dnsapi/dns_clouddns.sh \
@@ -93,6 +94,7 @@ ACME_SOURCES = \
 	dnsapi/dns_nsone.sh \
 	dnsapi/dns_nsupdate.sh \
 	dnsapi/dns_nw.sh \
+	dnsapi/dns_oci.sh \
 	dnsapi/dns_one.sh \
 	dnsapi/dns_online.sh \
 	dnsapi/dns_openprovider.sh \
diff --git a/src/acme.sh b/src/acme.sh
index 9293bcf..d84da5b 160000
--- a/src/acme.sh
+++ b/src/acme.sh
@@ -1 +1 @@
-Subproject commit 9293bcfb1cd5a56c6cede3f5f46af8529ee99624
+Subproject commit d84da5bdbf2ea190977ad66669173e3256380d2a
diff --git a/src/dns-challenge-schema.json b/src/dns-challenge-schema.json
index 559077f..ddfe2b2 100644
--- a/src/dns-challenge-schema.json
+++ b/src/dns-challenge-schema.json
@@ -90,6 +90,7 @@
       },
       "name" : "Amazon Route53 (AWS)"
    },
+   "azion" : {},
    "azure" : {},
    "cf" : {
       "description" : "Either provide global account key and email, or CF API token and Account ID.",
@@ -238,6 +239,7 @@
    "nsone" : {},
    "nsupdate" : {},
    "nw" : {},
+   "oci" : {},
    "one" : {},
    "online" : {},
    "openprovider" : {},
-- 
2.30.2

[pve-devel] [PATCH proxmox-acme 3/3] plugin-caller: pull in changes from upstream 3.0.0

[Stoiko Ivanov]

Commits ae3dda0f8fc3071495cd1e8dff0fe4a339febb1c and
d70b759cb9c5b413cce92e65e841a54a65813962

implementing retrying get and post requests seem worth pulling in.

From a quick look through the diff the remaining changes (between
2.9.0 and 3.0.0) should not be relevant for us

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/proxmox-acme | 62 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/src/proxmox-acme b/src/proxmox-acme
index 6cc7b5f..a00d23a 100644
--- a/src/proxmox-acme
+++ b/src/proxmox-acme
@@ -222,6 +222,8 @@ _resethttp() {
   :
 }
 
+_HTTP_MAX_RETRY=8
+
 # body  url [needbase64] [POST|PUT|DELETE] [ContentType]
 _post() {
   body="$1"
@@ -229,6 +231,33 @@ _post() {
   needbase64="$3"
   httpmethod="$4"
   _postContentType="$5"
+  _sleep_retry_sec=1
+  _http_retry_times=0
+  _hcode=0
+  while [ "${_http_retry_times}" -le "$_HTTP_MAX_RETRY" ]; do
+    [ "$_http_retry_times" = "$_HTTP_MAX_RETRY" ]
+    _lastHCode="$?"
+    _debug "Retrying post"
+    _post_impl "$body" "$_post_url" "$needbase64" "$httpmethod" "$_postContentType" "$_lastHCode"
+    _hcode="$?"
+    _debug _hcode "$_hcode"
+    if [ "$_hcode" = "0" ]; then
+      break
+    fi
+    _http_retry_times=$(_math $_http_retry_times + 1)
+    _sleep $_sleep_retry_sec
+  done
+  return $_hcode
+}
+
+# body  url [needbase64] [POST|PUT|DELETE] [ContentType] [displayError]
+_post_impl() {
+  body="$1"
+  _post_url="$2"
+  needbase64="$3"
+  httpmethod="$4"
+  _postContentType="$5"
+  displayError="$6"
 
   if [ -z "$httpmethod" ]; then
     httpmethod="POST"
@@ -272,7 +301,9 @@ _post() {
   fi
   _ret="$?"
   if [ "$_ret" != "0" ]; then
-    _err "Please refer to https://curl.haxx.se/libcurl/c/libcurl-errors.html for error code: $_ret"
+    if [ -z "$displayError" ] || [ "$displayError" = "0" ]; then
+      _err "Please refer to https://curl.haxx.se/libcurl/c/libcurl-errors.html for error code: $_ret"
+    fi
   fi
   printf "%s" "$response"
   return $_ret
@@ -283,6 +314,31 @@ _get() {
   url="$1"
   onlyheader="$2"
   t="$3"
+  _sleep_retry_sec=1
+  _http_retry_times=0
+  _hcode=0
+  while [ "${_http_retry_times}" -le "$_HTTP_MAX_RETRY" ]; do
+    [ "$_http_retry_times" = "$_HTTP_MAX_RETRY" ]
+    _lastHCode="$?"
+    _debug "Retrying GET"
+    _get_impl "$url" "$onlyheader" "$t" "$_lastHCode"
+    _hcode="$?"
+    _debug _hcode "$_hcode"
+    if [ "$_hcode" = "0" ]; then
+      break
+    fi
+    _http_retry_times=$(_math $_http_retry_times + 1)
+    _sleep $_sleep_retry_sec
+  done
+  return $_hcode
+}
+
+# url getheader timeout displayError
+_get_impl() {
+  url="$1"
+  onlyheader="$2"
+  t="$3"
+  displayError="$4"
 
   _CURL="curl -L --silent --dump-header $HTTP_HEADER -g "
   if [ "$HTTPS_INSECURE" ]; then
@@ -298,7 +354,9 @@ _get() {
   fi
   ret=$?
   if [ "$ret" != "0" ]; then
-    _err "Please refer to https://curl.haxx.se/libcurl/c/libcurl-errors.html for error code: $ret"
+    if [ -z "$displayError" ] || [ "$displayError" = "0" ]; then
+      _err "Please refer to https://curl.haxx.se/libcurl/c/libcurl-errors.html for error code: $ret"
+    fi
   fi
   return $ret
 }
-- 
2.30.2

[pve-devel] applied: [PATCH proxmox-acme 0/3] update acme.sh and fix #3536 and #3546

[Fabian Grünbichler]

series, thanks!

On August 6, 2021 5:44 pm, Stoiko Ivanov wrote:
> This patchset started out as attempt to add comfortable proxy-handling to
> our acme client(s) and to address #3536 and #3546, but in it's current form
> only fixes the two issues.
> 
> patch 1/3 is independent of the others and enables users to interact with an
> ACME provider via proxy on PVE on the commandline (by exporting the
> https_proxy environment variable).
> 
> the remaining patches simply update the acme.sh submodule, add the 2 new
> dns-plugins to our schema.json file and and port over retrying GET and POST
> requests from acme.sh.
> 
> Tested on my PVE-node with a domain of mine and the powerdns api:
> * setting https_proxy (and having a squid configured on a guest) does not
>   cause the `pvenode acme cert renew -force` to abort due to taint-checks
> * the content type of the PATCH requests is application/json insted of
>   application/x-www-form-urlencoded
> 
> Stoiko Ivanov (3):
>   acme client: fix #3536 untaint data returned from acme server
>   update to acme.sh dns plugins to 3.0.0
>   plugin-caller: pull in changes from upstream 3.0.0
> 
>  src/Makefile                  |  2 ++
>  src/PVE/ACME.pm               | 12 +++++--
>  src/acme.sh                   |  2 +-
>  src/dns-challenge-schema.json |  2 ++
>  src/proxmox-acme              | 62 +++++++++++++++++++++++++++++++++--
>  5 files changed, 75 insertions(+), 5 deletions(-)
> 
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>