Moodle
  1. Moodle
  2. MDL-34939

Regression: Clamav reporting errors when scanning external repo files

    Details

    • Testing Instructions:
      Hide
      1. Make sure that you are using clamdscan for virus checking in Moodle settings.
      2. Make sure that Moodle data directory permissions does not allow others read/execute (e.g. chmod 700 /path/to/moodledata).
      3. Make sure you have at least one external repo that allows files importing (e.g. Flickr public).
      4. Create file resource, choose adding new file and use external repo to chose file from. You should not get any errors, file should be successfully imported.
      5. Replicate steps above using clamscan
      Show
      Make sure that you are using clamdscan for virus checking in Moodle settings. Make sure that Moodle data directory permissions does not allow others read/execute (e.g. chmod 700 /path/to/moodledata). Make sure you have at least one external repo that allows files importing (e.g. Flickr public). Create file resource, choose adding new file and use external repo to chose file from. You should not get any errors, file should be successfully imported. Replicate steps above using clamscan
    • Workaround:
      Hide

      A couple of options here:

      1. Write a wrapper for clamdscan which adds the --fdpass option and use that instead; or
      2. Use clamscan instead. This is a last resort as it adds a massive performance hit.
      Show
      A couple of options here: Write a wrapper for clamdscan which adds the --fdpass option and use that instead; or Use clamscan instead. This is a last resort as it adds a massive performance hit.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-34939-master
    • Rank:
      43495

      Description

      when a user uploads/selects a file from an external repo (google docs, picasa, flickr)
      Clam av has trouble processing the file and sends an e-mail to the admin:

      Clam AV has failed to run. The return error message was An error occured. 
      Here is the output from Clam:
      
      /temp/download/filename.rtf: Can't open file or directory  ERROR 
      ----------- SCAN SUMMARY ----------- 
      Infected files: 0 
      Total errors: 1 
      Time: 0.005 sec (0 m 0 s) 
      

        Issue Links

          Activity

          Hide
          Dan Marsden added a comment -

          I haven't been able to reproduce this on a local machine yet - if anyone has any ideas - let me know!

          Show
          Dan Marsden added a comment - I haven't been able to reproduce this on a local machine yet - if anyone has any ideas - let me know!
          Hide
          Frédéric Massart added a comment -

          Hi Dan, I have tried to reproduce this using Google Docs on master and 2.3 and, as you, I can't. The files with/without viruses are properly scanned. Did you check the permissions on the directory itself?

          Show
          Frédéric Massart added a comment - Hi Dan, I have tried to reproduce this using Google Docs on master and 2.3 and, as you, I can't. The files with/without viruses are properly scanned. Did you check the permissions on the directory itself?
          Hide
          Dan Marsden added a comment -

          yeah - apparently it wasn't happening until I upgraded their site from 2.2 to 2.3 and it's only for external links, not for normal files which is stranger.

          I'll do some more work here and try to reproduce - maybe I've got the info wrong and it's happening with all repository files which might make a bit more sense!

          closing this as cannot reproduce for now - if I manage to find a way of reproducing and it points to Moodle as being the problem I'll re-open. Thanks for taking a look!

          Show
          Dan Marsden added a comment - yeah - apparently it wasn't happening until I upgraded their site from 2.2 to 2.3 and it's only for external links, not for normal files which is stranger. I'll do some more work here and try to reproduce - maybe I've got the info wrong and it's happening with all repository files which might make a bit more sense! closing this as cannot reproduce for now - if I manage to find a way of reproducing and it points to Moodle as being the problem I'll re-open. Thanks for taking a look!
          Hide
          Chris Wharton added a comment -

          I have replicated this issue on a test instance. This looks to be a server user privileges issue, and the simplest solution is to set Moodle to use clamscan instead of clamdscan. clamd runs as a different user to the www user by default.

          Show
          Chris Wharton added a comment - I have replicated this issue on a test instance. This looks to be a server user privileges issue, and the simplest solution is to set Moodle to use clamscan instead of clamdscan. clamd runs as a different user to the www user by default.
          Hide
          Ruslan Kabalin added a comment -

          I managed to replicate this, it is related to directory permissions. Basically when clamdscan is in use, clamd user need to be able to access the file, which is only the case when each directory it traverses on a way to file has exec permission for it and it is able to read the destination file. Currently we change the destination file permissions, which is obviously not sufficient, when, for example, access to moodle data directory is granted to webserver user only (has 600 permissions) for security reasons.

          What we need to do is to use --fdpass parameter with clamdscan, that passes the file descriptor permissions to clamd, which allows to scan given file irrespective of directory and file permissions. Notice, than when simple clamscan is in use, neither this parameter, nor directory permissions change is not required, as the scan is run by webserver user, who has access to Moodle data directory by definition.

          I have just encountered the same bug in Mahara as well https://bugs.launchpad.net/mahara/+bug/1168422

          Steps to reproduce:

          1. Make sure that you are using clamdscan for virus checking in Moodle settings.
          2. Make sure that Moodle data directory permissions does not allow others read/execute (e.g. chmod 700 /path/to/moodledata).
          3. Make sure you have at least one external repo that allows files importing (e.g. Flickr public).
          4. Create file resource, choose adding new file and use external repo to chose file from. You will get an error about unsuccessful scan. If messaging is configured correctly, you will also get an email with an error.
          Show
          Ruslan Kabalin added a comment - I managed to replicate this, it is related to directory permissions. Basically when clamdscan is in use, clamd user need to be able to access the file, which is only the case when each directory it traverses on a way to file has exec permission for it and it is able to read the destination file. Currently we change the destination file permissions, which is obviously not sufficient, when, for example, access to moodle data directory is granted to webserver user only (has 600 permissions) for security reasons. What we need to do is to use --fdpass parameter with clamdscan, that passes the file descriptor permissions to clamd, which allows to scan given file irrespective of directory and file permissions. Notice, than when simple clamscan is in use, neither this parameter, nor directory permissions change is not required, as the scan is run by webserver user, who has access to Moodle data directory by definition. I have just encountered the same bug in Mahara as well https://bugs.launchpad.net/mahara/+bug/1168422 Steps to reproduce: Make sure that you are using clamdscan for virus checking in Moodle settings. Make sure that Moodle data directory permissions does not allow others read/execute (e.g. chmod 700 /path/to/moodledata). Make sure you have at least one external repo that allows files importing (e.g. Flickr public). Create file resource, choose adding new file and use external repo to chose file from. You will get an error about unsuccessful scan. If messaging is configured correctly, you will also get an email with an error.
          Hide
          Ruslan Kabalin added a comment -

          Ready for review.

          Show
          Ruslan Kabalin added a comment - Ready for review.
          Hide
          Andrew Nicols added a comment -

          Chris Wharton clamdscan is preferable over clamscan as it is much more efficient and much faster. clamdscan runs as a daemon with all of the definitions in memory; whilst clamscan has to load the definitions each time.

          Show
          Andrew Nicols added a comment - Chris Wharton clamdscan is preferable over clamscan as it is much more efficient and much faster. clamdscan runs as a daemon with all of the definitions in memory; whilst clamscan has to load the definitions each time.
          Hide
          Andrew Nicols added a comment -

          [Y] Syntax
          [Y] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [N] Testing
          [Y] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          This took me a minute to get my head around but I think thta it does make sense.

          Could you add to the testing instructions to also test with clamscan please?

          Show
          Andrew Nicols added a comment - [Y] Syntax [Y] Output [Y] Whitespace [-] Language [-] Databases [N] Testing [Y] Security [-] Documentation [Y] Git [Y] Sanity check This took me a minute to get my head around but I think thta it does make sense. Could you add to the testing instructions to also test with clamscan please?
          Hide
          Andrew Nicols added a comment -

          Otherwise, +1 to go for integration.

          Show
          Andrew Nicols added a comment - Otherwise, +1 to go for integration.
          Hide
          Andrew Nicols added a comment -

          Adding Fred (component lead for Repositories), and Marina (Filepicker) as watchers so that they're aware of these changes.

          Show
          Andrew Nicols added a comment - Adding Fred (component lead for Repositories), and Marina (Filepicker) as watchers so that they're aware of these changes.
          Hide
          Ruslan Kabalin added a comment -

          I have tested it with clamscan already. Instructions are added as well.

          Show
          Ruslan Kabalin added a comment - I have tested it with clamscan already. Instructions are added as well.
          Hide
          Damyon Wiese added a comment -

          I did a bunch of testing on this to make sure it wasn't going to break anything and it looks OK.

          If clamd is configured to only allow TCP connections, passing this option does not give an error (which is good - because that is all that seems to work when I tested on windows).

          The extra option won't be passed on windows (because the filename will be clamdscan.exe) but it isn't useful on windows anyway.

          Show
          Damyon Wiese added a comment - I did a bunch of testing on this to make sure it wasn't going to break anything and it looks OK. If clamd is configured to only allow TCP connections, passing this option does not give an error (which is good - because that is all that seems to work when I tested on windows). The extra option won't be passed on windows (because the filename will be clamdscan.exe) but it isn't useful on windows anyway.
          Hide
          Damyon Wiese added a comment -

          Thanks, this is a bug fix and has been backported to 23, 24 and master branches. I did quite a bit of testing on this in integration, but will send it to another tester anyway because this is for stable branches so it's better to be really sure.

          Show
          Damyon Wiese added a comment - Thanks, this is a bug fix and has been backported to 23, 24 and master branches. I did quite a bit of testing on this in integration, but will send it to another tester anyway because this is for stable branches so it's better to be really sure.
          Hide
          Damyon Wiese added a comment -

          Pinching this one because I already have it setup.

          Show
          Damyon Wiese added a comment - Pinching this one because I already have it setup.
          Hide
          Damyon Wiese added a comment -

          Test passed on 23, 24 and master.

          Show
          Damyon Wiese added a comment - Test passed on 23, 24 and master.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I feel myself really alone tonight! So was time to push your fixes upstream!

          "Lest we forget. We will remember them."

          Thanks and ciao!

          Show
          Eloy Lafuente (stronk7) added a comment - I feel myself really alone tonight! So was time to push your fixes upstream! "Lest we forget. We will remember them." Thanks and ciao!
          Hide
          Mary Cooch added a comment -

          Removing docs_required label as I added a link to this in http://docs.moodle.org/en/Anti-virus for 2.5 and 2.4

          Show
          Mary Cooch added a comment - Removing docs_required label as I added a link to this in http://docs.moodle.org/en/Anti-virus for 2.5 and 2.4

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: