webserver: refuse to follow symlink when serving ACME challenge responses.

3 files changed, 12 insertions, 3 deletions

diff --git a/Changelog b/Changelog
index 59d5153..9b13c44 100644
--- a/Changelog
+++ b/Changelog
@@ -4,7 +4,7 @@ lacme (0.3) upstream;
     lacme-certs.conf.d"), import the default section of files read earlier.
   + new-cert: create certificate files atomically.
   + webserver: allow listening to multiple addresses (useful when
-    dual-stack IPv4/IPv6 is not supported).  Listen to a UNIX-domain
+    dual IPv4/IPv6 stack is not supported).  Listen to a UNIX-domain
     socket by default </var/run/lacme.socket>.
   + webserver: don't install temporary iptables by default.  Hosts
     without a public HTTP daemon listening on port 80 need to set the
@@ -21,6 +21,11 @@ lacme (0.3) upstream;
   - new-cert: mark the basicConstraints (CA:FALSE) and keyUsage x509v3
     extensions as critical in the CSR, following upstream fix of
     Boulder's issue #565.
+  - webserver: refuse to follow symlink when serving ACME challenge
+    responses.  When dropping privileges to a dedicated UID
+    (recommended) only the ACME client could write to its current
+    directory anyway, so following symlinks was not a serious
+    vulnerability.
 
  -- Guilhem Moulin <guilhem@guilhem.org>  Sun, 19 Feb 2017 13:08:41 +0100
 
diff --git a/lacme b/lacme
index d7a416e..7adc972 100755
--- a/lacme
+++ b/lacme
@@ -24,7 +24,7 @@ use warnings;
 our $VERSION = '0.0.1';
 my $NAME = 'lacme';
 
-use Errno qw/EADDRINUSE EINTR/;
+use Errno 'EINTR';
 use Fcntl qw/F_GETFD F_SETFD FD_CLOEXEC SEEK_SET/;
 use File::Temp ();
 use Getopt::Long qw/:config posix_default no_ignore_case gnu_getopt auto_version/;
diff --git a/webserver b/webserver
index 7914762..21486ae 100755
--- a/webserver
+++ b/webserver
@@ -91,7 +91,11 @@ while (1) {
     while (defined (my $h = $conn->getline())) { last if $h eq "\r\n" };
 
     my ($status_line, $content_type, $content);
-    if ($req =~ /\A\Q$ROOT\E\/([A-Za-z0-9_\-]+)\z/ and -f $1) {
+    if ($req =~ /\A\Q$ROOT\E\/([A-Za-z0-9_\-]+)\z/ and
+            ! -l $1 and -f _) { # reuse previous stat structure and save a syscall
+        # XXX calling lstat(2) before open(2) is racy; if O_NOFOLLOW was
+        # exposed to perl we would instead use it and later fstat(2) the
+        # file descriptor
         if (open my $fh, '<', $1) { # only open files in the cwd
             ($status_line, $content_type) = ('200 OK', 'application/jose+json');
             $content = do { local $/ = undef; $fh->getline() };