Bug 888571

Hi.

Taking this from Debian bug #693177:

The ipsets program is IMHO broken as it does not provide a real
restore command as iptables-restore does.
"ipsets restore" is merely and additive merge in most cases... actually
it just executes each line of the fed in file,... but it doesn't remove
any old sets (which is admittedly not possible when they're still in
use... but worse... it does not even remove old entries).
I had a longer discussion[0] with upstream but apparently he shows no
insight that his current restore operation is useless...

Now making a real restore is tricky to impossible.
Tricky, because a simple:
ipset flush
ipset destroy
ipset restore < foo
is not enough. Even though it seems to to just what is wanted... there
is a considerable amount of time where no entries are in place... and
connections would fail.
And it doesn't handle at all that sets which are in use cannot be
destroyed.


Attached is an init script, which does some smarter restore... it
normally adds new sets... it adds all sets who already exist with a
_tmp_ prefix and then swaps the contents (which is atomic) and then
tries to flush/destroy the now old _tmp_ sets (which should in principle
always work).
It then tries to flush/destroy completely gone sets... which may not
work, when these are still in use by iptables rules.


The init script MUST be loaded before the existing iptables init script...
RedHats mixed use of LSB and chkconf headers is quite crazy... and you don't provide X-Star/Stop-Before/After headers as Debian does...
So I hope my
# chkconfig: 2345 07 07
header makes the right thing...


Testing is welcomed.


Cheers,
Chris.

Created attachment 666830 [details]
/etc/init.d/ipsets

Comment on attachment 666830 [details]
/etc/init.d/ipsets

has some bugs... better version is coming

Christoph, do you have an updated version you could share?

The Mandriva version looks nice and simple:


http://mandriva.598463.n5.nabble.com/Bug-62639-New-request-adding-ipset-startup-script-td3404891.html


I liked that better than the one on attachement 66830, because it was much simpler.. Don't quite understand what you're doing in restore_ipsets().. My expectation is that stopping/starting it should only load a static configuration similar to iptables, not necessarily save/populate huge sets.

Hi.

Sorry I've lost a track on that little project a bit...

The script you mentioned is not very sophisticated... and may even fail in the pure stop/start situations... and a reload action is missing altogether.
When you run it only once at start (with no ipsets being used and/or existent yet)... it should work though.


The problem with ipset is, that it doesn't provide a real restore operation as iptables does.
It merely only executes the commands in the file, even without checking before whether those can succeed at all.


Many problems may arise, like (bot not limited to) removals of sets being in use (by iptables)... or changing of set types while the name stays the same, etc. pp.

This makes the restore such complicated.

I put mine in /sbin/ifup-local.  It's not the right way to do it, but as a temporary work-around it works for me.

> ipset -! create mySet1 hash:net
> ipset -! add    192.168.0.0/24
> ipset -! add    X.X.X.0/24 etc. etc. etc.
> /etc/init.d/iptables restart

I changed my method on this.  In /sbin/ifup-local I put these lines.  That way if the ipsets file is there, the rules get loaded, otherwise nothing happens.

if [ -x /etc/sysconfig/ipsets ]; then
  . /etc/sysconfig/ipsets
  /etc/init.d/iptables reload
fi

Created attachment 762314 [details]
ipset init script

There is working script for el6 finded in ipset.rpm in CentALT repo and modified.

(In reply to Dmitriy A Pyryakov from comment #11)
> Created attachment 762314 [details]
> ipset init script
> 
> There is working script for el6 finded in ipset.rpm in CentALT repo and
sorry, FlexBox repo, not CantALT.

Created attachment 783510 [details]
Patch to create systemd ipset service to load, save and unload ipset sets.

Currently loading of iptables at startup is failing when iptables uses ipsets, since they are not being loaded. This is causing a security problem at system startup.

The attached patch provides all the files necessary to add an ipset systemd service which will start up before iptables (and stop afterwards).

The structure is closely based on the iptables systemd files, since ipset is inextricably linked to iptables.

The ipset.init script is based on the iptables.init script, but uses the functionality in the earlier attachments to this bug, mainly Christoph Anton Mitterer's, with some modifications, which I believe resolve all the issues that have been raised in earlier comments.

When loading, if no ipset sets are configured, it does a simple restore. If sets are already installed, it destroys any sets not required by the config being loaded (and flushes them if they can't be destroyed), removes any entries which don't exist in the new config, and finally loads the saved config using ipset restore -! , to bring in any sets and entries in sets not already loaded.

The patch provides a /etc/sysconfig/ipset-config file, similar to iptables. It has one slightly unusual feature whereby the ip(6)tables entries IP(6)TABLES_SAVE_ON_STOP/RESTART, if set to "yes", override the IPSET_SAVE_ON_STOP/RESTART configuration, since ipset configuration must be updated if iptables configuration is updated.

Even if this patch isn't perfect, it would be really helpful if it could be implemented since it allows iptables to be used with ipsets and setup at boot time, and is better than what we have now. The default configuration is for the systemd ipset.service to be disabled so it will not alter existing functionality unless explicitly enabled.

This request was evaluated by Red Hat Product Management for
inclusion in the current release of Red Hat Enterprise Linux.
Because the affected component is not scheduled to be updated
in the current release, Red Hat is unable to address this
request at this time.

Red Hat invites you to ask your support representative to
propose this request, if appropriate, in the next release of
Red Hat Enterprise Linux.

Created attachment 896041 [details]
Porposed ipset script

This script is a mixture of the proposed Fedora and RHEL ipset scripts with some further adaptions (example status output and the ipset version in RHEL-6).

Please test and verify the proposed script in comment 21.

I can't say that I've thoroughly exercised it, but in the basic cases this script seems to work just dandy for me.

If I run /etc/init.d/ipset start without a configuration file, I am placed into some kind of emacs help page.

How is the configuration to be formatted?  Similar to the iptables formatting where we simply leave out the command part at the beginning?  This seems to work for me, so I guess that's the right format:

-! create mySet hash:net,port
-! add mySet 192.168.0.0/24,tcp:80
-! add mySet 192.168.0.0/24,tcp:443

(In reply to Bryan Harris from comment #24)
> If I run /etc/init.d/ipset start without a configuration file, I am placed
> into some kind of emacs help page.
> 
This has been fixed.

> How is the configuration to be formatted?  Similar to the iptables
> formatting where we simply leave out the command part at the beginning? 
> This seems to work for me, so I guess that's the right format:
> 
> -! create mySet hash:net,port
> -! add mySet 192.168.0.0/24,tcp:80
> -! add mySet 192.168.0.0/24,tcp:443

Yes, this is the file format for ipset.

Created attachment 908657 [details]
Final ipset script

(In reply to Thomas Woerner from comment #26)
> Created attachment 908657 [details]
> Final ipset script

The script appeared to have a security flaw in it by using $$ in temporary filenames located in /tmp, always use mktemp instead.

TMP_FIFO=/tmp/${IPSET}.$$

should be:

TMP_FIFO="$(mktemp /tmp/${IPSET}.XXXXXX)"



However looking at the script further it doesn't appear that TMP_FIFO is useful for anything at all because:

CLEAN_FILES=TMP_FIFO
            ^
            
Right now TMP_FIFO is set to "/tmp/ipset.$$", and CLEAN_FILES is set to literal text of "TMP_FIFO" and not the expansion of $TMP_FIFO, so at this point the variable $TMP_FIFO is set to an insecure filename but not actually used.



Typo here, should say "requires ipset" non-plural:

  echo -n $"${IPSET}: Current ip*tables configuration requires ipsets";


This could probably be worded a bit more clearly:
  echo -n $"${IPSET}: Flushing and destroying IP sets: "

as:
  echo -n $"${IPSET}: Flushing and destroying ipset sets: "



Then down below further:
    TMP_FILE=$(/bin/mktemp -q /tmp/$IPSET.XXXXXX) \
	&& CLEAN_FILES+=" $TMP_FILE" \
	&& chmod 600 "$TMP_FILE" \
	&& ${IPSET_BIN} save > $TMP_FILE 2>/dev/null \


Here it is now using mktemp correctly and adding the filename to CLEAN_FILES, which will now contain:

"TMP_FIFO /tmp/ipset.XXXXXX" where XXXXXX is random, however there is no file named TMP_FIFO, and the variable TMP_FIFO was just set, incorrectly used once then not actually used anywhere.  As far as I can tell just remove everything having to do with TMP_FIFO completely as it doesn't seem to serve any purpose except demonstrating bad temporary file naming (but not using it).  :)


Hope this helps Enterpriseinate(TM) it a bit. :)

*** Bug 1130570 has been marked as a duplicate of this bug. ***

The initscript does not save the ipsets at shutdown, instead it assumes that if iptables or ip6tables default configuration saves their configs on shutdown that ipset should too which is a bit short sighted.  It makes more sense to have it's own config /etc/sysconfig/ipset-config and have a variable in that to decide whether it will save it's config or not, that way it easily works on it's own without being tied to the default iptables/ip6tables configs.  Someone might want to have iptables/ip6tables /not/ save their config automatically at shutdown because they prefer to manually edit those configs by hand and want to retain their comments and file structure, but they do want to have ipset save it's config.  Or, someone might run an alternative firewall which can use ipsets but does not handle starting/stopping it directly instead relying on the system scripts.

I believe the ipset interfaces can potentially be used by other things as well, so it is best if it has it's own standalone configuration.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2014-1543.html

Hi guys.

Sorry for looking so late into this...


Back when I wrote this request, my main motivation was actually to get a proper reload function, i.e. something like iptable's iptables-restore...

Uhm... I've had a short look at the init script...

Some questions/issues:
1) I'm a bit unsure how far my original motivation is actually fulfilled.
Back when I stumbled across this, I tried to write a simple script myself, but it has proven to be quite difficult, at least when looking at some constraints:

a) IIRC, one couldn't delete an ipset, as long as it was used by iptables,... and of course (recursively) one cannot delete ipsets belonging to another ipset being still used.

b) I think, that a reload/restore should be basically atomic,... and I guess this is not possible from a shell script. Actually it most likely needs kernel support.

c) It shouldn't just be atomic in the sense of adding new sets, but also in the sense of everything that is added/removed.
That means, either a reload/restore should completely succeed i.e. being able to add all new sets with all their entries, and to remove all previous sets with their entries AND if necessary to change the type of sets.... or nothing should happen at all an the program should bail out with an error.


(b) and (c) are IMHO highly security relevant:
You never know what people actually do with the sets, and which crude rules they've come up with to protect their security.
So only adding some new sets, but not removing some others, or not being able to change the type of a set, or doing all this one by one and not in one atomic step, may already compromise security.

And even if this problem would be limited to "there's just a small window where the rules aren't consistent" this would be enough for an attacker... attacker's typically don't drink coffee when such window opens.
Moreover, many systems have the well known ESTABLISHED,RELATED -j ACCEPT rules... so one successful connection in such a short period of time may be enough.


That basically also means:

2) that the current way the reload operation is implemented is IMHO dangerous from a security POV, since first stop then start is executed, and stop flushes the sets (if possible) and AFAICS, there is no guarantee that the new sets at start will be able to be successfully loaded.



I guess this script is fine for loading the rules at system start and storing them at system shutdown.
Maybe I miss some important point, but I think it has security issues whenever used for reloading rules during the system running.




Any thought's about this from the RH side? Thomas?
I think especially in the enterprise field, this non-atomicity and the missing guarantee of either all or nothing,... is a big problem.
Or well at least it was for us here at the university, running extremely large clusters at the super computing centre in Munich.


Cheers,
Chris.