Hello Michael,
> Hi,
> 
> > On 20 Dec 2018, at 13:03, Stefan Schantl <stefan.schantl(a)ipfire.org
> > > wrote:
> > 
> > Hello Michael,
> > 
> > thanks for having a look at suricata and your feedback.
> > > Hi,
> > > 
> > > So I finally had a closer look at this, too. I have a couple of
> > > questions and would like to propose some changes to the UI:
> > > 
> > > Questions:
> > > 
> > > * Is it a good idea to have “No” rules as default and also “no”
> > > automatic rule updates? I guess we can configure at least
> > > Emerging
> > > Threats as a good default because they are free. I also think
> > > that
> > > when IDS is enabled, the system should update the ruleset by
> > > default
> > > once a day. When IDS is enabled, there should be no rule update.
> > > 
> > > * That also leads me to: Should no automatic updates even be an
> > > option?
> > > 
> > Making the free ruleset of emergingthreads as default is okay for
> > me -
> > ACK.

DONE - now the default selection, if no ruleset source has been choosen
yet is the free emergingthreads ruleset.

> > 
> > I'm also fine with doing a weekly ruleset update as default
> > setting.
> > 
> > My personal opinion is, it should be possible to disable the
> > automatic
> > rule update. There are some situations where this feature could be
> > quite usefull, for example if you have only volume limited internet
> > access or even other (which we currently do not have in mind....).
> 
> I guess if you don’t update the rules there is no point in running
> the IPS. It’s just like a virus scanner with outdated signatures:
> detecting even less.
> 
> Do we have more people with an opinion on this?
> 
> The rulesets are indeed quite large. But the only people who might
> want to disable it are people who run IPFire on an IoT appliance and
> those are too slow to run the IPS anyways. So it’s not a very strong
> argument in my opinion.
> 
> The best argument I can make for it is only that this is probably
> unwanted behaviour and therefore can easily be misconfugured.
> 

Default now is "weekly" until anything else has been selected.

I keept the feature for disabling the automatic update for now until we
have more opinions on this topic.

> > 
> > > Bugs:
> > > 
> > > * When I enabled IDS in IPS mode, I checked the box to activate
> > > it
> > > and selected the ET ruleset and hit save. Suricata starts but
> > > without
> > > rules which is definitely not what I wanted. Either the box
> > > should be
> > > greyed out or the downloader is launched when there are no rules,
> > > yet.
> > > 
> > > * It is possible to enable suricata without selecting any
> > > interfaces.
> > > That just wastes RAM and makes it easy to enable it without
> > > actually
> > > having the protection.
> > > 
> > > If you agree, I will create tickets for this.
> > 
> > Thanks, fixed.
> > 
> > https://git.ipfire.org/?p=people/stevee/ipfire-2.x.git;a=commit;h=ebdd0f9a90da800cc6173f6f30fb0621dddc354b
> 
> I am on a plane now, so I cannot see this, but thanks!
> 
> > > UI Stuff:
> > > 
> > > * I think that the settings to enable suricata up to and
> > > including
> > > “Traffic analysing” should be in the top box. The second box
> > > should
> > > only care about the rules. That might even solve the problem
> > > described above.
> > 
> > During development I had a very similar idea, of grouping all
> > suricata 
> > settings and all which are for the ruleset together into seperate
> > boxes. I dropped this idea, because of some big restrictions of the
> > code and I was afraid of how many work this would be to re-write
> > huge
> > parts of it to get the desired result.
> 
> I think that this is very little work.

DONE - Technically it was just to splitt the HTML <form> from a single
one into two seperate ones. But under the hood more code had to be
changed.

> 
> > > * I find it quite confusing that one has to “Activate Intrusion
> > > Detection System” and then again select IDS or IPS. It doesn’t
> > > seem
> > > to make much sense even to select the top box when IPS is
> > > checked. I
> > > would therefore prefer the following: one box that just says
> > > “Activate” and that switches on or off the whole thing, and then
> > > a
> > > second box that says “monitor only” or something along these
> > > lines.
> > > That box should be disabled by default which will run suricata in
> > > IPS
> > > mode (which is what everybody wants!) and when checked will
> > > downgrade
> > > it to IDS mode. Right now it looks like this is an equal level of
> > > both options but I think that there is a risk here that people
> > > enable
> > > IDS and think that they are protected when they are in fact not.
> > > I
> > > think the “monitor only” checkbox makes this a lot clearer.
> > 
> > Fair enough - I'm going now to rework the current section for
> > choosing
> > the runmode and move from the radio buttons to a select box which
> > will
> > be called "Only monitor traffic" as you suggested and will place it
> > right next to the box where you can enable the whole IPS/IDS
> > system.
> 
> I think the best wording is: Monitor traffic only

DONE - As promised, I've dropped the radio button selection for the
runmode and placed a checkbox for this.

> 
> > > * The Traffic Analyzing headline doesn’t make any sense. It
> > > should
> > > rather be “Monitored zones” or “Monitored interfaces”. Internet
> > > should be RED because it is called RED everywhere else.
> > 
> > ACK

DONE

> > 
> > > * We will need to grep for Snort everywhere and replace that
> > > phrase
> > > with “Intrusion Prevention System”. I do not think that we have
> > > to
> > > mention suricata here instead. It is confusing when the menu
> > > option
> > > is called IDS.
> > 
> > Most work on this already should be done, I'll remove the last
> > existing
> > snort entries if I'll found one. 
> 
> Cool.

This still needs to be done.

> 
> > > * The menu option should be called IPS instead of IDS I think
> > > since
> > > IPS mode is default. Spelled out obviously.
> > 
> > ACK - It should be a very easy task to switch the language string
> > from
> > "Intrusion Detection System" to the desired "Intrusion Prevention
> > System".

This also still needs to be done.

I've spent a lot of time in the past days to rework some code and
moving stuff to the "ids-functions.pl". The final goal was to get a
nice code-base for the "snort to suricata" converter on which I'm
currently working.

I hope I will finish this very soon, so I can go on fixing the last
minor issues and filed bugs.

Best regards,

-Stefan 

> 
> Cool. I am just saying that it needs to be done :) don’t forget the
> logging section.
> 
> > > Apart from that I have to say that this is really really good
> > > work,
> > > Stefan. The main thing is: it works. I think this is a huge
> > > improvement over what we had in the past and that this is
> > > bringing
> > > IPFire to the next level. It solves many problems that we didn’t
> > > dare
> > > to touch because snort was simply badly integrated into IPFire
> > > and
> > > did not really do much instead of wasting CPU cycles and memory.
> > > 
> > > I hope that we can continue working in this direction and maybe
> > > add
> > > some nice reports and so on and obviously carry over this work
> > > into
> > > IPFire 3 as soon as possible.
> > > 
> > > Please feel free to disagree with me above, but please do that
> > > via
> > > email if possible.
> > > 
> > > I want to target merging this into next as soon as possible in
> > > January. I hope that we can get the things above sorted out over
> > > the
> > > holidays since it should not be much work.
> > > 
> > Best regards,
> > 
> > -Stefan
> > > > On 17 Dec 2018, at 19:08, Stefan Schantl <
> > > > stefan.schantl(a)ipfire.org
> > > > > wrote:
> > > > > What is the bypass mark for? At least that should be set to
> > > > > nothing
> > > > > instead of one, because that will again conflict with the
> > > > > SNAT
> > > > > fix
> > > > > rule.
> > > > > 
> > > > > Best,
> > > > > -Michael
> > > > > 
> > > > Hello Michael,
> > > > 
> > > > taken from the official suricata.yaml documentation:
> > > > 
> > > > bypass mark and mask can be used to implement NFQ bypass. If
> > > > bypass
> > > > mark is set then the NFQ bypass is activated. Suricata will set
> > > > the
> > > > bypass mark/mask on packet of a flow that need to be bypassed.
> > > > The
> > > > Nefilter ruleset has to directly accept all packets of a flow
> > > > once
> > > > a
> > > > packet has been marked.
> > > 
> > > Weird. Why would you even pass this to the userspace process
> > > then?
> > > 
> > > Whatever, we don’t need it then...
> > > 
> > > > In my commit, I've disabled the bypass options to exactly
> > > > prevent
> > > > from
> > > > any issuse again with the SNAT packet mark.
> > > > 
> > > > Best regards,
> > > > 
> > > > -Stefan
> > > 
> > > Best,
> > > -Michael
> > > 
> > > > > > On 17 Dec 2018, at 14:21, Stefan Schantl <
> > > > > > stefan.schantl(a)ipfire.org
> > > > > > > wrote:
> > > > > > 
> > > > > > Am Sonntag, den 16.12.2018, 21:28 +0100 schrieb Peter
> > > > > > Müller:
> > > > > > > Hello Stefan,
> > > > > > > 
> > > > > > > to be a bit more precise about the NAT issue:
> > > > > > > 
> > > > > > > My setup is the IPFire Suricata test VM running in KVM,
> > > > > > > with
> > > > > > > two clients (Debian and OpenBSD) directly attached to it.
> > > > > > > 
> > > > > > > The Debian machine is located in GREEN, OpenBSD in
> > > > > > > ORANGE.
> > > > > > > RED interface is connected via bridge to my actual
> > > > > > > testing
> > > > > > > LAN;
> > > > > > > for the first testing, any outgoing traffic to the
> > > > > > > internet
> > > > > > > was allowed (I will test upstream proxy behaviour later).
> > > > > > > 
> > > > > > > While GREEN was using IPv4 range 192.168.100.0/24, with
> > > > > > > IPFire
> > > > > > > as 192.168.100.1, enabling Suricata caused packets coming
> > > > > > > from
> > > > > > > GREEN not to be NATted anymore: Instead of using the
> > > > > > > firewall's
> > > > > > > RED IP for destination, it was the internal GREEN IP.
> > > > > > > 
> > > > > > > Let me know whether is is useful or not. :-)
> > > > > > > 
> > > > > > > Thanks, and best regards,
> > > > > > > Peter Müller
> > > > > > > 
> > > > > > > > Hello Stefan,
> > > > > > > > 
> > > > > > > > back again. :-)
> > > > > > > > 
> > > > > > > > The new IDS WebUI looks quite good so far -
> > > > > > > > enabling/disabling
> > > > > > > > Suricata works as well as selecting the rule source and
> > > > > > > > the
> > > > > > > > operation mode (IDS/IPS).
> > > > > > > > 
> > > > > > > > I was also able to download the "Snort/VRT Community"
> > > > > > > > ruleset.
> > > > > > > > Trying to switch to the "Emerging Threats" ruleset is
> > > > > > > > possible,
> > > > > > > > but downloading its ruleset afterwards is not: The GUI
> > > > > > > > simply
> > > > > > > > stalls, printing a message that "Snort (!) is
> > > > > > > > performing a
> > > > > > > > task".
> > > > > > > > 
> > > > > > > > The WebUI services page still shows IDS status for each
> > > > > > > > interface,
> > > > > > > > which does not seem to work anymore (everything is
> > > > > > > > stopped,
> > > > > > > > but
> > > > > > > > Suricata was active on RED and GREEN).
> > > > > > > > 
> > > > > > > > Further, a client located in GREEN behind the test
> > > > > > > > firewall
> > > > > > > > instance is unable to browse the internet as soon as
> > > > > > > > Suricata
> > > > > > > > is
> > > > > > > > enabled. If disabled, downloading ET rulesets work as
> > > > > > > > well
> > > > > > > > as
> > > > > > > > internet traffic. At the moment, I am flying blind
> > > > > > > > here,
> > > > > > > > but it
> > > > > > > > looks
> > > > > > > > like packets are not NATted anymore if Suricata is
> > > > > > > > active.
> > > > > > > > 
> > > > > > > > Any outgoing connection is in state "SYN_SENT" if
> > > > > > > > Suricata
> > > > > > > > is
> > > > > > > > active.
> > > > > > > > 
> > > > > > > > A portscan against the firewall (GREEN interface) is
> > > > > > > > not
> > > > > > > > detected,
> > > > > > > > even though ET SCAN ruleset is enabled (used nmap with
> > > > > > > > NSE
> > > > > > > > active).
> > > > > > > > 
> > > > > > > > Especially the outgoing connection/NAT/? issue
> > > > > > > > mentioned
> > > > > > > > above
> > > > > > > > breaks things in my scenario. Anything else are minor
> > > > > > > > issues
> > > > > > > > (of
> > > > > > > > course, a portscan should be detected, this needs
> > > > > > > > further
> > > > > > > > investigation
> > > > > > > > indeed). WebUI works fine so far.
> > > > > > > > 
> > > > > > > > Thanks again for your work; I hope the feedback can
> > > > > > > > appreciate
> > > > > > > > it
> > > > > > > > somehow. :-)
> > > > > > > > 
> > > > > > > > Let me know if there are questions.
> > > > > > > > 
> > > > > > > > Thanks, and best regards,
> > > > > > > > Peter Müller
> > > > > > > > 
> > > > > > Hello Peter,
> > > > > > 
> > > > > > thanks for testing and your feedback.
> > > > > > 
> > > > > > I've setup a test environment and was able to re-produce
> > > > > > your
> > > > > > NAT
> > > > > > issue
> > > > > > very easy. After some research and help of Michael, we were
> > > > > > able to
> > > > > > find the real issue.
> > > > > > 
> > > > > > It was located in a identical mark on the packets which
> > > > > > already
> > > > > > have
> > > > > > been scanned by suricata and packets which should be
> > > > > > modified
> > > > > > for
> > > > > > SNAT
> > > > > > in the "Mangle" table of the routing logic by the kernel.
> > > > > > 
> > > > > > The changes can be found in my git repository:
> > > > > > 
> > > > > > https://git.ipfire.org/?p=people/stevee/ipfire-2.x.git;a=commit;h=f5ad510e3c0f416a1507999f5ad20ab171df9c07
> > > > > > 
> > > > > > I'll upload a fixed image very soon - please keep on
> > > > > > testing.
> > > > > > 
> > > > > > Best regards,
> > > > > > 
> > > > > > -Stefan