From: Guilhem Moulin Date: Thu, 13 Jun 2024 03:32:04 +0200 Subject: Fix post-issuance validation logic Rather than adding intermediates in the certificate bundle we now validate the leaf certificate with intermediates as untrusted (used for chain building only). Only the root certificates are used as trust anchor. Not pining intermediate certificates anymore is in line with Let's Encrypt's latest recommendations: Rotating the set of intermediates we issue from helps keep the Internet agile and more secure. It encourages automation and efficiency, and discourages outdated practices like key pinning. “Key Pinning” is a practice in which clients — either ACME clients getting certificates for their site, or apps connecting to their own backend servers — decide to trust only a single issuing intermediate certificate rather than delegating trust to the system trust store. Updating pinned keys is a manual process, which leads to an increased risk of errors and potential business continuity failures. — https://letsencrypt.org/2024/03/19/new-intermediate-certificates Origin: https://git.guilhem.org/lacme/commit/?id=9cb882a468843bf8ce9598de8769d5baaaaae3ea Bug-Debian: https://bugs.debian.org/1072847 --- Makefile | 8 ++---- lacme | 26 +++++++++++++---- tests/cert-install | 82 +++++++++++++++++++++++++++++++++++------------------- 3 files changed, 76 insertions(+), 40 deletions(-) diff --git a/Makefile b/Makefile index 10e55c5..cb2f4ed 100644 --- a/Makefile +++ b/Makefile @@ -16,17 +16,13 @@ $(MANUAL_FILES): $(BUILDDIR)/%: $(BUILDDIR)/%.md # used for validation, see https://letsencrypt.org/certificates/ $(BUILDDIR)/certs/ca-certificates.crt: \ certs/isrgrootx1.pem \ - certs/isrg-root-x2.pem \ - certs/lets-encrypt-r[34].pem \ - certs/lets-encrypt-e[12].pem + certs/isrg-root-x2.pem mkdir -pv -- $(@D) cat -- $^ >$@ # Staging Environment for tests, see https://letsencrypt.org/docs/staging-environment/ $(BUILDDIR)/certs-staging/ca-certificates.crt: \ - certs-staging/letsencrypt-stg-root-x[12].pem \ - certs-staging/letsencrypt-stg-int-r[34].pem \ - certs-staging/letsencrypt-stg-int-e[12].pem + certs-staging/letsencrypt-stg-root-x[12].pem mkdir -pv -- $(@D) cat -- $^ >$@ diff --git a/lacme b/lacme index 6284c66..86a0516 100755 --- a/lacme +++ b/lacme @@ -822,21 +822,31 @@ elsif ($COMMAND eq 'newOrder' or $COMMAND eq 'new-cert') { next; } - my $cert; + my @chain; eval { my $mem = Net::SSLeay::BIO_s_mem() or die; my $bio = Net::SSLeay::BIO_new($mem) or die; die "incomplete write" unless Net::SSLeay::BIO_write($bio, $chain) == length($chain); - my $x509 = Net::SSLeay::PEM_read_bio_X509($bio); - $cert = Net::SSLeay::PEM_get_string_X509($x509); + + my $sk_x509_info = Net::SSLeay::PEM_X509_INFO_read_bio($bio); + + my $n = Net::SSLeay::sk_X509_INFO_num($sk_x509_info); + for (my $i = 0; $i < $n; $i++) { + my $x509_info = Net::SSLeay::sk_X509_INFO_value($sk_x509_info, $i); + my $x509 = Net::SSLeay::P_X509_INFO_get_x509($x509_info); + my $cert = Net::SSLeay::PEM_get_string_X509($x509); + push @chain, $cert; + } + Net::SSLeay::BIO_free($bio) or die; }; - if ($@) { + if ($@ or !@chain) { print STDERR "[$s] Error: Received bogus X.509 certificate from ACME server!\n"; $rv = 1; next; } + my $cert = shift @chain; # leave only the intermediate in @chain # extract pubkeys from CSR and cert, and ensure they match # XXX would be nice to use X509_get_X509_PUBKEY and X509_REQ_get_X509_PUBKEY here, @@ -852,9 +862,15 @@ elsif ($COMMAND eq 'newOrder' or $COMMAND eq 'new-cert') { # verify certificate validity against the CA bundle if ((my $CAfile = $conf->{CAfile} // '@@datadir@@/lacme/ca-certificates.crt') ne '') { + my $chain_tmp = File::Temp::->new(SUFFIX => '.crt', TMPDIR => 1) // die; + $chain_tmp->say($_) foreach @chain; + $chain_tmp->flush(); + my %args = (in => $cert); $args{out} = \*STDERR if $OPTS{debug}; - my @options = ('-trusted', $CAfile, '-purpose', 'sslserver', '-x509_strict'); + my @options = ('-trusted', $CAfile); + push @options, '-untrusted', $chain_tmp->filename() if @chain; + push @options, ('-purpose', 'sslserver', '-x509_strict'); push @options, '-show_chain' if $OPTS{debug}; if (spawn(\%args, 'openssl', 'verify', @options)) { print STDERR "[$s] Error: Received invalid X.509 certificate from ACME server!\n"; diff --git a/tests/cert-install b/tests/cert-install index 4b3e820..c38f3cf 100644 --- a/tests/cert-install +++ b/tests/cert-install @@ -28,6 +28,55 @@ EOF grepstderr -Fxq "[bad3] Warning: Couldn't generate CSR, skipping" +check_spki() { + local p1="$1" p2="$2" s1 s2 + s1="$(openssl x509 -in "$p1" -noout -pubkey \ + | openssl pkey -pubin -outform DER \ + | openssl dgst -sha256 \ + | sed 's/.*=\s*//')" + s2="$(openssl pkey -in "$p2" -pubout -outform DER \ + | openssl dgst -sha256 \ + | sed 's/.*=\s*//')" + if [ -n "$s1" ] && [ "$s1" = "$s2" ]; then + return 0 + else + printf "%s != %s\\n" "$s1" "$s2" >&2 + return 1 + fi +} +check_chain() { + local priv="$1" chain="$2" leaf="${3-}" pem0 + + csplit -f "${chain%.crt}.chain.pem" "$chain" \ + "/-----BEGIN CERTIFICATE-----/" "{*}" + + pem0="${chain%.crt}.chain.pem00" + if [ ! -s "$pem0" ]; then + # 00 is empty, leaf cert is at 01 + rm -f -- "$pem0" + pem0="${chain%.crt}.chain.pem01" + fi + test -s "$pem0" || return 1 + check_spki "$pem0" "$priv" + + if [ -n "$leaf" ]; then + diff --ignore-blank-lines --unified "$pem0" "$leaf" || return 1 + fi + + leaf="${chain%.crt}.leaf.pem" + mv -T -- "$pem0" "$leaf" + + intermediates="${chain%.crt}.intermediates.pem" + sed "/^$/d" "${chain%.crt}.chain.pem"[0-9]* >"$intermediates" + test -s "$intermediates" || return 1 # ensure there is at least one intermediate + + openssl verify -trusted /usr/share/lacme/ca-certificates.crt \ + -untrusted "$intermediates" \ + -purpose sslserver -x509_strict \ + -show_chain \ + -- "$leaf" || return 1 +} + # 'certificate' installs only the leaf certificate openssl genpkey -algorithm RSA -out /etc/lacme/test1.key subject="/CN=$(head -c10 /dev/urandom | base32 -w0).$DOMAINNAME" @@ -42,23 +91,9 @@ lacme newOrder test1 2>"$STDERR" || fail newOrder test1 test /etc/lacme/test1.crt -nt /etc/lacme/test1.key sed -n "0,/^-----END CERTIFICATE-----$/ p" /etc/lacme/test1.crt >/etc/lacme/test1.pem diff --unified /etc/lacme/test1.crt /etc/lacme/test1.pem +check_spki /etc/lacme/test1.crt /etc/lacme/test1.key -check_hash() { - local p1="$1" p2 s1 s2 - s1="$(openssl x509 -noout -hash <"$p1")" - for p2 in /usr/share/lacme/ca-certificates.pem.*; do - s2="$(openssl x509 -noout -hash <"$p2")" - if [ "$s1" = "$s2" ]; then - return 0 - fi - done - return 1 -} -csplit -f /usr/share/lacme/ca-certificates.pem. /usr/share/lacme/ca-certificates.crt \ - "/-----BEGIN CERTIFICATE-----/" "{*}" -rm -f /usr/share/lacme/ca-certificates.pem.00 - # 'certificate-chain' appends the chain of trust openssl genpkey -algorithm RSA -out /etc/lacme/test2.key cat >"/etc/lacme/lacme-certs.conf.d/test2.conf" <<- EOF @@ -70,16 +105,7 @@ EOF lacme newOrder test2 2>"$STDERR" || fail newOrder test2 test /etc/lacme/test2.crt -nt /etc/lacme/test2.key -csplit -f /etc/lacme/test2.chain.pem /etc/lacme/test2.crt \ - "/-----BEGIN CERTIFICATE-----/" "{*}" -test -s /etc/lacme/test2.chain.pem01 # leaf cert (00 is empty) -rm -f /etc/lacme/test2.chain.pem0[01] -test -s /etc/lacme/test2.chain.pem02 # depth 1 - -# all certificates at depth >=1 must be in our CA bundle -for p in /etc/lacme/test2.chain.pem*; do - check_hash "$p" -done +check_chain /etc/lacme/test2.key /etc/lacme/test2.crt # 'certificate' + 'certificate-chain' openssl genpkey -algorithm RSA -out /etc/lacme/test3.key @@ -94,10 +120,8 @@ EOF lacme newOrder test3 2>"$STDERR" || fail newOrder test3 test /etc/lacme/test3.pem -nt /etc/lacme/test3.key test /etc/lacme/test3.crt -nt /etc/lacme/test3.key -csplit -f /etc/lacme/test3.chain.pem /etc/lacme/test3.crt \ - "/-----BEGIN CERTIFICATE-----/" "{*}" -sed -i "/^$/d" /etc/lacme/test3.chain.pem* -diff -q /etc/lacme/test3.chain.pem01 /etc/lacme/test3.pem +check_chain /etc/lacme/test3.key /etc/lacme/test3.crt /etc/lacme/test3.pem + st="$(stat -c "%U:%G %#a" /etc/lacme/test3.pem)" [ "$st" = "root:root 0644" ] st="$(stat -c "%U:%G %#a" /etc/lacme/test3.crt)"