From a903ea92dd736c560d21fe45063d4914765fa173 Mon Sep 17 00:00:00 2001 From: Guilhem Moulin Date: Sun, 14 Feb 2021 17:01:17 +0100 Subject: challenge-directory now needs to be set to an *existing* directory. Since lacme(8) spawns a builtin webserver by default the change doesn't affect default configurations. See https://bugs.debian.org/970800 for the rationale. --- Changelog | 7 +++++ config/lacme.conf | 13 ++++----- lacme | 82 ++++++++++++++++++++++++++++++----------------------- lacme.8.md | 16 +++++------ snippets/nginx.conf | 2 +- 5 files changed, 66 insertions(+), 54 deletions(-) diff --git a/Changelog b/Changelog index 34ace54..39249b4 100644 --- a/Changelog +++ b/Changelog @@ -1,5 +1,12 @@ lacme (0.7.1) upstream; + * Breaking change: 'challenge-directory' now needs to be set to an + *existing* directory (writable by the lacme client user). Since + lacme(8) spawns a builtin webserver by default the change doesn't + affect default configurations. + Thanks to Benjamin Tietz for the idea and initial patch. + * Breaking change: the 'iptables' option is now ignored unless the + builtin webserver is used. * Unprivileged user/group for the internal client resp. webserver are now configurable at install time. * lacme: new flag `--force`, which aliases to `--min-days=-1`, i.e., diff --git a/config/lacme.conf b/config/lacme.conf index 2955984..4c7dc86 100644 --- a/config/lacme.conf +++ b/config/lacme.conf @@ -64,17 +64,14 @@ # #listen = @@runstatedir@@/lacme-www.socket -# Non-existent directory under which an external HTTP daemon is -# configured to serve GET requests for challenge files under -# "/.well-known/acme-challenge/" (for each virtual host requiring -# authorization) as static files. +# Directory under which an external HTTP daemon is configured to serve +# GET requests for challenge files under "/.well-known/acme-challenge/" +# (for each virtual host requiring authorization) as static files. +# NOTE: the directory must exist and be writable by the lacme client +# user. # #challenge-directory = -# Do not symlink the challenge-directory, but copy the challenge-files -# explictly. -#hard-copy-challenge-directory = No - # username to drop privileges to (setting both effective and real uid). # Skip privilege drop if the value is empty (not recommended). # diff --git a/lacme b/lacme index d7ae8ce..7ad7aa8 100755 --- a/lacme +++ b/lacme @@ -26,9 +26,8 @@ our $VERSION = '0.3'; my $NAME = 'lacme'; use Errno 'EINTR'; -use Fcntl qw/F_GETFD F_SETFD FD_CLOEXEC SEEK_SET/; +use Fcntl qw/F_GETFD F_SETFD FD_CLOEXEC O_CREAT O_EXCL O_WRONLY SEEK_SET/; use File::Temp (); -use File::Path 'remove_tree'; use Getopt::Long qw/:config posix_default no_ignore_case gnu_getopt auto_version/; use List::Util 'first'; use POSIX (); @@ -105,7 +104,6 @@ do { webserver => { listen => '@@runstatedir@@/lacme-www.socket', 'challenge-directory' => undef, - 'hard-copy-challenge-directory' => 'No', user => '@@lacme_www_user@@', group => '@@lacme_www_group@@', command => '@@libexecdir@@/lacme/webserver', @@ -258,15 +256,6 @@ sub set_FD_CLOEXEC($$) { # The temporary challenge directory is returned. # sub spawn_webserver() { - # create a temporary directory; give write access to the ACME client - # and read access to the webserver - my $tmpdir = File::Temp::->newdir(CLEANUP => 1, TMPDIR => 1) // die; - chmod 0755, $tmpdir or die "Can't chmod: $!"; - if ((my $username = $CONFIG->{client}->{user}) ne '') { - my $uid = getpwnam($username) // die "Can't getpwnam($username): $!"; - chown($uid, -1, $tmpdir) or die "Can't chown: $!"; - } - my $conf = $CONFIG->{webserver}; # parse and pack addresses to listen to @@ -286,35 +275,56 @@ sub spawn_webserver() { push @sockaddr, $sockaddr; } - # symlink the 'challenge-directory' configuration option to the - # temporary challenge directory (so an existing httpd can directly - # serve ACME challenge reponses). + # Use existing HTTPd to serve challenge files using 'challenge-directory' + # as document root if (defined (my $dir = $conf->{'challenge-directory'})) { print STDERR "[$$] Using existing webserver on $dir\n" if $OPTS{debug}; - if (lc ($conf->{'hard-copy-challenge-directory'} // 'No') eq 'yes') { - mkdir $dir or die "Can't create directory $dir: $!"; - $tmpdir = $dir; - push @CLEANUP, sub() { - my $error = undef; - remove_tree($dir, { safe => 1, error => \$error }); - if ($error && @$error) { - foreach my $e (@$error) { - my ($file, $message) = %$e; - my $msghead = $file?"Error removing $file in":"Error while removing"; - warn "$msghead challenge dir $dir: $message\n"; - } + # lacme(8) doesn't have the list of challenge files to delete on + # cleanup -- instead, we unlink all files and fails at + # initialization stage when the challenge directory is not empty + + opendir my $dh, $dir or die "opendir($dir): $!\n"; + while (readdir $dh) { + die "Error: Refusing to use non-empty challenge directory $dir\n" + unless $_ eq '.' or $_ eq '..'; + } + closedir $dh or die "close: $!"; + undef $dh; + + # use a "lock file" (NFS-friendly) to avoid concurrent usages + my $lockfile = ".$NAME.lock"; + sysopen(my $fh, "$dir/$lockfile", O_CREAT|O_EXCL|O_WRONLY, 0600) + or die "Can't create lockfile in challenge directory: $!"; + print $fh $$, "\n"; + close $fh or die "close: $!"; + undef $fh; + + push @CLEANUP, sub() { + if (opendir(my $dh, $dir)) { + my @files = grep { $_ ne '.' and $_ ne '..' and $_ ne $lockfile } readdir $dh; + closedir $dh or warn "close: $!"; + push @files, $lockfile; # unlink $lockfile last + foreach (@files) { + die unless /\A(.+)\z/; # untaint + unlink "$dir/$1" or warn "unlink($dir/$1): $!"; } + } else { + warn "opendir($dir): $!\n"; } - } else { - symlink $tmpdir, $dir or die "Can't symlink $dir -> $tmpdir: $!"; - push @CLEANUP, sub() { - print STDERR "Unlinking $dir\n" if $OPTS{debug}; - unlink $dir or warn "Warning: Can't unlink $dir: $!"; - } - } + }; + return $dir; # ignore 'listen' and 'iptables' } - elsif (!@sockaddr) { - die "'challenge-directory' option of section [webserver] is required when 'listen' is empty\n"; + + die "'challenge-directory' option is required in section [webserver] when 'listen' is empty\n" + unless @sockaddr; + + # create a temporary directory; give write access to the ACME client + # and read access to the webserver + my $tmpdir = File::Temp::->newdir(CLEANUP => 1, TMPDIR => 1) // die; + chmod 0755, $tmpdir or die "Can't chmod: $!"; + if ((my $username = $CONFIG->{client}->{user}) ne '') { + my $uid = getpwnam($username) // die "Can't getpwnam($username): $!"; + chown($uid, -1, $tmpdir) or die "Can't chown: $!"; } # create socket(s) and spawn webserver(s) diff --git a/lacme.8.md b/lacme.8.md index 404180c..76cdd0d 100644 --- a/lacme.8.md +++ b/lacme.8.md @@ -238,16 +238,13 @@ served during certificate issuance. *challenge-directory* -: Specify a non-existent directory under which an external HTTP daemon - is configured to serve `GET` requests for challenge files under - `/.well-known/acme-challenge/` (for each virtual host requiring - authorization) as static files. - This option is required when *listen* is empty. - -*hard-copy-challenge-directory* +: Directory under which an external HTTP daemon is configured to serve `GET` + requests for challenge files under `/.well-known/acme-challenge/` (for + each virtual host requiring authorization) as static files. + `lacme` _must_ exist beforehand, _must_ be empty, and be writable by the + lacme client user (by default @@lacme_client_user@@). -: Do not symlink the challenge-directory, but copy the challenge-files - explictly. + This option is required when *listen* is empty. *user* @@ -275,6 +272,7 @@ served during certificate issuance. : Whether to automatically install temporary [`iptables`(8)] rules to open the `ADDRESS[:PORT]` specified with *listen*. The rules are automatically removed once `lacme` exits. + This option is ignored when *challenge-directory* is set. Default: `No`. `[accountd]` section diff --git a/snippets/nginx.conf b/snippets/nginx.conf index 6775489..af2e92e 100644 --- a/snippets/nginx.conf +++ b/snippets/nginx.conf @@ -13,6 +13,6 @@ location ^~ /.well-known/acme-challenge/ { ## lacme's configuration file # alias /var/www/acme-challenge/; # default_type application/jose+json; - # disable_symlinks on from=$document_root; + # disable_symlinks on; # autoindex off; } -- cgit v1.2.3