SNORT - Snort 2.9.2.3 pkg v. 2.2.1 process do not quit via update scripts or GUI
-
Fixed that for ya:
https://github.com/bsdperimeter/pfsense-packages/commit/cd645a1b11544eda4f3db68ba49caaec8bbf973e -
Huh. I spoke too soon. While it is true the rc_stop needed to be added to the snort.sh file, the problem is still not fixed.
There is one other problem in the /usr/local/pkg/snort/snort.inc file. Line number 894 currently reads as:
if [ "`/bin/pgrep -nF {$g['varrun_path']}/snort_{$if_real}{$snort_uuid}.pid`" = "0" ]; thenIt should be:
if [ "`/bin/pgrep -nF {$g['varrun_path']}/snort_{$if_real}{$snort_uuid}.pid`" != "0" ]; thenThe reason is that this line is part of the rc_stop() function and is checking to see if there are any existing instances of Snort running. "0" simply means the command completed. However, if an active pid is found in the pid file, that pid number is returned. Therefore, "=" should be "!=" in order for the pkill command to be run, thus stopping any running instances. This also needs to be updated in the snort.sh file and can be done by either making some arbitrary change to your snort config, saving, changing back, then resaving or manually editing /usr/local/etc/rc.d/snort.sh so that in the rc_stop() function, each instance of the above if statement is updated to "!=". Here's an example:
if [ "`/bin/pgrep -nF /var/run/snort_re027549.pid`" = "0" ]; thenbecomes:
if [ "`/bin/pgrep -nF /var/run/snort_re027549.pid`" != "0" ]; thenThe pid file name will be different for each instance, so don't worry about matching the name.
If you manually edit the snort.sh file DO NOT make this change to the rc_start() function. It messes things up if you do.
-
Fixed that for ya:
https://github.com/bsdperimeter/pfsense-packages/commit/cd645a1b11544eda4f3db68ba49caaec8bbf973eWhat does this mean? Do you mean that, although there isn't a new version of the snort package, you applied a fix to the problem where snort doesn't cleanup old instances during update?
-
What does this mean? Do you mean that, although there isn't a new version of the snort package, you applied a fix to the problem where snort doesn't cleanup old instances during update?
Yes, he fixed the code in the repository. Snort.inc is a support file that is downloaded during the post-install of Snort. So, anyone installing/reinstalling Snort will have the fixed snort.inc. However, I just applied the fixed listed in this thread (there are two separate posts that make for the entire fix, look for the ones with smiley faces) to my pfSense server manually rather than waiting for the repository to get corrected. As of this posting, only the first part, the missing rc_stop function call, is fixed in the repository.
-
Just realized all I needed to do was create a GitHub account and I could do the second part of the fix. So, I did. Just waiting for an admin to approve the change and pull it back into the master branch. Once that is done, this problem should be completely fixed.
-
What does this mean? Do you mean that, although there isn't a new version of the snort package, you applied a fix to the problem where snort doesn't cleanup old instances during update?
Yes, he fixed the code in the repository. Snort.inc is a support file that is downloaded during the post-install of Snort. So, anyone installing/reinstalling Snort will have the fixed snort.inc. However, I just applied the fixed listed in this thread (there are two separate posts that make for the entire fix, look for the ones with smiley faces) to my pfSense server manually rather than waiting for the repository to get corrected. As of this posting, only the first part, the missing rc_stop function call, is fixed in the repository.
Great! Thanks for investigating this issue. Unfortunately, I attempted the manual fix. While it appeared to work at first, I woke up only to once again find 4-6 instances of snort (I only have 2 interfaces) after the nightly update of definitions so it didn't work as I had hoped. By sounds of it, we shouldn't have to suffer very much longer.
-
Re-install Snort (I did remove + install from gui) and you should be ok. There's no need for the package version number to be bumped for these fixes to be applied.
-
Actually the fix was committed and the bump of the version was done for this specific issue.
-
@ermal:
Actually the fix was committed and the bump of the version was done for this specific issue.
Thank you SO MUCH Ermal. Am installing the latest SNORT now (2.9.2.3 pkg v. 2.2.2).
-
Re-install Snort (I did remove + install from gui) and you should be ok. There's no need for the package version number to be bumped for these fixes to be applied.
Sadly, I removed and re-installed snort from the packages list and the problem persists. After performing a manual update of snort definitions followed by a 'pgrep snort' command, it returns the following:
$ pgrep snort
26593
23394
43660
33218However I only have snort installed on 2 interfaces. In other words, it's still not killing the original process. After pgrep i should only have (2) PIDs. I must continue to issue a 'pkill snort' and then restart the snort service to fix. manually.
-
Sadly, I removed and re-installed snort from the packages list and the problem persists. After performing a manual update of snort definitions followed by a 'pgrep snort' command, it returns the following:
$ pgrep snort
26593
23394
43660
33218However I only have snort installed on 2 interfaces. In other words, it's still not killing the original process. After pgrep i should only have (2) PIDs. I must continue to issue a 'pkill snort' and then restart the snort service to fix. manually.
I'm not seeing this at all, reinstalled from previous version of package and all is good now with start and restart of snort only showing one instance.
-
@ermal:
Actually the fix was committed and the bump of the version was done for this specific issue.
Hey, Ermal;
Sorry to tell you this, but the changes that jimp and myself put into the repository are not there. Even jimp's is gone and that was there when I added my fix. As such, line 894 still needs the "!=" and rc_stop needs to be added at line 937. I've re-added them and requested a new merge to see if it takes this time:
https://github.com/bsdperimeter/pfsense-packages/pull/277
I figured this out by reinstalling the package and looking at the snort.sh file. It was missing the required changes.
-
I reinstalled before the package version number was bumped and mine is running fine so far since last morning. I guess I got lucky with timing :)
Available: 2.9.2.3 pkg v. 2.2.2
Installed: 2.9.2.3 pkg v. 2.2.1 -
@ermal:
Actually the fix was committed and the bump of the version was done for this specific issue.
Hey, Ermal;
Sorry to tell you this, but the changes that jimp and myself put into the repository are not there. Even jimp's is gone and that was there when I added my fix. As such, line 894 still needs the "!=" and rc_stop needs to be added at line 937. I've re-added them and requested a new merge to see if it takes this time:
https://github.com/bsdperimeter/pfsense-packages/pull/277
I figured this out by reinstalling the package and looking at the snort.sh file. It was missing the required changes.
Put a comment on the pull request but those were not the right fixes.
Especially the != 0 is wrong. -
The latest version Snort 2.9.2.3 pkg v. 2.2.2 has the same error.
Could not remove pid file /var/run/snort_pppoe013406.pid: No such file or directoryYesterday all works fine for me.
to reproduce: go to Services and klick on restart. I got twice snort pids. (Rule Update is the same…)
-
@HOD:
The latest version Snort 2.9.2.3 pkg v. 2.2.2 has the same error.
Could not remove pid file /var/run/snort_pppoe013406.pid: No such file or directoryYesterday all works fine for me.
to reproduce: go to Services and klick on restart. I got twice snort pids. (Rule Update is the same…)
Same issue here. Appears 2.9.2.3 v. 2.2.2 hasn't resolved the issue as we had hoped.
-
@ermal:
Put a comment on the pull request but those were not the right fixes.
Especially the != 0 is wrong.Then why is it this fixed my snort? Changing to != stopped my snort from not killing existing instances when the service is restarted. The "0" simply means the process exited without error. However, that if statement is looking for existing instances in the PID file. When pgrep finds a pid number in the pid file, it returns that number rather than 0. Therefore, it SHOULD be != or the script will not work. I suggest you try it, you'll see I'm right. Just issue that pgrep command as part of an echo statement at a command prompt and see what it returns when the PID file has a valid instance and when it does not. If an instance exists, you'll see the pid number, which is when you should run pkill to stop it, since we are looking to stop existing instances. If an instance does not exists, you will get 0. In that case, no running instances, so don't bother trying to stop them with pkill. Honestly, I did actually test this out before suggesting the fix.
-
2.2.2 fixes the issue for me :)
-
@ermal:
Put a comment on the pull request but those were not the right fixes.
Especially the != 0 is wrong.Then why is it this fixed my snort? Changing to != stopped my snort from not killing existing instances when the service is restarted. The "0" simply means the process exited without error. However, that if statement is looking for existing instances in the PID file. When pgrep finds a pid number in the pid file, it returns that number rather than 0. Therefore, it SHOULD be != or the script will not work. I suggest you try it, you'll see I'm right. Just issue that pgrep command as part of an echo statement at a command prompt and see what it returns when the PID file has a valid instance and when it does not. If an instance exists, you'll see the pid number, which is when you should run pkill to stop it, since we are looking to stop existing instances. If an instance does not exists, you will get 0. In that case, no running instances, so don't bother trying to stop them with pkill. Honestly, I did actually test this out before suggesting the fix.
Usually i do not go into this kind of debate but from pgrep(1)
EXIT STATUS
The pgrep and pkill utilities return one of the following values upon
exit:0 One or more processes were matched.
1 No processes were matched.
2 Invalid options were specified on the command line.
3 An internal error occurred.
And from testing this on the shell
Finidng the pid
pfsense-dev# pgrep -x cron
1519
pfsense-dev# echo $?
0
pfsense-dev#Not finding
pfsense-dev# pgrep -x init
pfsense-dev# echo $?
1
pfsense-dev# -
This is a modified version of my last post, those that read the earlier version, my apologies. I'm having a bad day and failed to check myself before making an ass of myself. I hope my attitude didn't put anyone off. That aside, here is the modified version:
The changes I made to snort.inc and, as a consequence, snort.sh fixed my snort. I now only have one copy of any given instance running on my system and when I update the rules, manually stop/start an instance, or even reboot the server.
Without these changes, including the most recent version you released (v.2.2.2) snort does not restart correctly.
You are not looking at the behavior of the test command in relation to it's output and pgrep's output. In truth, my solution is a bit wrong also. Look at this:
echo test "`/bin/pgrep -nF /var/run/snort_re027549.pid`" != "0" test 39615 != 0This is the command you are actually running. See how the pid is the first value being tested? With how you have the script setup, you are checking to see if "39615 = 0", which it will never be, so test fails, thus, code not run. With my original solution (used in the above example), the test is to see if "39615 != 0", which it always is, so the code is run. In other words, the exit code is not being checked, the return value is. If a pid exists, that number is returned. Now, let's look at when the pid file doesn't exist:
echo test "`/bin/pgrep -nF /var/run/snort_re0275491.pid`" != "0" pgrep: Cannot open pidfile `/var/run/snort_re0275491.pid': No such file or directory test != 0So, now we can't find the file and the test is invalid (since the first parameter doesn't even exist, which causes test to throw an "unexpected operator" error). So the code is not run; which is good since the pid file doesn't exist, so neither do any processes. So, the outcome is correct, but the code is wrong.
Okay, so we are both wrong. How do we fix it. The perfect solution would first check for an existing pid file, then perform the check to make sure the code returned is greater than 3 (since this is the highest error code pgrep uses, thus we can assume a valid pid was found). So:
if [ -e /var/run/snort_re027549.pid ]; then if [ `/bin/pgrep -nF /var/run/snort_re027549.pid` -gt 3 ]; thenNow we know the file exists, so the test can actually do something accurate and useful. Also, whatever the output is, it exceeds the highest exit code (0-3) which could be used as a return value, so we know we have a pid number. Since we are trying to stop an existing process before starting new ones, this is good, so run the code.
As for letting rc_start() handle the restart code all by itself, you would need to use the same code as I've explained above, and, rather than putting the start command inside an else{} statement, simply call rc_stop() inside of rc_start(). This would best handle the issue without code redundancy. I know it means losing the -HUP from the pkill command but, it isn't necessary and I've seen -HUP fail to stop a process if it is currently busy, which Snort tends to be. Also, I know this would mean barnyard2 is killed irregardless of situation. This could be settled by adding a boolean value: if true, kill barnyard; if false, don't kill barnyard. It might also be necessary to add to the block of code that actually starts snort and barnyard2, a check to make sure that the snort processes stopped by rc_stop() are actually done closing out before starting the new ones. This could be a looped code that, if pgrep finds processes, sleep one second, then check again, otherwise, start snort and barnyard2, then break out.
I'm going to put the more relevant parts of these suggestions into a corrected snort.inc file and post another pull request later today. My corrections will simply remove the if-statement part of the if-else and leave the else-statement part in tact from rc_start(). i.e., I'm going to disable the if-else statement while leaving the else part in tact. I'm also going to remove the line that deletes the pid file (since rc_stop takes care of this) and add at the very top of rc_start() a call to rc_stop.
In rc_stop(), I'm going to add the double if-statement listed in this post to check for the pid file, then run a check for instances in it. I'm also going to do a check for the pid file on the rm statement that happens right after the pkill commands are run.
Ermal, I ask that you test this out to see it actually fixes the problem. The changes I'm proposing are active on my pfsense server right now and they work. I no longer have issues with restarting or starting Snort.
