Skip to content

nexedi / slapos

Go to a project
Merge request !503 opened by  

Pure-FTPd SR

This SR is intended to be used in a demo next week. The purpose is to have a running pureftpd server which will call a script "/opt/pureftpd/upload_script" each time a file is uploaded. Pureftpd is used because it is already used in the current working environment of our partner and they don't want to change for now.

The script /opt/pureftpd/upload_script" is left out of the SR on purpose because it will be written by people outside Nexedi at first and contains sensitive information. Also, it is on purpose in /opt directory of the machine because this SR will be deployed on only one machine for now and the script will be shared by all pureftpd instances.

We don't merge this MR yet as we don't really know if this SR will be useful later...

/cc @jm @jerome

{{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved
  • Julien Muchembled @jm commented
    Owner

    TODO: missing dependencies ? probably openssl

  • Thomas Gambier @tomo

    added 1 commit

    Compare with previous version

    added 1 commit

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    Compare with previous version

    added 1 commit

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo commented
    Master

    @jm pureftpd is not using openssl. It uses libcrypt.so and not libcrypto.so. libcrypt comes from glibc.

    $ ldd /srv/slapgrid/slappart61/srv/runner/software/6d89de9ff91a81af09155426afb30041/parts/pure-ftpd/sbin/pure-ftpd
        linux-vdso.so.1 (0x00007ffc11912000)
        libcrypt.so.1 => /lib/x86_64-linux-gnu/libcrypt.so.1 (0x00007fd1f15bd000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fd1f1212000)
        /lib64/ld-linux-x86-64.so.2 (0x000055f4dba9a000)

$ dpkg -L libc6 | grep libcrypt
/lib/x86_64-linux-gnu/libcrypt-2.19.so
/lib/x86_64-linux-gnu/libcrypt.so.1
    Edited by Thomas Gambier
  • Jérome Perrin
    @jerome started a discussion on the diff
    Resolved by Jérome Perrin
    component/pure-ftpd/buildout.cfg
    7 7 md5sum = efce5529c1f0a39dafdd532c619503f1
    8 8
    9 9 # See https://download.pureftpd.org/pub/pure-ftpd/doc/README for more configurations
    10 # We need the trick about UPLOAD_PIPE_FILE and UPLOAD_PIPE_LOCK so that the files are created inside the $CWD/var/run
    11 # WARNING: this means that both pure-ftpd and pure-uploadscript binaries must be launched in $HOME !
    10 12 configure-options =
    11 13 --with-uploadscript
    12
    13 #environment =
    14 # Probably it is missing dependencies to be set here.
    14 --with-puredb
    15 --with-nonroot
    16 environment=
    17 CFLAGS=-DUPLOAD_PIPE_FILE='"/proc/self/cwd/var/run/pure-ftpd.upload.pipe"' -DUPLOAD_PIPE_LOCK='"/proc/self/cwd/var/run/pure-ftpd.upload.lock"'
    • Jérome Perrin @jerome commented
      Owner

      I guess this is not configurable at runtime

    • Thomas Gambier @tomo commented
      Master

      indeed it is not. I worked a lot to find a solution not requiring a patch (I even tried namespaces but JP is against it because it's not portable enough). I think this solution is the best compromise.

    • Jérome Perrin @jerome commented
      Owner

      I don't know but /proc/self/cwd might also be linux only :) anyway, that's a clever trick.

    Please register or sign in to reply
  • Jérome Perrin
    @jerome started a discussion on an old version of the diff
    Resolved by Thomas Gambier
    software/pureftpd/README.md 0 → 100644
    1 Pureftpd with sftp and upload script
    Please register or sign in to reply
  • Jérome Perrin
    @jerome started a discussion on an old version of the diff
    Resolved by Thomas Gambier
    software/pureftpd/instance-output-schema.json 0 → 100644
    1 {
    2 "$schema": "http://json-schema.org/draft-04/schema#",
    3 "description": "Values returned by Pure-FTPd instantiation",
    4 "additionalProperties": false,
    5 "properties": {
    6 "url": {
    7 "description": "URL of the FTP service",
    8 "pattern": "^sftp://",
    Please register or sign in to reply
  • Jérome Perrin
    @jerome started a discussion on the diff
    Resolved by Jérome Perrin
    software/pureftpd/instance.cfg.in 0 → 100644
    61 recipe =slapos.recipe.template:jinja2
    62 # WARNING pure-uploadscript must be launched AFTER pure-ftpd so keep them in the same wrapper
    63 # and make sure they are both killed if one of them is killed.
    64 template = inline:
    65 #!{{ bash_location }}/bin/bash
    66 {{ pureftpd_bin }} --uploadscript --customerproof --bind ${:host},${:ftp-port} --login puredb:${:auth-user-file} --pidfile ${:pid-file} &
    67 sleep 5
    68 {{ pureuploadscript_bin }} -r /opt/pureftpd/upload_script &
    69 wait -n
    70 kill 0
    71 rendered = ${directory:service}/pureftpd
    72 wrapper-path = ${:rendered}
    73
    74 [pure-pw]
    75 # command line to add a user, invoke with:
    76 # pure-pw useradd bob
    Please register or sign in to reply
  • Jérome Perrin
    @jerome started a discussion on an old version of the diff
    Resolved by Thomas Gambier
    software/pureftpd/instance.cfg.in 0 → 100644
    68 {{ pureuploadscript_bin }} -r /opt/pureftpd/upload_script &
    69 wait -n
    70 kill 0
    71 rendered = ${directory:service}/pureftpd
    72 wrapper-path = ${:rendered}
    73
    74 [pure-pw]
    75 # command line to add a user, invoke with:
    76 # pure-pw useradd bob
    77 # it will prompt for password twice
    78 recipe = slapos.recipe.template:jinja2
    79 mode = 0700
    80 extensions = jinja2.ext.do
    81 rendered = ${buildout:bin-directory}/${:_buildout_section_name_}
    82 wrapper-path = ${:rendered}
    83 # XXX: to workaround bug that pw-uid and gr-gir slapos.cookbook:userinfo cannot
    • Jérome Perrin @jerome commented
      Owner

      That bug (it comes from proftpd) was fixed in !321 (merged) . We probably have a release os slapos.cookbook including the fix now and can write this as:

       {{ purepw_bin }} useradd ${pureftpd-password:username} -d ${pureftpd:data-dir} -u ${pureftpd-userinfo:pw-uid} -g ${pureftpd-userinfo:gr-gid} -f ${auth-user-file:passwd-file} "$@"
    • Jérome Perrin @jerome commented
      Owner

      -> !504 (merged)

    • Thomas Gambier @tomo commented
      Master

      done, thanks for the info

    Please register or sign in to reply
  • Jérome Perrin
    @jerome started a discussion on an old version of the diff
    Resolved by Thomas Gambier
    software/pureftpd/instance.cfg.in 0 → 100644
    86 # because only the former is compatible with userinfo.
    87 # Because we are already in a jinja template, we have to escape jinja template
    88 # with {% raw %} / {% endraw %}
    89 template = inline:
    90 #!/bin/sh
    91 {{ purepw_bin }} useradd ${pureftpd-password:username} -d ${pureftpd:data-dir} -u {% raw %}{{ pw_uid }} -g {{ gr_gid }}{% endraw %} -f ${auth-user-file:passwd-file} "$@"
    92 context =
    93 key pw_uid pureftpd-userinfo:pw-uid
    94 key gr_gid pureftpd-userinfo:gr-gid
    95
    96 [auth-user-file]
    97 recipe = plone.recipe.command
    98 passwd-file = ${directory:etc}/pureftpd.passwd
    99 output = ${directory:etc}/pureftpd.pdb
    100 command =
    101 rm ${:passwd-file}
    • Jérome Perrin @jerome commented
      Owner

      This will run each time slapos node instance processes the partition isn't it ? it's probably better if we can update the password file in place.

    • Thomas Gambier @tomo commented
      Master

      yes indeed but we would have something like: if [ -f ${:passwd-file} ] then ( echo ${pureftpd-password:passwd} ; echo ${pureftpd-password:passwd}) | pure-pw passwd ${pureftpd-password:username} -f ${:passwd-file} else ( echo ${pureftpd-password:passwd} ; echo ${pureftpd-password:passwd}) | ${pure-pw:wrapper-path} fi

      I'm not sure it's better...

      Edited by Thomas Gambier
    • Jérome Perrin @jerome commented
      Owner

      What can be problematic is that if someone "manually" added a user, the user will be removed. Adding users manually means you enter the partition and run command, which is not something really supported, so maybe that's not an issue.

      I think this is what http://community.slapos.org/slapos_community/wiki/slapos-Wiki.Home/developer-SlapOS.Conventions.Guidelines recommend not to do:

      Instances profiles and recipes should be promise-based and shouldn't break/alter existing data of instance if any. I.e, all promises should check for existing content in instance before injecting/altering any content.

      Not acceptable:

      # Recipe code that blindly sets application password without checking if it has been changed
def install(self):
   set_password()

      Acceptable:

      def install(self):
  if not password_already_set:
    set_password()

      not something blocker anyway.

    • Thomas Gambier @tomo commented
      Master

      done

    Please register or sign in to reply
  • Jérome Perrin
    @jerome started a discussion on an old version of the diff
    Last updated by Jérome Perrin
    software/pureftpd/instance.cfg.in 0 → 100644
    52 ipv6 = ${slap-configuration:ipv6-random}
    53 ipv4 = ${slap-configuration:ipv4-random}
    54 host = ${:ipv6}
    55 ftp-port = ${slap-configuration:configuration.port}
    56 url = ftp://[${:host}]:${:ftp-port}
    57 data-dir = ${directory:pureftpd-dir}
    58 pid-file=${directory:run}/pureftpd.pid
    59 auth-user-file=${auth-user-file:output}
    60
    61 recipe =slapos.recipe.template:jinja2
    62 # WARNING pure-uploadscript must be launched AFTER pure-ftpd so keep them in the same wrapper
    63 # and make sure they are both killed if one of them is killed.
    64 template = inline:
    65 #!{{ bash_location }}/bin/bash
    66 {{ pureftpd_bin }} --uploadscript --customerproof --bind ${:host},${:ftp-port} --login puredb:${:auth-user-file} --pidfile ${:pid-file} &
    67 sleep 5
    • Jérome Perrin @jerome commented
      Owner

      For the records:

      It's a long time bug in supervisor that we cannot wait for a process to listen on port ( https://github.com/Supervisor/supervisor/issues/122 ). Some people use https://github.com/vishnubob/wait-for-it to workaround it. I don't think we tried using it in slapos.

      In grafana SR, we use a trick of running check port listening promise to only start the second process once the process it depends on is started. I did not know about wait-for-it at that time, so I did this silly "refuse to start when not ready".

    • Thomas Gambier @tomo commented
      Master

      Indeed, Julien already commented here : tomo/slapos@bc0ed7c4 (comment 73101)

      I'm waiting for the pipe to be created now but I just realized that the pipe is not removed when pureftpd is killed... so I will need to find a better condiion

    • Jérome Perrin @jerome commented
      Owner

      Maybe the wait-for-it logic could be added to slapos.recipe:wrapper, it already has "wait-for-files" it could have "wait-for-tcp-listen" or similar naming. But if the upload controller also have to be restarted when pure-ftp is restarted, then it's more complicated.

    Please register or sign in to reply
  • Jérome Perrin @jerome commented
    Owner

    It would be nice to have a few words explaining why we don't use existing proftpd and make a new software release. If I remember @rafael explained that the reason is that it's to be used in a context where the upload_script infrastructure is already used and switching to proftpd would be a "useless" change in this context. I think that's the reason, isn't it ?

    Also our proftpd only supports sftp ( a bit "on purpose" ).

    Note that proftpd has http://www.proftpd.org/docs/contrib/mod_exec.html that maybe can be used to achieve the same. proftpd was selected because it supports sftp (in that context we needed strong authentication and encryption) and virtual users. In my understanding PureFTP, did not support sftp, so I did not try it.

    Maybe the content /opt/pureftpd/upload_script can be an instance parameter ? this would make unit testing easier.

    This SR is intended to be used in a demo next week

    good luck :) I left lots of comments, but nothing seems "blocking" for that demo. Also, these are not comments to "request changes", just to "add information on this topic in the hope it would help"

  • Jérome Perrin @jerome commented
    Owner

    Small clarification: I'm perfectly OK that we make another "*ftp" SR, just think we should document why chose to make another one.

  • Thomas Gambier @tomo

    added 3 commits

    • c6733656 - 1 commit from branch nexedi:master
    • 4d3c51de - Move apache-libcloud and pyasn1 versions to stack/slapos.cfg as they are depende…
    • 34a7a178 - Add new SR: pureftpd

    Compare with previous version

    added 3 commits

    • c6733656 - 1 commit from branch nexedi:master
    • 4d3c51de - Move apache-libcloud and pyasn1 versions to stack/slapos.cfg as they are depende…
    • 34a7a178 - Add new SR: pureftpd

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    marked as a Work In Progress

    marked as a Work In Progress

    Toggle commit list
  • Julien Muchembled
    @jm started a discussion on the diff
    software/pureftpd/instance.cfg.in 0 → 100644
    82 wrapper-path =${buildout:bin-directory}/${:_buildout_section_name_}
    83 command-line =
    84 {{ purepw_bin }} useradd ${pureftpd-password:username} -d ${pureftpd:data-dir} -u ${pureftpd-userinfo:pw-uid} -g ${pureftpd-userinfo:gr-gid} -f ${auth-user-file:passwd-file} "$@"
    85
    86
    87 [auth-user-file]
    88 recipe = plone.recipe.command
    89 passwd-file = ${directory:etc}/pureftpd.passwd
    90 output = ${directory:etc}/pureftpd.pdb
    91 command =
    92 if [ -f ${:passwd-file} ] && {{ purepw_bin }} show ${pureftpd-password:username} -f ${:passwd-file}
    93 then
    94 ( echo ${pureftpd-password:passwd} ; echo ${pureftpd-password:passwd}) | {{ purepw_bin }} passwd ${pureftpd-password:username} -f ${:passwd-file}
    95 else
    96 ( echo ${pureftpd-password:passwd} ; echo ${pureftpd-password:passwd}) | ${pure-pw:wrapper-path}
    97 fi
    • Julien Muchembled @jm commented
      Owner 
      ( echo ${pureftpd-password:passwd} ; echo ${pureftpd-password:passwd}) | if ...
    Please register or sign in to reply
  • Rafael Monnerat @rafael commented
    Owner

    @jerome the reason to implement yet another FTP server, it is for a partner/prospect that requires pureftp specifically with the upload-script feature. Nothing wrong with proftpd, just it is not what they expect to have.

  • Thomas Gambier @tomo

    added 2 commits

    Compare with previous version

    added 2 commits

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 10 commits

    Compare with previous version

    added 10 commits

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    Compare with previous version

    added 1 commit

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    Compare with previous version

    added 1 commit

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    Compare with previous version

    added 1 commit

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    Compare with previous version

    added 1 commit

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    Compare with previous version

    added 1 commit

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    Compare with previous version

    added 1 commit

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    Compare with previous version

    added 1 commit

    Compare with previous version

    Toggle commit list
  • Thomas Gambier
    @tomo started a discussion on the diff
    software/pureftpd/instance.cfg.in 0 → 100644
    99 else
    100 ( echo ${pureftpd-password:passwd} ; echo ${pureftpd-password:passwd}) | ${pure-pw:wrapper-path}
    101 fi
    102 {{ purepw_bin }} mkdb ${:output} -f ${:passwd-file}
    103 update-command = ${:command}
    104
    105
    106 [promises]
    107 recipe =
    108 instance-promises =
    109 ${pureftpd-listen-promise:output}
    110
    111
    112 [publish-connection-parameter]
    113 recipe = slapos.cookbook:publish
    114 <= monitor-publish
    • Thomas Gambier @tomo commented
      Master

      @rafael I had this change in my webrunner but I forgot to push...

      Thanks for fixing it on the server.

    Please register or sign in to reply
  • Rafael Monnerat @rafael

    unmarked as a Work In Progress

    unmarked as a Work In Progress

    Toggle commit list
  • Rafael Monnerat @rafael commented
    Owner

    We could just merge this, it is ok for a proof of concept and worked for the demo we intend, so if nobody objects, I will just merge.

  • Jérome Perrin @jerome commented
    Owner

    +1 we already have the component on master ( https://lab.nexedi.com/nexedi/slapos/blob/master/component/pure-ftpd/buildout.cfg ) so it's good to also merge this, isn't it ?

  • Thomas Gambier @tomo

    merged

    merged

    Toggle commit list
  • Thomas Gambier @tomo

    mentioned in commit 4ea4d3ab

    mentioned in commit 4ea4d3ab

    Toggle commit list
  • Thomas Gambier @tomo commented
    Master

    thanks for the reminder @jerome I merged it.

  • Thomas Gambier @tomo

    mentioned in commit matevzg/slapos@a64ecb48

    mentioned in commit matevzg/slapos@a64ecb48

    Toggle commit list
  • Thomas Gambier @tomo

    mentioned in commit dbc2bf56

    mentioned in commit dbc2bf56

    Toggle commit list
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备14008524号