sshguard-users — User questions and answers, bugs, and general support

Re: [Sshguard-users] Scripting firewall backends

[Willem J. W. <wj...@di...>]

On 30/06/2015 15:01, Kevin Zheng wrote:
> On 06/30/2015 17:01, Willem Jan Withagen wrote:
>> I'm doing more or less the same but using the NULL firewall... and then
>> use the option -s to tell it what to really do....
>>
>> /usr/local/sbin/sshguard -e /usr/local/sbin/sshguard-ipfwtable
>>
>> Does the above mean that you are going to "kill" this facility for the 
>> time being?
> 
> No. Among other issues, I need to fix this before merging it back in.
> Since it looks like you're using IPFW, you're more than welcome (and
> encouraged!) to take a look at the work in progress in the 'newfw'
> branch. Feedback is welcome!

I've long patched older versions to get te ipfw implementation doing
what I now have in null/script. And with IPFW is see little advantage to
put the backend into compiled program... Adding rules is rarely
something that puts a heavy load on the system.

> If you also happen to run FreeBSD, and you're feeling adventurous, you
> can also try the 'capsicum' branch which adds preliminary sandboxing
> support using Capsicum. Many things are broken, namely logsucker, syslog
> logging, and probably procauth, but it appears to work.

I'm a FreeBSD user as of 1.0 :) So that's close to ~25 years...

Although I appreciate the Capsicum implementation it is not something to
start loading on my customers...

All that said, if I can find some spare time, I'll check it out...

Regards,
--Willem Jan

Re: [Sshguard-users] Scripting firewall backends

[Kevin Z. <kev...@gm...>]

On 06/30/2015 17:01, Willem Jan Withagen wrote:
> I'm doing more or less the same but using the NULL firewall... and then
> use the option -s to tell it what to really do....
> 
> /usr/local/sbin/sshguard -e /usr/local/sbin/sshguard-ipfwtable
> 
> Does the above mean that you are going to "kill" this facility for the 
> time being?

No. Among other issues, I need to fix this before merging it back in.
Since it looks like you're using IPFW, you're more than welcome (and
encouraged!) to take a look at the work in progress in the 'newfw'
branch. Feedback is welcome!

If you also happen to run FreeBSD, and you're feeling adventurous, you
can also try the 'capsicum' branch which adds preliminary sandboxing
support using Capsicum. Many things are broken, namely logsucker, syslog
logging, and probably procauth, but it appears to work.

Thanks,
Kevin Zheng

-- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090

Re: [Sshguard-users] Scripting firewall backends

[Willem J. W. <wj...@di...>]

On 28-6-2015 12:46, Kevin Zheng wrote:
> The current 'command' backend uses the shell to construct and
> execute firewall commands. Those are hard-coded in the binary and
> runs in the context of the SSHGuard process via the system() call.
>
> As the aix, ipf, and iptables backends show, those commands can get
> quite complex. In the interest of making life easier, I've created a
> new branch that delegates executing firewall commands to a separate
> 'sshg-fw' binary, currently implemented as a shell script. The
> script reads commands from standard input and issues the appropriate
> commands to the system firewall. This is a step towards privilege
> separation.
>
> Lots of work remains, in particular, doing this in a portable way,
> and actually re-implementing the aix, ipf, iptables, and hosts
> backends. Currently only null, pf, and ipfw are supported, and
> external commands simply aren't executed for the time being.

I'm doing more or less the same but using the NULL firewall... and then
use the option -s to tell it what to really do....

/usr/local/sbin/sshguard -e /usr/local/sbin/sshguard-ipfwtable

Does the above mean that you are going to "kill" this facility for the 
time being?

--WjW

Re: [Sshguard-users] Scripting firewall backends

[Rick H. <ri...@ta...>]

> In the interest of making life easier, I've created a new branch
> that delegates executing firewall commands to a separate 'sshg-fw'
> binary, currently implemented as a shell script. [...] This is a
> step towards privilege separation.

Thanks!

[Sshguard-users] Scripting firewall backends

[Kevin Z. <kev...@gm...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Greetings,

The current 'command' backend uses the shell to construct and execute
firewall commands. Those are hard-coded in the binary and runs in the
context of the SSHGuard process via the system() call.

As the aix, ipf, and iptables backends show, those commands can get
quite complex. In the interest of making life easier, I've created a
new branch that delegates executing firewall commands to a separate
'sshg-fw' binary, currently implemented as a shell script. The script
reads commands from standard input and issues the appropriate commands
to the system firewall. This is a step towards privilege separation.

Lots of work remains, in particular, doing this in a portable way, and
actually re-implementing the aix, ipf, iptables, and hosts backends.
Currently only null, pf, and ipfw are supported, and external commands
simply aren't executed for the time being.

Those interested should try out the 'newfw' branch, available on
Bitbucket. Comments and suggestions are welcome. If this idea is
sound, help will be appreciated, especially with re-writing the
scripts for the various backends and testing them on different systems.

Thanks,
Kevin Zheng

- -- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVj9B1AAoJEOrPD3bCLhCQSuYH/jQPd18jV3pyd/OH0/P28BuA
H8Mm4cBtWCLNUARIgCkHQr/w+l6Ga7JyjoZUA3Y68Vu66hHL2U3PANZQPgQUjPtQ
zdhwMxV+5e9TvRKIdPqFZVUFboBY+KCyRpzvAyRDNa9VlHazaeuYxSxjAJC3q78b
1hNrGtmyKAmKwKSWReKe53G/I6dTL5z9LAx8g/6ZRHkSRatgB6Qlyd02umBG3g7A
Pnd1NvEVJQmKotzBhjT4/L8YFWCO9uECG3Wx52ZLp0COtCkGph4F6cPfZwL/vHEW
8j80fwsdnZImlSym6Cq290gdXU6e9zwTCGKraVEMGUH+/ssEw2yrZew1UP8a6gg=
=/M22
-----END PGP SIGNATURE-----

Re: [Sshguard-users] New ipfw backend

[Kevin Z. <kev...@gm...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Greetings,

On 06/21/2015 23:43, SASAKI Katuhiro wrote:
> 1. In the viewpoint of ipfw , tables are specified by number (0 to 
> 65535). We can't assign the name like "sshguard" for tables. It 
> became necessary to replace "sshguard" with some number (22, for 
> example).

Thanks for pointing this out; I realized this eventually. The
implementation currently in the 'master' branch is correct.

> 2. Command "ipfw table [table number] add" can receive only one
> target (IP address, and some other search keys) at a time. Using
> loop in "COMMAND_BLOCK_LIST" looks reasonable for me.

I believe the original intent for using COMMAND_BLOCK_LIST was to
reduce the number of calls to system(), thus reducing the number of
shells that are spawned. So far this isn't an issue, so I'm erring on
the side of keeping this simple.

Thanks,
Kevin Zheng

- -- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJViBd0AAoJEOrPD3bCLhCQN4AIAJGLFd0XEZaPug2ki8DSlIFv
xVN6Ha4sAOadGIu7/Nlq7dTTFuADW2o0FvlMeRW1UiRUZ+mfKzgS5bHZtwUtcxa3
SAjNj/N2sCslYZZIpQSSexlLLWNBQtQvjF/yIycXGYfnnKxD6OIem3kbkzGMFhKt
hpbt9bDPRtFECXOG2xQCjbXp4fzokjgG6P4DORrzUmE/KFywG6rom4dVnNAmYqjn
tlJ5la80Rre8IlR1O8i6wGd6HG/hy3Bph5th3Mb3Ql2A1RrVRS6H6SzNl6xkuN80
0T7+WPGNDkyxxD4UPRGvFKNsPOsYRE/ImtKsgK68ZiMpapDCIVmVMJGgNnymUwA=
=El8h
-----END PGP SIGNATURE-----

Re: [Sshguard-users] New ipfw backend

[SASAKI K. <cr...@sa...>]

Hi.

> In light of the recent `ipfw` issues I've decided to re-implement the
> `ipfw` backend using the command framework that is used for nearly all
> of the other backends.
> 
Great!

> Please don't test this in a production environment, and if you test it
> at all, be aware that bad things can happen. Please take a look at the
> patch before you try to run this code.
> 
I tested the patch with 1.6.0 on my FreeBSD 10.1R/i386. Two problems
below are found.

1. In the viewpoint of ipfw , tables are specified by number (0 to
65535). We can't assign the name like "sshguard" for tables. It
became necessary to replace "sshguard" with some number (22, for
example).

2. Command "ipfw table [table number] add" can receive only one target
(IP address, and some other search keys) at a time. Using loop in
"COMMAND_BLOCK_LIST" looks reasonable for me.

Attached is patch for
0001-Reimplement-ipfw-backend-using-command-framework.patch.

Thank you.


-- 
                     SASAKI Katuhiro

                 mailto: cr...@sa...

Re: [Sshguard-users] Patch to fix blacklist loading with ipfw

[Greg P. <gr...@n0...>]

Kevin Zheng said:
> On 06/07/2015 10:39, Greg Putrich wrote:
> > I?ve been running this for the past week and its been working fine. When
> > sshguard is stopped, table 22 is cleared out, when its started, it
> > re-populates the table. My tables currently has 42 IP addresses and I
> > had started with nothing existing in blacklist.db (I did test with my
> > old list of 200+ addresses, but decided to start fresh to watch for new
> > IP addresses). 
> 
> Fantastic; good to hear it works. I should have this backported to the
> 1.6 branch so it's available when 1.6.1 rolls around.
> 
> Thanks,
> Kevin Zheng


Awesome, that's great news. 

   Greg

Re: [Sshguard-users] Patch to fix blacklist loading with ipfw

[Kevin Z. <kev...@gm...>]

On 06/07/2015 10:39, Greg Putrich wrote:
> I’ve been running this for the past week and its been working fine. When
> sshguard is stopped, table 22 is cleared out, when its started, it
> re-populates the table. My tables currently has 42 IP addresses and I
> had started with nothing existing in blacklist.db (I did test with my
> old list of 200+ addresses, but decided to start fresh to watch for new
> IP addresses). 

Fantastic; good to hear it works. I should have this backported to the
1.6 branch so it's available when 1.6.1 rolls around.

Thanks,
Kevin Zheng

-- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090

Re: [Sshguard-users] Patch to fix blacklist loading with ipfw

[Greg P. <gr...@n0...>]

On May 29, 2015, at 15:18 , Kevin Zheng <kev...@gm...> wrote:
> 
> Signed PGP part
> On 05/26/2015 22:08, Greg Putrich wrote:
> > It would certainly keep the rules tidier. At 200 rules from
> > sshguard, that's becoming a mess, but still manageable. For a
> > popular Internet host, it could easily become thousands/tens of
> > thousands which becomes silly in trying to manage the other normal
> > rules.
> >
> > I think the table would be a good option to keep order to the rule
> > set. While we would miss out on seeing which addresses are active,
> > I don't think that is all of that big of a concern (at worst, clear
> > the table to start over and the frequent pests would be blocked
> > quickly again).
> 
> The new backend is now available in the 'ipfw' branch of the Bitbucket
> repository. This time I was able to actually test it, and it appears
> to work reasonably well. In order to use it, you will need a rule like
> the following in your ipfw ruleset:
> 
> reset ip from table(22) to me
> 
> Currently, SSHGuard uses a fixed table number, '22', to store
> blacklisted addresses. This table is cleared when SSHGuard exits.
> 
> If there are no issues with this backend, it should appear in the
> 'master' branch and will be backported to 1.6.
> 
> Thanks,
> Kevin Zheng
> 
> --
> Kevin Zheng
> kev...@gm... | ke...@kd... | PGP: 0xC22E1090


I’ve been running this for the past week and its been working fine. When sshguard is stopped, table 22 is cleared out, when its started, it re-populates the table. My tables currently has 42 IP addresses and I had started with nothing existing in blacklist.db (I did test with my old list of 200+ addresses, but decided to start fresh to watch for new IP addresses).


Thanks,
Greg Putrich

Re: [Sshguard-users] Patch to fix blacklist loading with ipfw

[Greg P. <gr...@n0...>]

Hello Kevin, 

Compiled it on a test box (FreeBSD 10.1) and its working as expected. 

Now will put it on a machine that's connected to the outside and see
what it picks up. 

Greg



Kevin Zheng said:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> On 05/26/2015 22:08, Greg Putrich wrote:
> > It would certainly keep the rules tidier. At 200 rules from
> > sshguard, that's becoming a mess, but still manageable. For a
> > popular Internet host, it could easily become thousands/tens of
> > thousands which becomes silly in trying to manage the other normal
> > rules.
> > 
> > I think the table would be a good option to keep order to the rule
> > set. While we would miss out on seeing which addresses are active,
> > I don't think that is all of that big of a concern (at worst, clear
> > the table to start over and the frequent pests would be blocked
> > quickly again).
> 
> The new backend is now available in the 'ipfw' branch of the Bitbucket
> repository. This time I was able to actually test it, and it appears
> to work reasonably well. In order to use it, you will need a rule like
> the following in your ipfw ruleset:
> 
> reset ip from table(22) to me
> 
> Currently, SSHGuard uses a fixed table number, '22', to store
> blacklisted addresses. This table is cleared when SSHGuard exits.
> 
> If there are no issues with this backend, it should appear in the
> 'master' branch and will be backported to 1.6.
> 
> Thanks,
> Kevin Zheng
> 
> - -- 
> Kevin Zheng
> kev...@gm... | ke...@kd... | PGP: 0xC22E1090
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
> 
> iQEcBAEBCAAGBQJVaMmnAAoJEOrPD3bCLhCQ8M0IAJoxH6K6VeV8bnIO+jHsbajX
> h4pj24yCg21ADorHQHrMU9JBKVQZXGNjuCYH/q7Fc4MQfofvGwx63WgwYhfq/6O6
> /IEtlLuCQ2ri6+pxrzV3np6o0VMajBPZcyWsepGA0aJcqeXFcKnP/9qki7bedTE1
> qlL3SWt7nluJRkcgJ29ou0tYQt6x5xNst4/8FU00v+BxY2WEk5XbbJ/bWlS4lxIW
> t+XJHSLe/cqK9ylRvhXUw4f4Cs5epqWJTlP5fB0v4hTiZZ/hVYbpoVjbrlAHYmxa
> Lhl/rGjik6URsU9e7XmQDc7TnM2ec8Sl+26zfJm/OvUyzwnwTmKI2SQRikBuW5E=
> =wHdI
> -----END PGP SIGNATURE-----
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Sshguard-users mailing list
> Ssh...@li...
> https://lists.sourceforge.net/lists/listinfo/sshguard-users

Re: [Sshguard-users] Patch to fix blacklist loading with ipfw

[Kevin Z. <kev...@gm...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 05/26/2015 22:08, Greg Putrich wrote:
> It would certainly keep the rules tidier. At 200 rules from
> sshguard, that's becoming a mess, but still manageable. For a
> popular Internet host, it could easily become thousands/tens of
> thousands which becomes silly in trying to manage the other normal
> rules.
> 
> I think the table would be a good option to keep order to the rule
> set. While we would miss out on seeing which addresses are active,
> I don't think that is all of that big of a concern (at worst, clear
> the table to start over and the frequent pests would be blocked
> quickly again).

The new backend is now available in the 'ipfw' branch of the Bitbucket
repository. This time I was able to actually test it, and it appears
to work reasonably well. In order to use it, you will need a rule like
the following in your ipfw ruleset:

reset ip from table(22) to me

Currently, SSHGuard uses a fixed table number, '22', to store
blacklisted addresses. This table is cleared when SSHGuard exits.

If there are no issues with this backend, it should appear in the
'master' branch and will be backported to 1.6.

Thanks,
Kevin Zheng

- -- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVaMmnAAoJEOrPD3bCLhCQ8M0IAJoxH6K6VeV8bnIO+jHsbajX
h4pj24yCg21ADorHQHrMU9JBKVQZXGNjuCYH/q7Fc4MQfofvGwx63WgwYhfq/6O6
/IEtlLuCQ2ri6+pxrzV3np6o0VMajBPZcyWsepGA0aJcqeXFcKnP/9qki7bedTE1
qlL3SWt7nluJRkcgJ29ou0tYQt6x5xNst4/8FU00v+BxY2WEk5XbbJ/bWlS4lxIW
t+XJHSLe/cqK9ylRvhXUw4f4Cs5epqWJTlP5fB0v4hTiZZ/hVYbpoVjbrlAHYmxa
Lhl/rGjik6URsU9e7XmQDc7TnM2ec8Sl+26zfJm/OvUyzwnwTmKI2SQRikBuW5E=
=wHdI
-----END PGP SIGNATURE-----

Re: [Sshguard-users] On initiating sshguard

[Kevin Z. <kev...@gm...>]

Hi Rick,

On 05/21/2015 13:04, Rick Hanson wrote:
> My confusion is that I seem to see two different ways to launch/run
> sshguard.  One is from syslogd in syslog.conf, e.g.
> 
> auth.info <http://auth.info>;authpriv.info <http://authpriv.info>
> |/usr/local/sbin/sshguard [options here]
> 
> and another is to run sshguard as daemon from (the package installed)
> /usr/local/etc/rc.d/sshguard script, which tells sshguard to read
> auth.log. (And is a very nicely documented script to boot BTW.)
> 
> Both methods together *seem* redundant, but how should I be running
> sshguard: both ways or only one way (i.e. but not the other)?

Yep, you just pick one or another. SSHGuard can either read the log
files by itself, or have them piped in from syslogd.

Best,
Kevin Zheng

-- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090

Re: [Sshguard-users] New ipfw backend

[Rick H. <ri...@ta...>]

> and thus have sshguard call a script:
>         /usr/local/sbin/sshguard-ipfwtable

Very cool, Willem.  Thanks for sharing!

Re: [Sshguard-users] New ipfw backend

[Willem J. W. <wj...@di...>]

On 27-5-2015 00:32, Kevin Zheng wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Hi there,
>
> In light of the recent `ipfw` issues I've decided to re-implement the
> `ipfw` backend using the command framework that is used for nearly all
> of the other backends. Since I don't run `ipfw` on my machine, I'm
> unable to test this patch.
>
> If you are running `ipfw` and are willing to test-drive this new and
> more than likely broken backend, apply the attached patch, compile,
> and take it for a whirl. In particular, I'm not sure if the "add
> multiple addresses" part works, so if you have a large blacklist that
> crashed the original ipfw backend try it on the new one.
>
> The new backend operates on ipfw tables. You'll need to set up your
> firewall with a tabled named 'sshguard'. SSHGuard (should) add
> attackers to this table; you'll need to set up the rules yourself.
>
> Please don't test this in a production environment, and if you test it
> at all, be aware that bad things can happen. Please take a look at the
> patch before you try to run this code.

What I do is use the new backend called none.

The commandline looks like:
/usr/local/sbin/sshguard \
	-e /usr/local/sbin/sshguard-ipfwtable \
	-b 40:/var/db/sshguard/blacklist.db \
	-l /var/log/auth.log -l /var/log/maillog -l /var/log/messages \
	-a 40 -p 420 -s 1200 \
	-w /usr/local/etc/sshguard.whitelist \
	-i /var/run/sshguard.pid

and thus have sshguard call a script:
	/usr/local/sbin/sshguard-ipfwrable


Which contains the actual ipfw code to do FW managment.
Easier to maintain, easier to debug the ipfw stuff. And compared to the 
old ways, I get to put the rules in a table, which is more efficient in 
ipfw. And then in the firewall use the table in blocking rules...

Advantage of that is with an ipfw flush or service ipfw restart, .... it 
doesn't destroy the blacklist set. So I can tinker with the firewall 
without disrupting anything sshguard has added.

Not sure if the none backend also sufferes from the long-list problem 
that Kevin fixed.
One advantage at least here was that it would process all of the items 
in the list, except for the (possibly) corrupted last one.

--WjW

====
#!/bin/sh

IPFW=/sbin/ipfw
# for debugging
# IPFW=/bin/echo
IPFWTABLE=22

# echo  $*
# printenv | grep SSH

case $SSHG_ACTION in
         init)
                 # echo init
                 ;;
         flush)
                 # echo flush
                 ${IPFW} table ${IPFWTABLE} flush
                 ;;
         block)
                 # echo block
                 ${IPFW} table ${IPFWTABLE} add $SSHG_ADDR
                 ;;
         block_list)
                 # echo block_list
                 for a in `echo $SSHG_ADDR | sed 's/,/ /g'` ; do
                         ${IPFW} table ${IPFWTABLE} add $a
                 done
                 ;;
         fin)
                 # echo finish
		# ignore, and leave the blacklist as is
                 ;;
         *)
                 echo not implemented:
                 echo    $SSHG_ACTION
                 ;;
esac


exit 0
====

Re: [Sshguard-users] Patch to fix blacklist loading with ipfw

[Greg P. <gr...@n0...>]

Kevin Zheng said:
> You should consider checking out the latest sources from the Bitbucket
> repository. That might be easier to work with than running from ports.

Will give that a shot. 


> I'm not entirely certain why it was truncated, either.

979 characters from /sbin all the way to the last digit diplayed. 


> It would end up being similar to how `pf` is currently handled: you
> create a table 'sshguard', then SSHGuard would be responsible for adding
> and removing addresses from the table. This means that SSHGuard wouldn't
> have to fiddle with rule numbers. Thoughts?

It would certainly keep the rules tidier. At 200 rules from sshguard, 
that's becoming a mess, but still manageable. For a popular Internet
host, it could easily become thousands/tens of thousands which 
becomes silly in trying to manage the other normal rules. 

I think the table would be a good option to keep order to the rule set. 
While we would miss out on seeing which addresses are active, I don't think that
is all of that big of a concern (at worst, clear the table to start over
and the frequent pests would be blocked quickly again). 


Greg

Re: [Sshguard-users] Patch to fix blacklist loading with ipfw

[Kevin Z. <kev...@gm...>]

Hi Greg,

You should consider checking out the latest sources from the Bitbucket
repository. That might be easier to work with than running from ports.

On 05/26/2015 21:34, Greg Putrich wrote:
> While sshguard was still running, confirmed it did not add the entry. Then I 
> stopped sshguard. I copy & pasted the command from the log and it went in
> without issue. 

This seems odd.

> Copied a blacklist.db file with 213 entries. Upon start, it attempted to 
> insert, but failed. It made a long command, but was chopped off on the 
> 67th IP address (that address had its last digit cut off). and there are 
> still 146 more after that one that never made the list. 

I'm not entirely certain why it was truncated, either.

> The latter command clearly could not be added as it was truncated. However,
> unsure why it didn't like the first one.

If anything, I think this is an indication that the current `ipfw`
backend is unsalvagable and should be replaced with the command
framework thing. I'll start taking a closer look at that.

It would end up being similar to how `pf` is currently handled: you
create a table 'sshguard', then SSHGuard would be responsible for adding
and removing addresses from the table. This means that SSHGuard wouldn't
have to fiddle with rule numbers. Thoughts?

Thanks,
Kevin Zheng

-- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090

Re: [Sshguard-users] Patch to fix blacklist loading with ipfw

[Greg P. <gr...@n0...>]

After starting normally with an non-existent blacklist.db, poked at it 
from another host and it attempted to block, but failed: 

May 26 21:16:52 fbsd sshguard[18479]: Offender '192.168.1.53:4' scored 40 danger in 1 abuses (threshold 40) -> blacklisted.
May 26 21:16:52 fbsd sshguard[18479]: Blocking 192.168.1.53:4 for >0secs: 40 danger in 4 attacks over 1 seconds (all: 40d in 1 abuses over 1s).
May 26 21:16:52 fbsd sshguard[18479]: Command "/sbin/ipfw add 55003 drop ip from 192.168.1.53 to me" exited 64
May 26 21:16:52 fbsd sshguard[18479]: Blocking command failed. Exited: -1


While sshguard was still running, confirmed it did not add the entry. Then I 
stopped sshguard. I copy & pasted the command from the log and it went in
without issue. 


Copied a blacklist.db file with 213 entries. Upon start, it attempted to 
insert, but failed. It made a long command, but was chopped off on the 
67th IP address (that address had its last digit cut off). and there are 
still 146 more after that one that never made the list. 


The latter command clearly could not be added as it was truncated. However,
unsure why it didn't like the first one.


Greg



Kevin Zheng said:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> Hi there,
> 
> A patch that fixes blacklist loading when using the `ipfw` backend is
> available and attached here. It is mostly of interest to FreeBSD.
> 
> This patch has not been committed because it relies on the
> non-portable functions `strlcpy` and `strlcat`. While I work on
> bringing these to SSHGuard, FreeBSD users can enjoy a working
> blacklist now.
> 
> I've done rudimentary testing and this patch appears to work; before
> this hits the ports tree someone should really test it.
> 
> Thanks,
> Kevin Zheng
> 
> - -- 
> Kevin Zheng
> kev...@gm... | ke...@kd... | PGP: 0xC22E1090
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
> 
> iQEcBAEBCAAGBQJVZRxRAAoJEOrPD3bCLhCQN2MIAJOMmgslZPV5aYsYEnX1quC+
> IXMc6t/rpFDybZPKz4LC4YI+WcsQ+fykKQ3mFZfJ2HITqqyBorNUe8JKzR8p59tX
> sX5ePTq4Jld+LOFklKOSS3NSZauMi6zS8tcCpz5gVdQ0iBizDssW/f70ZTD927lB
> 44VgAdv8FrHXsPpgEgcrZCsNm3uK8j48eh3aAo3elThM4BAIhoMYobLZl1Jgnq59
> hjWVk49Z1njypiP2SYASXVdy5x8AINQDY4R8Wqa0/mNGfzFKT2y5HPw/70YbAm3M
> E1o/V9apCH3p1Trq/NshZwvP9sFxfV0oJtATRXUvJxuI0BDHIM5F+/w72TJCVU4=
> =SKWp
> -----END PGP SIGNATURE-----

> diff --git a/src/fwalls/ipfw.c b/src/fwalls/ipfw.c
> index 29045b0..9bee0ad 100644
> --- a/src/fwalls/ipfw.c
> +++ b/src/fwalls/ipfw.c
> @@ -20,6 +20,7 @@
>  
>  #include <assert.h>
>  #include <errno.h>
> +#include <limits.h>
>  #include <time.h>
>  #include <time.h>
>  #include <string.h>
> @@ -37,8 +38,6 @@
>  
>  #define IPFWMOD_ADDRESS_BULK_REPRESENTATIVE     "FF:FF:FF:FF:FF:FF:FF:FF"
>  
> -#define MAXIPFWCMDLEN           90
> -
>  #ifndef IPFW_RULERANGE_MIN
>  #define IPFW_RULERANGE_MIN      55000
>  #endif
> @@ -56,14 +55,14 @@ struct addr_ruleno_s {
>  };
>  
>  static list_t addrrulenumbers;
> -static char command[MAXIPFWCMDLEN], args[MAXIPFWCMDLEN];
> +static char command[PATH_MAX], args[ARG_MAX];
>  
>  /* generate an IPFW rule ID for inserting a rule */
>  static ipfw_rulenumber_t ipfwmod_getrulenumber(void);
>  /* execute an IPFW command */
> -static int ipfwmod_runcommand(char *command, char *args);
> +static int ipfwmod_runcommand(const char *command, const char *args);
>  /* build an IPFW rule for blocking a list of addresses, all of the given kind */
> -static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind, char *restrict command, char *restrict args);
> +static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind);
>  
>  static size_t ipfw_rule_meter(const void *el) { return sizeof(struct addr_ruleno_s); }
>  static int ipfw_rule_comparator(const void *a, const void *b) {
> @@ -95,7 +94,7 @@ int fw_block(const char *restrict addr, int addrkind, int service) {
>      ruleno = ipfwmod_getrulenumber();
>      addresses[0] = addr;
>      addresses[1] = NULL;
> -    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind, command, args) != FWALL_OK)
> +    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind) != FWALL_OK)
>          return FWALL_ERR;
>  
>      /* run command */
> @@ -108,7 +107,7 @@ int fw_block(const char *restrict addr, int addrkind, int service) {
>      sshguard_log(LOG_DEBUG, "Command exited %d.", ret);
>  
>      /* success, save rule number */
> -    strcpy(addendum.addr, addr);
> +    strlcpy(addendum.addr, addr, sizeof(addendum.addr));
>      addendum.ruleno = ruleno;
>      addendum.addrkind = addrkind;
>  
> @@ -134,7 +133,7 @@ int fw_block_list(const char *restrict addresses[], int addrkind, const int serv
>  
>      ruleno = ipfwmod_getrulenumber();
>      /* insert rules under this rule number (in chunks of max_addresses_per_rule) */
> -    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind, command, args) != FWALL_OK)
> +    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind) != FWALL_OK)
>          return FWALL_ERR;
>  
>      /* run command */
> @@ -147,7 +146,7 @@ int fw_block_list(const char *restrict addresses[], int addrkind, const int serv
>      sshguard_log(LOG_DEBUG, "Command exited %d.", ret);
>  
>      /* insert a placeholder for the bulk */
> -    strcpy(addendum.addr, IPFWMOD_ADDRESS_BULK_REPRESENTATIVE);
> +    strlcpy(addendum.addr, IPFWMOD_ADDRESS_BULK_REPRESENTATIVE, sizeof(addendum.addr));
>      addendum.ruleno = ruleno;
>      addendum.addrkind = addrkind;
>      list_append(& addrrulenumbers, & addendum);
> @@ -161,7 +160,7 @@ int fw_release(const char *restrict addr, int addrkind, int service) {
>      int pos, ret = 0;
>  
>      /* retrieve ID of rule blocking "addr" */
> -    strcpy(data.addr, addr);
> +    strlcpy(data.addr, addr, sizeof(data.addr));
>      data.addrkind = addrkind;
>      if ((pos = list_locate(& addrrulenumbers, &data)) < 0) {
>          sshguard_log(LOG_ERR, "could not get back rule ID for address %s", addr);
> @@ -172,22 +171,22 @@ int fw_release(const char *restrict addr, int addrkind, int service) {
>      switch (data.addrkind) {
>          case ADDRKIND_IPv4:
>              /* use ipfw */
> -            sprintf(command, IPFW_PATH "/ipfw");
> +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
>              break;
>          case ADDRKIND_IPv6:
>  #ifdef FWALL_HAS_IP6FW
>              /* use ip6fw if found */
> -            sprintf(command, IPFW_PATH "/ip6fw");
> +            strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
>  #else
>              /* use ipfw, assume it supports IPv6 rules as well */
> -            sprintf(command, IPFW_PATH "/ipfw");
> +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
>  #endif
>              break;
>          default:
>              return FWALL_UNSUPP;
>      }
>      /* build command arguments */
> -    snprintf(args, MAXIPFWCMDLEN, "delete %u", data.ruleno);
> +    snprintf(args, sizeof(args), "delete %u", data.ruleno);
>  
>      sshguard_log(LOG_DEBUG, "running: '%s %s'", command, args);
>  
> @@ -216,19 +215,19 @@ int fw_flush(void) {
>          data = (struct addr_ruleno_s *)list_iterator_next(& addrrulenumbers);
>          switch (data->addrkind) {
>              case ADDRKIND_IPv4:
> -                snprintf(command, MAXIPFWCMDLEN, IPFW_PATH "/ipfw");
> +                strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
>                  break;
>              case ADDRKIND_IPv6:
>  #ifdef FWALL_HAS_IP6FW
>                  /* use ip6fw if found */
> -                sprintf(command, IPFW_PATH "/ip6fw");
> +                strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
>  #else
>                  /* use ipfw, assume it supports IPv6 rules as well */
> -                sprintf(command, IPFW_PATH "/ipfw");
> +                strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
>  #endif
>                  break;
>          }
> -        sprintf(args, "delete %u", data->ruleno);
> +        snprintf(args, sizeof(args), "delete %u", data->ruleno);
>          sshguard_log(LOG_DEBUG, "running: '%s %s'", command, args);
>          ret = ipfwmod_runcommand(command, args);
>          if (ret != 0) {
> @@ -250,7 +249,7 @@ static ipfw_rulenumber_t ipfwmod_getrulenumber(void) {
>      return (rand() % (IPFW_RULERANGE_MAX - IPFW_RULERANGE_MIN)) + IPFW_RULERANGE_MIN;
>  }
>  
> -static int ipfwmod_runcommand(char *command, char *args) {
> +static int ipfwmod_runcommand(const char *command, const char *args) {
>      char *argsvec[20];
>      pid_t pid;
>      int i, j, ret;
> @@ -258,8 +257,8 @@ static int ipfwmod_runcommand(char *command, char *args) {
>  
>      sshguard_log(LOG_DEBUG, "Running command: '%s %s'.", command, args);
>  
> -    argsvec[0] = command;
> -    strcpy(locargs, args);
> +    argsvec[0] = strdup(command);
> +    strlcpy(locargs, args, sizeof(locargs));
>  
>      /* tokenize command */
>      argsvec[1] = locargs;
> @@ -280,6 +279,7 @@ static int ipfwmod_runcommand(char *command, char *args) {
>          sshguard_log(LOG_ERR, "Unable to run command: %s", strerror(errno));
>          _Exit(1);
>      }
> +    free(argsvec[0]);
>      free(locargs);
>      waitpid(pid, &ret, 0);
>      ret = WEXITSTATUS(ret);
> @@ -287,7 +287,7 @@ static int ipfwmod_runcommand(char *command, char *args) {
>      return ret;
>  }
>  
> -static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind, char *restrict command, char *restrict args) {
> +static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind) {
>      int i;
>  
>      assert(addresses != NULL);
> @@ -307,19 +307,19 @@ static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restr
>      switch (addrkind) {
>          case ADDRKIND_IPv4:
>              /* use ipfw */
> -            sprintf(command, IPFW_PATH "/ipfw");
> -            sprintf(args, "add %u drop ip", ruleno);
> +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> +            snprintf(args, sizeof(args), "add %u drop ip", ruleno);
>              break;
>  
>          case ADDRKIND_IPv6:
>  #ifdef FWALL_HAS_IP6FW
>              /* use ip6fw if found */
> -            sprintf(command, IPFW_PATH "/ip6fw");
> +            strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
>  #else
>              /* use ipfw, assume it supports IPv6 rules as well */
> -            sprintf(command, IPFW_PATH "/ipfw");
> +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
>  #endif
> -            sprintf(args, "add %u drop ipv6", ruleno);
> +            snprintf(args, sizeof(args), "add %u drop ipv6", ruleno);
>              break;
>  
>          default:
> @@ -327,13 +327,17 @@ static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restr
>      }
>  
>      /* add the rest of the rule */
> -    sprintf(args + strlen(args), " from %s", addresses[0]);
> +    strlcat(args, " from ", sizeof(args));
> +    strlcat(args, addresses[0], sizeof(args));
>      for (i = 1; addresses[i] != NULL; ++i) {
> -        sprintf(args + strlen(args), ",%s", addresses[i]);
> +        strlcat(args, ",", sizeof(args));
> +        strlcat(args, addresses[i], sizeof(args));
> +    }
> +    if (strlcat(args, " to me", sizeof(args)) >= sizeof(args)) {
> +        fprintf(stderr, "Fatal: Argument buffer too small\n");
> +        exit(EXIT_FAILURE);
>      }
> -    strcat(args, " to me");
>  
>      return FWALL_OK;
>  }
>  
> -


> ------------------------------------------------------------------------------

> _______________________________________________
> Sshguard-users mailing list
> Ssh...@li...
> https://lists.sourceforge.net/lists/listinfo/sshguard-users

Re: [Sshguard-users] Patch to fix blacklist loading with ipfw

[Greg P. <gr...@n0...>]

Looks like this was due to ipfw.c missing: 
#include <parser/address.h>

I've added that and it now compiles with only a couple of warnings:


Making all in fwalls
  CC       ipfw.o
ipfw.c:110:5: warning: implicitly declaring library function 'strlcpy' with type 'unsigned long (char *, const char *, unsigned long)'
    strlcpy(addendum.addr, addr, sizeof(addendum.addr));
    ^
ipfw.c:110:5: note: please include the header <string.h> or explicitly provide a declaration for 'strlcpy'
ipfw.c:330:5: warning: implicitly declaring library function 'strlcat' with type 'unsigned long (char *, const char *, unsigned long)'
    strlcat(args, " from ", sizeof(args));
    ^
ipfw.c:330:5: note: please include the header <string.h> or explicitly provide a declaration for 'strlcat'
2 warnings generated.
  AR       libfwall.a
  CC       sshguard_log.o
  CC       sshg_parser.o
  CCLD     sshg-parser
  CC       sshguard.o
  CC       seekers.o
  CC       sshguard_whitelist.o
  CC       sshguard_procauth.o
  CC       sshguard_blacklist.o
  CC       sshguard_options.o
  CC       sshguard_logsuck.o
  CC       simclist.o
  CC       hash_32a.o
  CCLD     sshguard



Greg





To ssh...@li... said:
> Hi Kevin,
> 
> I gave it a shot, but it failed to build. Did make a minor mod 
> to the diff. The file paths had a/ & b/, so removed those. 
> 
> 
> The output from the make: 
> 
> ===>  License BSD2CLAUSE accepted by the user
> ===>   sshguard-ipfw-1.6.0_1 depends on file: /usr/local/sbin/pkg - found
> ===> Fetching all distfiles required by sshguard-ipfw-1.6.0_1 for building
> ===>  Extracting for sshguard-ipfw-1.6.0_1
> => SHA256 Checksum OK for sshguard-1.6.0.tar.xz.
> ===>  Patching for sshguard-ipfw-1.6.0_1
> ===>  Applying FreeBSD patches for sshguard-ipfw-1.6.0_1
> ===>   sshguard-ipfw-1.6.0_1 depends on executable: autoconf-2.69 - found
> ===>   sshguard-ipfw-1.6.0_1 depends on executable: autoheader-2.69 - found
> ===>   sshguard-ipfw-1.6.0_1 depends on executable: autoreconf-2.69 - found
> ===>   sshguard-ipfw-1.6.0_1 depends on executable: aclocal-1.15 - found
> ===>   sshguard-ipfw-1.6.0_1 depends on executable: automake-1.15 - found
> ===>  Configuring for sshguard-ipfw-1.6.0_1
> configure: loading site script /usr/ports/Templates/config.site
> checking for a BSD-compatible install... /usr/bin/install -c
> checking whether build environment is sane... yes
> checking for a thread-safe mkdir -p... (cached) /bin/mkdir -p
> checking for gawk... (cached) /usr/bin/awk
> checking whether make sets $(MAKE)... yes
> checking whether make supports nested variables... yes
> checking whether make supports nested variables... (cached) yes
> checking for ipfw... /sbin
> checking for ip6fw... no
> configure: ip6fw program not found. Assuming ipfw supports IPv6 rules on its own.
> ## -------------- ##
> ## Program Checks ##
> ## -------------- ##
> checking for gawk... (cached) /usr/bin/awk
> checking for gcc... cc
> checking whether the C compiler works... yes
> checking for C compiler default output file name... a.out
> checking for suffix of executables... 
> checking whether we are cross compiling... no
> checking for suffix of object files... o
> checking whether we are using the GNU C compiler... yes
> checking whether cc accepts -g... yes
> checking for cc option to accept ISO C89... none needed
> checking whether cc understands -c and -o together... yes
> checking for style of include used by make... GNU
> checking dependency style of cc... gcc3
> checking for cc option to accept ISO C99... none needed
> checking for grep that handles long lines and -e... (cached) /usr/bin/grep
> checking for egrep... (cached) /usr/bin/egrep
> checking for ranlib... ranlib
> checking for bison... bison -y
> checking for flex... flex
> checking lex output file root... lex.yy
> checking lex library... -lfl
> checking whether yytext is a pointer... yes
> ## -------------- ##
> ## Library Checks ##
> ## -------------- ##
> checking for pthread_create in -lpthread... yes
> checking how to run the C preprocessor... cpp
> checking for ANSI C header files... (cached) yes
> checking for sys/wait.h that is POSIX.1 compatible... (cached) yes
> checking for sys/types.h... (cached) yes
> checking for sys/stat.h... (cached) yes
> checking for stdlib.h... (cached) yes
> checking for string.h... (cached) yes
> checking for memory.h... (cached) yes
> checking for strings.h... (cached) yes
> checking for inttypes.h... (cached) yes
> checking for stdint.h... (cached) yes
> checking for unistd.h... (cached) yes
> checking for arpa/inet.h... (cached) yes
> checking for malloc.h... (cached) no
> checking for netdb.h... (cached) yes
> checking for netinet/in.h... (cached) yes
> checking for stdlib.h... (cached) yes
> checking for string.h... (cached) yes
> checking for sys/socket.h... (cached) yes
> checking syslog.h usability... yes
> checking syslog.h presence... yes
> checking for syslog.h... yes
> checking for unistd.h... (cached) yes
> checking for getopt.h... (cached) yes
> checking for off_t... (cached) yes
> checking for pid_t... (cached) yes
> checking for size_t... (cached) yes
> checking for an ANSI C-conforming const... yes
> checking for inline... inline
> checking for C/C++ restrict keyword... __restrict
> checking build system type... amd64-portbld-freebsd10.1
> checking whether __SUNPRO_C is declared... no
> ## ----------------- ##
> ## Library Functions ##
> ## ----------------- ##
> checking for vfork.h... (cached) no
> checking for fork... (cached) yes
> checking for vfork... (cached) yes
> checking for working fork... yes
> checking for working vfork... (cached) yes
> checking for stdlib.h... (cached) yes
> checking for GNU libc compatible malloc... (cached) yes
> checking for gethostbyname... (cached) yes
> checking for inet_ntoa... (cached) yes
> checking for strerror... (cached) yes
> checking for strstr... yes
> checking for strtol... (cached) yes
> checking for library containing socket... none required
> checking for library containing gethostbyname... none required
> configure: Using /sbin as location for ipfw
> checking that generated files are newer than configure... done
> configure: creating ./config.status
> config.status: creating Makefile
> config.status: creating man/Makefile
> config.status: creating src/Makefile
> config.status: creating src/parser/Makefile
> config.status: creating src/fwalls/Makefile
> config.status: creating src/config.h
> config.status: executing depfiles commands
> ===>  Building for sshguard-ipfw-1.6.0_1
> Making all in src
> /usr/bin/make  all-recursive
> Making all in parser
> /usr/bin/make  all-am
>   LEX      attack_scanner.c
>   CC       attack_parser.o
>   CC       attack_scanner.o
> attack_scanner.c:27857:16: warning: function 'input' is not needed and will not be emitted [-Wunneeded-internal-declaration]
>     static int input  (void)
>                ^
> 1 warning generated.
>   AR       libparser.a
> Making all in fwalls
>   CC       ipfw.o
> ipfw.c:51:15: error: use of undeclared identifier 'ADDRLEN'
>     char addr[ADDRLEN];
>               ^
> ipfw.c:109:5: warning: implicitly declaring library function 'strlcpy' with type 'unsigned long (char *, const char *, unsigned long)'
>     strlcpy(addendum.addr, addr, sizeof(addendum.addr));
>     ^
> ipfw.c:109:5: note: please include the header <string.h> or explicitly provide a declaration for 'strlcpy'
> ipfw.c:171:14: error: use of undeclared identifier 'ADDRKIND_IPv4'
>         case ADDRKIND_IPv4:
>              ^
> ipfw.c:175:14: error: use of undeclared identifier 'ADDRKIND_IPv6'
>         case ADDRKIND_IPv6:
>              ^
> ipfw.c:216:18: error: use of undeclared identifier 'ADDRKIND_IPv4'
>             case ADDRKIND_IPv4:
>                  ^
> ipfw.c:219:18: error: use of undeclared identifier 'ADDRKIND_IPv6'
>             case ADDRKIND_IPv6:
>                  ^
> ipfw.c:307:14: error: use of undeclared identifier 'ADDRKIND_IPv4'
>         case ADDRKIND_IPv4:
>              ^
> ipfw.c:313:14: error: use of undeclared identifier 'ADDRKIND_IPv6'
>         case ADDRKIND_IPv6:
>              ^
> ipfw.c:329:5: warning: implicitly declaring library function 'strlcat' with type 'unsigned long (char *, const char *, unsigned long)'
>     strlcat(args, " from ", sizeof(args));
>     ^
> ipfw.c:329:5: note: please include the header <string.h> or explicitly provide a declaration for 'strlcat'
> 2 warnings and 7 errors generated.
> *** [ipfw.o] Error code 1
> 
> make[4]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src/fwalls
> 1 error
> 
> make[4]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src/fwalls
> *** [all-recursive] Error code 1
> 
> make[3]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src
> 1 error
> 
> make[3]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src
> *** [all] Error code 2
> 
> make[2]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src
> 1 error
> 
> make[2]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src
> *** [all-recursive] Error code 1
> 
> make[1]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0
> 1 error
> 
> make[1]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0
> ===> Compilation failed unexpectedly.
> Try to set MAKE_JOBS_UNSAFE=yes and rebuild before reporting the failure to
> the maintainer.
> *** Error code 1
> 
> Stop.
> make: stopped in /usr/ports/security/sshguard-ipfw
> 
> 
> 
> 
>  Greg
> 
> 
> 
> Kevin Zheng said:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA256
> > 
> > Hi there,
> > 
> > A patch that fixes blacklist loading when using the `ipfw` backend is
> > available and attached here. It is mostly of interest to FreeBSD.
> > 
> > This patch has not been committed because it relies on the
> > non-portable functions `strlcpy` and `strlcat`. While I work on
> > bringing these to SSHGuard, FreeBSD users can enjoy a working
> > blacklist now.
> > 
> > I've done rudimentary testing and this patch appears to work; before
> > this hits the ports tree someone should really test it.
> > 
> > Thanks,
> > Kevin Zheng
> > 
> > - -- 
> > Kevin Zheng
> > kev...@gm... | ke...@kd... | PGP: 0xC22E1090
> > -----BEGIN PGP SIGNATURE-----
> > Version: GnuPG v2
> > 
> > iQEcBAEBCAAGBQJVZRxRAAoJEOrPD3bCLhCQN2MIAJOMmgslZPV5aYsYEnX1quC+
> > IXMc6t/rpFDybZPKz4LC4YI+WcsQ+fykKQ3mFZfJ2HITqqyBorNUe8JKzR8p59tX
> > sX5ePTq4Jld+LOFklKOSS3NSZauMi6zS8tcCpz5gVdQ0iBizDssW/f70ZTD927lB
> > 44VgAdv8FrHXsPpgEgcrZCsNm3uK8j48eh3aAo3elThM4BAIhoMYobLZl1Jgnq59
> > hjWVk49Z1njypiP2SYASXVdy5x8AINQDY4R8Wqa0/mNGfzFKT2y5HPw/70YbAm3M
> > E1o/V9apCH3p1Trq/NshZwvP9sFxfV0oJtATRXUvJxuI0BDHIM5F+/w72TJCVU4=
> > =SKWp
> > -----END PGP SIGNATURE-----
> 
> > diff --git a/src/fwalls/ipfw.c b/src/fwalls/ipfw.c
> > index 29045b0..9bee0ad 100644
> > --- a/src/fwalls/ipfw.c
> > +++ b/src/fwalls/ipfw.c
> > @@ -20,6 +20,7 @@
> >  
> >  #include <assert.h>
> >  #include <errno.h>
> > +#include <limits.h>
> >  #include <time.h>
> >  #include <time.h>
> >  #include <string.h>
> > @@ -37,8 +38,6 @@
> >  
> >  #define IPFWMOD_ADDRESS_BULK_REPRESENTATIVE     "FF:FF:FF:FF:FF:FF:FF:FF"
> >  
> > -#define MAXIPFWCMDLEN           90
> > -
> >  #ifndef IPFW_RULERANGE_MIN
> >  #define IPFW_RULERANGE_MIN      55000
> >  #endif
> > @@ -56,14 +55,14 @@ struct addr_ruleno_s {
> >  };
> >  
> >  static list_t addrrulenumbers;
> > -static char command[MAXIPFWCMDLEN], args[MAXIPFWCMDLEN];
> > +static char command[PATH_MAX], args[ARG_MAX];
> >  
> >  /* generate an IPFW rule ID for inserting a rule */
> >  static ipfw_rulenumber_t ipfwmod_getrulenumber(void);
> >  /* execute an IPFW command */
> > -static int ipfwmod_runcommand(char *command, char *args);
> > +static int ipfwmod_runcommand(const char *command, const char *args);
> >  /* build an IPFW rule for blocking a list of addresses, all of the given kind */
> > -static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind, char *restrict command, char *restrict args);
> > +static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind);
> >  
> >  static size_t ipfw_rule_meter(const void *el) { return sizeof(struct addr_ruleno_s); }
> >  static int ipfw_rule_comparator(const void *a, const void *b) {
> > @@ -95,7 +94,7 @@ int fw_block(const char *restrict addr, int addrkind, int service) {
> >      ruleno = ipfwmod_getrulenumber();
> >      addresses[0] = addr;
> >      addresses[1] = NULL;
> > -    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind, command, args) != FWALL_OK)
> > +    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind) != FWALL_OK)
> >          return FWALL_ERR;
> >  
> >      /* run command */
> > @@ -108,7 +107,7 @@ int fw_block(const char *restrict addr, int addrkind, int service) {
> >      sshguard_log(LOG_DEBUG, "Command exited %d.", ret);
> >  
> >      /* success, save rule number */
> > -    strcpy(addendum.addr, addr);
> > +    strlcpy(addendum.addr, addr, sizeof(addendum.addr));
> >      addendum.ruleno = ruleno;
> >      addendum.addrkind = addrkind;
> >  
> > @@ -134,7 +133,7 @@ int fw_block_list(const char *restrict addresses[], int addrkind, const int serv
> >  
> >      ruleno = ipfwmod_getrulenumber();
> >      /* insert rules under this rule number (in chunks of max_addresses_per_rule) */
> > -    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind, command, args) != FWALL_OK)
> > +    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind) != FWALL_OK)
> >          return FWALL_ERR;
> >  
> >      /* run command */
> > @@ -147,7 +146,7 @@ int fw_block_list(const char *restrict addresses[], int addrkind, const int serv
> >      sshguard_log(LOG_DEBUG, "Command exited %d.", ret);
> >  
> >      /* insert a placeholder for the bulk */
> > -    strcpy(addendum.addr, IPFWMOD_ADDRESS_BULK_REPRESENTATIVE);
> > +    strlcpy(addendum.addr, IPFWMOD_ADDRESS_BULK_REPRESENTATIVE, sizeof(addendum.addr));
> >      addendum.ruleno = ruleno;
> >      addendum.addrkind = addrkind;
> >      list_append(& addrrulenumbers, & addendum);
> > @@ -161,7 +160,7 @@ int fw_release(const char *restrict addr, int addrkind, int service) {
> >      int pos, ret = 0;
> >  
> >      /* retrieve ID of rule blocking "addr" */
> > -    strcpy(data.addr, addr);
> > +    strlcpy(data.addr, addr, sizeof(data.addr));
> >      data.addrkind = addrkind;
> >      if ((pos = list_locate(& addrrulenumbers, &data)) < 0) {
> >          sshguard_log(LOG_ERR, "could not get back rule ID for address %s", addr);
> > @@ -172,22 +171,22 @@ int fw_release(const char *restrict addr, int addrkind, int service) {
> >      switch (data.addrkind) {
> >          case ADDRKIND_IPv4:
> >              /* use ipfw */
> > -            sprintf(command, IPFW_PATH "/ipfw");
> > +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> >              break;
> >          case ADDRKIND_IPv6:
> >  #ifdef FWALL_HAS_IP6FW
> >              /* use ip6fw if found */
> > -            sprintf(command, IPFW_PATH "/ip6fw");
> > +            strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
> >  #else
> >              /* use ipfw, assume it supports IPv6 rules as well */
> > -            sprintf(command, IPFW_PATH "/ipfw");
> > +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> >  #endif
> >              break;
> >          default:
> >              return FWALL_UNSUPP;
> >      }
> >      /* build command arguments */
> > -    snprintf(args, MAXIPFWCMDLEN, "delete %u", data.ruleno);
> > +    snprintf(args, sizeof(args), "delete %u", data.ruleno);
> >  
> >      sshguard_log(LOG_DEBUG, "running: '%s %s'", command, args);
> >  
> > @@ -216,19 +215,19 @@ int fw_flush(void) {
> >          data = (struct addr_ruleno_s *)list_iterator_next(& addrrulenumbers);
> >          switch (data->addrkind) {
> >              case ADDRKIND_IPv4:
> > -                snprintf(command, MAXIPFWCMDLEN, IPFW_PATH "/ipfw");
> > +                strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> >                  break;
> >              case ADDRKIND_IPv6:
> >  #ifdef FWALL_HAS_IP6FW
> >                  /* use ip6fw if found */
> > -                sprintf(command, IPFW_PATH "/ip6fw");
> > +                strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
> >  #else
> >                  /* use ipfw, assume it supports IPv6 rules as well */
> > -                sprintf(command, IPFW_PATH "/ipfw");
> > +                strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> >  #endif
> >                  break;
> >          }
> > -        sprintf(args, "delete %u", data->ruleno);
> > +        snprintf(args, sizeof(args), "delete %u", data->ruleno);
> >          sshguard_log(LOG_DEBUG, "running: '%s %s'", command, args);
> >          ret = ipfwmod_runcommand(command, args);
> >          if (ret != 0) {
> > @@ -250,7 +249,7 @@ static ipfw_rulenumber_t ipfwmod_getrulenumber(void) {
> >      return (rand() % (IPFW_RULERANGE_MAX - IPFW_RULERANGE_MIN)) + IPFW_RULERANGE_MIN;
> >  }
> >  
> > -static int ipfwmod_runcommand(char *command, char *args) {
> > +static int ipfwmod_runcommand(const char *command, const char *args) {
> >      char *argsvec[20];
> >      pid_t pid;
> >      int i, j, ret;
> > @@ -258,8 +257,8 @@ static int ipfwmod_runcommand(char *command, char *args) {
> >  
> >      sshguard_log(LOG_DEBUG, "Running command: '%s %s'.", command, args);
> >  
> > -    argsvec[0] = command;
> > -    strcpy(locargs, args);
> > +    argsvec[0] = strdup(command);
> > +    strlcpy(locargs, args, sizeof(locargs));
> >  
> >      /* tokenize command */
> >      argsvec[1] = locargs;
> > @@ -280,6 +279,7 @@ static int ipfwmod_runcommand(char *command, char *args) {
> >          sshguard_log(LOG_ERR, "Unable to run command: %s", strerror(errno));
> >          _Exit(1);
> >      }
> > +    free(argsvec[0]);
> >      free(locargs);
> >      waitpid(pid, &ret, 0);
> >      ret = WEXITSTATUS(ret);
> > @@ -287,7 +287,7 @@ static int ipfwmod_runcommand(char *command, char *args) {
> >      return ret;
> >  }
> >  
> > -static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind, char *restrict command, char *restrict args) {
> > +static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind) {
> >      int i;
> >  
> >      assert(addresses != NULL);
> > @@ -307,19 +307,19 @@ static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restr
> >      switch (addrkind) {
> >          case ADDRKIND_IPv4:
> >              /* use ipfw */
> > -            sprintf(command, IPFW_PATH "/ipfw");
> > -            sprintf(args, "add %u drop ip", ruleno);
> > +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> > +            snprintf(args, sizeof(args), "add %u drop ip", ruleno);
> >              break;
> >  
> >          case ADDRKIND_IPv6:
> >  #ifdef FWALL_HAS_IP6FW
> >              /* use ip6fw if found */
> > -            sprintf(command, IPFW_PATH "/ip6fw");
> > +            strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
> >  #else
> >              /* use ipfw, assume it supports IPv6 rules as well */
> > -            sprintf(command, IPFW_PATH "/ipfw");
> > +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> >  #endif
> > -            sprintf(args, "add %u drop ipv6", ruleno);
> > +            snprintf(args, sizeof(args), "add %u drop ipv6", ruleno);
> >              break;
> >  
> >          default:
> > @@ -327,13 +327,17 @@ static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restr
> >      }
> >  
> >      /* add the rest of the rule */
> > -    sprintf(args + strlen(args), " from %s", addresses[0]);
> > +    strlcat(args, " from ", sizeof(args));
> > +    strlcat(args, addresses[0], sizeof(args));
> >      for (i = 1; addresses[i] != NULL; ++i) {
> > -        sprintf(args + strlen(args), ",%s", addresses[i]);
> > +        strlcat(args, ",", sizeof(args));
> > +        strlcat(args, addresses[i], sizeof(args));
> > +    }
> > +    if (strlcat(args, " to me", sizeof(args)) >= sizeof(args)) {
> > +        fprintf(stderr, "Fatal: Argument buffer too small\n");
> > +        exit(EXIT_FAILURE);
> >      }
> > -    strcat(args, " to me");
> >  
> >      return FWALL_OK;
> >  }
> >  
> > -
> 
> 
> > ------------------------------------------------------------------------------
> 
> > _______________________________________________
> > Sshguard-users mailing list
> > Ssh...@li...
> > https://lists.sourceforge.net/lists/listinfo/sshguard-users
> 

Re: [Sshguard-users] Patch to fix blacklist loading with ipfw

[Greg P. <gr...@n0...>]

Hi Kevin,

I gave it a shot, but it failed to build. Did make a minor mod 
to the diff. The file paths had a/ & b/, so removed those. 


The output from the make: 

===>  License BSD2CLAUSE accepted by the user
===>   sshguard-ipfw-1.6.0_1 depends on file: /usr/local/sbin/pkg - found
===> Fetching all distfiles required by sshguard-ipfw-1.6.0_1 for building
===>  Extracting for sshguard-ipfw-1.6.0_1
=> SHA256 Checksum OK for sshguard-1.6.0.tar.xz.
===>  Patching for sshguard-ipfw-1.6.0_1
===>  Applying FreeBSD patches for sshguard-ipfw-1.6.0_1
===>   sshguard-ipfw-1.6.0_1 depends on executable: autoconf-2.69 - found
===>   sshguard-ipfw-1.6.0_1 depends on executable: autoheader-2.69 - found
===>   sshguard-ipfw-1.6.0_1 depends on executable: autoreconf-2.69 - found
===>   sshguard-ipfw-1.6.0_1 depends on executable: aclocal-1.15 - found
===>   sshguard-ipfw-1.6.0_1 depends on executable: automake-1.15 - found
===>  Configuring for sshguard-ipfw-1.6.0_1
configure: loading site script /usr/ports/Templates/config.site
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... (cached) /bin/mkdir -p
checking for gawk... (cached) /usr/bin/awk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking for ipfw... /sbin
checking for ip6fw... no
configure: ip6fw program not found. Assuming ipfw supports IPv6 rules on its own.
## -------------- ##
## Program Checks ##
## -------------- ##
checking for gawk... (cached) /usr/bin/awk
checking for gcc... cc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether cc accepts -g... yes
checking for cc option to accept ISO C89... none needed
checking whether cc understands -c and -o together... yes
checking for style of include used by make... GNU
checking dependency style of cc... gcc3
checking for cc option to accept ISO C99... none needed
checking for grep that handles long lines and -e... (cached) /usr/bin/grep
checking for egrep... (cached) /usr/bin/egrep
checking for ranlib... ranlib
checking for bison... bison -y
checking for flex... flex
checking lex output file root... lex.yy
checking lex library... -lfl
checking whether yytext is a pointer... yes
## -------------- ##
## Library Checks ##
## -------------- ##
checking for pthread_create in -lpthread... yes
checking how to run the C preprocessor... cpp
checking for ANSI C header files... (cached) yes
checking for sys/wait.h that is POSIX.1 compatible... (cached) yes
checking for sys/types.h... (cached) yes
checking for sys/stat.h... (cached) yes
checking for stdlib.h... (cached) yes
checking for string.h... (cached) yes
checking for memory.h... (cached) yes
checking for strings.h... (cached) yes
checking for inttypes.h... (cached) yes
checking for stdint.h... (cached) yes
checking for unistd.h... (cached) yes
checking for arpa/inet.h... (cached) yes
checking for malloc.h... (cached) no
checking for netdb.h... (cached) yes
checking for netinet/in.h... (cached) yes
checking for stdlib.h... (cached) yes
checking for string.h... (cached) yes
checking for sys/socket.h... (cached) yes
checking syslog.h usability... yes
checking syslog.h presence... yes
checking for syslog.h... yes
checking for unistd.h... (cached) yes
checking for getopt.h... (cached) yes
checking for off_t... (cached) yes
checking for pid_t... (cached) yes
checking for size_t... (cached) yes
checking for an ANSI C-conforming const... yes
checking for inline... inline
checking for C/C++ restrict keyword... __restrict
checking build system type... amd64-portbld-freebsd10.1
checking whether __SUNPRO_C is declared... no
## ----------------- ##
## Library Functions ##
## ----------------- ##
checking for vfork.h... (cached) no
checking for fork... (cached) yes
checking for vfork... (cached) yes
checking for working fork... yes
checking for working vfork... (cached) yes
checking for stdlib.h... (cached) yes
checking for GNU libc compatible malloc... (cached) yes
checking for gethostbyname... (cached) yes
checking for inet_ntoa... (cached) yes
checking for strerror... (cached) yes
checking for strstr... yes
checking for strtol... (cached) yes
checking for library containing socket... none required
checking for library containing gethostbyname... none required
configure: Using /sbin as location for ipfw
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating man/Makefile
config.status: creating src/Makefile
config.status: creating src/parser/Makefile
config.status: creating src/fwalls/Makefile
config.status: creating src/config.h
config.status: executing depfiles commands
===>  Building for sshguard-ipfw-1.6.0_1
Making all in src
/usr/bin/make  all-recursive
Making all in parser
/usr/bin/make  all-am
  LEX      attack_scanner.c
  CC       attack_parser.o
  CC       attack_scanner.o
attack_scanner.c:27857:16: warning: function 'input' is not needed and will not be emitted [-Wunneeded-internal-declaration]
    static int input  (void)
               ^
1 warning generated.
  AR       libparser.a
Making all in fwalls
  CC       ipfw.o
ipfw.c:51:15: error: use of undeclared identifier 'ADDRLEN'
    char addr[ADDRLEN];
              ^
ipfw.c:109:5: warning: implicitly declaring library function 'strlcpy' with type 'unsigned long (char *, const char *, unsigned long)'
    strlcpy(addendum.addr, addr, sizeof(addendum.addr));
    ^
ipfw.c:109:5: note: please include the header <string.h> or explicitly provide a declaration for 'strlcpy'
ipfw.c:171:14: error: use of undeclared identifier 'ADDRKIND_IPv4'
        case ADDRKIND_IPv4:
             ^
ipfw.c:175:14: error: use of undeclared identifier 'ADDRKIND_IPv6'
        case ADDRKIND_IPv6:
             ^
ipfw.c:216:18: error: use of undeclared identifier 'ADDRKIND_IPv4'
            case ADDRKIND_IPv4:
                 ^
ipfw.c:219:18: error: use of undeclared identifier 'ADDRKIND_IPv6'
            case ADDRKIND_IPv6:
                 ^
ipfw.c:307:14: error: use of undeclared identifier 'ADDRKIND_IPv4'
        case ADDRKIND_IPv4:
             ^
ipfw.c:313:14: error: use of undeclared identifier 'ADDRKIND_IPv6'
        case ADDRKIND_IPv6:
             ^
ipfw.c:329:5: warning: implicitly declaring library function 'strlcat' with type 'unsigned long (char *, const char *, unsigned long)'
    strlcat(args, " from ", sizeof(args));
    ^
ipfw.c:329:5: note: please include the header <string.h> or explicitly provide a declaration for 'strlcat'
2 warnings and 7 errors generated.
*** [ipfw.o] Error code 1

make[4]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src/fwalls
1 error

make[4]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src/fwalls
*** [all-recursive] Error code 1

make[3]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src
1 error

make[3]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src
*** [all] Error code 2

make[2]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src
1 error

make[2]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0/src
*** [all-recursive] Error code 1

make[1]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0
1 error

make[1]: stopped in /usr/ports/security/sshguard-ipfw/work/sshguard-1.6.0
===> Compilation failed unexpectedly.
Try to set MAKE_JOBS_UNSAFE=yes and rebuild before reporting the failure to
the maintainer.
*** Error code 1

Stop.
make: stopped in /usr/ports/security/sshguard-ipfw




 Greg



Kevin Zheng said:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> Hi there,
> 
> A patch that fixes blacklist loading when using the `ipfw` backend is
> available and attached here. It is mostly of interest to FreeBSD.
> 
> This patch has not been committed because it relies on the
> non-portable functions `strlcpy` and `strlcat`. While I work on
> bringing these to SSHGuard, FreeBSD users can enjoy a working
> blacklist now.
> 
> I've done rudimentary testing and this patch appears to work; before
> this hits the ports tree someone should really test it.
> 
> Thanks,
> Kevin Zheng
> 
> - -- 
> Kevin Zheng
> kev...@gm... | ke...@kd... | PGP: 0xC22E1090
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
> 
> iQEcBAEBCAAGBQJVZRxRAAoJEOrPD3bCLhCQN2MIAJOMmgslZPV5aYsYEnX1quC+
> IXMc6t/rpFDybZPKz4LC4YI+WcsQ+fykKQ3mFZfJ2HITqqyBorNUe8JKzR8p59tX
> sX5ePTq4Jld+LOFklKOSS3NSZauMi6zS8tcCpz5gVdQ0iBizDssW/f70ZTD927lB
> 44VgAdv8FrHXsPpgEgcrZCsNm3uK8j48eh3aAo3elThM4BAIhoMYobLZl1Jgnq59
> hjWVk49Z1njypiP2SYASXVdy5x8AINQDY4R8Wqa0/mNGfzFKT2y5HPw/70YbAm3M
> E1o/V9apCH3p1Trq/NshZwvP9sFxfV0oJtATRXUvJxuI0BDHIM5F+/w72TJCVU4=
> =SKWp
> -----END PGP SIGNATURE-----

> diff --git a/src/fwalls/ipfw.c b/src/fwalls/ipfw.c
> index 29045b0..9bee0ad 100644
> --- a/src/fwalls/ipfw.c
> +++ b/src/fwalls/ipfw.c
> @@ -20,6 +20,7 @@
>  
>  #include <assert.h>
>  #include <errno.h>
> +#include <limits.h>
>  #include <time.h>
>  #include <time.h>
>  #include <string.h>
> @@ -37,8 +38,6 @@
>  
>  #define IPFWMOD_ADDRESS_BULK_REPRESENTATIVE     "FF:FF:FF:FF:FF:FF:FF:FF"
>  
> -#define MAXIPFWCMDLEN           90
> -
>  #ifndef IPFW_RULERANGE_MIN
>  #define IPFW_RULERANGE_MIN      55000
>  #endif
> @@ -56,14 +55,14 @@ struct addr_ruleno_s {
>  };
>  
>  static list_t addrrulenumbers;
> -static char command[MAXIPFWCMDLEN], args[MAXIPFWCMDLEN];
> +static char command[PATH_MAX], args[ARG_MAX];
>  
>  /* generate an IPFW rule ID for inserting a rule */
>  static ipfw_rulenumber_t ipfwmod_getrulenumber(void);
>  /* execute an IPFW command */
> -static int ipfwmod_runcommand(char *command, char *args);
> +static int ipfwmod_runcommand(const char *command, const char *args);
>  /* build an IPFW rule for blocking a list of addresses, all of the given kind */
> -static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind, char *restrict command, char *restrict args);
> +static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind);
>  
>  static size_t ipfw_rule_meter(const void *el) { return sizeof(struct addr_ruleno_s); }
>  static int ipfw_rule_comparator(const void *a, const void *b) {
> @@ -95,7 +94,7 @@ int fw_block(const char *restrict addr, int addrkind, int service) {
>      ruleno = ipfwmod_getrulenumber();
>      addresses[0] = addr;
>      addresses[1] = NULL;
> -    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind, command, args) != FWALL_OK)
> +    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind) != FWALL_OK)
>          return FWALL_ERR;
>  
>      /* run command */
> @@ -108,7 +107,7 @@ int fw_block(const char *restrict addr, int addrkind, int service) {
>      sshguard_log(LOG_DEBUG, "Command exited %d.", ret);
>  
>      /* success, save rule number */
> -    strcpy(addendum.addr, addr);
> +    strlcpy(addendum.addr, addr, sizeof(addendum.addr));
>      addendum.ruleno = ruleno;
>      addendum.addrkind = addrkind;
>  
> @@ -134,7 +133,7 @@ int fw_block_list(const char *restrict addresses[], int addrkind, const int serv
>  
>      ruleno = ipfwmod_getrulenumber();
>      /* insert rules under this rule number (in chunks of max_addresses_per_rule) */
> -    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind, command, args) != FWALL_OK)
> +    if (ipfwmod_buildblockcommand(ruleno, addresses, addrkind) != FWALL_OK)
>          return FWALL_ERR;
>  
>      /* run command */
> @@ -147,7 +146,7 @@ int fw_block_list(const char *restrict addresses[], int addrkind, const int serv
>      sshguard_log(LOG_DEBUG, "Command exited %d.", ret);
>  
>      /* insert a placeholder for the bulk */
> -    strcpy(addendum.addr, IPFWMOD_ADDRESS_BULK_REPRESENTATIVE);
> +    strlcpy(addendum.addr, IPFWMOD_ADDRESS_BULK_REPRESENTATIVE, sizeof(addendum.addr));
>      addendum.ruleno = ruleno;
>      addendum.addrkind = addrkind;
>      list_append(& addrrulenumbers, & addendum);
> @@ -161,7 +160,7 @@ int fw_release(const char *restrict addr, int addrkind, int service) {
>      int pos, ret = 0;
>  
>      /* retrieve ID of rule blocking "addr" */
> -    strcpy(data.addr, addr);
> +    strlcpy(data.addr, addr, sizeof(data.addr));
>      data.addrkind = addrkind;
>      if ((pos = list_locate(& addrrulenumbers, &data)) < 0) {
>          sshguard_log(LOG_ERR, "could not get back rule ID for address %s", addr);
> @@ -172,22 +171,22 @@ int fw_release(const char *restrict addr, int addrkind, int service) {
>      switch (data.addrkind) {
>          case ADDRKIND_IPv4:
>              /* use ipfw */
> -            sprintf(command, IPFW_PATH "/ipfw");
> +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
>              break;
>          case ADDRKIND_IPv6:
>  #ifdef FWALL_HAS_IP6FW
>              /* use ip6fw if found */
> -            sprintf(command, IPFW_PATH "/ip6fw");
> +            strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
>  #else
>              /* use ipfw, assume it supports IPv6 rules as well */
> -            sprintf(command, IPFW_PATH "/ipfw");
> +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
>  #endif
>              break;
>          default:
>              return FWALL_UNSUPP;
>      }
>      /* build command arguments */
> -    snprintf(args, MAXIPFWCMDLEN, "delete %u", data.ruleno);
> +    snprintf(args, sizeof(args), "delete %u", data.ruleno);
>  
>      sshguard_log(LOG_DEBUG, "running: '%s %s'", command, args);
>  
> @@ -216,19 +215,19 @@ int fw_flush(void) {
>          data = (struct addr_ruleno_s *)list_iterator_next(& addrrulenumbers);
>          switch (data->addrkind) {
>              case ADDRKIND_IPv4:
> -                snprintf(command, MAXIPFWCMDLEN, IPFW_PATH "/ipfw");
> +                strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
>                  break;
>              case ADDRKIND_IPv6:
>  #ifdef FWALL_HAS_IP6FW
>                  /* use ip6fw if found */
> -                sprintf(command, IPFW_PATH "/ip6fw");
> +                strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
>  #else
>                  /* use ipfw, assume it supports IPv6 rules as well */
> -                sprintf(command, IPFW_PATH "/ipfw");
> +                strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
>  #endif
>                  break;
>          }
> -        sprintf(args, "delete %u", data->ruleno);
> +        snprintf(args, sizeof(args), "delete %u", data->ruleno);
>          sshguard_log(LOG_DEBUG, "running: '%s %s'", command, args);
>          ret = ipfwmod_runcommand(command, args);
>          if (ret != 0) {
> @@ -250,7 +249,7 @@ static ipfw_rulenumber_t ipfwmod_getrulenumber(void) {
>      return (rand() % (IPFW_RULERANGE_MAX - IPFW_RULERANGE_MIN)) + IPFW_RULERANGE_MIN;
>  }
>  
> -static int ipfwmod_runcommand(char *command, char *args) {
> +static int ipfwmod_runcommand(const char *command, const char *args) {
>      char *argsvec[20];
>      pid_t pid;
>      int i, j, ret;
> @@ -258,8 +257,8 @@ static int ipfwmod_runcommand(char *command, char *args) {
>  
>      sshguard_log(LOG_DEBUG, "Running command: '%s %s'.", command, args);
>  
> -    argsvec[0] = command;
> -    strcpy(locargs, args);
> +    argsvec[0] = strdup(command);
> +    strlcpy(locargs, args, sizeof(locargs));
>  
>      /* tokenize command */
>      argsvec[1] = locargs;
> @@ -280,6 +279,7 @@ static int ipfwmod_runcommand(char *command, char *args) {
>          sshguard_log(LOG_ERR, "Unable to run command: %s", strerror(errno));
>          _Exit(1);
>      }
> +    free(argsvec[0]);
>      free(locargs);
>      waitpid(pid, &ret, 0);
>      ret = WEXITSTATUS(ret);
> @@ -287,7 +287,7 @@ static int ipfwmod_runcommand(char *command, char *args) {
>      return ret;
>  }
>  
> -static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind, char *restrict command, char *restrict args) {
> +static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restrict addresses[], int addrkind) {
>      int i;
>  
>      assert(addresses != NULL);
> @@ -307,19 +307,19 @@ static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restr
>      switch (addrkind) {
>          case ADDRKIND_IPv4:
>              /* use ipfw */
> -            sprintf(command, IPFW_PATH "/ipfw");
> -            sprintf(args, "add %u drop ip", ruleno);
> +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
> +            snprintf(args, sizeof(args), "add %u drop ip", ruleno);
>              break;
>  
>          case ADDRKIND_IPv6:
>  #ifdef FWALL_HAS_IP6FW
>              /* use ip6fw if found */
> -            sprintf(command, IPFW_PATH "/ip6fw");
> +            strlcpy(command, IPFW_PATH "/ip6fw", sizeof(command));
>  #else
>              /* use ipfw, assume it supports IPv6 rules as well */
> -            sprintf(command, IPFW_PATH "/ipfw");
> +            strlcpy(command, IPFW_PATH "/ipfw", sizeof(command));
>  #endif
> -            sprintf(args, "add %u drop ipv6", ruleno);
> +            snprintf(args, sizeof(args), "add %u drop ipv6", ruleno);
>              break;
>  
>          default:
> @@ -327,13 +327,17 @@ static int ipfwmod_buildblockcommand(ipfw_rulenumber_t ruleno, const char *restr
>      }
>  
>      /* add the rest of the rule */
> -    sprintf(args + strlen(args), " from %s", addresses[0]);
> +    strlcat(args, " from ", sizeof(args));
> +    strlcat(args, addresses[0], sizeof(args));
>      for (i = 1; addresses[i] != NULL; ++i) {
> -        sprintf(args + strlen(args), ",%s", addresses[i]);
> +        strlcat(args, ",", sizeof(args));
> +        strlcat(args, addresses[i], sizeof(args));
> +    }
> +    if (strlcat(args, " to me", sizeof(args)) >= sizeof(args)) {
> +        fprintf(stderr, "Fatal: Argument buffer too small\n");
> +        exit(EXIT_FAILURE);
>      }
> -    strcat(args, " to me");
>  
>      return FWALL_OK;
>  }
>  
> -


> ------------------------------------------------------------------------------

> _______________________________________________
> Sshguard-users mailing list
> Ssh...@li...
> https://lists.sourceforge.net/lists/listinfo/sshguard-users

[Sshguard-users] Patch to fix blacklist loading with ipfw

[Kevin Z. <kev...@gm...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi there,

A patch that fixes blacklist loading when using the `ipfw` backend is
available and attached here. It is mostly of interest to FreeBSD.

This patch has not been committed because it relies on the
non-portable functions `strlcpy` and `strlcat`. While I work on
bringing these to SSHGuard, FreeBSD users can enjoy a working
blacklist now.

I've done rudimentary testing and this patch appears to work; before
this hits the ports tree someone should really test it.

Thanks,
Kevin Zheng

- -- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVZRxRAAoJEOrPD3bCLhCQN2MIAJOMmgslZPV5aYsYEnX1quC+
IXMc6t/rpFDybZPKz4LC4YI+WcsQ+fykKQ3mFZfJ2HITqqyBorNUe8JKzR8p59tX
sX5ePTq4Jld+LOFklKOSS3NSZauMi6zS8tcCpz5gVdQ0iBizDssW/f70ZTD927lB
44VgAdv8FrHXsPpgEgcrZCsNm3uK8j48eh3aAo3elThM4BAIhoMYobLZl1Jgnq59
hjWVk49Z1njypiP2SYASXVdy5x8AINQDY4R8Wqa0/mNGfzFKT2y5HPw/70YbAm3M
E1o/V9apCH3p1Trq/NshZwvP9sFxfV0oJtATRXUvJxuI0BDHIM5F+/w72TJCVU4=
=SKWp
-----END PGP SIGNATURE-----

[Sshguard-users] New ipfw backend

[Kevin Z. <kev...@gm...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi there,

In light of the recent `ipfw` issues I've decided to re-implement the
`ipfw` backend using the command framework that is used for nearly all
of the other backends. Since I don't run `ipfw` on my machine, I'm
unable to test this patch.

If you are running `ipfw` and are willing to test-drive this new and
more than likely broken backend, apply the attached patch, compile,
and take it for a whirl. In particular, I'm not sure if the "add
multiple addresses" part works, so if you have a large blacklist that
crashed the original ipfw backend try it on the new one.

The new backend operates on ipfw tables. You'll need to set up your
firewall with a tabled named 'sshguard'. SSHGuard (should) add
attackers to this table; you'll need to set up the rules yourself.

Please don't test this in a production environment, and if you test it
at all, be aware that bad things can happen. Please take a look at the
patch before you try to run this code.

Best,
Kevin Zheng

- -- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVZPR5AAoJEOrPD3bCLhCQ4VsH/3mugV40W5hmj3sfVOV+AYDl
0hUssAIOdapw0yOoaRnGYP/2+IZNbtw1737iH10BBX1S59xmWDuqPT/Wi00kHuLv
WttvPCUuHBBJcJS6H0C+VG0yeQepFdmvln6zb7bKBAbarVb8z4Oq5sOBPDMtby9t
hAfYWr4CEKe1MI9x0wHY8y2Lz9yVBc+bPUJzYj+WS7a1qwdYqzyLIfB5NWMsRpdF
lv6ktXZYWwb/Gkw/ALTwPHm5xepz5suBjsyPS2eQgVnBMaNAzjsGy349BvKYOdkX
Q5wKaVwBWs0RcpfR0GmbYoSbT3Ya1Q+ToNl/9Ep8BUMbC/XuR+Py7u1kGghIlHA=
=JaPC
-----END PGP SIGNATURE-----

[Sshguard-users] Errata Notice: Crash when loading blacklists using the ipfw backend

[Kevin Z. <kev...@gm...>]

This is an errata notice for SSHGuard. This issue impacts the 1.6.0
release, but has actually been around for quite some time.

## Problem ##

When blocking attackers from a loaded blacklist file, SSHGuard will
write past the boundaries of a fixed-length buffer.

This problem only affects users running SSHGuard with blacklisting
enabled, while using the `ipfw` backend.

## Impact ##

SSHGuard will crash with a segmentation fault upon startup when loading
a blacklist with enough (less than 100) entries. Because the blacklist
file is generally owned by the superuser, it is unlikely that this
vulnerability could be used to gain superuser privileges. If you are
affected, please consider using one of the workarounds:

## Workaround ##

Any one of these should work around the issue:

1. Don't use blacklisting.

2. Don't use the `ipfw` backend.

3. If you need blacklisting, delete the blacklist file before starting.

## Solution ##

We're working on one. The "long-term" solution is to switch `ipfw` to
the "command" backend and use ipfw tables instead of individual rules.
For the time being:

1. Increase the length of the fixed buffer. Eventually, though, this
will run into the same problem.

2. There is a patch on the mailing list that adds the blacklisted
addresses one at a time. I haven't taken a look at it yet.

## Credits ##

Thanks to Greg Putrich <gr...@n0...> for analyzing and proposing a
fix to this issue. Thanks to the many people who have reported this
issue beforehand, even though I never got around to acting on them.

-- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090

Re: [Sshguard-users] Core dump on FreeBSD 10.1 with sshguard-ipfw 1.6.0

[Kevin Z. <kev...@gm...>]

Hi Greg,

Thanks for tracking down the problem and providing a fix. This issue has
been around for quite some time and I never had enough motivation to
track it down (I never used the blacklist with ipfw).

(The original patch is attached with this message.)

On 05/26/2015 12:11, Greg Putrich wrote:
> On sshguard 1.6.0 (and 1.5.0) on FreeBSD 10.1 with ipfw, when starting up 
> sshguard with a "large" blacklist.db file, it would crash with a segmentation 
> fault & dump its core. Tracked this down to MAXIPFWCMDLEN being set to 90.

That would do it. The culprits are short fixed-length buffers used with
unbounded string functions. All the sprintf's should be taken out.

> Set it to 100 and it worked with a slightly larger blacklist.db file, but the
> problem is, changing that number is fine for a time, but my blacklist.db file
> for running for a couple of weeks is 212 entries and that would be one really
> long rule. I found this the hard way when I patched my system, rebooted and
> didn't check sshguard. I looked at it by chance later and it wasn't running
> and wouldn't start. Cleared out blacklist.db and it was fine. As you can see,
> this is not an ideal condition and makes blacklist.db useless.

A "fix" would be to bump the buffer up to something ridiculous like 2048
(or something in sys/limits.h). But you're right; that doesn't solve the
problem at hand.

> I decided to fix it by looping through each entry & adding a separate rule.

This was originally avoided to stop incurring the penalty of a system()
call for every IP. But this fix is better than crashing.

> What this also does is keeps the counters meaningful as can tell which IP
> addresses are actively being a pest.

The ipfw backend has been rotting in lots of different places. A while
ago someone pointed out some vulnerabilities concerning how the ipfw
backend assigns attackers to firewall rules, but that hasn't been fixed.

> Attached is the patch for 1.6.0. For the most part, I copied the code from two
> sections within ipfw.c then wrapped it in a for loop.
> 
> Also included in that diff is the existing patch for ipfw.c in sshguard-ipfw
> on FreeBSD.
> 
> I'm not much of a C coder, so this may not be the ideal way of doing it, but
> its been working here and no more core dumps when loading a big blacklist.db.

I'll take a look. In the future, you're more than welcome to post
patches to the mailing list for more eyes to look at it. Also, in case I
never get around to actually looking at it.

Thanks,
Kevin Zheng

-- 
Kevin Zheng
kev...@gm... | ke...@kd... | PGP: 0xC22E1090

Re: [Sshguard-users] On initiating sshguard

[Rick H. <ri...@ta...>]

Please ignore this issue I brought up.  I tried the service method (only,
i.e. no changes to syslog.conf) and everything seems to be working fine.
Sorry for the noise.  Cheers!