* [PATCH proxmox-acme v3 1/1] fix #7749: always serialize in same order
@ 2026-08-12 11:51 Thomas Ellmenreich
2026-08-14 10:11 ` Elias Huhsovitz
0 siblings, 1 reply; 2+ messages in thread
From: Thomas Ellmenreich @ 2026-08-12 11:51 UTC (permalink / raw)
To: pve-devel; +Cc: Thomas Ellmenreich
All objects are now serialized with "canonical => 1" so that the
resulting bytes are directly comparable. Perls randomized hash key
orders lead to different serialization outputs even with the same input.
The 'tojs' function also now explicitly overrides the utf8 and canonical
options to make sure that they are always set.
Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
Serializing acme responses to json, because of perls randomized hash key order,
would lead to unequal serialized responses if they were compared as bytes. By
setting "canonical => 1" the serializations now always have the same order and
can thus be compared bytewise.
I have tested the patch against a local instance of HashiCorp Vault PKI and the
EAB registration works.
It seems that "canonical" can have performance implications when serialising
larger objects, but since the objects produced in our acme client are
relatively small, I consider this acceptable.
Changes since v2 (thanks @Elias)
--------------------------------
- The options were beeing passed to the 'to_json' function as a hash
instead of a hash reference. This is now fixed.
src/PVE/ACME.pm | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
index e6fb9c2..3756ac0 100644
--- a/src/PVE/ACME.pm
+++ b/src/PVE/ACME.pm
@@ -64,9 +64,13 @@ sub encode($) { # acme requires 'base64url' encoding
return encode_base64url($_[0]);
}
-sub tojs($;%) { # shortcut for to_json with utf8=>1
- my ($data, %data) = @_;
- return to_json($data, { utf8 => 1, %data });
+sub tojs($;%) { # shortcut for to_json with utf8=>1 and canonical=>1
+ my ($data, %opts) = @_;
+
+ $opts{utf8} = 1;
+ $opts{canonical} = 1;
+
+ return to_json($data, \%opts);
}
sub fromjs($) {
@@ -155,7 +159,7 @@ sub save {
}
# pretty => 1 for readability
# canonical => 1 to reduce churn
- file_set_contents($self->{path}, tojs($o, pretty => 1, canonical => 1));
+ file_set_contents($self->{path}, tojs($o, pretty => 1));
}
# Load serialized account JSON file into $self
@@ -196,7 +200,7 @@ sub jwk {
sub jwk_thumbprint {
my ($self) = @_;
my $jwk = $self->jwk(1); # $pure = 1
- return encode(sha256(tojs($jwk, canonical => 1))); # canonical sorts
+ return encode(sha256(tojs($jwk)));
}
# A key authorization string in acme is a challenge token dot-connected with
--
2.47.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH proxmox-acme v3 1/1] fix #7749: always serialize in same order
2026-08-12 11:51 [PATCH proxmox-acme v3 1/1] fix #7749: always serialize in same order Thomas Ellmenreich
@ 2026-08-14 10:11 ` Elias Huhsovitz
0 siblings, 0 replies; 2+ messages in thread
From: Elias Huhsovitz @ 2026-08-14 10:11 UTC (permalink / raw)
To: Thomas Ellmenreich, pve-devel
I tested the changes in 2 ways:
Test Process
============
Calling tojs
------------
I directly called the updated tojs subroutine with different data (hash
references) and optional paramters.
The serialization order remained constant.
pebble (letsencrypt)
--------------------
I installed pebble from [1].
Ran
`PEBBLE_VA_ALWAYS_VALID=1 PEBBLE_WFE_NONCEREJECT=0 pebble -config ./test/config/pebble-config.json`
initialized a new ACME client againt pebble using
`my $acme = PVE::ACME->new('/tmp/pebble_test_account.json', "https://192.168.16.88:14000/dir")`
then called `acme->new_account` and `$acme->new_order()`
Pebble output seems fine:
Pebble 2026/08/14 12:08:26 GET /dir -> calling handler()
Pebble 2026/08/14 12:08:26 GET /nonce-plz -> calling handler()
Pebble 2026/08/14 12:08:26 POST /sign-me-up -> calling handler()
Pebble 2026/08/14 12:08:26 There are now 1 accounts in memory
Pebble 2026/08/14 12:08:26 POST /order-plz -> calling handler()
Pebble 2026/08/14 12:08:26 There are now 1 authorizations in the db
Pebble 2026/08/14 12:08:26 Added order "d9M7evJN3JqyVwvKZxCBCrun1l9gKV7cegZDTYRKIRY" to the db
Pebble 2026/08/14 12:08:26 There are now 1 orders in the db
[1] https://github.com/letsencrypt/pebble
Therefore consider this patch.
Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Tested-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
On Wed Aug 12, 2026 at 1:51 PM CEST, Thomas Ellmenreich wrote:
> All objects are now serialized with "canonical => 1" so that the
> resulting bytes are directly comparable. Perls randomized hash key
> orders lead to different serialization outputs even with the same input.
>
> The 'tojs' function also now explicitly overrides the utf8 and canonical
> options to make sure that they are always set.
>
> Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
> ---
> Serializing acme responses to json, because of perls randomized hash key order,
> would lead to unequal serialized responses if they were compared as bytes. By
> setting "canonical => 1" the serializations now always have the same order and
> can thus be compared bytewise.
>
> I have tested the patch against a local instance of HashiCorp Vault PKI and the
> EAB registration works.
>
> It seems that "canonical" can have performance implications when serialising
> larger objects, but since the objects produced in our acme client are
> relatively small, I consider this acceptable.
>
> Changes since v2 (thanks @Elias)
> --------------------------------
>
> - The options were beeing passed to the 'to_json' function as a hash
> instead of a hash reference. This is now fixed.
>
>
> src/PVE/ACME.pm | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
> index e6fb9c2..3756ac0 100644
> --- a/src/PVE/ACME.pm
> +++ b/src/PVE/ACME.pm
> @@ -64,9 +64,13 @@ sub encode($) { # acme requires 'base64url' encoding
> return encode_base64url($_[0]);
> }
>
> -sub tojs($;%) { # shortcut for to_json with utf8=>1
> - my ($data, %data) = @_;
> - return to_json($data, { utf8 => 1, %data });
> +sub tojs($;%) { # shortcut for to_json with utf8=>1 and canonical=>1
> + my ($data, %opts) = @_;
> +
> + $opts{utf8} = 1;
> + $opts{canonical} = 1;
> +
> + return to_json($data, \%opts);
> }
>
> sub fromjs($) {
> @@ -155,7 +159,7 @@ sub save {
> }
> # pretty => 1 for readability
> # canonical => 1 to reduce churn
> - file_set_contents($self->{path}, tojs($o, pretty => 1, canonical => 1));
> + file_set_contents($self->{path}, tojs($o, pretty => 1));
> }
>
> # Load serialized account JSON file into $self
> @@ -196,7 +200,7 @@ sub jwk {
> sub jwk_thumbprint {
> my ($self) = @_;
> my $jwk = $self->jwk(1); # $pure = 1
> - return encode(sha256(tojs($jwk, canonical => 1))); # canonical sorts
> + return encode(sha256(tojs($jwk)));
> }
>
> # A key authorization string in acme is a challenge token dot-connected with
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-14 10:11 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12 11:51 [PATCH proxmox-acme v3 1/1] fix #7749: always serialize in same order Thomas Ellmenreich
2026-08-14 10:11 ` Elias Huhsovitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox