From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 86E779C463 for ; Tue, 24 Oct 2023 10:33:01 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6F2651D08C for ; Tue, 24 Oct 2023 10:33:01 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 24 Oct 2023 10:33:00 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 87CEC44B00 for ; Tue, 24 Oct 2023 10:33:00 +0200 (CEST) Date: Tue, 24 Oct 2023 10:32:53 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20231023131808.172494-1-f.gleumes@proxmox.com> <20231023131808.172494-2-f.gleumes@proxmox.com> In-Reply-To: <20231023131808.172494-2-f.gleumes@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1698133256.b0dljkont3.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.062 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH acme 1/5] fix #4497: add support for external account bindings X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Oct 2023 08:33:01 -0000 On October 23, 2023 3:18 pm, Folke Gleumes wrote: > implementation acording to rfc855 section 7.3.4 >=20 > Signed-off-by: Folke Gleumes > --- > src/PVE/ACME.pm | 43 +++++++++++++++++++++++++++++++++++-------- > 1 file changed, 35 insertions(+), 8 deletions(-) >=20 > diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm > index 3f66182..f65729a 100644 > --- a/src/PVE/ACME.pm > +++ b/src/PVE/ACME.pm > @@ -7,10 +7,10 @@ use POSIX; > =20 > use Data::Dumper; > use Date::Parse; > -use MIME::Base64 qw(encode_base64url); > +use MIME::Base64 qw(encode_base64url decode_base64); > use File::Path qw(make_path); > use JSON; > -use Digest::SHA qw(sha256 sha256_hex); > +use Digest::SHA qw(sha256 sha256_hex hmac_sha256); > =20 > use HTTP::Request; > use LWP::UserAgent; > @@ -251,6 +251,28 @@ sub jws { > }; > } > =20 > +# EAB signing using the HS256 alg (HMAC/SHA256). > +sub eab { > + my ($self, $eab_kid, $eab_hmac_key, $url) =3D @_; > + > + my $protected =3D { > + alg =3D> 'HS256', > + kid =3D> $eab_kid, > + url =3D> $url, > + }; > + $protected =3D encode(tojs($protected)); > + > + my $payload =3D encode(tojs($self->jwk())); > + my $signdata =3D "$protected.$payload"; > + my $signature =3D encode(hmac_sha256($signdata, $eab_hmac_key)); > + > + return { > + protected =3D> $protected, > + payload =3D> $payload, > + signature =3D> $signature, > + }; > +} > + > sub __get_result { > my ($resp, $code, $plain) =3D @_; > =20 > @@ -300,8 +322,8 @@ sub list_methods { > } > =20 > # return (optional) meta directory entry. > -# this is public because it might contain the ToS, which should be displ= ayed > -# and agreed to before creating an account > +# this is public because it might contain the ToS and EAB requirements, > +# which have to be considered before creating an account > sub get_meta { > my ($self) =3D @_; > my $methods =3D $self->__get_methods(); > @@ -329,21 +351,26 @@ sub __new_account { > return $self->{account}; > } > =20 > -# Create a new account using data in %info. > +# Create a new account using data in $info. this change (and the related ones below) are actually not required (and would be inconsistent with the signature of the other account methods here) - see comment in the patch for pve-manager. > # Optionally pass $tos_url to agree to the given Terms of Service > # POST to newAccount endpoint > # Expects a '201 Created' reply > # Saves and returns the account data > sub new_account { > - my ($self, $tos_url, %info) =3D @_; > + my ($self, $tos_url, $info) =3D @_; > my $url =3D $self->_method('newAccount'); > =20 > + if ($info->{'eab'}) { > + my $eab_hmac_key =3D decode_base64($info->{'eab'}->{hmac_key}); > + $info->{externalAccountBinding} =3D $self->eab($info->{'eab'}->{kid}, $= eab_hmac_key, $url); this means that `info` now contains both the binding, but also the input including the KID (okay, this is contained in the binding as well, so just duplicate info) and the HMAC key, which is supposed to be secret. granted, it is a secret given to the user by the CA over some channel, and we only send it back to the CA, but some ACME implementations might still reject the request because of the unexpected contents. and if the user ever mixes up the CAs they are talking to, they might accidentally leak the secret to the wrong entitiy. since `info` is directly translated to the new_account request contents, it might be better to pass in the EAB parameters on their own, and make this sub take my ($self, $tos_url, $eab, %info) =3D @_; if $eab is undef -> no EAB. if it is set, generate the binding and put it into %info for further passing to the ACME provider. alternatively, it would also work to combine $tos_url and $eab into a new $account_params or $params hash, if we think that more parameters might be added in the future, or if we plan on merging new_account and init, like we do in PMG/PBS. or, as third option, we could switch the public sub new_account to take my ($self, $tos_url, $contact, $eab) =3D @_; or my ($self, $tos_url, $contact, $eab, $rsa_bits) =3D @_; which would be more aligned with how PMG and PBS look like. almost all of the variants are a breaking change (except if we keep the signature as is, and properly extract the eab member if it is set) that require a double check for any reverse dependencies. there's at least one internal tool that uses this as well that would need to be updated, for example. > + } > + > if ($tos_url) { > $self->{tos} =3D $tos_url; > - $info{termsOfServiceAgreed} =3D JSON::true; > + $info->{termsOfServiceAgreed} =3D JSON::true; > } > =20 > - return $self->__new_account(201, $url, 1, %info); > + return $self->__new_account(201, $url, 1, %{$info}); > } > =20 > # Update existing account with new %info > --=20 > 2.39.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20