403023 - Extract NSIS (windows) installers and MAR files when virus scanning

Status: RESOLVED DUPLICATE of bug 72410

(Infrastructure & Operations :: Infrastructure: Other, task, P3)

Description

[Nick Thomas [:nthomas] (UTC+12)]

clamav apparently doesn't know how to un-7-zip our windows installers, if you scan for viruses on non-windows platforms. Please add scripting to do this so that we improve our coverage on the most virus prone platform.

Comment 1

[Dave Miller [:justdave]]

Any idea how?

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

Can clamav be given a location to scan ? If so

# get files with something like: find <layer> -type f -name '*.exe'
for each <file>
 <make tmpdir>
 7z x -o<tmpdir> <file>
 <call clamav with tmpdir>
done

You can also use -so to send the output to stdout.

Hopefully old school SeaMonkey installers will also be extracted by 7z.

Comment 3

[Daniel Veditz [:dveditz]]

"old-school" seamonkey installers are not 7-zip. They're a Netscape-custom self-extractor based on zip.  Call those with "sm-install.exe -u" to extract in-place without running the installer. The resulting files will be a few executables/libraries and zip-format .xpi files which the scanner should already be able to handle.

Comment 4

[Dave Miller [:justdave]]

ok, so basically, we have to wrap the scanning script in something that pre-extracts the files, and remembers where they came from, so if one of those files is found to be infected, we move the correct file from the tree into quarantine...  and the extracted versions have to be put somewhere outside of the tree so they don't accidently get posted in extracted form after they pass.

(In reply to comment #3)
> "old-school" seamonkey installers are not 7-zip. They're a Netscape-custom
> self-extractor based on zip.  Call those with "sm-install.exe -u" to extract
> in-place without running the installer.

So you're saying in order to extract them I have to run the code we're scanning.  Yeah, that's safe. :)

Comment 5

[Daniel Veditz [:dveditz]]

It's our own custom self-extractor, I'm sure the tool could be adapted fairly easily. The contained archives are zip-compressed and stored as individual windows "resources". Look at the EnumResourceNames() call in nsztool.c and the ExtractFilesProc callback function.

Same code appears in mozilla/xpinstall/wizard/windows/nsinstall/nsinstall.cpp, if they're not identical I'm not sure which version would be more up-to-date.

Comment 6

[Dave Miller [:justdave]]

So I'm not generally a C programmer...  can someone write me a utility that unpacks our self-extracting installer archives? or explain how to script it if it's doable with existing linux utilities? I'm not going to trust uploaded files to actually run them.

Comment 7

[Dave Miller [:justdave]]

Reassign this back to me when you get a volunteer for comment 6.

Comment 8

[Dave Miller [:justdave]]

Nick says build can make sure someone gets this utility written

Comment 9

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #8)
> Nick says build can make sure someone gets this utility written

Hmm, I don't recall saying that. The original intent here was to handle NSIS/7zip installers, so I've filed bug 421159 for the netscape ones. Also bug 421158 for mar files.

Comment 10

[Dave Miller [:justdave]]

Ah, yes, 7zip there's a utility for already.  No reason to wait on the Netscape stuff to do that one.

Comment 11

[Jeremy Orem [:oremj]]

Changing QA Contact.

Comment 12

[Reed Loden [:reed]]

Moving to RE in order to get the script written.

Comment 13

[[e-mail] (:bhearsum)]

Does this even matter until we have virus scanning on stage?

Comment 14

[Chris AtLee [:catlee]]

Clamav claims they support 7zip now: http://oss.netfarm.it/clamav/

Comment 15

[Dave Miller [:justdave]]

we just picked up 0.95.1 this last week, so we should indeed have this now.

Comment 16

[Dave Miller [:justdave]]

(In reply to comment #14)
> Clamav claims they support 7zip now: http://oss.netfarm.it/clamav/

Only on Windows it turns out.  The Linux version doesn't support it yet.

Comment 17

[Chris Cooper [:coop] (he/him)]

Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.

Comment 18

[Nick Thomas [:nthomas] (UTC+12)]

https://wwws.clamav.net/bugzilla/show_bug.cgi?id=1222 and http://www.clamav.net/about/roadmap/ say that 7-zip archives will be scannable in the ClamAV 0.96 release. A RC for that is due March 10.

Comment 19

[Chris Cooper [:coop] (he/him)]

Dave: have we tried the RC yet to see whether it works? Looks like they're on rc2 now:

http://sourceforge.net/projects/clamav/files/clamav/0.96rc2/clamav-0.96rc2.tar.gz/download

Comment 20

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to comment #18)
> https://wwws.clamav.net/bugzilla/show_bug.cgi?id=1222 and
> http://www.clamav.net/about/roadmap/ say that 7-zip archives will be scannable
> in the ClamAV 0.96 release. A RC for that is due March 10.

Is this ClamAV 0.96 available, and does it scan 7-zip archives as expected?

Comment 21

[matthew zeier [:mrz]]

Pushing this over to Chris.  He has ideas on which scanners we should be using or moving to so I'll let him comment on this.

Comment 22

[Chris Lyon [:clyon]]

(In reply to comment #21)
> Pushing this over to Chris.  He has ideas on which scanners we should be using
> or moving to so I'll let him comment on this.

So besides ClamAV, the other Linux based scanners that we have are McAfee and AVG. These are both acceptable for scanning but not sure if it is going to solve the root of this problem. Would need to do further research on that. The other one which we should be getting a copy of soon is Kapersky.

Comment 23

[matthew zeier [:mrz]]

I no longer know what this bug is asking IT/infrasec to do.  Help?

Comment 24

[Chris Lyon [:clyon]]

closing this out since it doesn't seem there is any real direction. reopen and comment if somebody disagrees.

Comment 25

[Reed Loden [:reed]]

We need to be able to scan inside our windows installers before releases get pushed out in case one of our build machines gets infected and corrupts one of our releases. It's that simple.

Comment 26

[Chris Lyon [:clyon]]

(In reply to comment #25)
> We need to be able to scan inside our windows installers before releases get
> pushed out in case one of our build machines gets infected and corrupts one of
> our releases. It's that simple.

At what point are we going to do these scans? RegEng has stated they will not hold a release pending a scan before it goes public. Their stance is it will be done after the fact and this is already in place on f.m.o. 

FWIW, I don't agree with that and I think they need to scan before stuff goes public and the more scanners the better. I can't tell you off the top of my head which scanners vs 7z are better or worse but we probably shouldn't just use one of them.

Comment 27

[Chris Lyon [:clyon]]

btw, moving to releng as this is a process issue that needs to be discussed further and they need to agree upon where to scan in the process.

Comment 28

[Chris AtLee [:catlee]]

Virus scanning is currently an automated part of our release automation which happens automatically after updates are generated, and manually before pushing to mirrors, and we do block pushing builds to mirrors based on the virus scan results.

Looks like we're currently running 0.97 on stage, does that do what we want for scanning?

Comment 29

[Dave Miller [:justdave]]

I don't think clamav knows how to extract 7zip self-installer packages, I think that was the point of this bug.

Comment 30

[Chris AtLee [:catlee]]

(In reply to comment #29)
> I don't think clamav knows how to extract 7zip self-installer packages, I think
> that was the point of this bug.

Ok, so using clamav 0.97 on my machine confirms this. As far as I can tell it's not unpacking the setup.exe file and looking inside of it.

Back over to RelEng.

Comment 31

[[e-mail] (:bhearsum)]

Attachment: bash script which runs a command on a list of files, while extracting 7z archives

Catlee, what do you think of this approach? I've only done a mild local test, but it seems to work.

Comment 32

[Chris AtLee [:catlee]]

Comment on attachment 517738 [details]
bash script which runs a command on a list of files, while extracting 7z archives

Looks good.  Be sure to sign the installer as a whole as well though!

Comment 33

[[e-mail] (:bhearsum)]

I realized that we don't look inside of MAR files either, updating summary to include that.

Comment 34

[[e-mail] (:bhearsum)]

Can someone from IT tell me how the normal virus scanning is done? What invokes it, and how?

Comment 35

[[e-mail] (:bhearsum)]

Per mrz, moving to Server Ops to get comment on this:

(In reply to comment #34)
> Can someone from IT tell me how the normal virus scanning is done? What invokes
> it, and how?

Comment 36

[[e-mail] (:bhearsum)]

Attachment: patch, including mar tool and updated script that handles MARs and EXEs

I've been testing this on surf in the release-scenario, where we trigger a scan. Invocation looks like:
./extract-and-run-command clamdscan -m --no-summary -- /pub/mozilla.org/firefox/nightly/4.0rc1-candidates/build1/

This seems to work well, but has a couple of drawbacks:
- We only scan a single file per invocation
- "-m" is now useless, because of the above
I think these are OK, because of how early we run the virus scan these days. Even in a chemspill, I suspect update verify and QA will still be the long pole. I'm doing a re-run of the 4.0rc1 virus scan right now to get an idea of how long this is going to take.

If it's an issue, I can try to build some sort of batching in...but that may get hairy due to the temp dirs.

I've dropped -v and added --no-summary because the level of verboseness we currently use doesn't make sense here, because we only scan a single file per invocation. New output looks like:
/pub/mozilla.org/firefox/nightly/4.0rc1-candidates/build1/linux-i686/fa/firefox-4.0rc1.tar.bz2.asc: OK
/pub/mozilla.org/firefox/nightly/4.0rc1-candidates/build1/linux-i686/fi/firefox-4.0rc1.checksums: OK
/pub/mozilla.org/firefox/nightly/4.0rc1-candidates/build1/linux-i686/fi/firefox-4.0rc1.tar.bz2: OK
/pub/mozilla.org/firefox/nightly/4.0rc1-candidates/build1/linux-i686/fi/firefox-4.0rc1.tar.bz2.asc: OK
...
/tmp/tmp.elNPO27899/core/plugin-container.exe: OK
/tmp/tmp.elNPO27899/core/README.txt: OK
/tmp/tmp.elNPO27899/core/removed-files: OK
/tmp/tmp.elNPO27899/core/searchplugins/eBay.xml: OK
/tmp/tmp.elNPO27899/core/searchplugins/eki-ee.xml: OK
/tmp/tmp.elNPO27899/core/searchplugins/google.xml: OK
/tmp/tmp.elNPO27899/core/searchplugins/neti-ee.xml: OK
/tmp/tmp.elNPO27899/core/searchplugins/osta-ee.xml: OK


Still waiting to hear from IT about how the normal virus scans are run, to ensure this can be integrated there.

Comment 37

[Chris AtLee [:catlee]]

Comment on attachment 517866 [details] [diff] [review]
patch, including mar tool and updated script that handles MARs and EXEs

Can we print out which file we fail on if we do fail? Printing out which file we failed on will be very useful when debugging I imagine!

How long does this take to run now?  Does it preclude running it on release day?  If so, do we need to verify that nothing has changed before pushing to mirrors?  we should probably be doing that anyway...

Comment 38

[[e-mail] (:bhearsum)]

(In reply to comment #37)
> Comment on attachment 517866 [details] [diff] [review]
> patch, including mar tool and updated script that handles MARs and EXEs
> 
> Can we print out which file we fail on if we do fail? Printing out which file
> we failed on will be very useful when debugging I imagine!

The output already shows each file and pass/fail for it, are you looking to have these summarized at the end? Do you want just the high level files to be highlighted, or even files that were scanned from an unpacked EXE or MAR? Those come out as "/tmp/tmp.whatever/blah/blah", and are fairly noisy. Would grepping out the "PASS" messages from the existing output be good enough?

> How long does this take to run now?

Not sure..I forgot to start this in screen last night, so my first try never finished :(.

> Does it preclude running it on release
> day?

It certainly precludes doing a scan after we get a "push to mirrors" mail, but if we preempt it by enough, we could do it.

> If so, do we need to verify that nothing has changed before pushing to
> mirrors?  we should probably be doing that anyway...

We definitely should be doing that, I'll file a follow-up on it. I don't think that should block this, because we already do releases in which we scan a week ahead of going to mirrors, and never rescan.

Comment 39

[Chris AtLee [:catlee]]

(In reply to comment #38)
> (In reply to comment #37)
> > Comment on attachment 517866 [details] [diff] [review]
> > patch, including mar tool and updated script that handles MARs and EXEs
> > 
> > Can we print out which file we fail on if we do fail? Printing out which file
> > we failed on will be very useful when debugging I imagine!
> 
> The output already shows each file and pass/fail for it, are you looking to
> have these summarized at the end? Do you want just the high level files to be
> highlighted, or even files that were scanned from an unpacked EXE or MAR? Those
> come out as "/tmp/tmp.whatever/blah/blah", and are fairly noisy. Would grepping
> out the "PASS" messages from the existing output be good enough?

I just want to make sure if we detect a virus, that we know exactly which file it's in, including the container (setup.exe, .mar) if applicable

Comment 40

[[e-mail] (:bhearsum)]

(In reply to comment #35)
> Per mrz, moving to Server Ops to get comment on this:
> 
> (In reply to comment #34)
> > Can someone from IT tell me how the normal virus scanning is done? What invokes
> > it, and how?

ping

Comment 41

[Dave Miller [:justdave]]

Heh, I started to answer this the other day, and then in doing my fact checking, discovered it was all broken, and got distracted trying to fix it (we broke it all when we changed the firefox ftp mounts around, which I knew at the time and was intending to fix within a couple days after that, and promptly forgot :|  )

We have it set up according to the "Ongoing Scanning" section on https://intranet.mozilla.org/Security/Virus_Scanning

We have a scheduler and queue runners on various scan engines that run the scan jobs.  The dashboard is at http://im-virusscan01.mozilla.org/cgi-bin/listqueue.cgi for now (VPN required, URL will probably change eventually)

It should be fairly easy to get this integrated into the queue runners to handle files of these types when they're hit.

Comment 42

[[e-mail] (:bhearsum)]

Thanks for your reply, Dave. Just to spell it out explicitly, this is the current interface to the extractor:
extract_and_run_command.py -j4 [command that causes files to be scanned] -- [files to be scanned]

Will the ongoing scanning system be able to integrate that?

Comment 43

[[e-mail] (:bhearsum)]

(In reply to comment #42)
> Thanks for your reply, Dave. Just to spell it out explicitly, this is the
> current interface to the extractor:
> extract_and_run_command.py -j4 [command that causes files to be scanned] --
> [files to be scanned]
> 
> Will the ongoing scanning system be able to integrate that?

Justdave confirmed that this should be fine on IRC. I'm going to take this bug back to finish up the RelEng parts. I'll throw it back again once it's all landed and ready to be used.

Comment 44

[[e-mail] (:bhearsum)]

Attachment: python version

I ran this version on stage with -j4 and -j12. -j4 took 90min, -j12 took 77min. I kept on an eye on running jobs at the time, and neither seemed to hammer the system hard enough to cause any failures. Load shot up pretty high, but I suspect that's normal when you're working this much.

For the on-demand scans, we're going to want to change our invocation to something like:
extract_and_run_command.py -jN clamdscan -m --no-summary -- /pub/mozilla.org/firefox/nightly/4.0rc1-candidates/build1

I'm not sure what -j should end up being yet, maybe we'll stick with 4 for now?

KeyboardInterrupt still doesn't seem to be working right, despite what the comments say it only seems to be killing a single thread. Any idea how to fix that? Should I just rip it out and make it clear that you need to use 'kill' to terminate mid-run?

Comment 45

[[e-mail] (:bhearsum)]

Attachment: get rid of KeyboardInterrupt handling

After a few more attempts I still couldn't get KeyboardInterrupt handling to work properly, so I ripped it out entirely and added a big warning about killing it.

This patch is 100% done now, as far as I know.

Comment 46

[[e-mail] (:bhearsum)]

Attachment: [untested] adjust invocation

Comment 47

[[e-mail] (:bhearsum)]

Attachment: [untested] add extract and run to puppet stage class

Comment 48

[[e-mail] (:bhearsum)]

Attachment: fix sys.path addition, push-to-mirrors.py

Small updates here:
- Fix sys.path appending to account for symlinks, because that's how we'll be using it.
- Fix bug in push-to-mirrors.py patch

This has been running fine in staging, albeit really slowly. We may have to move staging-stage to a better disk/ESX host for this to actually work.

Comment 49

[[e-mail] (:bhearsum)]

Attachment: add p7zip to stage class, change fake clamdscan

We need p7zip to extract installers, and I changed the fake clamdscan to improve the output a bit.

Comment 50

[[e-mail] (:bhearsum)]

Attachment: updated puppet patch to use the right RPM (el4 vs el5)

Comment 51

[Rail Aliiev [:rail]]

Comment on attachment 518734 [details] [diff] [review]
fix sys.path addition, push-to-mirrors.py

r+ with the following 2 nits:
1. mar.py: use "#/usr/bin/env python" shebang
2. extract_and_run_command.py: use os.path.splitext instead of filename.split('.')[-1]

Otherwise looks good.

Comment 52

[Rail Aliiev [:rail]]

BTW, dmg files could be also extracted by mounting them:

bzcat file.dmg > file.hfs
sudo mount -t hfsplus -o loop file.hfs /mnt/dmg_dir

what requires sudo or a special fstab entry :/

Comment 53

[[e-mail] (:bhearsum)]

Attachment: updated puppet patch

Uses the EL4 RPM, also has extract_and_run_command.py / updated clmadscan. I tested the $@ on a Linux staging slave, and it worked fine.

Comment 54

[[e-mail] (:bhearsum)]

(In reply to comment #52)
> BTW, dmg files could be also extracted by mounting them:
> 
> bzcat file.dmg > file.hfs
> sudo mount -t hfsplus -o loop file.hfs /mnt/dmg_dir
> 
> what requires sudo or a special fstab entry :/

Don't think we'll be getting that on stage, and I'm going to resist further scope-creep here and explicitly defer it :).

Comment 55

[[e-mail] (:bhearsum)]

Attachment: v4, deadlocking fixed

This version fixes the potential deadlock caused by using proc.wait() without sending stdout somewhere.

Comment 56

[[e-mail] (:bhearsum)]

Dave, the Releng side of this is done now. Can you:
- Add a symlink from /usr/local/bin/extract_and_run_command.py -> /home/ffxbld/bin/extract_and_run_command.py
- Adjust the continuous virus scan to use the following command:
/usr/local/bin/extract_and_run_command.py -j4 [your normal clamd invocation] -- [files to scan]

Let me know if you have any questions!

Comment 57

[Rail Aliiev [:rail]]

Attachment: chmod extract_and_run_command.py

extract_and_run_command.py should be chmoded 755

Comment 58

[[e-mail] (:bhearsum)]

Any chance that the symlink part of this can be done today, so that the 4.0rc2 virus scan works correctly?

Comment 59

[Rail Aliiev [:rail]]

Comment on attachment 520162 [details] [diff] [review]
chmod extract_and_run_command.py

Pushed http://hg.mozilla.org/build/tools/rev/e28488b561ea

[~ffxbld/checkouts/hg-build-tools@stage.m.o]
hg pull -u

Comment 60

[Dave Miller [:justdave]]

symlink is in place on surf.  Working on initial checkout/setup on one of the virus scan machines to test there.

Comment 61

[[e-mail] (:bhearsum)]

Attachment: handle files, too

justdave found a bug in extract_and_run_command.py: it doesn't handle files, only dirs. This patch fixes it:
[] bhearsum@voot:~/Mozilla/checkouts/tools/stage$ python extract_and_run_command.py -j4 echo -- ~/tmp/abcd/setup.exe
START 10:28:56: /home/bhearsum/tmp/abcd/setup.exe
/home/bhearsum/tmp/abcd/setup.exe
END 10:28:56 (0 seconds elapsed): /home/bhearsum/tmp/abcd/setup.exe

Comment 62

[[e-mail] (:bhearsum)]

Attachment: append dirname to tempdir

Justdave says that this will make the output much easier to parse/understand.

Comment 63

[Dave Miller [:justdave]]

Comment on attachment 520205 [details] [diff] [review]
append dirname to tempdir

Should include a directory named after the file being unpacked, too.

Comment 64

[Dave Miller [:justdave]]

i.e. if you're unpacking /pub/mozilla.org/foo.exe then the tempdir should be
 {tempdir}/pub/mozilla.org/foo.exe/{unpacked files}
not
 {tempdir}/pub/mozilla.org/{unpacked files}

Comment 65

[[e-mail] (:bhearsum)]

Attachment: v2

good catch!

Comment 66

[[e-mail] (:bhearsum)]

Attachment: v3

Better way of appending the filename, also fixes relative paths to work better, by using abspath() in find_files().

Comment 67

[[e-mail] (:bhearsum)]

Comment on attachment 520210 [details] [diff] [review]
v3

This version doesn't clean up very well....going to fix that.

Comment 68

[[e-mail] (:bhearsum)]

Attachment: clean up properly

Comment 69

[[e-mail] (:bhearsum)]

Dave, I'm all done here, anything else you need to do?

Comment 70

[Nick Thomas [:nthomas] (UTC+12)]

Apparently we are running scan-uploaded-files.sh somewhere (this file lives at, eg, upload1.dmz.scl3:/usr/local/bin/) because there is an up-to-date timestamps in /data/. That scripts calls out to scanfile, which is just doing a clamdscan. It could be calling extract_and_run_command.py to handle this. For example, our explicit scanning of release files does this:
 extract_and_run_command.py -j2 clamdscan -m --no-summary -- /pub/mozilla.org/thunderbird/nightly/13.0-candidates/build1/