aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGuilhem Moulin <guilhem@fripost.org>2021-02-24 20:03:44 +0100
committerGuilhem Moulin <guilhem@fripost.org>2021-02-24 21:09:02 +0100
commitcdd025133a306cd8d3e81aa832ac056119d65f3a (patch)
tree70124d1307c6bb9f49fb9b2440521a7fe15786b9
parentfaab30461b0f2b920e3dd19489ce458c0b38e6d9 (diff)
lacme: Don't write certificate(-chain) file on chown/chmod failure.
Otherwise we end up with files with mode 0644 owned by root:root, and subsequent lacme(8) invocations will likely not renew them for a while. This change also saves a chown(2) call. And the new logic (chown resp. chmod from root:root resp. 0600) is safe if we ever include private key material in there too.
-rw-r--r--Changelog1
-rwxr-xr-xlacme54
-rw-r--r--tests/cert-install34
3 files changed, 44 insertions, 45 deletions
diff --git a/Changelog b/Changelog
index e047ac5..2a027f1 100644
--- a/Changelog
+++ b/Changelog
@@ -2,6 +2,7 @@ lacme (0.8.1) upstream;
+ lacme-accountd: improve log messages and refactor logging logic.
+ lacme-accountd: refuse to sign JWS with an invalid Protected Header.
+ + lacme: don't write certificate(-chain) file on chown/chmod failure.
- lacme: in the [accountd] config, let lacme-accountd(1) do the
%-expansion for 'config', not lacme(8) when building the command.
- lacme-accountd: don't log debug messages unless --debug is set.
diff --git a/lacme b/lacme
index fb19646..2366830 100755
--- a/lacme
+++ b/lacme
@@ -657,8 +657,9 @@ sub spawn($@) {
#############################################################################
# Install the certificate (optionally excluding the chain of trust)
#
-sub install_cert($$;$) {
- my ($filename, $chain, $leafonly) = @_;
+sub install_cert(%) {
+ my %args = @_;
+ my $filename = $args{path} // die;
my ($dirname, $basename) =
$filename =~ /\A(.*)\/([^\/]+)\z/ ? ($1, $2) : ('.', $filename);
@@ -666,10 +667,8 @@ sub install_cert($$;$) {
TEMPLATE => "$basename.XXXXXX") // die;
eval {
- my $umask = umask() // die "umask: $!";
- chmod(0644 &~ $umask, $fh) or die "chmod: $!";
- if ($leafonly) {
- # keep only the leaf certificate
+ if ($args{nochain}) {
+ # keep only the first certificate in the file
pipe my $rd, my $wd or die "pipe: $!";
my $pid = fork // die "fork: $!";
unless ($pid) {
@@ -678,16 +677,34 @@ sub install_cert($$;$) {
exec qw/openssl x509 -outform PEM/ or die;
}
$rd->close() or die "close: $!";
- $wd->print($chain);
+ $wd->print($args{content});
$wd->close() or die "close: $!";
waitpid $pid => 0;
die $? if $? > 0;
} else {
- $fh->print($chain) or die "print: $!";
+ $fh->print($args{content}) or die "print: $!";
}
+
+ my $mode;
+ if ((my $m = $args{mode}) ne "") {
+ $mode = oct($m) // die;
+ } else {
+ my $umask = umask() // die;
+ $mode = 0644 &~ $umask;
+ }
+ chmod($mode, $fh) or die "chown: $!";
+
+ if ((my $owner = $args{owner}) ne "") {
+ my ($user, $group) = split /:/, $owner, 2;
+ my $uid = getpwnam($user) // die "getpwnam($user)", ($! ? ": $!" : "\n");
+ my $gid = getgrnam($group) // die "getgrnam($group)", ($! ? ": $!" : "\n") if defined $group;
+ chown($uid, $gid // -1, $fh) or die "chown: $!";
+ }
+
$fh->close() or die "close: $!";
};
+
my $path = $fh->filename();
if ($@) {
print STDERR "Unlinking $path\n" if $OPTS{debug};
@@ -837,29 +854,16 @@ elsif ($COMMAND eq 'newOrder' or $COMMAND eq 'new-cert') {
}
}
+ my %install = ( content => $x509, mode => $conf->{chmod} // "", owner => $conf->{chown} // "" );
+
# install certificate
if ((my $path = $conf->{'certificate'} // "") ne "") {
print STDERR "Installing X.509 certificate $path\n";
- install_cert($path, $x509, 1);
+ install_cert(%install, path => $path, nochain => 1);
}
if ((my $path = $conf->{'certificate-chain'} // "") ne "") {
print STDERR "Installing X.509 certificate chain $path\n";
- install_cert($path, $x509);
- }
-
- if ((my $own = $conf->{chown} // "") ne "") {
- my ($user, $group) = split /:/, $own, 2;
- my $uid = getpwnam($user) // die "getpwnam($user)", ($! ? ": $!" : "\n");
- my $gid = getgrnam($group) // die "getgrnam($group)", ($! ? ": $!" : "\n") if defined $group;
- foreach (@certs) {
- chown($uid, $gid // -1, $_) or die "chown: $!";
- }
- }
- if ((my $mode = $conf->{chmod} // "") ne "") {
- my $mode = oct($mode) // die;
- foreach (@certs) {
- chmod($mode, $_) or die "chown: $!";
- }
+ install_cert(%install, path => $path);
}
my @certopts = join ',', qw/no_header no_version no_pubkey no_sigdump/;
diff --git a/tests/cert-install b/tests/cert-install
index 39110f4..5d8a239 100644
--- a/tests/cert-install
+++ b/tests/cert-install
@@ -110,25 +110,22 @@ cat >"/etc/lacme/lacme-certs.conf.d/test4.conf" <<- EOF
certificate-key = /etc/lacme/test4.key
certificate = /etc/lacme/test4.pem
certificate-chain = /etc/lacme/test4.crt
- chown = nobody
+ chown = nonexistent-user
subject = $subject
EOF
+! lacme newOrder test4 2>"$STDERR" || fail newOrder test4
+grepstderr -Fxq "getpwnam(nonexistent-user)"
+! test -e /etc/lacme/test4.pem
+! test -e /etc/lacme/test4.crt
+
+sed -ri "s/^chown\\s*=.*/chown = nobody/" /etc/lacme/lacme-certs.conf.d/test4.conf
lacme newOrder test4 2>"$STDERR" || fail newOrder test4
st="$(stat -c "%U:%G %#a" /etc/lacme/test4.pem)"
[ "$st" = "nobody:root 0644" ]
st="$(stat -c "%U:%G %#a" /etc/lacme/test4.crt)"
[ "$st" = "nobody:root 0644" ]
-rm -f /etc/lacme/test4.pem /etc/lacme/test4.crt
-sed -ri "s/^chown\\s*=.*/chown = nonexistent-user/" /etc/lacme/lacme-certs.conf.d/test4.conf
-! lacme newOrder test4 2>"$STDERR" || fail newOrder test4
-grepstderr -Fxq "getpwnam(nonexistent-user)"
-st="$(stat -c "%U:%G %#a" /etc/lacme/test4.pem)"
-[ "$st" = "root:root 0644" ]
-st="$(stat -c "%U:%G %#a" /etc/lacme/test4.crt)"
-[ "$st" = "root:root 0644" ]
-
# chown user:group
openssl genpkey -algorithm RSA -out /etc/lacme/test5.key
cat >"/etc/lacme/lacme-certs.conf.d/test5.conf" <<- EOF
@@ -136,25 +133,22 @@ cat >"/etc/lacme/lacme-certs.conf.d/test5.conf" <<- EOF
certificate-key = /etc/lacme/test5.key
certificate = /etc/lacme/test5.pem
certificate-chain = /etc/lacme/test5.crt
- chown = nobody:nogroup
+ chown = nobody:nonexistent-group
subject = $subject
EOF
+! lacme newOrder test5 2>"$STDERR" || fail newOrder test5
+grepstderr -Fxq "getgrnam(nonexistent-group)"
+! test -e /etc/lacme/test5.pem
+! test -e /etc/lacme/test5.crt
+
+sed -ri "s/^chown\\s*=.*/chown = nobody:nogroup/" /etc/lacme/lacme-certs.conf.d/test5.conf
lacme newOrder test5 2>"$STDERR" || fail newOrder test5
st="$(stat -c "%U:%G %#a" /etc/lacme/test5.pem)"
[ "$st" = "nobody:nogroup 0644" ]
st="$(stat -c "%U:%G %#a" /etc/lacme/test5.crt)"
[ "$st" = "nobody:nogroup 0644" ]
-rm -f /etc/lacme/test5.pem /etc/lacme/test5.crt
-sed -ri "s/^chown\\s*=.*/chown = nobody:nonexistent-group/" /etc/lacme/lacme-certs.conf.d/test5.conf
-! lacme newOrder test5 2>"$STDERR" || fail newOrder test5
-grepstderr -Fxq "getgrnam(nonexistent-group)"
-st="$(stat -c "%U:%G %#a" /etc/lacme/test5.pem)"
-[ "$st" = "root:root 0644" ]
-st="$(stat -c "%U:%G %#a" /etc/lacme/test5.crt)"
-[ "$st" = "root:root 0644" ]
-
# chmod
openssl genpkey -algorithm RSA -out /etc/lacme/test6.key
cat >"/etc/lacme/lacme-certs.conf.d/test6.conf" <<- EOF