dovecot-auth-proxy: replace directory traversal with LDAP lookups.

This provides better isolation opportunity as the service doesn't need
to run as ‘vmail’ user.  We use a dedicated system user instead, and
LDAP ACLs to limit its access to the strict minimum.

The new solution is also more robust to quoting/escaping, and doesn't
depend on ‘home=/home/mail/virtual/%d/%n’ (we might use $entryUUID
instead of %d/%n at some point to make user renaming simpler).

OTOH we no longer lists users that have been removed from LDAP but still
have a mailstore lingering around.  This is fair.

5 files changed, 99 insertions, 90 deletions

diff --git a/roles/IMAP/files/etc/systemd/system/dovecot-auth-proxy.service b/roles/IMAP/files/etc/systemd/system/dovecot-auth-proxy.service
index d20f9c2..3ac0b31 100644
--- a/roles/IMAP/files/etc/systemd/system/dovecot-auth-proxy.service
+++ b/roles/IMAP/files/etc/systemd/system/dovecot-auth-proxy.service
@@ -1,27 +1,25 @@
 [Unit]
 Description=Dovecot authentication proxy
 After=dovecot.target
 Requires=dovecot-auth-proxy.socket
 
 [Service]
-User=vmail
-Group=vmail
+User=_dovecot-auth-proxy
 StandardInput=null
 SyslogFacility=mail
 ExecStart=/usr/local/bin/dovecot-auth-proxy.pl
 
 # Hardening
 NoNewPrivileges=yes
 PrivateDevices=yes
-ProtectSystem=strict
-ProtectHome=read-only
-PrivateDevices=yes
 PrivateNetwork=yes
+ProtectHome=yes
+ProtectSystem=strict
 ProtectControlGroups=yes
 ProtectKernelModules=yes
 ProtectKernelTunables=yes
-RestrictAddressFamilies=
+RestrictAddressFamilies=AF_UNIX
 
 [Install]
 WantedBy=multi-user.target
 Also=postfix-sender-login.socket
diff --git a/roles/IMAP/files/usr/local/bin/dovecot-auth-proxy.pl b/roles/IMAP/files/usr/local/bin/dovecot-auth-proxy.pl
index 350bb2c..39d3762 100755
--- a/roles/IMAP/files/usr/local/bin/dovecot-auth-proxy.pl
+++ b/roles/IMAP/files/usr/local/bin/dovecot-auth-proxy.pl
@@ -1,131 +1,171 @@
-#!/usr/bin/perl
+#!/usr/bin/perl -T
 
 #----------------------------------------------------------------------
 # Dovecot userdb lookup proxy table for user iteration
-# Copyright © 2017 Guilhem Moulin <guilhem@fripost.org>
+# Copyright © 2017,2020 Guilhem Moulin <guilhem@fripost.org>
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
 # the Free Software Foundation, either version 3 of the License, or
 # (at your option) any later version.
 #
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #----------------------------------------------------------------------
 
 use warnings;
 use strict;
 
-use Errno 'EINTR';
+use Errno qw/EINTR/;
+use Net::LDAPI;
+use Net::LDAP::Constant qw/LDAP_CONTROL_PAGED LDAP_SUCCESS/;
+use Net::LDAP::Control::Paged ();
+use Net::LDAP::Util qw/ldap_explode_dn/;
+use Authen::SASL;
+
+my $BASE = "ou=virtual,dc=fripost,dc=org";
 
 # clean up PATH
 $ENV{PATH} = join ':', qw{/usr/bin /bin};
 delete @ENV{qw/IFS CDPATH ENV BASH_ENV/};
 
-# number of pre-forked servers
+# number of pre-forked servers and maximum requests per worker
 my $nProc = 1;
+my $maxRequests = 1;
 sub server();
 
 # fdopen(3) the file descriptor FD
 die "This service must be socket-activated.\n"
     unless defined $ENV{LISTEN_PID} and $ENV{LISTEN_PID} == $$
        and defined $ENV{LISTEN_FDS} and $ENV{LISTEN_FDS} == 1;
 open my $S, '+<&=', 3 or die "fdopen: $!";
 
-do {
-    my $dir = (getpwnam('vmail'))[7] // die "No such user: vmail";
-    $dir .= '/virtual';
-    chdir($dir) or die "chdir($dir): $!";
-};
-
 my @CHILDREN;
 for (my $i = 0; $i < $nProc-1; $i++) {
     my $pid = fork() // die "fork: $!";
     if ($pid) {
         push @CHILDREN, $pid;
     } else {
         server(); # child, never return
         exit;
     }
 }
 server();
 waitpid $_ => 0 foreach @CHILDREN;
 exit $?;
 
 
 #############################################################################
 
 sub server() {
-    for (my $n = 0; $n < 1; $n++) {
+    for (my $n = 0; $n < $maxRequests; $n++) {
         accept(my $conn, $S) or do {
             next if $! == EINTR;
             die "accept: $!";
         };
 
         my $hello = $conn->getline() // '';
         unless ($hello =~ /\AH(\d+)\t(\d+)\t(\d+)(?:\t.*)?\n\z/) {
             warn "Invalid greeting line: $hello\n";
             close $conn or warn "Can't close: $!";
             next;
         }
         # <major-version> <minor-version> <value type>
         unless ($1 == 2 and $2 == 2 and $3 == 0) {
             warn "Unsupported protocol version $1.$2 (or value type $3)\n";
             close $conn or warn "Can't close: $!";
             next;
         }
 
         my $cmd = $conn->getline() // '';
         if ($cmd =~ /\AI(\d+)\t(\d+)\t(.*)\n\z/) {
             iterate($conn, $1, $2, $3);
         }
         else {
             fail($conn => "Unknown command line: $cmd");
         }
         close $conn or warn "Can't close: $!";
     }
 }
 
 sub fail($;$) {
     my ($fh, $msg) = @_;
     $fh->printflush("F\n");
-    warn "$msg\n" if defined $msg;
+    print STDERR $msg, "\n" if defined $msg;
 }
 
-# list all users, even the inactive ones
+sub dn2user($) {
+    my $dn = shift;
+    $dn = ldap_explode_dn($dn, casefold => "lower");
+    if (defined $dn and $#$dn == 4
+            and defined (my $l = $dn->[0]->{fvl})
+            and defined (my $d = $dn->[1]->{fvd})) {
+        return $l ."@". $d;
+    }
+}
+
+# list all users (even the inactive ones)
 sub iterate($$$$) {
     my ($fh, $flags, $max_rows, $prefix) = @_;
     unless ($flags == 0) {
         fail($fh => "Unsupported iterate flags $flags");
         return;
     }
 
-    opendir my $dh, '.' or do {
-        fail($fh => "opendir: $!");
-        return;
-    };
-    my $count = 0;
-    while (defined (my $d = readdir $dh)) {
-        next if $d eq '.' or $d eq '..';
-        opendir my $dh, $d or do {
-            fail($fh => "opendir: $!");
-            return;
-        };
-        while (defined (my $l = readdir $dh) and ($max_rows <= 0 or $count < $max_rows)) {
-            next if $l eq '.' or $l eq '..';
-            my $user = $l.'@'.$d;
-            next unless $user =~ /\A[a-zA-Z0-9\.\-_@]+\z/; # skip invalid user names
+    my $ldap = Net::LDAPI::->new();
+    $ldap->bind( undef, sasl => Authen::SASL::->new(mechanism => "EXTERNAL") )
+        or do { fail($fh => "Error: Couldn't bind"); return; };
+    my $page = Net::LDAP::Control::Paged::->new(size => 100);
+
+    my $callback = sub($$) {
+        my ($mesg, $entry) = @_;
+        return unless defined $entry;
+
+        my $dn = $entry->dn();
+        if (defined (my $user = dn2user($dn))) {
             $fh->printf("O%s%s\t\n", $prefix, $user);
-            $count++;
+        } else {
+            print STDERR "Couldn't extract username from dn: ", $dn, "\n";
         }
-        closedir $dh or warn "closedir: $!";
+        $mesg->pop_entry;
+    };
+
+    my @search_args = (
+          base => $BASE,
+        , scope => "children"
+        , deref => "never"
+        , filter => "(objectClass=FripostVirtualUser)"
+        , sizelimit => $max_rows
+        , control => [$page]
+        , callback => $callback
+        , attrs => ["1.1"]
+    );
+
+    my $cookie;
+    while (1) {
+        my $mesg = $ldap->search(@search_args);
+        last unless $mesg->code == LDAP_SUCCESS;
+
+        my ($resp) = $mesg->control(LDAP_CONTROL_PAGED) or last;
+        $cookie = $resp->cookie();
+        goto SEARCH_DONE unless defined $cookie and length($cookie) > 0;
+
+        $page->cookie($cookie);
+    }
+
+    if (defined $cookie and length($cookie) > 0) {
+        fail($fh => "Abnormal exit from LDAP search, aborting");
+        $page->cookie($cookie);
+        $page->size(0);
+        $ldap->search(@search_args);
     }
-    closedir $dh or warn "closedir: $!";
 
+    SEARCH_DONE:
+    $ldap->unbind();
     $fh->printflush("\n");
 }
diff --git a/roles/IMAP/files/usr/local/bin/list-users.pl b/roles/IMAP/files/usr/local/bin/list-users.pl
deleted file mode 100755
index 1bcab35..0000000
--- a/roles/IMAP/files/usr/local/bin/list-users.pl
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/usr/bin/perl
-
-# Copyright © 2017 Guilhem Moulin <guilhem@fripost.org>
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-use warnings;
-use strict;
-use Net::LDAPI;
-use Net::LDAP::Util qw/ldap_explode_dn escape_dn_value/;
-use Authen::SASL;
-
-my $BASE = 'ou=virtual,dc=fripost,dc=org';
-
-my $LDAP = Net::LDAPI::->new();
-$LDAP->bind( undef, sasl => Authen::SASL::->new(mechanism => 'EXTERNAL') )
-    or die "Error: Couldn't bind";
-
-my $mesg = $LDAP->search( base => $BASE, scope => 'children', deref => 'never'
-                        , filter => '(objectClass=FripostVirtualUser)'
-                        , attrs => ['1.1']
-                        );
-die $mesg->error if $mesg->code;
-
-while (defined (my $entry = $mesg->pop_entry())) {
-    my $dn = $entry->dn() // next;
-    $dn = ldap_explode_dn($dn, casefold => 'lower');
-    next unless defined $dn and $#$dn == 4;
-    my $l = $dn->[0]->{fvl} // next;
-    my $d = $dn->[1]->{fvd} // next;
-    printf "%s@%s\n", $l, $d;
-}
-
-$LDAP->unbind;
diff --git a/roles/IMAP/tasks/imap.yml b/roles/IMAP/tasks/imap.yml
index 429854e..4a157af 100644
--- a/roles/IMAP/tasks/imap.yml
+++ b/roles/IMAP/tasks/imap.yml
@@ -1,52 +1,68 @@
 - name: Install Dovecot
   apt: pkg={{ packages }}
   vars:
     packages:
     - dovecot-core
     - dovecot-ldap
     - dovecot-imapd
     - dovecot-lmtpd
     - dovecot-antispam
     - dovecot-managesieved
     - dovecot-sieve
 
 - name: Create a user 'vmail'
   user: name=vmail system=yes
         createhome=no
         home=/home/mail
         shell=/usr/sbin/nologin
         password=!
         state=present
 
+- name: Install Net::LDAP and Authen::SASL
+  apt: pkg={{ packages }}
+  vars:
+    packages:
+    - libnet-ldap-perl
+    - libauthen-sasl-perl
+
 - name: Copy dovecot auth proxy
   copy: src=usr/local/bin/dovecot-auth-proxy.pl
         dest=/usr/local/bin/dovecot-auth-proxy.pl
         owner=root group=staff
         mode=0755
 
 # Required for IDLE as all imap processes have the same UID (vmail).
 - name: Set per user maximum number of inotify instances to 512
   sysctl: name=fs.inotify.max_user_instances value=512 sysctl_set=yes
   tags:
    - sysctl
 
+- name: Create '_dovecot-auth-proxy' user
+  user: name=_dovecot-auth-proxy system=yes
+        group=nogroup
+        createhome=no
+        home=/nonexistent
+        shell=/usr/sbin/nologin
+        password=!
+        state=present
+
 - name: Copy dovecot auth proxy systemd unit files
   copy: src=etc/systemd/system/{{ item }}
         dest=/etc/systemd/system/{{ item }}
         owner=root group=root
         mode=0644
   with_items:
     - dovecot-auth-proxy.service
     - dovecot-auth-proxy.socket
   notify:
     - systemctl daemon-reload
 
 - meta: flush_handlers
 
 - name: Enable dovecot auth proxy
   service: name=dovecot-auth-proxy.socket state=started enabled=yes
 
 # The ownership and permissions ensure that dovecot won't try to
 # deliver mails under an umounted mountpoint.
 - name: Create a home directory for user 'vmail'
   file: path=/home/mail
diff --git a/roles/common-LDAP/templates/etc/ldap/database.ldif.j2 b/roles/common-LDAP/templates/etc/ldap/database.ldif.j2
index 1be00cb..b640cbf 100644
--- a/roles/common-LDAP/templates/etc/ldap/database.ldif.j2
+++ b/roles/common-LDAP/templates/etc/ldap/database.ldif.j2
@@ -240,85 +240,85 @@ olcAccess: to dn.subtree="ou=virtual,dc=fripost,dc=org"
     by dn.onelevel="ou=syncRepl,dc=fripost,dc=org" tls_ssf=128 =rsd
     by *                                                       =0
 #
 # * They may also read entries (ie, the attributes they have access to
 #   as per the ACL below) in that subtree, when using a TLS-protected
 #   connection.  Listing entries (their DN) is required to replicate
 #   deletions properly.
 olcAccess: to dn.subtree="ou=virtual,dc=fripost,dc=org"
         attrs=entry,objectClass
     by dn.onelevel="ou=syncRepl,dc=fripost,dc=org" tls_ssf=128 =rsd
     by group.exact="cn=admin,ou=groups,dc=fripost,dc=org"      =wrsd
     by users                                                   =0   break
 olcAccess: to dn.children="ou=virtual,dc=fripost,dc=org"
     by group.exact="cn=admin,ou=groups,dc=fripost,dc=org"      =wrsd
     by users                                                   =0   break
 {% endif -%}
 #
 # * Postfix may use the base as a searchBase on the MX:es, when
 #   connecting a local ldapi:// socket from the 'private' directory in
 #   one of the non-default instance's chroot.
-# * So may Dovecot on the MDA (needed for the iterate filter), when
-#   SASL-binding using the EXTERNAL mechanism and connecting to a local
-#   ldapi:// socket.
+# * So may _dovecot-auth-proxy on the MDA (needed for the iterate
+#   logic), when SASL-binding using the EXTERNAL mechanism and
+#   connecting to a local ldapi:// socket.
 # * So may Nextcloud on the LDAP provider
 olcAccess: to dn.exact="ou=virtual,dc=fripost,dc=org"
         attrs=entry,objectClass
         filter=(objectClass=FripostVirtual)
     {% if 'MDA' in group_names -%}
-    by dn.exact="username=dovecot,cn=peercred,cn=external,cn=auth" sockurl.regex="^ldapi://"                                                    =sd
+    by dn.exact="username=_dovecot-auth-proxy,cn=peercred,cn=external,cn=auth" sockurl.regex="^ldapi://"                                        =sd
     {% endif -%}
     {% if 'MX' in group_names or 'MSA' in group_names -%}
     by dn.exact="username=postfix,cn=peercred,cn=external,cn=auth" sockurl.regex="^ldapi://%2Fvar%2Fspool%2Fpostfix-[-[:alnum:]]+%2Fprivate%2F" =sd
     {% endif -%}
     {% if 'LDAP_provider' in group_names -%}
     by dn.exact="cn=nextcloud,ou=services,dc=fripost,dc=org"       tls_ssf=128                                                                  =sd
     {% endif -%}
     by users                                                                                                                                    =0 break
 #
 # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
 # Domain entries
 #
 # * The SyncRepl replicates have read access to the entry itself, when
 #   using a TLS-protected connection.
 # * So has Postfix, when connecting a local ldapi:// socket from the
 #   'private' directory in one of the non-default instance's chroot.
-# * So has Dovecot on the MDA (for the iterate filter), when
+# * So has _dovecot-auth-proxy on the MDA (for the iterate logic), when
 #   SASL-binding using the EXTERNAL mechanism and connecting to a local
 #   ldapi:// socket.
 # * Amavis may use the entry as searchBase (required to look for the
 #   per-user preferences) but doesn't have read access to the entry.
 # * The 'nobody' UNIX user has read access on the MX:es, when using
 #   SASL-binding using the EXTERNAL mechanism and connecting to a local
 #   ldapi:// socket.  This is required for the 'reserved-alias.pl'
 #   script.
 olcAccess: to dn.regex="^fvd=[^,]+,ou=virtual,dc=fripost,dc=org$"
         attrs=entry,objectClass,fvd
         filter=(&(objectClass=FripostVirtualDomain)(!(objectClass=FripostPendingEntry)))
     {% if 'LDAP_provider' in group_names -%}
     {% if groups.MX | difference([inventory_hostname]) -%}
     by dn.exact="cn=mX,ou=syncRepl,dc=fripost,dc=org"              tls_ssf=128                                                                  =rsd
     {% endif -%}
     {% endif -%}
     by dn.exact="username=postfix,cn=peercred,cn=external,cn=auth" sockurl.regex="^ldapi://%2Fvar%2Fspool%2Fpostfix-[-[:alnum:]]+%2Fprivate%2F" =rsd
     {% if 'MDA' in group_names -%}
-    by dn.exact="username=dovecot,cn=peercred,cn=external,cn=auth" sockurl.regex="^ldapi://"                                                    =rsd
+    by dn.exact="username=_dovecot-auth-proxy,cn=peercred,cn=external,cn=auth" sockurl.regex="^ldapi://"                                        =rsd
     by dn.exact="username=amavis,cn=peercred,cn=external,cn=auth"  sockurl.regex="^ldapi://"                                                    =sd
     {% endif -%}
     {% if 'MX' in group_names -%}
     by dn.exact="username=nobody,cn=peercred,cn=external,cn=auth"  sockurl.regex="^ldapi://"                                                    =rsd
     {% endif -%}
     by users                                                                                                                                    =0 break
 #
 # * The SyncRepl MX replicates can check whether a virtual domain is
 #   active, and read the destination address for catch-alls, when using
 #   a TLS-protected connection.
 # * So can Postfix on the MX:es, when connecting a local ldapi:// socket
 #   from the 'private' directory in one of the non-default instance's
 #   chroot.
 {% if 'MX' in group_names or ('LDAP_provider' in group_names and groups.MX | difference([inventory_hostname])) %}
 olcAccess: to dn.regex="^fvd=[^,]+,ou=virtual,dc=fripost,dc=org$"
         attrs=fripostIsStatusActive,fripostOptionalMaildrop
         filter=(&(objectClass=FripostVirtualDomain)(!(objectClass=FripostPendingEntry)))
     {% if 'LDAP_provider' in group_names and groups.MX | difference([inventory_hostname]) -%}
     by dn.exact="cn=mX,ou=syncRepl,dc=fripost,dc=org"              tls_ssf=128                                                                  =rsd
     {% endif -%}
@@ -355,54 +355,54 @@ olcAccess: to dn.regex="^fvd=[^,]+,ou=virtual,dc=fripost,dc=org$"
 {% if 'MX' in group_names or ('LDAP_provider' in group_names and groups.MX | difference([inventory_hostname])) %}
 olcAccess: to dn.regex="^fvd=[^,]+,ou=virtual,dc=fripost,dc=org$"
         attrs=entry,fripostMaildrop
         filter=(&(objectClass=FripostVirtualAliasDomain)(!(objectClass=FripostPendingEntry)))
     {% if 'LDAP_provider' in group_names and groups.MX | difference([inventory_hostname]) -%}
     by dn.exact="cn=mX,ou=syncRepl,dc=fripost,dc=org"              tls_ssf=128                                                                  =rsd
     {% endif -%}
     {% if 'MX' in group_names -%}
     by dn.exact="username=postfix,cn=peercred,cn=external,cn=auth" sockurl.regex="^ldapi://%2Fvar%2Fspool%2Fpostfix-[-[:alnum:]]+%2Fprivate%2F" =rsd
     {% endif -%}
     by users                                                                                                                                    =0 break
 {% endif %}
 #
 # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
 # User entries
 #
 # * The SyncRepl replicates have read access to the entry itself, when
 #   using a TLS-protected connection.
 # * So has Postfix, when connecting a local ldapi:// socket from the
 #   'private' directory in one of the non-default instance's chroot.
-# * So has Dovecot on the MDA (for the iterate filter), when
+# * So has _dovecot-auth-proxy on the MDA (for the iterate logic), when
 #   SASL-binding using the EXTERNAL mechanism and connecting to a local
 #   ldapi:// socket.
 # * So has Amavis on the MDA, when SASL-binding using the EXTERNAL
 #   mechanism and connecting to a local ldapi:// socket.
 olcAccess: to dn.regex="^fvl=[^,]+,fvd=[^,]+,ou=virtual,dc=fripost,dc=org$"
         attrs=entry,objectClass,fvl
         filter=(objectClass=FripostVirtualUser)
     {% if 'LDAP_provider' in group_names and groups.MX | difference([inventory_hostname]) -%}
     by dn.exact="cn=mX,ou=syncRepl,dc=fripost,dc=org"                      tls_ssf=128                                                                  =rsd
     {% endif -%}
     by dn.exact="username=postfix,cn=peercred,cn=external,cn=auth"         sockurl.regex="^ldapi://%2Fvar%2Fspool%2Fpostfix-[-[:alnum:]]+%2Fprivate%2F" =rsd
     {% if 'MDA' in group_names -%}
-    by dn.exact="username=dovecot,cn=peercred,cn=external,cn=auth"         sockurl.regex="^ldapi://"                                                    =rsd
+    by dn.exact="username=_dovecot-auth-proxy,cn=peercred,cn=external,cn=auth" sockurl.regex="^ldapi://"                                                =rsd
     by dn.exact="username=amavis,cn=peercred,cn=external,cn=auth"          sockurl.regex="^ldapi://"                                                    =rsd
     {% endif -%}
     by users                                                                                                                                            =0 break
 #
 # * The SyncRepl MX replicates can check whether a virtual user is
 #   active, when using a TLS-protected connection.
 # * So can Postfix on the MX:es, when connecting a local ldapi:// socket
 #   from the 'private' directory in one of the non-default instance's
 #   chroot.
 {% if 'MX' in group_names or ('LDAP_provider' in group_names and groups.MX | difference([inventory_hostname])) %}
 olcAccess: to dn.regex="^fvl=[^,]+,fvd=[^,]+,ou=virtual,dc=fripost,dc=org$"
         attrs=fripostIsStatusActive,fripostUseContentFilter
         filter=(objectClass=FripostVirtualUser)
     {% if 'LDAP_provider' in group_names and groups.MX | difference([inventory_hostname]) -%}
     by dn.exact="cn=mX,ou=syncRepl,dc=fripost,dc=org"      tls_ssf=128                                                                          =rsd
     {% endif -%}
     {% if 'MX' in group_names -%}
     by dn.exact="username=postfix,cn=peercred,cn=external,cn=auth" sockurl.regex="^ldapi://%2Fvar%2Fspool%2Fpostfix-[-[:alnum:]]+%2Fprivate%2F" =rsd
     {% endif -%}
     by users                                                                                                                                    =0 break