Re: IPCop Banish Addon for IPFire

[Matthias Fischer <matthias.fischer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 4622 bytes --]

On 07.01.2019 20:28, Bob Brewer wrote:
> Hi Matthias,
> 
> Matthias Fischer wrote:
> 
> Thank you for your response

No problem - you're welcome... ;-)

>> 
>> Some findings:
>> 
>> 1. For keeping the "naming conventions" in '/etc/init.d', I'd suggest to
>> name this script just 'banish', not 'rc.banish'. This way it can be
>> found more easily. Every other script in this dir is named accordingly
>> to the program its starting/stopping...
>> 
> rc.banish is the original IPCop name for the init script, but I can follow 
> convention and change it to banish. 

For further testing, I changed the name to '/etc/init.d/banish' - and
renamed the corresponding symlinks.

But still I can't add any entries. Page reloads, list stays empty. GUI
stays empty and 'iptables' chains, too. Rule(s) seem(s) to be deleted
right before/after adding!?

Here I tried to add '50.22.0.0/15:
...
22:08:56	banish	: Remove Banish rules
22:08:56	banish	: Removing Banished entries
22:08:56	banish	: Load Banish rules
22:08:56	banish	: Adding Banished entries to DROP
22:08:56	banish	: Regel hinzugefügt: 50.22.0.0/15
22:08:56	banish	: Remove Banish rules
22:08:56	banish	: Removing Banished entries
22:08:56	banish	: Load Banish rules
22:08:56	banish	: Adding Banished entries to DROP
22:08:56	banish	: Regel gelöscht:
...

For entries to add, must the testmachine be online!?

>> 2. With '1.0.2', installation throws no errors anymore, but I can't add
>> any IP-Addresses. List stays empty. And everytime 'BanishGeo.cgi' is
>> opened, '/var/log/httpd/error_log' says:
>> 
>> ***SNIP***
>> ...
>> Usage: host [-aCdilrTvVw] [-c class] [-N ndots] [-t type] [-W time]
>>             [-R number] [-m flag] hostname [server]
>> ...
>> ***SNAP**
>> 
>> It seems there are some syntax errors in 'BanishGeo.cgi' (line 773!?).
>> 
> After a bit of investigation, this is caused by a default blank entry 
> creeping into the banish_config file presumably a CR/LF that shouldn't be 
> there. If you delete the blank line in the BanishGeo.cgi display  (with the 
> dustbin icon all should be OK. I used a test banish_config file for my 
> original testing and didn't see this error.

Ok, after deleting the empty line, this error is gone, but: see above.

> 
>> 3. $Lang::tr{'apply'} - used in 'BanishGeo.cgi' line 232 - is missing in
>> the language-files.
>> 
> This is a problem with the original banish distribution files. Maybe there 
> was a translation in IPCop which IPFire doesn't have, I'll add one to these 
> lang files if I can find the appropriate translations.  
> 
>> 4. Uninstaller didn't remove '/var/ipfire/menu.d/EX-banish.menu' and
>> 'EX-Banishlog.menu'. They're not shown anymore, but files are still there.
>> 
> 
> Looks like I missed those in the uninstall script. I'll add them.

;-)

>> 5. None of the given URLs is working here:
>> http://wiki.sidsolutions.net/ => "Can't be found".
>> http://banish.sidsolutions.net
>> is redirected to
>> ww2.banish.sidsolutions.net => no answer...
>>
> I am aware that sidsolutions is no longer there and removed quite a lot of 
> code with links to his site and must have missed these. Thank you for 
> finding them.

Most of them are in the language files...

Besides, all "umlauts" are looking weird in the log, as seen above:

...
21:49:50	banish	: Adding Banished entries to DROP
21:49:50	banish	: Regel hinzugefügt: 50.22.0.0/15
                                        ^^^
...
21:50:41	banish	: Regel gelöscht:
                                   ^^^
...

> In the original Banish there were 2 cgi control pages  a Banish_Settings.cgi 
> and the BanishGeo.cgi. For IPFire I combined the 2 pages which puts the 
> Viewing Options box at the top of the BanishGeo.cgi page. Also in the 
> original there were 2 optional Banish.cgi pages and the BanishGeo.cgi page 
> was only installed if Geo::IP::PurePerl was installed. As IPFire seems to 
> always install this I didn't bother with the non-Geo page.
>  
>> HTH,
> Yes it has, thank you very for your time to look at this. Look out for 0.3 
> which should have the errors corrected
> ...
> As an aside I'm running Core Update 126 on my test box and noticed what I 
> think is a typo in log.dat for the captive translation entry.  I think line 
> 84 should be:
> 
> 'captive' => $Lang::tr{'Captive'},
> with a capital 'C' and not:
> 
> 'captive' => $Lang::tr{'captive'},
>...

It seems so. I'm running heavily tuned language files here, so I
overlooked this. I'll take a look.

Best,
Matthias