Merge pull request #1060 from MarcWeber/submit/vsftpd

small vsftpd improvements

2 files changed, 121 insertions, 57 deletions

diff --git a/nixos/.topmsg b/nixos/.topmsg
new file mode 100644
index 00000000000..9632e592631
--- /dev/null
+++ b/nixos/.topmsg
@@ -0,0 +1 @@
+improvements to vsftpd module
diff --git a/nixos/modules/services/networking/vsftpd.nix b/nixos/modules/services/networking/vsftpd.nix
index 1b2432401de..265afb3618c 100644
--- a/nixos/modules/services/networking/vsftpd.nix
+++ b/nixos/modules/services/networking/vsftpd.nix
@@ -4,12 +4,92 @@ with pkgs.lib;
 
 let
 
+  /* minimal secure setup:
+
+   enable = true;
+   forceLocalLoginsSSL = true;
+   forceLocalDataSSL = true;
+   userlistDeny = false;
+   localUsers = true;
+   userlist = ["non-root-user" "other-non-root-user"];
+   rsaCertFile = "/var/vsftpd/vsftpd.pem";
+
+  */
+
   cfg = config.services.vsftpd;
 
   inherit (pkgs) vsftpd;
 
-  yesNoOption = p : name :
-    "${name}=${if p then "YES" else "NO"}";
+  yesNoOption = nixosName: vsftpdName: default: description: {
+    cfgText = "${vsftpdName}=${if getAttr nixosName cfg then "YES" else "NO"}";
+
+    nixosOption = {
+      name = nixosName;
+      value = mkOption {
+        inherit description default;
+        type = types.bool;
+      };
+    };
+  };
+
+  optionDescription = [
+
+    (yesNoOption "anonymousUser" "anonymous_enable" false ''
+     Whether to enable the anonymous FTP user.
+    '')
+    (yesNoOption "localUsers" "local_enable" false ''
+     Whether to enable FTP for local users.
+    '')
+    (yesNoOption "writeEnable" "write_enable" false ''
+    Whether any write activity is permitted to users.
+    '')
+    (yesNoOption "anonymousUploadEnable" "anon_upload_enable" false ''
+    Whether any uploads are permitted to anonymous users.
+    '')
+    (yesNoOption "anonymousMkdirEnable" "anon_mkdir_write_enable" false ''
+    Whether any uploads are permitted to anonymous users.
+    '')
+    (yesNoOption "chrootlocalUser" "chroot_local_user" false ''
+    Whether local users are confined to their home directory.
+    '')
+    (yesNoOption "userlistEnable" "userlist_enable" false ''
+    Whether users are included.
+    '')
+    (yesNoOption "userlistDeny" "userlist_deny" false ''
+      Specifies whether <option>userlistFile</option> is a list of user
+      names to allow or deny access.
+      The default <literal>false</literal> means whitelist/allow.
+    '')
+    (yesNoOption "forceLocalLoginsSSL" "force_local_logins_ssl" true ''
+    Only applies if <option>sslEnable</option> is true. Non anonymous (local) users
+    must use a secure SSL connection to send a password.
+    '')
+    (yesNoOption "forceLocalDataSSL" "force_local_data_ssl" true ''
+    Only applies if <option>sslEnable</option> is true. Non anonymous (local) users
+    must use a secure SSL connection for sending/receiving data on data connection.
+    '')
+    (yesNoOption "ssl_tlsv1" "ssl_tlsv1" true  '' '')
+    (yesNoOption "ssl_sslv2" "ssl_sslv2" false '' '')
+    (yesNoOption "ssl_sslv3" "ssl_sslv3" false '' '')
+
+    {
+      cfgText = if cfg.rsaCertFile == null then ""
+        else ''
+        sslEnable=YES
+        rsa_cert_file=${cfg.rsaCertFile}
+      '';
+
+      nixosOption = {
+        name = "rsaCertFile";
+        value = mkOption {
+          default = null;
+          description = ''
+            rsa certificate file.
+          '';
+        };
+      };
+    }
+    ];
 
 in
 
@@ -26,52 +106,27 @@ in
         description = "Whether to enable the vsftpd FTP server.";
       };
 
-      anonymousUser = mkOption {
-        default = false;
-        description = "Whether to enable the anonymous FTP user.";
-      };
-
-      anonymousUserHome = mkOption {
-        default = "/home/ftp";
-        description = "Path to anonymous user data.";
-      };
-
-      localUsers = mkOption {
-        default = false;
-        description = "Whether to enable FTP for local users.";
-      };
-
-      writeEnable = mkOption {
-        default = false;
-        description = "Whether any write activity is permitted to users.";
-      };
+      userlist = mkOption {
+        default = [];
 
-      anonymousUploadEnable = mkOption {
-        default = false;
-        description = "Whether any uploads are permitted to anonymous users.";
+        description = ''
+          See <option>userlistFile</option>.
+        '';
       };
 
-      anonymousMkdirEnable = mkOption {
-        default = false;
-        description = "Whether mkdir is permitted to anonymous users.";
-      };
+      userlistFile = mkOption {
+        default = pkgs.writeText "userlist" (concatMapStrings (x: "${x}\n") cfg.userlist);
+        description = ''
+          Newline separated list of names to be allowed/denied if <option>userlistEnable</option>
+          is <literal>true</literal>. Meaning see <option>userlistDeny</option>.
 
-      chrootlocalUser = mkOption {
-        default = false;
-        description = "Whether local users are confined to their home directory.";
-      };
+          The default is a file containing the users from <option>userlist</option>.
 
-      userlistEnable = mkOption {
-        default = false;
-        description = "Whether users are included.";
-      };
-
-      userlistDeny = mkOption {
-        default = false;
-        description = "Whether users are excluded.";
+          If explicitely set to null userlist_file will not be set in vsftpd's config file.
+        '';
       };
 
-    };
+    } // (listToAttrs (catAttrs "nixosOption" optionDescription)) ;
 
   };
 
@@ -80,6 +135,15 @@ in
 
   config = mkIf cfg.enable {
 
+    assertions = [
+      {
+        assertion =
+              (cfg.forceLocalLoginsSSL -> cfg.rsaCertFile != null)
+          &&  (cfg.forceLocalDataSSL -> cfg.rsaCertFile != null);
+        message = "vsftpd: If forceLocalLoginsSSL or forceLocalDataSSL is true then a rsaCertFile must be provided!";
+      }
+    ];
+
     users.extraUsers =
       [ { name = "vsftpd";
           uid = config.ids.uids.vsftpd;
@@ -99,6 +163,21 @@ in
         gid = config.ids.gids.ftp;
       };
 
+    # If you really have to access root via FTP use mkOverride or userlistDeny
+    # = false and whitelist root
+    services.vsftpd.userlist = if cfg.userlistDeny then ["root"] else [];
+
+    environment.etc."vsftpd.conf".text =
+      concatMapStrings (x: "${x.cfgText}\n") optionDescription
+      + ''
+      ${if cfg.userlistFile == null then ""
+        else "userlist_file=${cfg.userlistFile}"}
+      background=NO
+      listen=YES
+      nopriv_user=vsftpd
+      secure_chroot_dir=/var/empty
+    '';
+
     jobs.vsftpd =
       { description = "vsftpd server";
 
@@ -107,22 +186,6 @@ in
 
         preStart =
           ''
-            # !!! Why isn't this generated in the normal way?
-            cat > /etc/vsftpd.conf <<EOF
-            ${yesNoOption cfg.anonymousUser "anonymous_enable"}
-            ${yesNoOption cfg.localUsers "local_enable"}
-            ${yesNoOption cfg.writeEnable "write_enable"}
-            ${yesNoOption cfg.anonymousUploadEnable "anon_upload_enable"}
-            ${yesNoOption cfg.anonymousMkdirEnable "anon_mkdir_write_enable"}
-            ${yesNoOption cfg.chrootlocalUser "chroot_local_user"}
-            ${yesNoOption cfg.userlistEnable "userlist_enable"}
-            ${yesNoOption cfg.userlistDeny "userlist_deny"}
-            background=NO
-            listen=YES
-            nopriv_user=vsftpd
-            secure_chroot_dir=/var/empty
-            EOF
-
             ${if cfg.anonymousUser then ''
               mkdir -p -m 555 ${cfg.anonymousUserHome}
               chown -R ftp:ftp ${cfg.anonymousUserHome}