From 15639f5b1aa607ccb4fec1a41643a3b916e0e44a Mon Sep 17 00:00:00 2001
From: Guilhem Moulin <guilhem@fripost.org>
Date: Thu, 29 Jun 2017 10:48:35 +0200
Subject: webserver: refuse to follow symlink when serving ACME challenge
 responses.

---
 webserver | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

(limited to 'webserver')

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() };
-- 
cgit v1.2.3


From a71cce0e99270492dbfa1567c046cd7db79ffd64 Mon Sep 17 00:00:00 2001
From: Guilhem Moulin <guilhem@fripost.org>
Date: Thu, 29 Jun 2017 12:06:35 +0200
Subject: webserver: open ACME challenge files with O_NOFOLLOW.

---
 webserver | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

(limited to 'webserver')

diff --git a/webserver b/webserver
index 21486ae..211f646 100755
--- a/webserver
+++ b/webserver
@@ -38,6 +38,7 @@ use warnings;
 # not a problem since FD can be bound as root prior to the execve(2).
 
 use Errno 'EINTR';
+use Fcntl qw/O_NOFOLLOW O_RDONLY/;
 use Socket qw/AF_UNIX AF_INET AF_INET6/;
 
 # Untaint and fdopen(3) the listening socket
@@ -91,12 +92,9 @@ 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
-            ! -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
+    if ($req =~ /\A\Q$ROOT\E\/([A-Za-z0-9_\-]+)\z/ and -f $1) {
+        # only open files in the cwd, and refuse to follow symlinks
+        if (sysopen(my $fh, $1, O_NOFOLLOW|O_RDONLY)) {
             ($status_line, $content_type) = ('200 OK', 'application/jose+json');
             $content = do { local $/ = undef; $fh->getline() };
             $fh->close() or die "close: $!";
-- 
cgit v1.2.3


From 97b4aad955ea816d7cc2273c1fd85fe139ec6207 Mon Sep 17 00:00:00 2001
From: Guilhem Moulin <guilhem@fripost.org>
Date: Thu, 29 Jun 2017 14:53:40 +0200
Subject: webserver: improve serving logic for ACME challenge responses.

In particular, we now return "403 Forbidden" for
/.well-known/acme-challenge/
---
 webserver | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

(limited to 'webserver')

diff --git a/webserver b/webserver
index 211f646..90be70c 100755
--- a/webserver
+++ b/webserver
@@ -37,7 +37,7 @@ use warnings;
 # as "www-data:www-data"; bind(2)'ing to a privileged port such as 80 is
 # not a problem since FD can be bound as root prior to the execve(2).
 
-use Errno 'EINTR';
+use Errno qw/EINTR ENOENT/;
 use Fcntl qw/O_NOFOLLOW O_RDONLY/;
 use Socket qw/AF_UNIX AF_INET AF_INET6/;
 
@@ -92,19 +92,23 @@ 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) {
-        # only open files in the cwd, and refuse to follow symlinks
-        if (sysopen(my $fh, $1, O_NOFOLLOW|O_RDONLY)) {
+    if ($req =~ /\A\Q$ROOT\E\/([A-Za-z0-9_\-]+)\z/) {
+        # only serve base64-encoded filenames (tokens) in the cwd
+        # XXX stat(2) followed by open(2) is racy; open(2) would hang if
+        # an attacker manages to replace a regular file with a FIFO
+        # between the syscalls, leading to denial of service; however
+        # there shouldn't be any risk of information disclosure as we're
+        # not following symlinks (and the cwd is not owned by us)
+        if (-f $1 and sysopen(my $fh, $1, O_NOFOLLOW|O_RDONLY)) {
             ($status_line, $content_type) = ('200 OK', 'application/jose+json');
             $content = do { local $/ = undef; $fh->getline() };
             $fh->close() or die "close: $!";
-        }
-        else {
-            $status_line = '403 Forbidden';
+        } elsif ($! == ENOENT) { # errno from either stat(2) or open(2)
+            $status_line = '404 Not Found';
         }
     }
 
-    $conn->print( "HTTP/$proto ", ($status_line // '404 Not Found'), "\r\n" );
+    $conn->print( "HTTP/$proto ", ($status_line // '403 Forbidden'), "\r\n" );
     $conn->print( "Content-Type: $content_type\r\n" ) if defined $content_type;
     $conn->print( "Content-Length: ".length($content)."\r\n" ) if defined $content;
     $conn->print( "Connection: close\r\n" );
-- 
cgit v1.2.3