Re: [pve-devel] Firewall hooks
Am Donnerstag, den 22.03.2018, 12:28 +0100 schrieb Harald Leithner: > Hi, > > it seams that there are no firewall hooks in pve-firewall is this > correct? IIRC, yes. > I would like to add my own action before, after the firewall > configuration for a VM is stop,started or reloaded. [..] > Is there any point I could attach my own script? when I was dealing with the firewall code cleanups last year, I had ideas like this in the back of my head. Or at least ideas that would make implementing such hooks easier. My plans to further work on this still exist - free time doesn't currently :( Tom ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] pve-firewall : can't have log on drop/reject rules
Am Mittwoch, den 21.11.2018, 18:40 +0100 schrieb Alexandre DERUMIER: > Hi, > > I'm not sure it was working before, > > but I can't get any log for a vm rule with a drop/reject. > > It's only works with default vm drop/reject action. Yes. > I found an old patch about adding log by rules > https://pve.proxmox.com/pipermail/pve-devel/2017-September/028816.htm > l > > But I don't see anywhere how to define it in rules. > > any idea ? Yes, this was a preparing patch towards the features you're looking for. Unfortunately real life work caught me up all year, so I couldn't spent more time on this. This isn't implemented (though still somewhere on my todo). Tom ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] Request for improvement of Network handling regarding LXC
Hi there,
i'm currently evaluating the PVE environment as a replacement for my
custom KVM+LXC+DRBD setup I'm running so far.
Playing with (privileged) containers I figured that IP configuration is
always done from inside the container.
My usual setup is setting the (static) IP of the container from the
outside (and applying firewall rules) and dropping capabilities for the
container itself so this can't be changed from inside the container.
Currently this seems to be impossible with PVE as it comes.
Attached is a little patch that sets the IP from the 'outside' (if
defined as a static one). Once I manually add the lxc.cap.drop lines to
the CT config, I can't change this from the inside anymore.
It's only for IPv4 (can't test v6 on this setup) but I think it's
rather trivial to add this.
Unless you drop net_admin the CT will still be able to change
networking and behave like before - or work with DHCP.
Regards,
Tom
--- /usr/share/perl5/PVE/LXC.pm.orig2017-07-20 12:03:52.949344829 +0200
+++ /usr/share/perl5/PVE/LXC.pm 2017-07-20 14:12:09.022119871 +0200
@@ -428,6 +428,11 @@
$raw .= "lxc.network.type = veth\n";
$raw .= "lxc.network.veth.pair = veth${vmid}i${ind}\n";
$raw .= "lxc.network.hwaddr = $d->{hwaddr}\n" if defined($d->{hwaddr});
+ if (defined($d->{ip}) and ($d->{ip} ne "dhcp")) {
+ $raw .= "lxc.network.ipv4 = $d->{ip}\n";
+ $raw .= "lxc.network.ipv4.gateway = $d->{gw}\n" if
defined($d->{gw});
+ $raw .= "lxc.network.flags = up\n" if defined($d->{ip});
+ }
$raw .= "lxc.network.name = $d->{name}\n" if defined($d->{name});
$raw .= "lxc.network.mtu = $d->{mtu}\n" if defined($d->{mtu});
}
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Request for improvement of Network handling regarding LXC
Am Donnerstag, den 20.07.2017, 13:31 +0200 schrieb Michael Rasmussen:
> On Thu, 20 Jul 2017 13:22:58 +0200
> Tom Weber wrote:
>
> >
> > + if (defined($d->{ip}) and ($d->{ip} ne "dhcp")) {
> > + $raw .= "lxc.network.ipv4 = $d->{ip}\n";
> > + $raw .= "lxc.network.ipv4.gateway = $d->{gw}\n" if
> > defined($d->{gw});
> > + $raw .= "lxc.network.flags = up\n" if defined($d-
> > >{ip});
> The if defined($d->{ip}) in this line is not needed since this is
> already true when entering this condition.
Indeed.
restover from the first version which didn't have the bigger if block
with dhcp checking.
thanks,
Tom
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Request for improvement of Network handling regarding LXC
Am Donnerstag, den 20.07.2017, 15:00 +0200 schrieb Wolfgang Bumiller: > On Thu, Jul 20, 2017 at 01:22:58PM +0200, Tom Weber wrote: > > > > Hi there, > > > > i'm currently evaluating the PVE environment as a replacement for > > my > > custom KVM+LXC+DRBD setup I'm running so far. > > > > Playing with (privileged) containers I figured that IP > > configuration is > > always done from inside the container. > > > > My usual setup is setting the (static) IP of the container from the > > outside (and applying firewall rules) and dropping capabilities for > > the > > container itself so this can't be changed from inside the > > container. > > > > Currently this seems to be impossible with PVE as it comes. > > > > Attached is a little patch that sets the IP from the 'outside' (if > > defined as a static one). Once I manually add the lxc.cap.drop > > lines to > > the CT config, I can't change this from the inside anymore. > > > > It's only for IPv4 (can't test v6 on this setup) but I think it's > > rather trivial to add this. > > > > Unless you drop net_admin the CT will still be able to change > > networking and behave like before - or work with DHCP. > No objection to adding this as a separate option. > > There's still the idea of adding feature flags to containers floating > around (initially for allowing things like fuse or mounting of > network > shares (nfs, cifs)), and this would definitely be another useful flag > to add. > > Note that dropping net_admin also prevents containers from > configuring > their inner firewall or using tunnels/vpns/etc., so it would > definitely > need to be a separate option rather than a general change of behavior > like in this patch, but you probably know that. As far as I can see this patch alone wouldn't change the normal behavior: step 1) lxc sets the IP from outside step 2) container itself sets/overrides the IP from the inside. Only if I manually add lxc.cap.drop = net_admin to the config of the container it will prevent step 2. Preventing the container from messing with networking/firewall settings is exactly why I want/need this. A feature switch for this (maybe even in the UI would be nice too) but thats far beyond my 2 days knowledge of playing with pve :) Tom ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] debian 9.1 LXC unsupported
With debian bumping it's stretch Version Number to 9.1, the check in
/usr/share/perl5/PVE/LXC/Setup/Debian.pm
fails.
# lxc-start -F -n 14433
unsupported debian version '9.1'
...
I fixed it for me like this:
--- /usr/share/perl5/PVE/LXC/Setup/Debian.pm.orig 2017-07-24
11:25:37.601390691 +0200
+++ /usr/share/perl5/PVE/LXC/Setup/Debian.pm2017-07-24 11:25:59.305206733
+0200
@@ -28,7 +28,7 @@
$version = $1;
die "unsupported debian version '$version'\n"
- if !($version >= 4 && $version <= 9);
+ if !($version >= 4 && $version < 10);
my $self = { conf => $conf, rootdir => $rootdir, version => $version };
Tom
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH Storage] LVM Striping
Am Mittwoch, den 26.07.2017, 20:44 +0200 schrieb Martin Lablans: > Dear all, > > this patch will change the LVM storage plugin to create striped > rather > than linear logical volumes, which can multiply the throughput for > volume groups backed by several controllers or network paths. > > The number of stripes is set equal to the number of physical volumes > in > the volume group (which is determined using vgs). If this number is > 1 > (one), linear volumes are used as before. You're making an assumption that wouldn't hold on most of my machines (event though they're not converted to pve yet). - I often have SSD and HDD based physical volumes in a volume group, so I can decide which LV I put on which PV - speed vs space - Huge drives - esp. HDD based ones, I usually partition to get smaller PVs (each partition a PV) for 2 reasons: 1) the PVs at the beginning of the disk are usually faster than the ones at the end - this makes it easier to place a LV 2) Handling of PVs (think about pvmove) is easier. This might or might not be of interest for the regular pve user (and I guess I'll end up hand-optimizing for my setup anyway when using pve ;) ) but I don't think automatic striping by default, just based on the number of PVs is a good idea. Regards, Tom ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH Storage] Newly created LVM volumes are now created with a number of stripes equal to the number of physical volumes in a volume group.
Am Sonntag, den 30.07.2017, 10:52 +0200 schrieb Martin Lablans: > Of course it would be preferable to leave this in the admin's hand > via > system-wide LVM configuration. This would also give Tom the > flexibility > for his setup. However, I don't know a way to achieve striping in > LVM > without using the -i option. raid_stripe_all_devices=1 does not > achieve > that result, cf. https://serverfault.com/questions/849088 > > If someone knows how to configure striping in LVM, then I'll be happy > to > write a few lines for the Proxmox documentation or wiki. If not, > using > -i seems to be the only way to achieve striping. > > Basically, we are discussing to change the current behaviour from > "use 1 > stripe" to "use stripes". The latter option seems to > make a lot more sense to me, although I can see that there are > special > setups requiring exactly one stripe. I think huge (and affordable) Server SSDs and approaches like ceph etc. will move storage back from external solutions into the local servers again (hyper converged servers). And that will make local storage setups way more complex and maybe the rule (and my setups not so special anymore ;-) ). > I see three options: > 1) We find a way to configure that using LVM; I'll contribute > documentation > 2) We add a "-stripes" option to pct create, with options > 0 ==> old behaviour (no -i) > n>0 ==> n stripes > auto==> number of pvs > 3) Like 2), but define it as an option of the PVE storage > (/etc/pve/storage.cfg) I don't know what the design goals are for pve (still new to it) but I personally would prefer to have such an option well documented (with or without UI) over a (silent) default for the whole lvm setup. So i'd prefer 2) (maybe 3) but that will be difficult in complex setups). Regards, Tom ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] better firewall logging possible?
Hi there, today I had to figure the hard way that the Firewall Option 'IP filter' (at least in PVE 5.0 for Containers) drops packets silently without any logging at all, even if the log_level_* is set. If I set the log_level, I'd expect to see _all_ dropped packets in the Log. (This gave me a hell of a time today with a DHCP Server inside a container). Regards, Tom ps: is this the right place for such a feature request? ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] better firewall logging possible?
Hi,
thanks for the quick reply.
it doesn't seem to be that easy though.
this one compiles:
--- Firewall.pm.ORIG2017-09-06 11:27:00.158674622 +0200
+++ Firewall.pm 2017-09-06 11:39:07.801620128 +0200
@@ -2119,8 +2119,13 @@
if ($ipversion == 6 && !$options->{radv}) {
ruleset_addrule($ruleset, $chain, '-p icmpv6 --icmpv6-type
router-advertisement -j DROP');
}
+# XXX
if ($ipfilter_ipset) {
ruleset_addrule($ruleset, $chain, "-m set ! --match-set
$ipfilter_ipset src -j DROP");
+ my $lc_direction = lc($direction);
+ my $loglevel = get_option_log_level($options,
"log_level_${lc_direction}");
+ #ruleset_addlog($ruleset, $chain, $vmid, "policy $policy: ",
$loglevel);
+ ruleset_addlog($ruleset, $chain, "FIXME-vmid", "policy:
FIXME-policy", $loglevel);
}
ruleset_addrule($ruleset, $chain, "-j MARK --set-mark
$FWACCEPTMARK_OFF"); # clear mark
}
- $vmid and $policy are not (yet) available in ruleset_create_vm_chain
- $direction is always OUT in this block
- It doesn't produce anything in the log (doesn't it stop processing
after the DROP?)
this is the generated chain:
Chain veth144010i2-OUT (1 references)
pkts bytes target prot opt in out source destination
0 0 DROP all -- anyany anywhere anywhere
! match-set PVEFW-1DB4EE2A src
0 0 NFLOG all -- anyany anywhere anywhere
nflog-prefix ":FIXME-vmid:4:veth144010i2-OUT: policy: FIXME-policy"
0 0 MARK all -- anyany anywhere anywhere
MARK and 0x7fff
0 0 GROUP-dns-server-OUT all -- anyany anywhere
anywhere
0 0 RETURN all -- anyany anywhere anywhere
mark match 0x8000/0x8000
0 0 GROUP-default-guests-OUT all -- anyany anywhere
anywhere
0 0 RETURN all -- anyany anywhere anywhere
mark match 0x8000/0x8000
0 0 GROUP-dhcp-server-OUT all -- anyany anywhere
anywhere
0 0 RETURN all -- anyany anywhere anywhere
mark match 0x8000/0x8000
0 0 GROUP-mail-sender-OUT all -- anyany anywhere
anywhere
0 0 RETURN all -- anyany anywhere anywhere
mark match 0x8000/0x8000
0 0 PVEFW-SET-ACCEPT-MARK udp -- anyany anywhere
anywhere[goto] udp dpt:discard
0 0 PVEFW-Reject all -- anyany anywhere anywhere
0 0 NFLOG all -- anyany anywhere anywhere
nflog-prefix ":144010:4:veth144010i2-OUT: policy REJECT: "
0 0 PVEFW-reject all -- anyany anywhere anywhere
[goto]
0 0all -- anyany anywhere anywhere
/* PVESIG:j/bfq13umaUL2xMG35f2FLY+hPU */
Tom
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] better firewall logging possible?
Hi,
this patch compiles, but it won't work.
it still DROPs without logging.
Now it logs the packets that don't get dropped.
The first DROP stops the evaluation of the chain. Everything else gets
logged.
Chain veth144010i2-OUT (1 references)
pkts bytes target prot opt in out source destination
0 0 DROP all -- anyany anywhere anywhere
! match-set PVEFW-1DB4EE2A src
0 0 NFLOG all -- anyany anywhere anywhere
nflog-prefix ":144010:4:veth144010i2-OUT: policyXXX: DROP"
changing the order will make it log everything.
I think the blacklist Option does it right:
sub ruleset_chain_add_input_filters {
my ($ruleset, $chain, $ipversion, $options, $cluster_conf, $loglevel) = @_;
if ($cluster_conf->{ipset}->{blacklist}){
if (!ruleset_chain_exist($ruleset, "PVEFW-blacklist")) {
ruleset_create_chain($ruleset, "PVEFW-blacklist");
ruleset_addlog($ruleset, "PVEFW-blacklist", 0, "DROP: ", $loglevel)
if $loglevel;
ruleset_addrule($ruleset, "PVEFW-blacklist", "-j DROP");
}
my $ipset_chain = compute_ipset_chain_name(0, 'blacklist', $ipversion);
ruleset_addrule($ruleset, $chain, "-m set --match-set ${ipset_chain}
src -j PVEFW-blacklist");
}
Unfortunately I'm too short on time right now to create a proper patch
myself.
this is the non working version so far:
--- Firewall.pm.ORIG2017-09-06 11:27:00.158674622 +0200
+++ Firewall.pm 2017-09-06 15:51:27.850452259 +0200
@@ -2081,8 +2081,9 @@
}
}
+#XXX
sub ruleset_create_vm_chain {
-my ($ruleset, $chain, $ipversion, $options, $macaddr,
$ipfilter_ipset, $direction) = @_;
+my ($ruleset, $chain, $ipversion, $options, $macaddr,
$ipfilter_ipset, $direction, $vmid) = @_;
ruleset_create_chain($ruleset, $chain);
my $accept = generate_nfqueue($options);
@@ -2119,8 +2120,11 @@
if ($ipversion == 6 && !$options->{radv}) {
ruleset_addrule($ruleset, $chain, '-p icmpv6 --icmpv6-type
router-advertisement -j DROP');
}
+# XXX
if ($ipfilter_ipset) {
ruleset_addrule($ruleset, $chain, "-m set ! --match-set
$ipfilter_ipset src -j DROP");
+ my $loglevel = get_option_log_level($options,
"log_level_out");
+ ruleset_addlog($ruleset, $chain, $vmid, "policyXXX: DROP",
$loglevel);
}
ruleset_addrule($ruleset, $chain, "-j MARK --set-mark
$FWACCEPTMARK_OFF"); # clear mark
}
@@ -2232,7 +2236,8 @@
if $options->{ipfilter} || $vmfw_conf->{ipset}-
>{$ipfilter_name};
# create chain with mac and ip filter
-ruleset_create_vm_chain($ruleset, $tapchain, $ipversion, $options,
$macaddr, $ipfilter_ipset, $direction);
+#XXX
+ruleset_create_vm_chain($ruleset, $tapchain, $ipversion, $options,
$macaddr, $ipfilter_ipset, $direction, $vmid);
if ($options->{enable}) {
ruleset_generate_vm_rules($ruleset, $rules, $cluster_conf,
$vmfw_conf, $tapchain, $netid, $direction, $options, $ipversion);
Am Mittwoch, den 06.09.2017, 13:39 +0200 schrieb Alexandre DERUMIER:
> >
> > >
> > > - $vmid and $policy are not (yet) available in
> > > ruleset_create_vm_chain
> for $vmid,
> --
> edit
>
> # create chain with mac and ip filter
> ruleset_create_vm_chain($ruleset, $tapchain, $ipversion,
> $options, $macaddr, $ipfilter_ipset, $direction);
>
> ->
>
> # create chain with mac and ip filter
> ruleset_create_vm_chain($ruleset, $tapchain, $ipversion,
> $options, $macaddr, $ipfilter_ipset, $direction, $vmid);
>
>
>
>
> then
>
> sub ruleset_create_vm_chain {
> my ($ruleset, $chain, $ipversion, $options, $macaddr,
> $ipfilter_ipset, $direction) = @_;
>
>
> ->
>
> sub ruleset_create_vm_chain {
> my ($ruleset, $chain, $ipversion, $options, $macaddr,
> $ipfilter_ipset, $direction, $vmid) = @_;
>
>
>
>
>
> for
> "policy $policy: "
> ---
> it's just a string, keep it empty for now, or "policy DROP"
>
>
>
>
> >
> > >
> > > - $direction is always OUT in this block
> yes, but it need to be use lowercase for the rule
>
> I think this should be ok
> + my $lc_direction = lc($direction);
> + my $loglevel = get_option_log_level($options,
> "log_level_${lc_direction}");
>
>
> or
>
> my $loglevel = "log_level_out";
>
>
> >
> > >
> > > - It doesn't produce anything in the log (doesn't it stop
> > > processing
> > > after the DROP?)
> in others rules it's added after the DROP.
> try to add it before to test. (but I think that the missing $vmid
> was the problem, to log in correct logfile)
>
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] better firewall logging possible?
Attached patch works for me regarding and tested with ipfilter Option.
I also added logging for the 2 other silent DROPs above - untested
though.
Maybe someone could verify and even commit (no git repository for pve
over here - yet)
Tom
Am Mittwoch, den 06.09.2017, 16:24 +0200 schrieb Tom Weber:
>
>
> Hi,
>
> this patch compiles, but it won't work.
>
> it still DROPs without logging.
>
> Now it logs the packets that don't get dropped.
> The first DROP stops the evaluation of the chain. Everything else
> gets
> logged.
>
> Chain veth144010i2-OUT (1 references)
> pkts bytes target prot opt
> in out source destination
> 0 0 DROP all
> -- anyany anywhere anywhere ! match-
> set PVEFW-1DB4EE2A src
> 0 0 NFLOG all
> -- anyany anywhere anywhere nflog-
> prefix ":144010:4:veth144010i2-OUT: policyXXX: DROP"
>
> changing the order will make it log everything.
>
> I think the blacklist Option does it right:
>
> sub ruleset_chain_add_input_filters {
> my ($ruleset, $chain, $ipversion, $options, $cluster_conf,
> $loglevel) = @_;
>
> if ($cluster_conf->{ipset}->{blacklist}){
> if (!ruleset_chain_exist($ruleset, "PVEFW-blacklist")) {
> ruleset_create_chain($ruleset, "PVEFW-blacklist");
> ruleset_addlog($ruleset, "PVEFW-blacklist", 0, "DROP: ",
> $loglevel) if $loglevel;
> ruleset_addrule($ruleset, "PVEFW-blacklist", "-j DROP");
> }
> my $ipset_chain = compute_ipset_chain_name(0, 'blacklist',
> $ipversion);
> ruleset_addrule($ruleset, $chain, "-m set --match-set
> ${ipset_chain} src -j PVEFW-blacklist");
> }
[...]
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] add log for ipfilter, macfilter && ipv6 router-advertisement
Hi Alexandre,
i can test it later, thanks. 2 comments though.
Am Donnerstag, den 07.09.2017, 03:22 +0200 schrieb Alexandre Derumier:
> +my ($ruleset, $chain, $ipversion, $options, $macaddr,
> $ipfilter_ipset, $direction, $vmid) = @_;
> +
> +my $lc_direction = lc($direction);
> +my $loglevel = get_option_log_level($options,
> "log_level_${lc_direction}");
in this function we're only logging for outgoing. it's always
log_level_out if we need it.
> - ruleset_addrule($ruleset, $chain, "-m mac ! --mac-
> source
> $macaddr -j DROP");
> + my $rule = "-m mac ! --mac-source $macaddr";
> + ruleset_addlog($ruleset, $chain, $vmid, "policy DROP: ",
> $loglevel, $rule);
> + ruleset_addrule($ruleset, $chain, "$rule -j DROP");
you are aware that $rule is used elsewhere and in a totally different
way? just look in ruleset_add_group_rule. Thats why I named it
$matchrule initially to avoid confusion.
Tom
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] add log for ipfilter, macfilter && ipv6 router-advertisement
Am Donnerstag, den 07.09.2017, 09:30 +0200 schrieb Alexandre DERUMIER:
> >
> > >
> > > you are aware that $rule is used elsewhere and in a totally
> > > different
> > > way? just look in ruleset_add_group_rule. Thats why I named it
> > > $matchrule initially to avoid confusion.
> we already used it like this in:
>
>
>
> sub ruleset_addlog {
> ...
> $logrule = "$rule $logrule" if defined($rule);
> }
>
> so I think it's fine.
well.. do a
$ grep '$rule->' Firewall.pm
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH] Firewall Improvements
See mail Firewall Improvements Tom Weber (1): prepare code for more generic firewall logging src/PVE/Firewall.pm | 168 +++- 1 file changed, 99 insertions(+), 69 deletions(-) ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] Firewall Improvements
Hi all, last week I reported a problem with firewall logging. After looking deeper into Firewall.pm I have a better understanding of the problems I first had with using the Firewall as a rather fresh PVE User: - the different levels of log_level_in / out don't make sense to me. Firewall.pm uses them as boolean to decide wether to log or not. The level is is never ever used (which i'd expect from the UI and the docs). Or did I miss something? - I don't see a clear consistent way on how packets are getting logged (and in what direction). From a user point of view i was expecting more logging with increasing log levels up to logging accepted packets for a loglevel of debug. Now I know that this asumption is wrong. - Macros can't be edited or extended since they are hardwired in the code. - Same for the Standard Set of Rules I was thinking on how to improve the situation. My first thought was to give the log_levels a better meaning (like start logging drops at a certain log_level and accepts a the highest loglevel). But I think, with lots of chains used by all guests, this is no good approach. IMHO a flag to toggle logging for a rule or security group in general would be the most usefull thing to have. This would even make it possible to log accepted packages. I'd also like the Idea of having the macro definitions outside the code, where an admin can change them (same for the standard set of rules). But before I start putting too much time into this I'd like to ask for comments and if there is interest at all :-). I'll also send a first patch that makes all (except for one place) iptables rule generation in Firewall.pm differenciate between the match and the action part of the rule, which gives us one central place to generate logging rules. This is meant as an interim patch on the way to the final goal - so some of the code probably looks clumsy - but the goal was to generate exactly the same rules as before - which appears to work on my setups (comparing pve-firewall compile outputs) except for a few additional spaces sometimes. And since this is my first patch here, I'd appreciate feedback for the patch in any case :) Tom ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH] prepare code for more generic firewall logging
making ruleset generation aware of a match and action
part in iptable rules.
code will generate the same iptables as before! (except for
a few additional spaces between match and action).
---
src/PVE/Firewall.pm | 168 +++-
1 file changed, 99 insertions(+), 69 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index cc81325..61f07e0 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1648,8 +1648,6 @@ sub enable_bridge_firewall {
$bridge_firewall_enabled = 1;
}
-my $rule_format = "%-15s %-30s %-30s %-15s %-15s %-15s\n";
-
sub iptables_restore_cmdlist {
my ($cmdlist) = @_;
@@ -1778,7 +1776,7 @@ sub ipset_get_chains {
return $res;
}
-sub ruleset_generate_cmdstr {
+sub ruleset_generate_match {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
return if defined($rule->{enable}) && !$rule->{enable};
@@ -1909,6 +1907,14 @@ sub ruleset_generate_cmdstr {
push @cmd, "-m addrtype --dst-type $rule->{dsttype}" if $rule->{dsttype};
+return scalar(@cmd) ? join(' ', @cmd) : undef;
+}
+
+sub ruleset_generate_action {
+my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
+
+my @cmd = ();
+
if (my $action = $rule->{action}) {
$action = $actions->{$action} if defined($actions->{$action});
$goto = 1 if !defined($goto) && $action eq 'PVEFW-SET-ACCEPT-MARK';
@@ -1918,6 +1924,17 @@ sub ruleset_generate_cmdstr {
return scalar(@cmd) ? join(' ', @cmd) : undef;
}
+sub ruleset_generate_cmdstr {
+my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
+my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
+my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
+
+return undef if !(defined($match) or defined($action));
+my $ret = defined($match) ? $match : "";
+$ret = "$ret $action" if defined($action);
+return $ret;
+}
+
sub ruleset_generate_rule {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
@@ -1931,15 +1948,19 @@ sub ruleset_generate_rule {
# update all or nothing
-my @cmds = ();
+my @mstrs = ();
+my @astrs = ();
foreach my $tmp (@$rules) {
- if (my $cmdstr = ruleset_generate_cmdstr($ruleset, $chain, $ipversion,
$tmp, $actions, $goto, $cluster_conf, $fw_conf)) {
- push @cmds, $cmdstr;
+ my $m = ruleset_generate_match($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
+ my $a = ruleset_generate_action($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
+ if (defined $m or defined $a) {
+ push @mstrs, defined($m) ? $m : "";
+ push @astrs, defined($a) ? $a : "";
}
}
-foreach my $cmdstr (@cmds) {
- ruleset_addrule($ruleset, $chain, $cmdstr);
+for my $i (0 .. $#mstrs) {
+ ruleset_addrule($ruleset, $chain, $mstrs[$i], $astrs[$i]);
}
}
@@ -1948,8 +1969,10 @@ sub ruleset_generate_rule_insert {
die "implement me" if $rule->{macro}; # not implemented, because not
needed so far
-if (my $cmdstr = ruleset_generate_cmdstr($ruleset, $chain, $ipversion,
$rule, $actions, $goto)) {
- ruleset_insertrule($ruleset, $chain, $cmdstr);
+my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
+my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
+if (defined $match && defined $action) {
+ ruleset_insertrule($ruleset, $chain, $match, $action);
}
}
@@ -1970,7 +1993,7 @@ sub ruleset_chain_exist {
return $ruleset->{$chain} ? 1 : undef;
}
-sub ruleset_addrule {
+sub ruleset_addrule_old {
my ($ruleset, $chain, $rule) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
@@ -1978,12 +2001,20 @@ sub ruleset_addrule {
push @{$ruleset->{$chain}}, "-A $chain $rule";
}
+sub ruleset_addrule {
+ my ($ruleset, $chain, $match, $action, $log) = @_;
+
+ die "no such chain '$chain'\n" if !$ruleset->{$chain};
+
+ push @{$ruleset->{$chain}}, "-A $chain $match $action";
+}
+
sub ruleset_insertrule {
- my ($ruleset, $chain, $rule) = @_;
+ my ($ruleset, $chain, $match, $action, $log) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
- unshift @{$ruleset->{$chain}}, "-A $chain $rule";
+ unshift @{$ruleset->{$chain}}, "-A $chain $match $action";
}
sub get_log_rule_base {
@@ -2000,15 +2031,14 @@ sub get_log_rule_base {
}
sub ruleset_addlog {
-my ($ruleset, $chain, $vmid, $msg, $loglevel, $rule) = @_;
+my ($ruleset, $chain, $vmid, $msg, $loglevel, $match) = @_;
return if !defined($loglevel);
-my $logrule = get_log_rule_ba
Re: [pve-devel] [PATCH] prepare code for more generic firewall logging
Am Montag, den 18.09.2017, 12:21 +0200 schrieb Wolfgang Bumiller:
> Improving logging makes sense, the current state might be confuse for
> some (given that drop-rules simply generate a `-j DROP` iptables
> rules
> and therefore don't get logged).
> This seems to be a good first step, although I'd be much happier if
> iptables would allow setting the log-prefix and performing the log
> action separately, then we could simply introduce a log-drop chain
> instead.
well, once we have one single place that actually creates the rules, it
should be way easier to implement there.
My intention is to get away from hardwired "rulegeneration" which we
have in the code on several places to separate ruledefinitions and code
that only generates the rules from that definitions.
> I'm assuming your intention is to be able to duplicate the matching
> part
> of a rule so that you can first add it with `-j NFLOG` and afterwards
> its `-j DROP` action (or whatever action was requested). In this
> case,
> also note that with groups the actions may not be executed
> immediately
> and instead set a mark and return out of the current chain.
yes thats the idea.
And I'm aware of the marks.
> With that in mind, I have no objections to this patch (or a version
> of
> it, see the inline comments below).
>
> But first things first: please read https://pve.proxmox.com/wiki/Deve
> loper_Documentation
> for details about patches and CLA (which is required for us to apply
> external patches).
Martin already has the signed CLA.
> Also, the spaces in your patch have been replaced by non-breaking-
> space
> characters, causing git-am to fail on it. You should check your
> mailer
> settings to avoid this.
if you don't mind i'll send the next version to you directly just to
let you verify the cosmetic things before cluttering the list again :)
> On Thu, Sep 14, 2017 at 07:08:54PM +0200, Tom Weber wrote:
> >
> > making ruleset generation aware of a match and action
> > part in iptable rules.
> > code will generate the same iptables as before! (except for
> > a few additional spaces between match and action).
> Note that these spaces are currently not accepted by the testcases
> and
> requires:
> -$rule =~ s/^-A $chain // || die "got strange rule: $rule";
> +$rule =~ s/^-A $chain +// || die "got strange rule: $rule";
> in FirewallSimulator.pm's rule_match()
>
> Please use `make check` in the future to check your changes ;-)
To be honest, I didn't catch what the use of FirewallSimumlator
actually was/is :-)
It's not that there's much comments or dokumentation at all.
> > ---
> > src/PVE/Firewall.pm | 168 +++-
> >
> > 1 file changed, 99 insertions(+), 69 deletions(-)
> >
> > diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> > index cc81325..61f07e0 100644
> > --- a/src/PVE/Firewall.pm
> > +++ b/src/PVE/Firewall.pm
> > @@ -1648,8 +1648,6 @@ sub enable_bridge_firewall {
> > $bridge_firewall_enabled = 1;
> > }
> >
> > -my $rule_format = "%-15s %-30s %-30s %-15s %-15s %-15s\n";
> > -
> Removes an unrelated unused variable. Such cleanups are preferred as
> separate patches.
ok.
> > -my @cmds = ();
> > +my @mstrs = ();
> > +my @astrs = ();
> > foreach my $tmp (@$rules) {
> > - if (my $cmdstr = ruleset_generate_cmdstr($ruleset, $chain,
> > $ipversion, $tmp, $actions, $goto, $cluster_conf, $fw_conf)) {
> > - push @cmds, $cmdstr;
> > + my $m = ruleset_generate_match($ruleset, $chain,
> > $ipversion, $tmp, $actions, $goto, $cluster_conf, $fw_conf);
> > + my $a = ruleset_generate_action($ruleset, $chain,
> > $ipversion, $tmp, $actions, $goto, $cluster_conf, $fw_conf);
> > + if (defined $m or defined $a) {
> > + push @mstrs, defined($m) ? $m : "";
> > + push @astrs, defined($a) ? $a : "";
> While this is all part of a small chunk of code, I'd prefer a single
> array containing pairs of [$match, $action] as elements, rather than
> worrying about future changes possibly bringing @mstrs and @astrs out
> of
> sync.
agreed. I consider this ugly too, but this is code that will probably
be replaced in further steps.
The intention of this patch was to only separate match and action and
show where i'm heading to with as few changes as possible to the actual
logic.
> > -sub ruleset_addrule {
> > +sub ruleset_addrule_old {
> The name suggests that you plan on removing this later on. If this is
yes. Only one place in the code where I couldn't easily split action
from match, hence I had to keep it. Will also be eliminated in a later
step.
thanks for your feedback!
Tom
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] prepare code for more generic firewall logging
Am Montag, den 18.09.2017, 13:34 +0200 schrieb Dietmar Maurer: > > > > With that in mind, I have no objections to this patch (or a version > > of > > it, see the inline comments below). > But logging all Dropped package would produce an incredible amount of > logs? That's why I'd like to have a flag to turn logging on/off for every single rule, security group, etc.. see my other mail. Tom ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH container] Fix restore with multiple mountpoints
If you use mountpoints inside a container, and change ownership of these, a restore of the CT will reset them to root:root again. That's because the whole CT FS gets prepared and mounted together with (of course) an owner of root:root for the mp. As tar uses the option --skip-old-files it won't touch these dirs then. I don't see why one would need --skip-old-files for a restore job (or did I miss something?) Tom Weber (1): remove --skip-old-files from tar restore options src/PVE/LXC/Create.pm | 5 - 1 file changed, 5 deletions(-) -- 2.7.4 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH container] remove --skip-old-files from tar restore options
this breaks ownership of mountpoints in containers
(leaves them at root:root)
---
src/PVE/LXC/Create.pm | 5 -
1 file changed, 5 deletions(-)
diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
index 1f21e06..ac632de 100644
--- a/src/PVE/LXC/Create.pm
+++ b/src/PVE/LXC/Create.pm
@@ -77,11 +77,6 @@ sub restore_archive {
@PVE::Storage::Plugin::COMMON_TAR_FLAGS,
'-C', $rootdir];
-# skip-old-files doesn't have anything to do with time (old/new), but is
-# simply -k (annoyingly also called --keep-old-files) without the 'treat
-# existing files as errors' part... iow. it's bsdtar's interpretation of -k
-# *sigh*, gnu...
-push @$cmd, '--skip-old-files';
push @$cmd, '--anchored';
push @$cmd, '--exclude' , './dev/*';
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 firewall 4/4] convert string based rule definitions to hashes
---
src/PVE/Firewall.pm | 220
1 file changed, 117 insertions(+), 103 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index f8a9300..179617a 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -142,6 +142,20 @@ my $log_level_hash = {
emerg => 0,
};
+# %rule
+#
+# name => optional
+# action =>
+# proto =>
+# sport =>
+# dport =>
+# log => optional, loglevel
+# logmsg => optional, logmsg - overwrites default
+# iface_in
+# iface_out
+# match => optional, overwrites generation of match
+# target => optional, overwrites action
+
# we need to overwrite some macros for ipv6
my $pve_ipv6fw_macros = {
'Ping' => [
@@ -536,7 +550,7 @@ my $FWACCEPTMARK_OFF = "0x/0x8000";
my $pve_std_chains = {};
$pve_std_chains->{4} = {
'PVEFW-SET-ACCEPT-MARK' => [
- "-j MARK --set-mark $FWACCEPTMARK_ON",
+ { target => "-j MARK --set-mark $FWACCEPTMARK_ON" },
],
'PVEFW-DropBroadcast' => [
# same as shorewall 'Broadcast'
@@ -552,10 +566,10 @@ $pve_std_chains->{4} = {
{ action => 'DROP', dsttype => 'BROADCAST' },
{ action => 'DROP', source => '224.0.0.0/4' },
{ action => 'DROP', proto => 'icmp' },
- "-p tcp -j REJECT --reject-with tcp-reset",
- "-p udp -j REJECT --reject-with icmp-port-unreachable",
- "-p icmp -j REJECT --reject-with icmp-host-unreachable",
- "-j REJECT --reject-with icmp-host-prohibited",
+ { match => '-p tcp', target => '-j REJECT --reject-with tcp-reset' },
+ { match => '-p udp', target => '-j REJECT --reject-with
icmp-port-unreachable' },
+ { match => '-p icmp', target => '-j REJECT --reject-with
icmp-host-unreachable' },
+ { target => '-j REJECT --reject-with icmp-host-prohibited' },
],
'PVEFW-Drop' => [
# same as shorewall 'Drop', which is equal to DROP,
@@ -568,7 +582,7 @@ $pve_std_chains->{4} = {
{ action => 'ACCEPT', proto => 'icmp', dport => 'fragmentation-needed'
},
{ action => 'ACCEPT', proto => 'icmp', dport => 'time-exceeded' },
# Drop packets with INVALID state
- "-m conntrack --ctstate INVALID -j DROP",
+ { action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
{ action => 'DROP', proto => 'udp', dport => '135,445', nbdport => 2 },
{ action => 'DROP', proto => 'udp', dport => '137:139'},
@@ -576,7 +590,7 @@ $pve_std_chains->{4} = {
{ action => 'DROP', proto => 'tcp', dport => '135,139,445', nbdport =>
3 },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
- "-p tcp -m tcp ! --tcp-flags FIN,SYN,RST,ACK SYN -j DROP",
+ { action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
# Drop DNS replies
{ action => 'DROP', proto => 'udp', sport => 53 },
],
@@ -591,7 +605,7 @@ $pve_std_chains->{4} = {
{ action => 'ACCEPT', proto => 'icmp', dport => 'fragmentation-needed'
},
{ action => 'ACCEPT', proto => 'icmp', dport => 'time-exceeded' },
# Drop packets with INVALID state
- "-m conntrack --ctstate INVALID -j DROP",
+ { action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
{ action => 'PVEFW-reject', proto => 'udp', dport => '135,445', nbdport
=> 2 },
{ action => 'PVEFW-reject', proto => 'udp', dport => '137:139'},
@@ -599,111 +613,115 @@ $pve_std_chains->{4} = {
{ action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445',
nbdport => 3 },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
- "-p tcp -m tcp ! --tcp-flags FIN,SYN,RST,ACK SYN -j DROP",
+ { action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
# Drop DNS replies
{ action => 'DROP', proto => 'udp', sport => 53 },
],
'PVEFW-tcpflags' => [
# same as shorewall tcpflags action.
# Packets arriving on this interface are checked for som illegal
combinations of TCP flags
- "-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG FIN,PSH,URG -g
PVEFW-logflags",
- "-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG NONE -g
PVEFW-logflags",
- "-p tcp -m tcp --tcp-flags SYN,RST SYN,RST -g PVEFW-logflags",
- "-p tcp -m tcp --tcp-flags FIN,SYN FIN,SYN -g PVEFW-logflags",
- "-p tcp -m tcp --sport 0 --tcp-flags FIN,SYN,RST,ACK SYN -g
PVEFW-logflags",
+ { match => '-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG
FIN,PSH,URG', target => '-g PVEFW-logflags' },
+ { match => '-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG NONE',
target => '-g PVEFW-logflags' },
+ { match => '-p tcp -m tcp --tcp-flags SYN,RST SYN,RST', target => '-g
PVEFW-logflags' },
+ { match => '-p
[pve-devel] [PATCH v2 firewall 2/4] prepare code for more generic firewall logging
making ruleset generation aware of a match and action
part in iptable rules.
code will generate the same iptables as before! (except for
a few additional spaces between match and action).
---
src/PVE/Firewall.pm | 166 ++-
src/PVE/FirewallSimulator.pm | 2 +-
2 files changed, 100 insertions(+), 68 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 5d78686..f1aecef 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1776,7 +1776,7 @@ sub ipset_get_chains {
return $res;
}
-sub ruleset_generate_cmdstr {
+sub ruleset_generate_match {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
return if defined($rule->{enable}) && !$rule->{enable};
@@ -1907,6 +1907,14 @@ sub ruleset_generate_cmdstr {
push @cmd, "-m addrtype --dst-type $rule->{dsttype}" if $rule->{dsttype};
+return scalar(@cmd) ? join(' ', @cmd) : undef;
+}
+
+sub ruleset_generate_action {
+my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
+
+my @cmd = ();
+
if (my $action = $rule->{action}) {
$action = $actions->{$action} if defined($actions->{$action});
$goto = 1 if !defined($goto) && $action eq 'PVEFW-SET-ACCEPT-MARK';
@@ -1916,6 +1924,17 @@ sub ruleset_generate_cmdstr {
return scalar(@cmd) ? join(' ', @cmd) : undef;
}
+sub ruleset_generate_cmdstr {
+my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
+my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
+my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
+
+return undef if !(defined($match) or defined($action));
+my $ret = defined($match) ? $match : "";
+$ret = "$ret $action" if defined($action);
+return $ret;
+}
+
sub ruleset_generate_rule {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
@@ -1929,15 +1948,19 @@ sub ruleset_generate_rule {
# update all or nothing
-my @cmds = ();
+my @mstrs = ();
+my @astrs = ();
foreach my $tmp (@$rules) {
- if (my $cmdstr = ruleset_generate_cmdstr($ruleset, $chain, $ipversion,
$tmp, $actions, $goto, $cluster_conf, $fw_conf)) {
- push @cmds, $cmdstr;
+ my $m = ruleset_generate_match($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
+ my $a = ruleset_generate_action($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
+ if (defined $m or defined $a) {
+ push @mstrs, defined($m) ? $m : "";
+ push @astrs, defined($a) ? $a : "";
}
}
-foreach my $cmdstr (@cmds) {
- ruleset_addrule($ruleset, $chain, $cmdstr);
+for my $i (0 .. $#mstrs) {
+ ruleset_addrule($ruleset, $chain, $mstrs[$i], $astrs[$i]);
}
}
@@ -1946,8 +1969,10 @@ sub ruleset_generate_rule_insert {
die "implement me" if $rule->{macro}; # not implemented, because not
needed so far
-if (my $cmdstr = ruleset_generate_cmdstr($ruleset, $chain, $ipversion,
$rule, $actions, $goto)) {
- ruleset_insertrule($ruleset, $chain, $cmdstr);
+my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
+my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
+if (defined $match && defined $action) {
+ ruleset_insertrule($ruleset, $chain, $match, $action);
}
}
@@ -1968,7 +1993,7 @@ sub ruleset_chain_exist {
return $ruleset->{$chain} ? 1 : undef;
}
-sub ruleset_addrule {
+sub ruleset_addrule_old {
my ($ruleset, $chain, $rule) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
@@ -1976,12 +2001,20 @@ sub ruleset_addrule {
push @{$ruleset->{$chain}}, "-A $chain $rule";
}
+sub ruleset_addrule {
+ my ($ruleset, $chain, $match, $action, $log) = @_;
+
+ die "no such chain '$chain'\n" if !$ruleset->{$chain};
+
+ push @{$ruleset->{$chain}}, "-A $chain $match $action";
+}
+
sub ruleset_insertrule {
- my ($ruleset, $chain, $rule) = @_;
+ my ($ruleset, $chain, $match, $action, $log) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
- unshift @{$ruleset->{$chain}}, "-A $chain $rule";
+ unshift @{$ruleset->{$chain}}, "-A $chain $match $action";
}
sub get_log_rule_base {
@@ -1998,15 +2031,14 @@ sub get_log_rule_base {
}
sub ruleset_addlog {
-my ($ruleset, $chain, $vmid, $msg, $loglevel, $rule) = @_;
+my ($ruleset, $chain, $vmid, $msg, $loglevel, $match) = @_;
return if !defined($loglevel);
-my $logrule = get_log_rule_base($chain, $vmid, $msg, $loglevel);
-
-$logrule = "$rule $logrule" if defined($rule);
+my $logaction = get_log_rule_base($chain, $vmid, $msg, $loglevel);
-ru
[pve-devel] [PATCH v2 firewall 0/4] Firewall improvements
second version, far from finished but trying to reorganize things without breaking what exists. generates the same rules as before. feedback welcome. Tom Weber (4): remove unused $rule_format prepare code for more generic firewall logging integrate logging into ruleset_addrule convert string based rule definitions to hashes src/PVE/Firewall.pm | 387 +++ src/PVE/FirewallSimulator.pm | 2 +- 2 files changed, 210 insertions(+), 179 deletions(-) -- 2.7.4 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 firewall 3/4] integrate logging into ruleset_addrule
---
src/PVE/Firewall.pm | 33 ++---
1 file changed, 10 insertions(+), 23 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index f1aecef..f8a9300 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2002,10 +2002,14 @@ sub ruleset_addrule_old {
}
sub ruleset_addrule {
- my ($ruleset, $chain, $match, $action, $log) = @_;
+ my ($ruleset, $chain, $match, $action, $log, $logmsg, $vmid) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
+ if (defined($log) && $log) {
+ my $logaction = get_log_rule_base($chain, $vmid, $logmsg, $log);
+ push @{$ruleset->{$chain}}, "-A $chain $match $logaction";
+ }
push @{$ruleset->{$chain}}, "-A $chain $match $action";
}
@@ -2020,27 +2024,15 @@ sub ruleset_insertrule {
sub get_log_rule_base {
my ($chain, $vmid, $msg, $loglevel) = @_;
-die "internal error - no log level" if !defined($loglevel);
-
$vmid = 0 if !defined($vmid);
+$msg = "" if !defined($msg);
# Note: we use special format for prefix to pass further
-# info to log daemon (VMID, LOGVELEL and CHAIN)
+# info to log daemon (VMID, LOGLEVEL and CHAIN)
return "-j NFLOG --nflog-prefix \":$vmid:$loglevel:$chain: $msg\"";
}
-sub ruleset_addlog {
-my ($ruleset, $chain, $vmid, $msg, $loglevel, $match) = @_;
-
-return if !defined($loglevel);
-
-my $logaction = get_log_rule_base($chain, $vmid, $msg, $loglevel);
-
-$match = "" if !defined $match;
-ruleset_addrule($ruleset, $chain, $match, $logaction);
-}
-
sub ruleset_add_chain_policy {
my ($ruleset, $chain, $ipversion, $vmid, $policy, $loglevel,
$accept_action) = @_;
@@ -2053,15 +2045,11 @@ sub ruleset_add_chain_policy {
ruleset_addrule($ruleset, $chain, "", "-j PVEFW-Drop");
- ruleset_addlog($ruleset, $chain, $vmid, "policy $policy: ", $loglevel);
-
- ruleset_addrule($ruleset, $chain, "", "-j DROP");
+ ruleset_addrule($ruleset, $chain, "", "-j DROP", $loglevel, "policy
$policy: ", $vmid);
} elsif ($policy eq 'REJECT') {
ruleset_addrule($ruleset, $chain, "", "-j PVEFW-Reject");
- ruleset_addlog($ruleset, $chain, $vmid, "policy $policy: ", $loglevel);
-
- ruleset_addrule($ruleset, $chain, "", "-g PVEFW-reject");
+ ruleset_addrule($ruleset, $chain, "", "-g PVEFW-reject", $loglevel,
"policy: $policy", $vmid);
} else {
# should not happen
die "internal error: unknown policy '$policy'";
@@ -2093,8 +2081,7 @@ sub ruleset_chain_add_input_filters {
if ($cluster_conf->{ipset}->{blacklist}){
if (!ruleset_chain_exist($ruleset, "PVEFW-blacklist")) {
ruleset_create_chain($ruleset, "PVEFW-blacklist");
- ruleset_addlog($ruleset, "PVEFW-blacklist", 0, "DROP: ", $loglevel)
if $loglevel;
- ruleset_addrule($ruleset, "PVEFW-blacklist", "", "-j DROP");
+ ruleset_addrule($ruleset, "PVEFW-blacklist", "", "-j DROP",
$loglevel, "DROP: ");
}
my $ipset_chain = compute_ipset_chain_name(0, 'blacklist', $ipversion);
ruleset_addrule($ruleset, $chain, "-m set --match-set ${ipset_chain}
src", "-j PVEFW-blacklist");
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 firewall 1/4] remove unused $rule_format
---
src/PVE/Firewall.pm | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index cc81325..5d78686 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1648,8 +1648,6 @@ sub enable_bridge_firewall {
$bridge_firewall_enabled = 1;
}
-my $rule_format = "%-15s %-30s %-30s %-15s %-15s %-15s\n";
-
sub iptables_restore_cmdlist {
my ($cmdlist) = @_;
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 firewall 4/4] convert string based rule definitions to hashes
First of all, this all is work in progress. I thought I commit for
easier understanding of the way i'm heading - instead of one huge
commit which turns everything inside out. and for feedback of course.
My goal are defined structures for rules, chains, macros (which i think
are just arrays of "rule templates") etc and code which deals with
these structures instead of printing out iptables commands in various
places.
While doing this, keeping "input" and "output" identical to whats there
doesn't make this an easy task and therefore I sometimes use these
'temporary ugliness' approaches at places that I intend to replace
anyway.
Am Mittwoch, den 27.09.2017, 09:53 +0200 schrieb Wolfgang Bumiller:
> On Wed, Sep 27, 2017 at 12:02:33AM +0200, Tom Weber wrote:
> >
> > ---
> > src/PVE/Firewall.pm | 220 --
> > --
> > 1 file changed, 117 insertions(+), 103 deletions(-)
> >
> > diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> > index f8a9300..179617a 100644
> > --- a/src/PVE/Firewall.pm
> > +++ b/src/PVE/Firewall.pm
> > @@ -142,6 +142,20 @@ my $log_level_hash = {
> > emerg => 0,
> > };
> >
> > +# %rule
> > +#
> +1 for the documentation, but what happened to the right sides of
> most
> of these arrows? ;-) (yeah some of these options are being abused a
> bit...)
still in the progress of defining this :-).. Right now it's more like a
scratchpad where I collect what I encounter.
> > +# name => optional
> > +# action =>
> > +# proto =>
> > +# sport =>
> > +# dport =>
> > +# log => optional, loglevel
> Why not call it loglevel then? Also, with this being new it should be
> mentioned in the commit message.
this is for now and for backward compatibility. I'd prefer this as a
boolean, but I'm still carrying the loglevel around since it's used in
the frontend and the logformat for the pvefw-logger.
Do we really need the loglevel as it is or would a boolean be
sufficient? And how would we get rid of it?
> > +# logmsg => optional, logmsg - overwrites default
> And this.
> >
> > +# iface_in
> > +# iface_out
> > +# match => optional, overwrites generation of match
> > +# target => optional, overwrites action
> > +
> > # we need to overwrite some macros for ipv6
> > my $pve_ipv6fw_macros = {
> > 'Ping' => [
> > @@ -536,7 +550,7 @@ my $FWACCEPTMARK_OFF = "0x/0x8000";
>
>
> >
> > @@ -1948,19 +1970,24 @@ sub ruleset_generate_rule {
> >
> > # update all or nothing
> >
> > +# fixme: lots of temporary ugliness
> +1 for the 'temporary' in there ;-)
>
> >
> > my @mstrs = ();
> > my @astrs = ();
> > +my @logging = ();
> > +my @logmsg = ();
> > foreach my $tmp (@$rules) {
> > my $m = ruleset_generate_match($ruleset, $chain,
> > $ipversion, $tmp, $actions, $goto, $cluster_conf, $fw_conf);
> > my $a = ruleset_generate_action($ruleset, $chain,
> > $ipversion, $tmp, $actions, $goto, $cluster_conf, $fw_conf);
> > if (defined $m or defined $a) {
> > push @mstrs, defined($m) ? $m : "";
> > push @astrs, defined($a) ? $a : "";
> > + push @logging, $tmp->{log};
> > + push @logmsg, $tmp->{logmsg};
> > }
> > }
> >
> > for my $i (0 .. $#mstrs) {
> > - ruleset_addrule($ruleset, $chain, $mstrs[$i], $astrs[$i]);
> > + ruleset_addrule($ruleset, $chain, $mstrs[$i], $astrs[$i],
> > $logging[$i], $logmsg[$i]);
> > }
> > }
> >
> > @@ -1993,14 +2020,6 @@ sub ruleset_chain_exist {
> > return $ruleset->{$chain} ? 1 : undef;
> > }
> >
> > -sub ruleset_addrule_old {
> > - my ($ruleset, $chain, $rule) = @_;
> > -
> > - die "no such chain '$chain'\n" if !$ruleset->{$chain};
> > -
> > - push @{$ruleset->{$chain}}, "-A $chain $rule";
> > -}
> > -
> > sub ruleset_addrule {
> > my ($ruleset, $chain, $match, $action, $log, $logmsg, $vmid) =
> > @_;
> >
> > @@ -3127,26 +3146,21 @@ sub generate_std_chains {
> > my $std_chains = $pve_std_chains->{$ipversion} || die
> > "internal error";
> >
> > my $loglevel = get_option_log_level($options,
> > 'smurf_log_level');
> > -
> > -my $chain;
> > -
> > -if ($ipversion == 4) {
> > - # same as shorewall
Re: [pve-devel] [PATCH v2 firewall 4/4] convert string based rule definitions to hashes
Am Mittwoch, den 27.09.2017, 11:53 +0200 schrieb Wolfgang Bumiller:
> On Wed, Sep 27, 2017 at 12:02:33AM +0200, Tom Weber wrote:
> >
> > ---
> > +'PVEFW-smurflog' => [
> > + { action => 'DROP', logmsg => 'DROP: ' },
> > +],
> > +'PVEFW-logflags' => [
> > + { action => 'DROP', logmsg => 'DROP: ' },
> > ],
> > };
>
> Just noticed this is missing in the ipv6 part below it (my ip6tables
> aren't updating anymore).
yes, missed it. but only the PVEFW-logflags? smurflog has never been
part of ipv6.
Tom
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 firewall 4/4] convert string based rule definitions to hashes
Am Mittwoch, den 27.09.2017, 11:51 +0200 schrieb Wolfgang Bumiller:
> On Wed, Sep 27, 2017 at 11:09:29AM +0200, Tom Weber wrote:
> >
> > My goal are defined structures for rules, chains, macros (which i
> > think
> > are just arrays of "rule templates") etc and code which deals with
> > these structures instead of printing out iptables commands in
> > various
> > places.
> That's long overdue anyway and might be useful later on when we want
> to
> start using nftables.
>
> >
> > While doing this, keeping "input" and "output" identical to whats
> > there
> > doesn't make this an easy task and therefore I sometimes use these
> > 'temporary ugliness' approaches at places that I intend to replace
> > anyway.
> That's fine.
>
> >
> > >
> > > >
> > > > @@ -3127,26 +3146,21 @@ sub generate_std_chains {
> > > > my $std_chains = $pve_std_chains->{$ipversion} || die
> > > > "internal error";
> > > >
> > > > my $loglevel = get_option_log_level($options,
> > > > 'smurf_log_level');
> > > > -
> > > > -my $chain;
> > > > -
> > > > -if ($ipversion == 4) {
> > > > - # same as shorewall smurflog.
> > > > - $chain = 'PVEFW-smurflog';
> > > > - $std_chains->{$chain} = [];
> > > > -
> > > > - push @{$std_chains->{$chain}},
> > > > get_log_rule_base($chain,
> > > > 0, "DROP: ", $loglevel) if $loglevel;
> > > > - push @{$std_chains->{$chain}}, "-j DROP";
> > > > +my $chain = 'PVEFW-smurflog';
> > > > +if ( $loglevel && $std_chains->{$chain} ) {
> > > This shouldn't check for $loglevel as it can also have changed
> > > from
> > > non-zero to zero in which case the change would happen. Iow. you
> > > cannot
> > > turn off the smurf logs anymore.
> > huh? if $loglevel == 0 it will not turn on logging.
> This modifies the fields in the global $pve_std_chains, so it doesn't
> need to not turn it on, it needs to turn if off. Such modifications
> are
> something I'd prefer to reduce if possible.
yes you're right - didn't look at it as running service.
so, the ultimate solution would be to have the std_chains template
defined in external config files (which i have in mind from the
beginning) from which we'll generate our working internal tree which
we'd then manipulate according to various settings?
> > > > + foreach my $r (@{$std_chains->{$chain}}) {
> > > > + $r->{log} = $loglevel;
> > > Wouldn't it be better to just give ruleset_generate_rule an
> > > optional
> > > default loglevel parameter for when rules don't define their own
> > > {log}?
> > how would this help with turning smurf / tcpflags logging
> > separately on
> > and off then? (not that I like the current way of setting these -
> > but
> > that's what's in the frontend right now)
> Could be passed depending on $chain in the loop below, then we could
> avoid touching the global hashes entirely.
> The function would still prefer the rule's ->{log} member over the
> default if it exists.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 firewall 08/13] implement ipt_rule_to_cmds, ruleset_add_ipt_cmd
ipt_rule_to_cmds converts a %rule to an array of iptables commands
ruleset_add_ipt_cmd adds such an iptables command to a chain
ruleset_generate_rule uses these now
ruleset_generate_rule_old is an interim workaround
---
src/PVE/Firewall.pm | 151
1 file changed, 140 insertions(+), 11 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 89a8ef3..b492086 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -146,14 +146,15 @@ my $log_level_hash = {
# %rule
#
# name => optional
+# enable => [0|1]
# action =>
# proto =>
-# sport =>
-# dport =>
+# sport => port[,port[,port]].. or port:port
+# dport => port[,port[,port]].. or port:port
# log => optional, loglevel
# logmsg => optional, logmsg - overwrites default
-# iface_in
-# iface_out
+# iface_in => incomin interface
+# iface_out => outgoing interface
# match => optional, overwrites generation of match
# target => optional, overwrites action
@@ -1844,6 +1845,104 @@ sub ipt_gen_src_or_dst_match {
return $match;
}
+# convert a %rule to an array of iptables commands
+sub ipt_rule_to_cmds {
+my ($rule, $chain, $ipversion, $cluster_conf, $fw_conf, $vmid) = @_;
+
+die "ipt_rule_to_cmds unable to handle macro" if $rule->{macro}; #should
not happen
+
+my @match = ();
+
+if (defined $rule->{match}) {
+ push @match, $rule->{match};
+} else {
+ push @match, "-i $rule->{iface_in}" if $rule->{iface_in};
+ push @match, "-o $rule->{iface_out}" if $rule->{iface_out};
+
+ if ($rule->{source}) {
+ push @match, ipt_gen_src_or_dst_match($rule->{source}, 's',
$ipversion, $cluster_conf, $fw_conf);
+ }
+ if ($rule->{dest}) {
+ push @match, ipt_gen_src_or_dst_match($rule->{dest}, 'd',
$ipversion, $cluster_conf, $fw_conf);
+ }
+
+ if (my $proto = $rule->{proto}) {
+ push @match, "-p $proto";
+
+ my $nbdport = defined($rule->{dport}) ?
parse_port_name_number_or_range($rule->{dport}, 1) : 0;
+ my $nbsport = defined($rule->{sport}) ?
parse_port_name_number_or_range($rule->{sport}, 0) : 0;
+
+ my $multiport = 0;
+ $multiport++ if $nbdport > 1;
+ $multiport++ if $nbsport > 1;
+
+ push @match, "--match multiport" if $multiport;
+
+ die "multiport: option '--sports' cannot be used together with
'--dports'\n"
+ if ($multiport == 2) && ($rule->{dport} ne $rule->{sport});
+
+ if ($rule->{dport}) {
+ if ($proto eq 'icmp') {
+ # Note: we use dport to store --icmp-type
+ die "unknown icmp-type '$rule->{dport}'\n"
+ if $rule->{dport} !~ /^\d+$/ &&
!defined($icmp_type_names->{$rule->{dport}});
+ push @match, "-m icmp --icmp-type $rule->{dport}";
+ } elsif ($proto eq 'icmpv6') {
+ # Note: we use dport to store --icmpv6-type
+ die "unknown icmpv6-type '$rule->{dport}'\n"
+ if $rule->{dport} !~ /^\d+$/ &&
!defined($icmpv6_type_names->{$rule->{dport}});
+ push @match, "-m icmpv6 --icmpv6-type $rule->{dport}";
+ } elsif (!$PROTOCOLS_WITH_PORTS->{$proto}) {
+ die "protocol $proto does not have ports\n";
+ } else {
+ if ($nbdport > 1) {
+ if ($multiport == 2) {
+ push @match, "--ports $rule->{dport}";
+ } else {
+ push @match, "--dports $rule->{dport}";
+ }
+ } else {
+ push @match, "--dport $rule->{dport}";
+ }
+ }
+ }
+
+ if ($rule->{sport}) {
+ die "protocol $proto does not have ports\n"
+ if !$PROTOCOLS_WITH_PORTS->{$proto};
+ if ($nbsport > 1) {
+ push @match, "--sports $rule->{sport}" if $multiport != 2;
+ } else {
+ push @match, "--sport $rule->{sport}";
+ }
+ }
+ } elsif ($rule->{dport} || $rule->{sport}) {
+ die "destination port '$rule->{dport}', but no protocol
specified\n" if $rule->{dport};
+ die "source port '$rule->{sport}', but no protocol specified\n" if
$rule->{sport};
+ }
+
+ push @match, "-m addrtype --dst-type $rule->{dsttype}" if
$rule->{dsttype};
+}
+my $matchstr = scalar(@match) ? join(' ', @match) : "";
+
+my $targetstr;
+if (defined $rule->{target}) {
+ $targetstr = $rule->{target};
+} else {
+ my $action = (defined $rule->{action}) ? $rule->{action} : "";
+ my $goto = 1 if $action eq 'PVEFW-SET-ACCEPT-MARK';
+ $targetstr = ($goto) ? "-g $action" : "-j $action";
+}
+
+my @iptcmds;
+if (defined $rule->{log} && $rule->{log}) {
[pve-devel] [PATCH v3 firewall 12/13] remove unused ruleset_generate_rule_insert
---
src/PVE/Firewall.pm | 12
1 file changed, 12 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index d249f7a..65ea132 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2070,18 +2070,6 @@ sub ruleset_generate_rule {
}
}
-sub ruleset_generate_rule_insert {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto) = @_;
-
-die "implement me" if $rule->{macro}; # not implemented, because not
needed so far
-
-my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
-my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
-if (defined $match && defined $action) {
- ruleset_insertrule($ruleset, $chain, $match, $action);
-}
-}
-
sub ruleset_create_chain {
my ($ruleset, $chain) = @_;
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 firewall 13/13] remove ruleset_generate_match, ruleset_generate_action
ruleset_generate_match and ruleset_generate_action not used anymore
---
src/PVE/Firewall.pm | 97 -
1 file changed, 97 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 65ea132..9b78acb 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1952,103 +1952,6 @@ sub ipt_rule_to_cmds {
return @iptcmds;
}
-sub ruleset_generate_match {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
-
-return if defined($rule->{enable}) && !$rule->{enable};
-return if $rule->{errors};
-
-return $rule->{match} if defined $rule->{match};
-
-die "unable to emit macro - internal error" if $rule->{macro}; # should
not happen
-
-my $nbdport = defined($rule->{dport}) ?
parse_port_name_number_or_range($rule->{dport}, 1) : 0;
-my $nbsport = defined($rule->{sport}) ?
parse_port_name_number_or_range($rule->{sport}, 0) : 0;
-
-my @cmd = ();
-
-push @cmd, "-i $rule->{iface_in}" if $rule->{iface_in};
-push @cmd, "-o $rule->{iface_out}" if $rule->{iface_out};
-
-my $source = $rule->{source};
-my $dest = $rule->{dest};
-
-push @cmd, ipt_gen_src_or_dst_match($source, 's', $ipversion,
$cluster_conf, $fw_conf) if $source;
-push @cmd, ipt_gen_src_or_dst_match($dest, 'd', $ipversion, $cluster_conf,
$fw_conf) if $dest;
-
-if (my $proto = $rule->{proto}) {
- push @cmd, "-p $proto";
-
- my $multiport = 0;
- $multiport++ if $nbdport > 1;
- $multiport++ if $nbsport > 1;
-
- push @cmd, "--match multiport" if $multiport;
-
- die "multiport: option '--sports' cannot be used together with
'--dports'\n"
- if ($multiport == 2) && ($rule->{dport} ne $rule->{sport});
-
- if ($rule->{dport}) {
- if ($proto eq 'icmp') {
- # Note: we use dport to store --icmp-type
- die "unknown icmp-type '$rule->{dport}'\n"
- if $rule->{dport} !~ /^\d+$/ &&
!defined($icmp_type_names->{$rule->{dport}});
- push @cmd, "-m icmp --icmp-type $rule->{dport}";
- } elsif ($proto eq 'icmpv6') {
- # Note: we use dport to store --icmpv6-type
- die "unknown icmpv6-type '$rule->{dport}'\n"
- if $rule->{dport} !~ /^\d+$/ &&
!defined($icmpv6_type_names->{$rule->{dport}});
- push @cmd, "-m icmpv6 --icmpv6-type $rule->{dport}";
- } elsif (!$PROTOCOLS_WITH_PORTS->{$proto}) {
- die "protocol $proto does not have ports\n";
- } else {
- if ($nbdport > 1) {
- if ($multiport == 2) {
- push @cmd, "--ports $rule->{dport}";
- } else {
- push @cmd, "--dports $rule->{dport}";
- }
- } else {
- push @cmd, "--dport $rule->{dport}";
- }
- }
- }
-
- if ($rule->{sport}) {
- die "protocol $proto does not have ports\n"
-if !$PROTOCOLS_WITH_PORTS->{$proto};
- if ($nbsport > 1) {
- push @cmd, "--sports $rule->{sport}" if $multiport != 2;
- } else {
- push @cmd, "--sport $rule->{sport}";
- }
- }
-} elsif ($rule->{dport} || $rule->{sport}) {
- die "destination port '$rule->{dport}', but no protocol specified\n" if
$rule->{dport};
- die "source port '$rule->{sport}', but no protocol specified\n" if
$rule->{sport};
-}
-
-push @cmd, "-m addrtype --dst-type $rule->{dsttype}" if $rule->{dsttype};
-
-return scalar(@cmd) ? join(' ', @cmd) : undef;
-}
-
-sub ruleset_generate_action {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
-
-return $rule->{target} if defined $rule->{target};
-
-my @cmd = ();
-
-if (my $action = $rule->{action}) {
- $action = $actions->{$action} if defined($actions->{$action});
- $goto = 1 if !defined($goto) && $action eq 'PVEFW-SET-ACCEPT-MARK';
- push @cmd, $goto ? "-g $action" : "-j $action";
-}
-
-return scalar(@cmd) ? join(' ', @cmd) : undef;
-}
-
sub ruleset_generate_rule {
my ($ruleset, $chain, $ipversion, $rule, $cluster_conf, $fw_conf) = @_;
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 firewall 10/13] rule_substitude_action, remove ruleset_generate_rule_old
implement rule_substitude_action
eliminate use of ruleset_genereate_rule_old and remove it
---
src/PVE/Firewall.pm | 73 ++---
1 file changed, 24 insertions(+), 49 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 633aa7a..d9c2347 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1800,6 +1800,15 @@ sub ipset_get_chains {
return $res;
}
+# substitude action of rule according to action hash
+sub rule_substitude_action {
+my ($rule, $actions) = @_;
+
+if (my $action = $rule->{action}) {
+ $rule->{action} = $actions->{$action} if defined($actions->{$action});
+}
+}
+
# generate a src or dst match
# $dir(ection) is either d or s
sub ipt_gen_src_or_dst_match {
@@ -2061,40 +2070,6 @@ sub ruleset_generate_rule {
}
}
-sub ruleset_generate_rule_old {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
-
-my $rules;
-
-if ($rule->{macro}) {
- $rules = &$apply_macro($rule->{macro}, $rule, 0, $ipversion);
-} else {
- $rules = [ $rule ];
-}
-
-# update all or nothing
-
-# fixme: lots of temporary ugliness
-my @mstrs = ();
-my @astrs = ();
-my @logging = ();
-my @logmsg = ();
-foreach my $tmp (@$rules) {
- my $m = ruleset_generate_match($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
- my $a = ruleset_generate_action($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
- if (defined $m or defined $a) {
- push @mstrs, defined($m) ? $m : "";
- push @astrs, defined($a) ? $a : "";
- push @logging, $tmp->{log};
- push @logmsg, $tmp->{logmsg};
- }
-}
-
-for my $i (0 .. $#mstrs) {
- ruleset_addrule($ruleset, $chain, $mstrs[$i], $astrs[$i], $logging[$i],
$logmsg[$i]);
-}
-}
-
sub ruleset_generate_rule_insert {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto) = @_;
@@ -2170,8 +2145,9 @@ sub ruleset_add_chain_policy {
if ($policy eq 'ACCEPT') {
- ruleset_generate_rule_old($ruleset, $chain, $ipversion, { action =>
'ACCEPT' },
- { ACCEPT => $accept_action});
+ my $rule = { action => 'ACCEPT' };
+ rule_substitude_action($rule, { ACCEPT => $accept_action});
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule);
} elsif ($policy eq 'DROP') {
@@ -2317,12 +2293,12 @@ sub ruleset_generate_vm_rules {
next if $rule->{type} ne $lc_direction;
eval {
if ($direction eq 'OUT') {
- ruleset_generate_rule_old($ruleset, $chain, $ipversion,
$rule,
- { ACCEPT => "PVEFW-SET-ACCEPT-MARK",
REJECT => "PVEFW-reject" },
+ rule_substitude_action($rule, { ACCEPT =>
"PVEFW-SET-ACCEPT-MARK", REJECT => "PVEFW-reject" });
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
undef, $cluster_conf, $vmfw_conf);
} else {
- ruleset_generate_rule_old($ruleset, $chain, $ipversion,
$rule,
- { ACCEPT => $in_accept , REJECT =>
"PVEFW-reject" },
+ rule_substitude_action($rule, { ACCEPT => $in_accept ,
REJECT => "PVEFW-reject" });
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
undef, $cluster_conf, $vmfw_conf);
}
};
@@ -2451,8 +2427,8 @@ sub enable_host_firewall {
if ($rule->{type} eq 'group') {
ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule,
'IN', $accept_action, $ipversion);
} elsif ($rule->{type} eq 'in') {
- ruleset_generate_rule_old($ruleset, $chain, $ipversion, $rule,
- { ACCEPT => $accept_action, REJECT =>
"PVEFW-reject" },
+ rule_substitude_action($rule, { ACCEPT => $accept_action,
REJECT => "PVEFW-reject" });
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
undef, $cluster_conf, $hostfw_conf);
}
};
@@ -2508,8 +2484,8 @@ sub enable_host_firewall {
if ($rule->{type} eq 'group') {
ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule,
'OUT', $accept_action, $ipversion);
} elsif ($rule->{type} eq 'out') {
- ruleset_generate_rule_old($ruleset, $chain, $ipversion,
- $rule, { ACCEPT => $accept_action, REJECT
=> "PVEFW-reject" },
+ rule_substitude_action($rule, { ACCEPT => $accept_action,
REJECT => "PVEFW-reject" });
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
[pve-devel] [PATCH v3 firewall 09/13] remove unused ruleset_generate_cmdstr
---
src/PVE/Firewall.pm | 11 ---
1 file changed, 11 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index b492086..633aa7a 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2040,17 +2040,6 @@ sub ruleset_generate_action {
return scalar(@cmd) ? join(' ', @cmd) : undef;
}
-sub ruleset_generate_cmdstr {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
-my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
-my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
-
-return undef if !(defined($match) or defined($action));
-my $ret = defined($match) ? $match : "";
-$ret = "$ret $action" if defined($action);
-return $ret;
-}
-
sub ruleset_generate_rule {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 firewall 00/13] Firewall code cleanups
third version. mostly converting rules into structures. reorganized ruleset_generate_rule and everything around it. please note that some of the stuff implemented in the first patches gets eliminated later. So maybe it's worth reading all patches before flaming me ;-) Tom Weber (13): remove unused $rule_format prepare code for more generic firewall logging integrate logging into ruleset_addrule convert string based rule definitions to hashes make $pve_std_chains a copy of $pve_std_chains_conf eliminate unused nbdport in pve_std_chains_conf iptables address matching in own subroutine implement ipt_rule_to_cmds, ruleset_add_ipt_cmd remove unused ruleset_generate_cmdstr rule_substitude_action, remove ruleset_generate_rule_old cleanup parameters to ruleset_generate_rule remove unused ruleset_generate_rule_insert remove ruleset_generate_match, ruleset_generate_action src/PVE/Firewall.pm | 674 ++- src/PVE/FirewallSimulator.pm | 2 +- 2 files changed, 342 insertions(+), 334 deletions(-) -- 2.7.4 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 firewall 06/13] eliminate unused nbdport in pve_std_chains_conf
---
src/PVE/Firewall.pm | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index c7ddd10..f009e58 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -586,10 +586,10 @@ $pve_std_chains_conf->{4} = {
# Drop packets with INVALID state
{ action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
- { action => 'DROP', proto => 'udp', dport => '135,445', nbdport => 2 },
- { action => 'DROP', proto => 'udp', dport => '137:139'},
+ { action => 'DROP', proto => 'udp', dport => '135,445' },
+ { action => 'DROP', proto => 'udp', dport => '137:139' },
{ action => 'DROP', proto => 'udp', dport => '1024:65535', sport => 137
},
- { action => 'DROP', proto => 'tcp', dport => '135,139,445', nbdport =>
3 },
+ { action => 'DROP', proto => 'tcp', dport => '135,139,445' },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
{ action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
@@ -609,10 +609,10 @@ $pve_std_chains_conf->{4} = {
# Drop packets with INVALID state
{ action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
- { action => 'PVEFW-reject', proto => 'udp', dport => '135,445', nbdport
=> 2 },
+ { action => 'PVEFW-reject', proto => 'udp', dport => '135,445' },
{ action => 'PVEFW-reject', proto => 'udp', dport => '137:139'},
{ action => 'PVEFW-reject', proto => 'udp', dport => '1024:65535',
sport => 137 },
- { action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445',
nbdport => 3 },
+ { action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445' },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
{ action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
@@ -682,10 +682,10 @@ $pve_std_chains_conf->{6} = {
# Drop packets with INVALID state
{ action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
- { action => 'DROP', proto => 'udp', dport => '135,445', nbdport => 2 },
+ { action => 'DROP', proto => 'udp', dport => '135,445' },
{ action => 'DROP', proto => 'udp', dport => '137:139'},
{ action => 'DROP', proto => 'udp', dport => '1024:65535', sport => 137
},
- { action => 'DROP', proto => 'tcp', dport => '135,139,445', nbdport =>
3 },
+ { action => 'DROP', proto => 'tcp', dport => '135,139,445' },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
{ action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
@@ -706,10 +706,10 @@ $pve_std_chains_conf->{6} = {
# Drop packets with INVALID state
{ action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
- { action => 'PVEFW-reject', proto => 'udp', dport => '135,445', nbdport
=> 2 },
- { action => 'PVEFW-reject', proto => 'udp', dport => '137:139'},
+ { action => 'PVEFW-reject', proto => 'udp', dport => '135,445' },
+ { action => 'PVEFW-reject', proto => 'udp', dport => '137:139' },
{ action => 'PVEFW-reject', proto => 'udp', dport => '1024:65535',
sport => 137 },
- { action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445',
nbdport => 3 },
+ { action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445' },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
{ action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 firewall 07/13] iptables address matching in own subroutine
put generation of iptables source/destination address matching
in own subroutine and use this in ruleset_generate_match
---
src/PVE/Firewall.pm | 104
1 file changed, 47 insertions(+), 57 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index f009e58..89a8ef3 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1799,6 +1799,51 @@ sub ipset_get_chains {
return $res;
}
+# generate a src or dst match
+# $dir(ection) is either d or s
+sub ipt_gen_src_or_dst_match {
+my ($adr, $dir, $ipversion, $cluster_conf, $fw_conf) = @_;
+
+my $srcdst;
+if ($dir eq 's') {
+ $srcdst = "src";
+} elsif ($dir eq 'd') {
+ $srcdst = "dst";
+} else {
+ die "ipt_gen_src_or_dst_match: invalid direction $dir \n";
+}
+
+my $match;
+if ($adr =~ m/^\+/) {
+ if ($adr =~ m/^\+(${ipset_name_pattern})$/) {
+ my $name = $1;
+ my $ipset_chain;
+ if ($fw_conf && $fw_conf->{ipset}->{$name}) {
+ $ipset_chain = compute_ipset_chain_name($fw_conf->{vmid},
$name, $ipversion);
+ } elsif ($cluster_conf && $cluster_conf->{ipset}->{$name}) {
+ $ipset_chain = compute_ipset_chain_name(0, $name, $ipversion);
+ } else {
+ die "no such ipset '$name'\n";
+ }
+ $match = "-m set --match-set ${ipset_chain} ${srcdst}";
+ } else {
+ die "invalid security group name '$adr'\n";
+ }
+} elsif ($adr =~ m/^${ip_alias_pattern}$/){
+ my $alias = lc($adr);
+ my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef;
+ $e = $cluster_conf->{aliases}->{$alias} if !$e && $cluster_conf;
+ die "no such alias '$adr'\n" if !$e;
+ $match = "-${dir} $e->{cidr}";
+} elsif ($adr =~ m/\-/){
+ $match = "-m iprange --${srcdst}-range $adr";
+} else {
+ $match = "-${dir} $adr";
+}
+
+return $match;
+}
+
sub ruleset_generate_match {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
@@ -1820,63 +1865,8 @@ sub ruleset_generate_match {
my $source = $rule->{source};
my $dest = $rule->{dest};
-if ($source) {
-if ($source =~ m/^\+/) {
- if ($source =~ m/^\+(${ipset_name_pattern})$/) {
- my $name = $1;
- if ($fw_conf && $fw_conf->{ipset}->{$name}) {
- my $ipset_chain =
compute_ipset_chain_name($fw_conf->{vmid}, $name, $ipversion);
- push @cmd, "-m set --match-set ${ipset_chain} src";
- } elsif ($cluster_conf && $cluster_conf->{ipset}->{$name}) {
- my $ipset_chain = compute_ipset_chain_name(0, $name,
$ipversion);
- push @cmd, "-m set --match-set ${ipset_chain} src";
- } else {
- die "no such ipset '$name'\n";
- }
- } else {
- die "invalid security group name '$source'\n";
- }
- } elsif ($source =~ m/^${ip_alias_pattern}$/){
- my $alias = lc($source);
- my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef;
- $e = $cluster_conf->{aliases}->{$alias} if !$e && $cluster_conf;
- die "no such alias '$source'\n" if !$e;
- push @cmd, "-s $e->{cidr}";
-} elsif ($source =~ m/\-/){
- push @cmd, "-m iprange --src-range $source";
- } else {
- push @cmd, "-s $source";
-}
-}
-
-if ($dest) {
-if ($dest =~ m/^\+/) {
- if ($dest =~ m/^\+(${ipset_name_pattern})$/) {
- my $name = $1;
- if ($fw_conf && $fw_conf->{ipset}->{$name}) {
- my $ipset_chain =
compute_ipset_chain_name($fw_conf->{vmid}, $name, $ipversion);
- push @cmd, "-m set --match-set ${ipset_chain} dst";
- } elsif ($cluster_conf && $cluster_conf->{ipset}->{$name}) {
- my $ipset_chain = compute_ipset_chain_name(0, $name,
$ipversion);
- push @cmd, "-m set --match-set ${ipset_chain} dst";
- } else {
- die "no such ipset '$name'\n";
- }
- } else {
- die "invalid security group name '$dest'\n";
- }
- } elsif ($dest =~ m/^${ip_alias_pattern}$/){
- my $alias = lc($dest);
- my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef;
- $e = $cluster_conf->{aliases}->{$alias} if !$e && $cluster_conf;
- die "no such alias '$dest'\n" if !$e;
- push @cmd, "-d $e->{cidr}";
-} elsif ($dest =~
m/^(\d+)\.(\d+).(\d+).(\d+)\-(\d+)\.(\d+).(\d+).(\d+)$/){
- push @cmd, "-m iprange --dst-range $dest";
- } else {
- push @cmd, "-d $dest";
-}
-}
+push @cmd, ipt_gen_src_or_dst_match($source, 's', $ipversion,
$cluster_conf, $fw_conf) if $source;
+pus
[pve-devel] [PATCH v3 firewall 05/13] make $pve_std_chains a copy of $pve_std_chains_conf
create a new $pve_std_chains with $pve_std_chains_conf as template on
every compilation of the rules. This avoids persitant changes to the
$pve_std_chains and makes it easier to read the std_chains configuration
from external config files (later to implement).
---
src/PVE/Firewall.pm | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 634ff90..c7ddd10 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -20,6 +20,7 @@ use IO::File;
use Net::IP;
use PVE::Tools qw(run_command lock_file dir_glob_foreach);
use Encode;
+use Storable qw(dclone);
my $hostfw_conf_filename = "/etc/pve/local/host.fw";
my $pvefw_conf_dir = "/etc/pve/firewall";
@@ -548,7 +549,8 @@ my $FWACCEPTMARK_ON = "0x8000/0x8000";
my $FWACCEPTMARK_OFF = "0x/0x8000";
my $pve_std_chains = {};
-$pve_std_chains->{4} = {
+my $pve_std_chains_conf = {};
+$pve_std_chains_conf->{4} = {
'PVEFW-SET-ACCEPT-MARK' => [
{ target => "-j MARK --set-mark $FWACCEPTMARK_ON" },
],
@@ -641,7 +643,7 @@ $pve_std_chains->{4} = {
],
};
-$pve_std_chains->{6} = {
+$pve_std_chains_conf->{6} = {
'PVEFW-SET-ACCEPT-MARK' => [
{ target => "-j MARK --set-mark $FWACCEPTMARK_ON" },
],
@@ -3354,6 +3356,9 @@ sub compile {
my $vmfw_configs;
+# fixme: once we read standard chains from config this needs to be put in
test/standard cases below
+$pve_std_chains = dclone($pve_std_chains_conf);
+
if ($vmdata) { # test mode
my $testdir = $vmdata->{testdir} || die "no test directory specified";
my $filename = "$testdir/cluster.fw";
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 firewall 01/13] remove unused $rule_format
---
src/PVE/Firewall.pm | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index cc81325..5d78686 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1648,8 +1648,6 @@ sub enable_bridge_firewall {
$bridge_firewall_enabled = 1;
}
-my $rule_format = "%-15s %-30s %-30s %-15s %-15s %-15s\n";
-
sub iptables_restore_cmdlist {
my ($cmdlist) = @_;
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 firewall 11/13] cleanup parameters to ruleset_generate_rule
remove $actions and $goto - not used anymore
---
src/PVE/Firewall.pm | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index d9c2347..d249f7a 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2050,7 +2050,7 @@ sub ruleset_generate_action {
}
sub ruleset_generate_rule {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
+my ($ruleset, $chain, $ipversion, $rule, $cluster_conf, $fw_conf) = @_;
my $rules;
@@ -2294,12 +2294,10 @@ sub ruleset_generate_vm_rules {
eval {
if ($direction eq 'OUT') {
rule_substitude_action($rule, { ACCEPT =>
"PVEFW-SET-ACCEPT-MARK", REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
- undef, $cluster_conf, $vmfw_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf, $vmfw_conf);
} else {
rule_substitude_action($rule, { ACCEPT => $in_accept ,
REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
- undef, $cluster_conf, $vmfw_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf, $vmfw_conf);
}
};
warn $@ if $@;
@@ -2428,8 +2426,7 @@ sub enable_host_firewall {
ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule,
'IN', $accept_action, $ipversion);
} elsif ($rule->{type} eq 'in') {
rule_substitude_action($rule, { ACCEPT => $accept_action,
REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
- undef, $cluster_conf, $hostfw_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf, $hostfw_conf);
}
};
warn $@ if $@;
@@ -2485,8 +2482,7 @@ sub enable_host_firewall {
ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule,
'OUT', $accept_action, $ipversion);
} elsif ($rule->{type} eq 'out') {
rule_substitude_action($rule, { ACCEPT => $accept_action,
REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
- undef, $cluster_conf, $hostfw_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf, $hostfw_conf);
}
};
warn $@ if $@;
@@ -2532,7 +2528,7 @@ sub generate_group_rules {
next if $rule->{type} ne 'in';
next if $rule->{ipversion} && $rule->{ipversion} ne $ipversion;
rule_substitude_action($rule, { ACCEPT => "PVEFW-SET-ACCEPT-MARK",
REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, undef,
undef, $cluster_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf);
}
$chain = "GROUP-${group}-OUT";
@@ -2546,8 +2542,7 @@ sub generate_group_rules {
# we use PVEFW-SET-ACCEPT-MARK (Instead of ACCEPT) because we need to
# check also other tap rules later
rule_substitude_action($rule, { ACCEPT => 'PVEFW-SET-ACCEPT-MARK',
REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, undef,
- undef, $cluster_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf);
}
}
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 firewall 03/13] integrate logging into ruleset_addrule
---
src/PVE/Firewall.pm | 33 ++---
1 file changed, 10 insertions(+), 23 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index f1aecef..ad59267 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2002,10 +2002,14 @@ sub ruleset_addrule_old {
}
sub ruleset_addrule {
- my ($ruleset, $chain, $match, $action, $log) = @_;
+ my ($ruleset, $chain, $match, $action, $log, $logmsg, $vmid) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
+ if (defined($log) && $log) {
+ my $logaction = get_log_rule_base($chain, $vmid, $logmsg, $log);
+ push @{$ruleset->{$chain}}, "-A $chain $match $logaction";
+ }
push @{$ruleset->{$chain}}, "-A $chain $match $action";
}
@@ -2020,27 +2024,15 @@ sub ruleset_insertrule {
sub get_log_rule_base {
my ($chain, $vmid, $msg, $loglevel) = @_;
-die "internal error - no log level" if !defined($loglevel);
-
$vmid = 0 if !defined($vmid);
+$msg = "" if !defined($msg);
# Note: we use special format for prefix to pass further
-# info to log daemon (VMID, LOGVELEL and CHAIN)
+# info to log daemon (VMID, LOGLEVEL and CHAIN)
return "-j NFLOG --nflog-prefix \":$vmid:$loglevel:$chain: $msg\"";
}
-sub ruleset_addlog {
-my ($ruleset, $chain, $vmid, $msg, $loglevel, $match) = @_;
-
-return if !defined($loglevel);
-
-my $logaction = get_log_rule_base($chain, $vmid, $msg, $loglevel);
-
-$match = "" if !defined $match;
-ruleset_addrule($ruleset, $chain, $match, $logaction);
-}
-
sub ruleset_add_chain_policy {
my ($ruleset, $chain, $ipversion, $vmid, $policy, $loglevel,
$accept_action) = @_;
@@ -2053,15 +2045,11 @@ sub ruleset_add_chain_policy {
ruleset_addrule($ruleset, $chain, "", "-j PVEFW-Drop");
- ruleset_addlog($ruleset, $chain, $vmid, "policy $policy: ", $loglevel);
-
- ruleset_addrule($ruleset, $chain, "", "-j DROP");
+ ruleset_addrule($ruleset, $chain, "", "-j DROP", $loglevel, "policy
$policy: ", $vmid);
} elsif ($policy eq 'REJECT') {
ruleset_addrule($ruleset, $chain, "", "-j PVEFW-Reject");
- ruleset_addlog($ruleset, $chain, $vmid, "policy $policy: ", $loglevel);
-
- ruleset_addrule($ruleset, $chain, "", "-g PVEFW-reject");
+ ruleset_addrule($ruleset, $chain, "", "-g PVEFW-reject", $loglevel,
"policy $policy:", $vmid);
} else {
# should not happen
die "internal error: unknown policy '$policy'";
@@ -2093,8 +2081,7 @@ sub ruleset_chain_add_input_filters {
if ($cluster_conf->{ipset}->{blacklist}){
if (!ruleset_chain_exist($ruleset, "PVEFW-blacklist")) {
ruleset_create_chain($ruleset, "PVEFW-blacklist");
- ruleset_addlog($ruleset, "PVEFW-blacklist", 0, "DROP: ", $loglevel)
if $loglevel;
- ruleset_addrule($ruleset, "PVEFW-blacklist", "", "-j DROP");
+ ruleset_addrule($ruleset, "PVEFW-blacklist", "", "-j DROP",
$loglevel, "DROP: ");
}
my $ipset_chain = compute_ipset_chain_name(0, 'blacklist', $ipversion);
ruleset_addrule($ruleset, $chain, "-m set --match-set ${ipset_chain}
src", "-j PVEFW-blacklist");
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 firewall 02/13] prepare code for more generic firewall logging
making ruleset generation aware of a match and action
part in iptable rules.
code will generate the same iptables as before! (except for
a few additional spaces between match and action).
---
src/PVE/Firewall.pm | 166 ++-
src/PVE/FirewallSimulator.pm | 2 +-
2 files changed, 100 insertions(+), 68 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 5d78686..f1aecef 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1776,7 +1776,7 @@ sub ipset_get_chains {
return $res;
}
-sub ruleset_generate_cmdstr {
+sub ruleset_generate_match {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
return if defined($rule->{enable}) && !$rule->{enable};
@@ -1907,6 +1907,14 @@ sub ruleset_generate_cmdstr {
push @cmd, "-m addrtype --dst-type $rule->{dsttype}" if $rule->{dsttype};
+return scalar(@cmd) ? join(' ', @cmd) : undef;
+}
+
+sub ruleset_generate_action {
+my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
+
+my @cmd = ();
+
if (my $action = $rule->{action}) {
$action = $actions->{$action} if defined($actions->{$action});
$goto = 1 if !defined($goto) && $action eq 'PVEFW-SET-ACCEPT-MARK';
@@ -1916,6 +1924,17 @@ sub ruleset_generate_cmdstr {
return scalar(@cmd) ? join(' ', @cmd) : undef;
}
+sub ruleset_generate_cmdstr {
+my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
+my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
+my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
+
+return undef if !(defined($match) or defined($action));
+my $ret = defined($match) ? $match : "";
+$ret = "$ret $action" if defined($action);
+return $ret;
+}
+
sub ruleset_generate_rule {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
@@ -1929,15 +1948,19 @@ sub ruleset_generate_rule {
# update all or nothing
-my @cmds = ();
+my @mstrs = ();
+my @astrs = ();
foreach my $tmp (@$rules) {
- if (my $cmdstr = ruleset_generate_cmdstr($ruleset, $chain, $ipversion,
$tmp, $actions, $goto, $cluster_conf, $fw_conf)) {
- push @cmds, $cmdstr;
+ my $m = ruleset_generate_match($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
+ my $a = ruleset_generate_action($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
+ if (defined $m or defined $a) {
+ push @mstrs, defined($m) ? $m : "";
+ push @astrs, defined($a) ? $a : "";
}
}
-foreach my $cmdstr (@cmds) {
- ruleset_addrule($ruleset, $chain, $cmdstr);
+for my $i (0 .. $#mstrs) {
+ ruleset_addrule($ruleset, $chain, $mstrs[$i], $astrs[$i]);
}
}
@@ -1946,8 +1969,10 @@ sub ruleset_generate_rule_insert {
die "implement me" if $rule->{macro}; # not implemented, because not
needed so far
-if (my $cmdstr = ruleset_generate_cmdstr($ruleset, $chain, $ipversion,
$rule, $actions, $goto)) {
- ruleset_insertrule($ruleset, $chain, $cmdstr);
+my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
+my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
+if (defined $match && defined $action) {
+ ruleset_insertrule($ruleset, $chain, $match, $action);
}
}
@@ -1968,7 +1993,7 @@ sub ruleset_chain_exist {
return $ruleset->{$chain} ? 1 : undef;
}
-sub ruleset_addrule {
+sub ruleset_addrule_old {
my ($ruleset, $chain, $rule) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
@@ -1976,12 +2001,20 @@ sub ruleset_addrule {
push @{$ruleset->{$chain}}, "-A $chain $rule";
}
+sub ruleset_addrule {
+ my ($ruleset, $chain, $match, $action, $log) = @_;
+
+ die "no such chain '$chain'\n" if !$ruleset->{$chain};
+
+ push @{$ruleset->{$chain}}, "-A $chain $match $action";
+}
+
sub ruleset_insertrule {
- my ($ruleset, $chain, $rule) = @_;
+ my ($ruleset, $chain, $match, $action, $log) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
- unshift @{$ruleset->{$chain}}, "-A $chain $rule";
+ unshift @{$ruleset->{$chain}}, "-A $chain $match $action";
}
sub get_log_rule_base {
@@ -1998,15 +2031,14 @@ sub get_log_rule_base {
}
sub ruleset_addlog {
-my ($ruleset, $chain, $vmid, $msg, $loglevel, $rule) = @_;
+my ($ruleset, $chain, $vmid, $msg, $loglevel, $match) = @_;
return if !defined($loglevel);
-my $logrule = get_log_rule_base($chain, $vmid, $msg, $loglevel);
-
-$logrule = "$rule $logrule" if defined($rule);
+my $logaction = get_log_rule_base($chain, $vmid, $msg, $loglevel);
-ru
[pve-devel] [PATCH v3 firewall 04/13] convert string based rule definitions to hashes
also extending %rule with log,logmsg,match,target
---
src/PVE/Firewall.pm | 223
1 file changed, 120 insertions(+), 103 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index ad59267..634ff90 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -142,6 +142,20 @@ my $log_level_hash = {
emerg => 0,
};
+# %rule
+#
+# name => optional
+# action =>
+# proto =>
+# sport =>
+# dport =>
+# log => optional, loglevel
+# logmsg => optional, logmsg - overwrites default
+# iface_in
+# iface_out
+# match => optional, overwrites generation of match
+# target => optional, overwrites action
+
# we need to overwrite some macros for ipv6
my $pve_ipv6fw_macros = {
'Ping' => [
@@ -536,7 +550,7 @@ my $FWACCEPTMARK_OFF = "0x/0x8000";
my $pve_std_chains = {};
$pve_std_chains->{4} = {
'PVEFW-SET-ACCEPT-MARK' => [
- "-j MARK --set-mark $FWACCEPTMARK_ON",
+ { target => "-j MARK --set-mark $FWACCEPTMARK_ON" },
],
'PVEFW-DropBroadcast' => [
# same as shorewall 'Broadcast'
@@ -552,10 +566,10 @@ $pve_std_chains->{4} = {
{ action => 'DROP', dsttype => 'BROADCAST' },
{ action => 'DROP', source => '224.0.0.0/4' },
{ action => 'DROP', proto => 'icmp' },
- "-p tcp -j REJECT --reject-with tcp-reset",
- "-p udp -j REJECT --reject-with icmp-port-unreachable",
- "-p icmp -j REJECT --reject-with icmp-host-unreachable",
- "-j REJECT --reject-with icmp-host-prohibited",
+ { match => '-p tcp', target => '-j REJECT --reject-with tcp-reset' },
+ { match => '-p udp', target => '-j REJECT --reject-with
icmp-port-unreachable' },
+ { match => '-p icmp', target => '-j REJECT --reject-with
icmp-host-unreachable' },
+ { target => '-j REJECT --reject-with icmp-host-prohibited' },
],
'PVEFW-Drop' => [
# same as shorewall 'Drop', which is equal to DROP,
@@ -568,7 +582,7 @@ $pve_std_chains->{4} = {
{ action => 'ACCEPT', proto => 'icmp', dport => 'fragmentation-needed'
},
{ action => 'ACCEPT', proto => 'icmp', dport => 'time-exceeded' },
# Drop packets with INVALID state
- "-m conntrack --ctstate INVALID -j DROP",
+ { action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
{ action => 'DROP', proto => 'udp', dport => '135,445', nbdport => 2 },
{ action => 'DROP', proto => 'udp', dport => '137:139'},
@@ -576,7 +590,7 @@ $pve_std_chains->{4} = {
{ action => 'DROP', proto => 'tcp', dport => '135,139,445', nbdport =>
3 },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
- "-p tcp -m tcp ! --tcp-flags FIN,SYN,RST,ACK SYN -j DROP",
+ { action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
# Drop DNS replies
{ action => 'DROP', proto => 'udp', sport => 53 },
],
@@ -591,7 +605,7 @@ $pve_std_chains->{4} = {
{ action => 'ACCEPT', proto => 'icmp', dport => 'fragmentation-needed'
},
{ action => 'ACCEPT', proto => 'icmp', dport => 'time-exceeded' },
# Drop packets with INVALID state
- "-m conntrack --ctstate INVALID -j DROP",
+ { action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
{ action => 'PVEFW-reject', proto => 'udp', dport => '135,445', nbdport
=> 2 },
{ action => 'PVEFW-reject', proto => 'udp', dport => '137:139'},
@@ -599,111 +613,118 @@ $pve_std_chains->{4} = {
{ action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445',
nbdport => 3 },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
- "-p tcp -m tcp ! --tcp-flags FIN,SYN,RST,ACK SYN -j DROP",
+ { action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
# Drop DNS replies
{ action => 'DROP', proto => 'udp', sport => 53 },
],
'PVEFW-tcpflags' => [
# same as shorewall tcpflags action.
# Packets arriving on this interface are checked for som illegal
combinations of TCP flags
- "-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG FIN,PSH,URG -g
PVEFW-logflags",
- "-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG NONE -g
PVEFW-logflags",
- "-p tcp -m tcp --tcp-flags SYN,RST SYN,RST -g PVEFW-logflags",
- "-p tcp -m tcp --tcp-flags FIN,SYN FIN,SYN -g PVEFW-logflags",
- "-p tcp -m tcp --sport 0 --tcp-flags FIN,SYN,RST,ACK SYN -g
PVEFW-logflags",
+ { match => '-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG
FIN,PSH,URG', target => '-g PVEFW-logflags' },
+ { match => '-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG NONE',
target => '-g PVEFW-logflags' },
+ { match => '-p tcp -m tcp --tcp-flags SYN,RST SYN,RST', target
Re: [pve-devel] [PATCH container] Fix restore with multiple mountpoints
Anyone care about this? I have two cases of containers were i have to manually fix after a restore. Tom Am Dienstag, den 26.09.2017, 15:29 +0200 schrieb Tom Weber: > If you use mountpoints inside a container, and change ownership of > these, a restore of the CT will reset them to root:root again. > > That's because the whole CT FS gets prepared and mounted together > with > (of course) an owner of root:root for the mp. > As tar uses the option --skip-old-files it won't touch these dirs > then. > > I don't see why one would need --skip-old-files for a restore job (or > did I miss something?) > > > Tom Weber (1): > remove --skip-old-files from tar restore options > > src/PVE/LXC/Create.pm | 5 - > 1 file changed, 5 deletions(-) > ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v3 firewall 00/13] Firewall code cleanups
Am Mittwoch, den 18.10.2017, 12:44 +0200 schrieb Wolfgang Bumiller: > On Mon, Oct 09, 2017 at 12:16:18PM +0200, Tom Weber wrote: > > > > third version. mostly converting rules into structures. > > reorganized ruleset_generate_rule and everything around it. > > please note that some of the stuff implemented in the first patches > > gets eliminated later. So maybe it's worth reading all patches > > before > > flaming me ;-) > Where would be the fun in that? > > Anyway, the patches seem fine. > Only thing I'm not too happy about currently is that $pve_std_chains > is > still a global. Currently we could clone it in generate_std_chains() > directly as this is both what modifies and uses it, unless this > conflicts with later changes of yours - then it would still be a nice > finish up to this point in the series and change it into a parameter > passed from the outside later on. The cloning of the still hardwired _conf is just to make the behavior of $pve_std_chains similar to what it'd be if we build it from parsing configuration files. That means various switches/settings would default and be reset to whats in the config on every rebuild of the rules and not be carried around if code changes them in the internal structure - remember what hit me when I thought I only need to turn on logging? For now, and the code as it is, it doesn't make a difference but it makes my thinking and maybe the future a bit easier ;) Just tell me if I should keep it or drop it for v4 > OTOH the _conf+clone patch could just be skipped for now as well > until > we actually need it, as the rest of the series doesn't strictly > depend > on that change to be there. Partly due to the length of the series. > I don't want you to have to drag along the entire patch set with each > version. Apart from the above I have no objections to applying the > series as it is. > (Although we do still miss the Signed-off-by lines which I forgot to > mention the last couple of times, sorry.) > So if you can send a v4 with the above changes we could apply it and > continue from there. Sounds great. So i'll try to make a v4 this evening. Tom ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 firewall 10/13] rule_substitude_action, remove ruleset_generate_rule_old
implement rule_substitude_action
eliminate use of ruleset_genereate_rule_old and remove it
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 73 ++---
1 file changed, 24 insertions(+), 49 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 633aa7a..95e00bd 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1800,6 +1800,15 @@ sub ipset_get_chains {
return $res;
}
+# substitude action of rule according to action hash
+sub rule_substitude_action {
+my ($rule, $actions) = @_;
+
+if (my $action = $rule->{action}) {
+ $rule->{action} = $actions->{$action} if defined($actions->{$action});
+}
+}
+
# generate a src or dst match
# $dir(ection) is either d or s
sub ipt_gen_src_or_dst_match {
@@ -2061,40 +2070,6 @@ sub ruleset_generate_rule {
}
}
-sub ruleset_generate_rule_old {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
-
-my $rules;
-
-if ($rule->{macro}) {
- $rules = &$apply_macro($rule->{macro}, $rule, 0, $ipversion);
-} else {
- $rules = [ $rule ];
-}
-
-# update all or nothing
-
-# fixme: lots of temporary ugliness
-my @mstrs = ();
-my @astrs = ();
-my @logging = ();
-my @logmsg = ();
-foreach my $tmp (@$rules) {
- my $m = ruleset_generate_match($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
- my $a = ruleset_generate_action($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
- if (defined $m or defined $a) {
- push @mstrs, defined($m) ? $m : "";
- push @astrs, defined($a) ? $a : "";
- push @logging, $tmp->{log};
- push @logmsg, $tmp->{logmsg};
- }
-}
-
-for my $i (0 .. $#mstrs) {
- ruleset_addrule($ruleset, $chain, $mstrs[$i], $astrs[$i], $logging[$i],
$logmsg[$i]);
-}
-}
-
sub ruleset_generate_rule_insert {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto) = @_;
@@ -2170,8 +2145,9 @@ sub ruleset_add_chain_policy {
if ($policy eq 'ACCEPT') {
- ruleset_generate_rule_old($ruleset, $chain, $ipversion, { action =>
'ACCEPT' },
- { ACCEPT => $accept_action});
+ my $rule = { action => 'ACCEPT' };
+ rule_substitude_action($rule, { ACCEPT => $accept_action});
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule);
} elsif ($policy eq 'DROP') {
@@ -2317,12 +2293,12 @@ sub ruleset_generate_vm_rules {
next if $rule->{type} ne $lc_direction;
eval {
if ($direction eq 'OUT') {
- ruleset_generate_rule_old($ruleset, $chain, $ipversion,
$rule,
- { ACCEPT => "PVEFW-SET-ACCEPT-MARK",
REJECT => "PVEFW-reject" },
+ rule_substitude_action($rule, { ACCEPT =>
"PVEFW-SET-ACCEPT-MARK", REJECT => "PVEFW-reject" });
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
undef, $cluster_conf, $vmfw_conf);
} else {
- ruleset_generate_rule_old($ruleset, $chain, $ipversion,
$rule,
- { ACCEPT => $in_accept , REJECT =>
"PVEFW-reject" },
+ rule_substitude_action($rule, { ACCEPT => $in_accept ,
REJECT => "PVEFW-reject" });
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
undef, $cluster_conf, $vmfw_conf);
}
};
@@ -2451,8 +2427,8 @@ sub enable_host_firewall {
if ($rule->{type} eq 'group') {
ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule,
'IN', $accept_action, $ipversion);
} elsif ($rule->{type} eq 'in') {
- ruleset_generate_rule_old($ruleset, $chain, $ipversion, $rule,
- { ACCEPT => $accept_action, REJECT =>
"PVEFW-reject" },
+ rule_substitude_action($rule, { ACCEPT => $accept_action,
REJECT => "PVEFW-reject" });
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
undef, $cluster_conf, $hostfw_conf);
}
};
@@ -2508,8 +2484,8 @@ sub enable_host_firewall {
if ($rule->{type} eq 'group') {
ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule,
'OUT', $accept_action, $ipversion);
} elsif ($rule->{type} eq 'out') {
- ruleset_generat
[pve-devel] [PATCH v4 firewall 04/13] convert string based rule definitions to hashes
also extending %rule with log,logmsg,match,target
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 223
1 file changed, 120 insertions(+), 103 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index ad59267..634ff90 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -142,6 +142,20 @@ my $log_level_hash = {
emerg => 0,
};
+# %rule
+#
+# name => optional
+# action =>
+# proto =>
+# sport =>
+# dport =>
+# log => optional, loglevel
+# logmsg => optional, logmsg - overwrites default
+# iface_in
+# iface_out
+# match => optional, overwrites generation of match
+# target => optional, overwrites action
+
# we need to overwrite some macros for ipv6
my $pve_ipv6fw_macros = {
'Ping' => [
@@ -536,7 +550,7 @@ my $FWACCEPTMARK_OFF = "0x/0x8000";
my $pve_std_chains = {};
$pve_std_chains->{4} = {
'PVEFW-SET-ACCEPT-MARK' => [
- "-j MARK --set-mark $FWACCEPTMARK_ON",
+ { target => "-j MARK --set-mark $FWACCEPTMARK_ON" },
],
'PVEFW-DropBroadcast' => [
# same as shorewall 'Broadcast'
@@ -552,10 +566,10 @@ $pve_std_chains->{4} = {
{ action => 'DROP', dsttype => 'BROADCAST' },
{ action => 'DROP', source => '224.0.0.0/4' },
{ action => 'DROP', proto => 'icmp' },
- "-p tcp -j REJECT --reject-with tcp-reset",
- "-p udp -j REJECT --reject-with icmp-port-unreachable",
- "-p icmp -j REJECT --reject-with icmp-host-unreachable",
- "-j REJECT --reject-with icmp-host-prohibited",
+ { match => '-p tcp', target => '-j REJECT --reject-with tcp-reset' },
+ { match => '-p udp', target => '-j REJECT --reject-with
icmp-port-unreachable' },
+ { match => '-p icmp', target => '-j REJECT --reject-with
icmp-host-unreachable' },
+ { target => '-j REJECT --reject-with icmp-host-prohibited' },
],
'PVEFW-Drop' => [
# same as shorewall 'Drop', which is equal to DROP,
@@ -568,7 +582,7 @@ $pve_std_chains->{4} = {
{ action => 'ACCEPT', proto => 'icmp', dport => 'fragmentation-needed'
},
{ action => 'ACCEPT', proto => 'icmp', dport => 'time-exceeded' },
# Drop packets with INVALID state
- "-m conntrack --ctstate INVALID -j DROP",
+ { action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
{ action => 'DROP', proto => 'udp', dport => '135,445', nbdport => 2 },
{ action => 'DROP', proto => 'udp', dport => '137:139'},
@@ -576,7 +590,7 @@ $pve_std_chains->{4} = {
{ action => 'DROP', proto => 'tcp', dport => '135,139,445', nbdport =>
3 },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
- "-p tcp -m tcp ! --tcp-flags FIN,SYN,RST,ACK SYN -j DROP",
+ { action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
# Drop DNS replies
{ action => 'DROP', proto => 'udp', sport => 53 },
],
@@ -591,7 +605,7 @@ $pve_std_chains->{4} = {
{ action => 'ACCEPT', proto => 'icmp', dport => 'fragmentation-needed'
},
{ action => 'ACCEPT', proto => 'icmp', dport => 'time-exceeded' },
# Drop packets with INVALID state
- "-m conntrack --ctstate INVALID -j DROP",
+ { action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
{ action => 'PVEFW-reject', proto => 'udp', dport => '135,445', nbdport
=> 2 },
{ action => 'PVEFW-reject', proto => 'udp', dport => '137:139'},
@@ -599,111 +613,118 @@ $pve_std_chains->{4} = {
{ action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445',
nbdport => 3 },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
- "-p tcp -m tcp ! --tcp-flags FIN,SYN,RST,ACK SYN -j DROP",
+ { action => 'DROP', match =>
[pve-devel] [PATCH v4 firewall 07/13] iptables address matching in own subroutine
put generation of iptables source/destination address matching
in own subroutine and use this in ruleset_generate_match
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 104
1 file changed, 47 insertions(+), 57 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index f009e58..89a8ef3 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1799,6 +1799,51 @@ sub ipset_get_chains {
return $res;
}
+# generate a src or dst match
+# $dir(ection) is either d or s
+sub ipt_gen_src_or_dst_match {
+my ($adr, $dir, $ipversion, $cluster_conf, $fw_conf) = @_;
+
+my $srcdst;
+if ($dir eq 's') {
+ $srcdst = "src";
+} elsif ($dir eq 'd') {
+ $srcdst = "dst";
+} else {
+ die "ipt_gen_src_or_dst_match: invalid direction $dir \n";
+}
+
+my $match;
+if ($adr =~ m/^\+/) {
+ if ($adr =~ m/^\+(${ipset_name_pattern})$/) {
+ my $name = $1;
+ my $ipset_chain;
+ if ($fw_conf && $fw_conf->{ipset}->{$name}) {
+ $ipset_chain = compute_ipset_chain_name($fw_conf->{vmid},
$name, $ipversion);
+ } elsif ($cluster_conf && $cluster_conf->{ipset}->{$name}) {
+ $ipset_chain = compute_ipset_chain_name(0, $name, $ipversion);
+ } else {
+ die "no such ipset '$name'\n";
+ }
+ $match = "-m set --match-set ${ipset_chain} ${srcdst}";
+ } else {
+ die "invalid security group name '$adr'\n";
+ }
+} elsif ($adr =~ m/^${ip_alias_pattern}$/){
+ my $alias = lc($adr);
+ my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef;
+ $e = $cluster_conf->{aliases}->{$alias} if !$e && $cluster_conf;
+ die "no such alias '$adr'\n" if !$e;
+ $match = "-${dir} $e->{cidr}";
+} elsif ($adr =~ m/\-/){
+ $match = "-m iprange --${srcdst}-range $adr";
+} else {
+ $match = "-${dir} $adr";
+}
+
+return $match;
+}
+
sub ruleset_generate_match {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
@@ -1820,63 +1865,8 @@ sub ruleset_generate_match {
my $source = $rule->{source};
my $dest = $rule->{dest};
-if ($source) {
-if ($source =~ m/^\+/) {
- if ($source =~ m/^\+(${ipset_name_pattern})$/) {
- my $name = $1;
- if ($fw_conf && $fw_conf->{ipset}->{$name}) {
- my $ipset_chain =
compute_ipset_chain_name($fw_conf->{vmid}, $name, $ipversion);
- push @cmd, "-m set --match-set ${ipset_chain} src";
- } elsif ($cluster_conf && $cluster_conf->{ipset}->{$name}) {
- my $ipset_chain = compute_ipset_chain_name(0, $name,
$ipversion);
- push @cmd, "-m set --match-set ${ipset_chain} src";
- } else {
- die "no such ipset '$name'\n";
- }
- } else {
- die "invalid security group name '$source'\n";
- }
- } elsif ($source =~ m/^${ip_alias_pattern}$/){
- my $alias = lc($source);
- my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef;
- $e = $cluster_conf->{aliases}->{$alias} if !$e && $cluster_conf;
- die "no such alias '$source'\n" if !$e;
- push @cmd, "-s $e->{cidr}";
-} elsif ($source =~ m/\-/){
- push @cmd, "-m iprange --src-range $source";
- } else {
- push @cmd, "-s $source";
-}
-}
-
-if ($dest) {
-if ($dest =~ m/^\+/) {
- if ($dest =~ m/^\+(${ipset_name_pattern})$/) {
- my $name = $1;
- if ($fw_conf && $fw_conf->{ipset}->{$name}) {
- my $ipset_chain =
compute_ipset_chain_name($fw_conf->{vmid}, $name, $ipversion);
- push @cmd, "-m set --match-set ${ipset_chain} dst";
- } elsif ($cluster_conf && $cluster_conf->{ipset}->{$name}) {
- my $ipset_chain = compute_ipset_chain_name(0, $name,
$ipversion);
- push @cmd, "-m set --match-set ${ipset_chain} dst";
- } else {
- die "no such ipset '$name'\n";
- }
- } else {
- die "invalid security group name '$dest'\n";
- }
- } elsif ($dest =~ m/^${ip_alias_pattern}$/){
- my $alias = lc($dest);
- my $e = $fw_conf ? $fw_conf->{aliases}->{$alia
[pve-devel] [PATCH v4 firewall 06/13] eliminate unused nbdport in pve_std_chains_conf
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index c7ddd10..f009e58 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -586,10 +586,10 @@ $pve_std_chains_conf->{4} = {
# Drop packets with INVALID state
{ action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
- { action => 'DROP', proto => 'udp', dport => '135,445', nbdport => 2 },
- { action => 'DROP', proto => 'udp', dport => '137:139'},
+ { action => 'DROP', proto => 'udp', dport => '135,445' },
+ { action => 'DROP', proto => 'udp', dport => '137:139' },
{ action => 'DROP', proto => 'udp', dport => '1024:65535', sport => 137
},
- { action => 'DROP', proto => 'tcp', dport => '135,139,445', nbdport =>
3 },
+ { action => 'DROP', proto => 'tcp', dport => '135,139,445' },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
{ action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
@@ -609,10 +609,10 @@ $pve_std_chains_conf->{4} = {
# Drop packets with INVALID state
{ action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
- { action => 'PVEFW-reject', proto => 'udp', dport => '135,445', nbdport
=> 2 },
+ { action => 'PVEFW-reject', proto => 'udp', dport => '135,445' },
{ action => 'PVEFW-reject', proto => 'udp', dport => '137:139'},
{ action => 'PVEFW-reject', proto => 'udp', dport => '1024:65535',
sport => 137 },
- { action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445',
nbdport => 3 },
+ { action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445' },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
{ action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
@@ -682,10 +682,10 @@ $pve_std_chains_conf->{6} = {
# Drop packets with INVALID state
{ action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
- { action => 'DROP', proto => 'udp', dport => '135,445', nbdport => 2 },
+ { action => 'DROP', proto => 'udp', dport => '135,445' },
{ action => 'DROP', proto => 'udp', dport => '137:139'},
{ action => 'DROP', proto => 'udp', dport => '1024:65535', sport => 137
},
- { action => 'DROP', proto => 'tcp', dport => '135,139,445', nbdport =>
3 },
+ { action => 'DROP', proto => 'tcp', dport => '135,139,445' },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
{ action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
@@ -706,10 +706,10 @@ $pve_std_chains_conf->{6} = {
# Drop packets with INVALID state
{ action => 'DROP', match => '-m conntrack --ctstate INVALID', },
# Drop Microsoft SMB noise
- { action => 'PVEFW-reject', proto => 'udp', dport => '135,445', nbdport
=> 2 },
- { action => 'PVEFW-reject', proto => 'udp', dport => '137:139'},
+ { action => 'PVEFW-reject', proto => 'udp', dport => '135,445' },
+ { action => 'PVEFW-reject', proto => 'udp', dport => '137:139' },
{ action => 'PVEFW-reject', proto => 'udp', dport => '1024:65535',
sport => 137 },
- { action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445',
nbdport => 3 },
+ { action => 'PVEFW-reject', proto => 'tcp', dport => '135,139,445' },
{ action => 'DROP', proto => 'udp', dport => 1900 }, # UPnP
# Drop new/NotSyn traffic so that it doesn't get logged
{ action => 'DROP', match => '-p tcp -m tcp ! --tcp-flags
FIN,SYN,RST,ACK SYN' },
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 firewall 12/13] remove unused ruleset_generate_rule_insert
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 12
1 file changed, 12 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 4821759..8d36175 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2070,18 +2070,6 @@ sub ruleset_generate_rule {
}
}
-sub ruleset_generate_rule_insert {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto) = @_;
-
-die "implement me" if $rule->{macro}; # not implemented, because not
needed so far
-
-my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
-my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
-if (defined $match && defined $action) {
- ruleset_insertrule($ruleset, $chain, $match, $action);
-}
-}
-
sub ruleset_create_chain {
my ($ruleset, $chain) = @_;
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 firewall 11/13] cleanup parameters to ruleset_generate_rule
remove $actions and $goto - not used anymore
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 95e00bd..4821759 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2050,7 +2050,7 @@ sub ruleset_generate_action {
}
sub ruleset_generate_rule {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
+my ($ruleset, $chain, $ipversion, $rule, $cluster_conf, $fw_conf) = @_;
my $rules;
@@ -2294,12 +2294,10 @@ sub ruleset_generate_vm_rules {
eval {
if ($direction eq 'OUT') {
rule_substitude_action($rule, { ACCEPT =>
"PVEFW-SET-ACCEPT-MARK", REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
- undef, $cluster_conf, $vmfw_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf, $vmfw_conf);
} else {
rule_substitude_action($rule, { ACCEPT => $in_accept ,
REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
- undef, $cluster_conf, $vmfw_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf, $vmfw_conf);
}
};
warn $@ if $@;
@@ -2428,8 +2426,7 @@ sub enable_host_firewall {
ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule,
'IN', $accept_action, $ipversion);
} elsif ($rule->{type} eq 'in') {
rule_substitude_action($rule, { ACCEPT => $accept_action,
REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
- undef, $cluster_conf, $hostfw_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf, $hostfw_conf);
}
};
warn $@ if $@;
@@ -2485,8 +2482,7 @@ sub enable_host_firewall {
ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule,
'OUT', $accept_action, $ipversion);
} elsif ($rule->{type} eq 'out') {
rule_substitude_action($rule, { ACCEPT => $accept_action,
REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
undef,
- undef, $cluster_conf, $hostfw_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf, $hostfw_conf);
}
};
warn $@ if $@;
@@ -2532,7 +2528,7 @@ sub generate_group_rules {
next if $rule->{type} ne 'in';
next if $rule->{ipversion} && $rule->{ipversion} ne $ipversion;
rule_substitude_action($rule, { ACCEPT => "PVEFW-SET-ACCEPT-MARK",
REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, undef,
undef, $cluster_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf);
}
$chain = "GROUP-${group}-OUT";
@@ -2546,8 +2542,7 @@ sub generate_group_rules {
# we use PVEFW-SET-ACCEPT-MARK (Instead of ACCEPT) because we need to
# check also other tap rules later
rule_substitude_action($rule, { ACCEPT => 'PVEFW-SET-ACCEPT-MARK',
REJECT => "PVEFW-reject" });
- ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, undef,
- undef, $cluster_conf);
+ ruleset_generate_rule($ruleset, $chain, $ipversion, $rule,
$cluster_conf);
}
}
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 firewall 13/13] remove ruleset_generate_match, ruleset_generate_action
ruleset_generate_match and ruleset_generate_action not used anymore
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 97 -
1 file changed, 97 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 8d36175..c858b85 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1952,103 +1952,6 @@ sub ipt_rule_to_cmds {
return @iptcmds;
}
-sub ruleset_generate_match {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
-
-return if defined($rule->{enable}) && !$rule->{enable};
-return if $rule->{errors};
-
-return $rule->{match} if defined $rule->{match};
-
-die "unable to emit macro - internal error" if $rule->{macro}; # should
not happen
-
-my $nbdport = defined($rule->{dport}) ?
parse_port_name_number_or_range($rule->{dport}, 1) : 0;
-my $nbsport = defined($rule->{sport}) ?
parse_port_name_number_or_range($rule->{sport}, 0) : 0;
-
-my @cmd = ();
-
-push @cmd, "-i $rule->{iface_in}" if $rule->{iface_in};
-push @cmd, "-o $rule->{iface_out}" if $rule->{iface_out};
-
-my $source = $rule->{source};
-my $dest = $rule->{dest};
-
-push @cmd, ipt_gen_src_or_dst_match($source, 's', $ipversion,
$cluster_conf, $fw_conf) if $source;
-push @cmd, ipt_gen_src_or_dst_match($dest, 'd', $ipversion, $cluster_conf,
$fw_conf) if $dest;
-
-if (my $proto = $rule->{proto}) {
- push @cmd, "-p $proto";
-
- my $multiport = 0;
- $multiport++ if $nbdport > 1;
- $multiport++ if $nbsport > 1;
-
- push @cmd, "--match multiport" if $multiport;
-
- die "multiport: option '--sports' cannot be used together with
'--dports'\n"
- if ($multiport == 2) && ($rule->{dport} ne $rule->{sport});
-
- if ($rule->{dport}) {
- if ($proto eq 'icmp') {
- # Note: we use dport to store --icmp-type
- die "unknown icmp-type '$rule->{dport}'\n"
- if $rule->{dport} !~ /^\d+$/ &&
!defined($icmp_type_names->{$rule->{dport}});
- push @cmd, "-m icmp --icmp-type $rule->{dport}";
- } elsif ($proto eq 'icmpv6') {
- # Note: we use dport to store --icmpv6-type
- die "unknown icmpv6-type '$rule->{dport}'\n"
- if $rule->{dport} !~ /^\d+$/ &&
!defined($icmpv6_type_names->{$rule->{dport}});
- push @cmd, "-m icmpv6 --icmpv6-type $rule->{dport}";
- } elsif (!$PROTOCOLS_WITH_PORTS->{$proto}) {
- die "protocol $proto does not have ports\n";
- } else {
- if ($nbdport > 1) {
- if ($multiport == 2) {
- push @cmd, "--ports $rule->{dport}";
- } else {
- push @cmd, "--dports $rule->{dport}";
- }
- } else {
- push @cmd, "--dport $rule->{dport}";
- }
- }
- }
-
- if ($rule->{sport}) {
- die "protocol $proto does not have ports\n"
-if !$PROTOCOLS_WITH_PORTS->{$proto};
- if ($nbsport > 1) {
- push @cmd, "--sports $rule->{sport}" if $multiport != 2;
- } else {
- push @cmd, "--sport $rule->{sport}";
- }
- }
-} elsif ($rule->{dport} || $rule->{sport}) {
- die "destination port '$rule->{dport}', but no protocol specified\n" if
$rule->{dport};
- die "source port '$rule->{sport}', but no protocol specified\n" if
$rule->{sport};
-}
-
-push @cmd, "-m addrtype --dst-type $rule->{dsttype}" if $rule->{dsttype};
-
-return scalar(@cmd) ? join(' ', @cmd) : undef;
-}
-
-sub ruleset_generate_action {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
-
-return $rule->{target} if defined $rule->{target};
-
-my @cmd = ();
-
-if (my $action = $rule->{action}) {
- $action = $actions->{$action} if defined($actions->{$action});
- $goto = 1 if !defined($goto) && $action eq 'PVEFW-SET-ACCEPT-MARK';
- push @cmd, $goto ? "-g $action" : "-j $action";
-}
-
-return scalar(@cmd) ? join(' ', @cmd) : undef;
-}
-
sub ruleset_generate_rule {
my ($ruleset, $chain, $ipversion, $rule, $cluster_conf, $fw_conf) = @_;
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 firewall 00/13] Firewall code cleanups
4th version. third version with signed off lines make $pve_std_chains a copy of $pve_std_chains_conf is optional Tom Weber (13): remove unused $rule_format prepare code for more generic firewall logging integrate logging into ruleset_addrule convert string based rule definitions to hashes make $pve_std_chains a copy of $pve_std_chains_conf eliminate unused nbdport in pve_std_chains_conf iptables address matching in own subroutine implement ipt_rule_to_cmds, ruleset_add_ipt_cmd remove unused ruleset_generate_cmdstr rule_substitude_action, remove ruleset_generate_rule_old cleanup parameters to ruleset_generate_rule remove unused ruleset_generate_rule_insert remove ruleset_generate_match, ruleset_generate_action src/PVE/Firewall.pm | 674 ++- src/PVE/FirewallSimulator.pm | 2 +- 2 files changed, 342 insertions(+), 334 deletions(-) -- 2.7.4 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 firewall 09/13] remove unused ruleset_generate_cmdstr
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 11 ---
1 file changed, 11 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index b492086..633aa7a 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2040,17 +2040,6 @@ sub ruleset_generate_action {
return scalar(@cmd) ? join(' ', @cmd) : undef;
}
-sub ruleset_generate_cmdstr {
-my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
-my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
-my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
-
-return undef if !(defined($match) or defined($action));
-my $ret = defined($match) ? $match : "";
-$ret = "$ret $action" if defined($action);
-return $ret;
-}
-
sub ruleset_generate_rule {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 firewall 01/13] remove unused $rule_format
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index cc81325..5d78686 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1648,8 +1648,6 @@ sub enable_bridge_firewall {
$bridge_firewall_enabled = 1;
}
-my $rule_format = "%-15s %-30s %-30s %-15s %-15s %-15s\n";
-
sub iptables_restore_cmdlist {
my ($cmdlist) = @_;
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 firewall 02/13] prepare code for more generic firewall logging
making ruleset generation aware of a match and action
part in iptable rules.
code will generate the same iptables as before! (except for
a few additional spaces between match and action).
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 166 ++-
src/PVE/FirewallSimulator.pm | 2 +-
2 files changed, 100 insertions(+), 68 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 5d78686..f1aecef 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1776,7 +1776,7 @@ sub ipset_get_chains {
return $res;
}
-sub ruleset_generate_cmdstr {
+sub ruleset_generate_match {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
return if defined($rule->{enable}) && !$rule->{enable};
@@ -1907,6 +1907,14 @@ sub ruleset_generate_cmdstr {
push @cmd, "-m addrtype --dst-type $rule->{dsttype}" if $rule->{dsttype};
+return scalar(@cmd) ? join(' ', @cmd) : undef;
+}
+
+sub ruleset_generate_action {
+my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
+
+my @cmd = ();
+
if (my $action = $rule->{action}) {
$action = $actions->{$action} if defined($actions->{$action});
$goto = 1 if !defined($goto) && $action eq 'PVEFW-SET-ACCEPT-MARK';
@@ -1916,6 +1924,17 @@ sub ruleset_generate_cmdstr {
return scalar(@cmd) ? join(' ', @cmd) : undef;
}
+sub ruleset_generate_cmdstr {
+my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
+my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
+my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto, $cluster_conf, $fw_conf);
+
+return undef if !(defined($match) or defined($action));
+my $ret = defined($match) ? $match : "";
+$ret = "$ret $action" if defined($action);
+return $ret;
+}
+
sub ruleset_generate_rule {
my ($ruleset, $chain, $ipversion, $rule, $actions, $goto, $cluster_conf,
$fw_conf) = @_;
@@ -1929,15 +1948,19 @@ sub ruleset_generate_rule {
# update all or nothing
-my @cmds = ();
+my @mstrs = ();
+my @astrs = ();
foreach my $tmp (@$rules) {
- if (my $cmdstr = ruleset_generate_cmdstr($ruleset, $chain, $ipversion,
$tmp, $actions, $goto, $cluster_conf, $fw_conf)) {
- push @cmds, $cmdstr;
+ my $m = ruleset_generate_match($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
+ my $a = ruleset_generate_action($ruleset, $chain, $ipversion, $tmp,
$actions, $goto, $cluster_conf, $fw_conf);
+ if (defined $m or defined $a) {
+ push @mstrs, defined($m) ? $m : "";
+ push @astrs, defined($a) ? $a : "";
}
}
-foreach my $cmdstr (@cmds) {
- ruleset_addrule($ruleset, $chain, $cmdstr);
+for my $i (0 .. $#mstrs) {
+ ruleset_addrule($ruleset, $chain, $mstrs[$i], $astrs[$i]);
}
}
@@ -1946,8 +1969,10 @@ sub ruleset_generate_rule_insert {
die "implement me" if $rule->{macro}; # not implemented, because not
needed so far
-if (my $cmdstr = ruleset_generate_cmdstr($ruleset, $chain, $ipversion,
$rule, $actions, $goto)) {
- ruleset_insertrule($ruleset, $chain, $cmdstr);
+my $match = ruleset_generate_match($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
+my $action = ruleset_generate_action($ruleset, $chain, $ipversion, $rule,
$actions, $goto);
+if (defined $match && defined $action) {
+ ruleset_insertrule($ruleset, $chain, $match, $action);
}
}
@@ -1968,7 +1993,7 @@ sub ruleset_chain_exist {
return $ruleset->{$chain} ? 1 : undef;
}
-sub ruleset_addrule {
+sub ruleset_addrule_old {
my ($ruleset, $chain, $rule) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
@@ -1976,12 +2001,20 @@ sub ruleset_addrule {
push @{$ruleset->{$chain}}, "-A $chain $rule";
}
+sub ruleset_addrule {
+ my ($ruleset, $chain, $match, $action, $log) = @_;
+
+ die "no such chain '$chain'\n" if !$ruleset->{$chain};
+
+ push @{$ruleset->{$chain}}, "-A $chain $match $action";
+}
+
sub ruleset_insertrule {
- my ($ruleset, $chain, $rule) = @_;
+ my ($ruleset, $chain, $match, $action, $log) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
- unshift @{$ruleset->{$chain}}, "-A $chain $rule";
+ unshift @{$ruleset->{$chain}}, "-A $chain $match $action";
}
sub get_log_rule_base {
@@ -1998,15 +2031,14 @@ sub get_log_rule_base {
}
sub ruleset_addlog {
-my ($ruleset, $chain, $vmid, $msg, $loglevel, $rule) = @_;
+my ($ruleset,
[pve-devel] [PATCH v4 firewall 03/13] integrate logging into ruleset_addrule
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 33 ++---
1 file changed, 10 insertions(+), 23 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index f1aecef..ad59267 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2002,10 +2002,14 @@ sub ruleset_addrule_old {
}
sub ruleset_addrule {
- my ($ruleset, $chain, $match, $action, $log) = @_;
+ my ($ruleset, $chain, $match, $action, $log, $logmsg, $vmid) = @_;
die "no such chain '$chain'\n" if !$ruleset->{$chain};
+ if (defined($log) && $log) {
+ my $logaction = get_log_rule_base($chain, $vmid, $logmsg, $log);
+ push @{$ruleset->{$chain}}, "-A $chain $match $logaction";
+ }
push @{$ruleset->{$chain}}, "-A $chain $match $action";
}
@@ -2020,27 +2024,15 @@ sub ruleset_insertrule {
sub get_log_rule_base {
my ($chain, $vmid, $msg, $loglevel) = @_;
-die "internal error - no log level" if !defined($loglevel);
-
$vmid = 0 if !defined($vmid);
+$msg = "" if !defined($msg);
# Note: we use special format for prefix to pass further
-# info to log daemon (VMID, LOGVELEL and CHAIN)
+# info to log daemon (VMID, LOGLEVEL and CHAIN)
return "-j NFLOG --nflog-prefix \":$vmid:$loglevel:$chain: $msg\"";
}
-sub ruleset_addlog {
-my ($ruleset, $chain, $vmid, $msg, $loglevel, $match) = @_;
-
-return if !defined($loglevel);
-
-my $logaction = get_log_rule_base($chain, $vmid, $msg, $loglevel);
-
-$match = "" if !defined $match;
-ruleset_addrule($ruleset, $chain, $match, $logaction);
-}
-
sub ruleset_add_chain_policy {
my ($ruleset, $chain, $ipversion, $vmid, $policy, $loglevel,
$accept_action) = @_;
@@ -2053,15 +2045,11 @@ sub ruleset_add_chain_policy {
ruleset_addrule($ruleset, $chain, "", "-j PVEFW-Drop");
- ruleset_addlog($ruleset, $chain, $vmid, "policy $policy: ", $loglevel);
-
- ruleset_addrule($ruleset, $chain, "", "-j DROP");
+ ruleset_addrule($ruleset, $chain, "", "-j DROP", $loglevel, "policy
$policy: ", $vmid);
} elsif ($policy eq 'REJECT') {
ruleset_addrule($ruleset, $chain, "", "-j PVEFW-Reject");
- ruleset_addlog($ruleset, $chain, $vmid, "policy $policy: ", $loglevel);
-
- ruleset_addrule($ruleset, $chain, "", "-g PVEFW-reject");
+ ruleset_addrule($ruleset, $chain, "", "-g PVEFW-reject", $loglevel,
"policy $policy:", $vmid);
} else {
# should not happen
die "internal error: unknown policy '$policy'";
@@ -2093,8 +2081,7 @@ sub ruleset_chain_add_input_filters {
if ($cluster_conf->{ipset}->{blacklist}){
if (!ruleset_chain_exist($ruleset, "PVEFW-blacklist")) {
ruleset_create_chain($ruleset, "PVEFW-blacklist");
- ruleset_addlog($ruleset, "PVEFW-blacklist", 0, "DROP: ", $loglevel)
if $loglevel;
- ruleset_addrule($ruleset, "PVEFW-blacklist", "", "-j DROP");
+ ruleset_addrule($ruleset, "PVEFW-blacklist", "", "-j DROP",
$loglevel, "DROP: ");
}
my $ipset_chain = compute_ipset_chain_name(0, 'blacklist', $ipversion);
ruleset_addrule($ruleset, $chain, "-m set --match-set ${ipset_chain}
src", "-j PVEFW-blacklist");
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 firewall 05/13] make $pve_std_chains a copy of $pve_std_chains_conf
create a new $pve_std_chains with $pve_std_chains_conf as template on
every compilation of the rules. This avoids persitant changes to the
$pve_std_chains and makes it easier to read the std_chains configuration
from external config files (later to implement).
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 634ff90..c7ddd10 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -20,6 +20,7 @@ use IO::File;
use Net::IP;
use PVE::Tools qw(run_command lock_file dir_glob_foreach);
use Encode;
+use Storable qw(dclone);
my $hostfw_conf_filename = "/etc/pve/local/host.fw";
my $pvefw_conf_dir = "/etc/pve/firewall";
@@ -548,7 +549,8 @@ my $FWACCEPTMARK_ON = "0x8000/0x8000";
my $FWACCEPTMARK_OFF = "0x/0x8000";
my $pve_std_chains = {};
-$pve_std_chains->{4} = {
+my $pve_std_chains_conf = {};
+$pve_std_chains_conf->{4} = {
'PVEFW-SET-ACCEPT-MARK' => [
{ target => "-j MARK --set-mark $FWACCEPTMARK_ON" },
],
@@ -641,7 +643,7 @@ $pve_std_chains->{4} = {
],
};
-$pve_std_chains->{6} = {
+$pve_std_chains_conf->{6} = {
'PVEFW-SET-ACCEPT-MARK' => [
{ target => "-j MARK --set-mark $FWACCEPTMARK_ON" },
],
@@ -3354,6 +3356,9 @@ sub compile {
my $vmfw_configs;
+# fixme: once we read standard chains from config this needs to be put in
test/standard cases below
+$pve_std_chains = dclone($pve_std_chains_conf);
+
if ($vmdata) { # test mode
my $testdir = $vmdata->{testdir} || die "no test directory specified";
my $filename = "$testdir/cluster.fw";
--
2.7.4
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v4 firewall 08/13] implement ipt_rule_to_cmds, ruleset_add_ipt_cmd
ipt_rule_to_cmds converts a %rule to an array of iptables commands
ruleset_add_ipt_cmd adds such an iptables command to a chain
ruleset_generate_rule uses these now
ruleset_generate_rule_old is an interim workaround
Signed-off-by: Tom Weber
---
src/PVE/Firewall.pm | 151
1 file changed, 140 insertions(+), 11 deletions(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 89a8ef3..b492086 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -146,14 +146,15 @@ my $log_level_hash = {
# %rule
#
# name => optional
+# enable => [0|1]
# action =>
# proto =>
-# sport =>
-# dport =>
+# sport => port[,port[,port]].. or port:port
+# dport => port[,port[,port]].. or port:port
# log => optional, loglevel
# logmsg => optional, logmsg - overwrites default
-# iface_in
-# iface_out
+# iface_in => incomin interface
+# iface_out => outgoing interface
# match => optional, overwrites generation of match
# target => optional, overwrites action
@@ -1844,6 +1845,104 @@ sub ipt_gen_src_or_dst_match {
return $match;
}
+# convert a %rule to an array of iptables commands
+sub ipt_rule_to_cmds {
+my ($rule, $chain, $ipversion, $cluster_conf, $fw_conf, $vmid) = @_;
+
+die "ipt_rule_to_cmds unable to handle macro" if $rule->{macro}; #should
not happen
+
+my @match = ();
+
+if (defined $rule->{match}) {
+ push @match, $rule->{match};
+} else {
+ push @match, "-i $rule->{iface_in}" if $rule->{iface_in};
+ push @match, "-o $rule->{iface_out}" if $rule->{iface_out};
+
+ if ($rule->{source}) {
+ push @match, ipt_gen_src_or_dst_match($rule->{source}, 's',
$ipversion, $cluster_conf, $fw_conf);
+ }
+ if ($rule->{dest}) {
+ push @match, ipt_gen_src_or_dst_match($rule->{dest}, 'd',
$ipversion, $cluster_conf, $fw_conf);
+ }
+
+ if (my $proto = $rule->{proto}) {
+ push @match, "-p $proto";
+
+ my $nbdport = defined($rule->{dport}) ?
parse_port_name_number_or_range($rule->{dport}, 1) : 0;
+ my $nbsport = defined($rule->{sport}) ?
parse_port_name_number_or_range($rule->{sport}, 0) : 0;
+
+ my $multiport = 0;
+ $multiport++ if $nbdport > 1;
+ $multiport++ if $nbsport > 1;
+
+ push @match, "--match multiport" if $multiport;
+
+ die "multiport: option '--sports' cannot be used together with
'--dports'\n"
+ if ($multiport == 2) && ($rule->{dport} ne $rule->{sport});
+
+ if ($rule->{dport}) {
+ if ($proto eq 'icmp') {
+ # Note: we use dport to store --icmp-type
+ die "unknown icmp-type '$rule->{dport}'\n"
+ if $rule->{dport} !~ /^\d+$/ &&
!defined($icmp_type_names->{$rule->{dport}});
+ push @match, "-m icmp --icmp-type $rule->{dport}";
+ } elsif ($proto eq 'icmpv6') {
+ # Note: we use dport to store --icmpv6-type
+ die "unknown icmpv6-type '$rule->{dport}'\n"
+ if $rule->{dport} !~ /^\d+$/ &&
!defined($icmpv6_type_names->{$rule->{dport}});
+ push @match, "-m icmpv6 --icmpv6-type $rule->{dport}";
+ } elsif (!$PROTOCOLS_WITH_PORTS->{$proto}) {
+ die "protocol $proto does not have ports\n";
+ } else {
+ if ($nbdport > 1) {
+ if ($multiport == 2) {
+ push @match, "--ports $rule->{dport}";
+ } else {
+ push @match, "--dports $rule->{dport}";
+ }
+ } else {
+ push @match, "--dport $rule->{dport}";
+ }
+ }
+ }
+
+ if ($rule->{sport}) {
+ die "protocol $proto does not have ports\n"
+ if !$PROTOCOLS_WITH_PORTS->{$proto};
+ if ($nbsport > 1) {
+ push @match, "--sports $rule->{sport}" if $multiport != 2;
+ } else {
+ push @match, "--sport $rule->{sport}";
+ }
+ }
+ } elsif ($rule->{dport} || $rule->{sport}) {
+ die "destination port '$rule->{dport}', but no protocol
specified\n" if $rule->{dport};
+ die "source port '$rule->{sport}', but no protocol specified\n" if
$rule->{sport};
+ }
+
Re: [pve-devel] pve-firewall : nftables ?
Am Dienstag, den 27.11.2018, 14:55 +0100 schrieb Wolfgang Bumiller: > The pve-firewall code is very iptables-oriented though, and I'm not > sure > if maybe we're not better off splitting the rule-generating part out > and write the nftables variant from scratch... The iptables part > would > be considered feature-frozen from that point on I'd say/hope/think... Yes, I think in the long term rule generation really needs to be separated completely from rule definition. Right now there's a lot of implicit iptable rule generation inside pve-firewall, which makes it a real pain. Just to throw in another idea: How about using something like shorewall (shorewall.net) to handle the whole firewall generation code from a higher level. I'm using it for in really complex setups for years and i am very happy with it. (I know this won't solve the nftables problem right now). Tom ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] firewall: Razor Macro broken and opens Firewall
Hi, in the middle of a weekend migration i realized that the 'Razor'
Macro is broken and basically disables ALL firewalling for a Container,
at least when used in a Security Group.
Looking at Firewall.pm
..
'RNDC' => [
"BIND remote management protocol",
{ action => 'PARAM', proto => 'tcp', dport => '953' },
],
'Razor' => [
"Razor Antispam System",
{ action => 'ACCEPT', proto => 'tcp', dport => '2703' },
],
'Rdate' => [
"Remote time retrieval (rdate)",
{ action => 'PARAM', proto => 'tcp', dport => '37' },
],
..
The Problem seems obvious (might have even missed that one myself when
I was working on this some time ago).
As mentioned, I'm in the middle of a bigger migration so just a short
notice and no patch (fix seems obvious)...
I consider this serious because it silently disables ALL firewalling
(at least for me). Even though Razor Macro probably isn't used often.
Regards,
Tom
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] rfc : pve-network : idea to generate and reload config accross the nodes
Am Mittwoch, den 03.04.2019, 07:03 +0200 schrieb Dietmar Maurer: > > I think, something easy, is that we could have a copy of each > > /etc/network/interfaces of each node in > > /etc/pve/nodes//interfaces. > > (could be done we a change is done in gui local netowrk, or local > > network daemon copy it at regular interval in case of manual change > > for example). > > > > > > Like this, it's very easy, when a network change is one at > > datacenter level, we can directly test it on all network interfaces > > of all nodes ( /etc/pve/nodes/*/interfaces). (in the api endpoint), /etc/network/interfaces is only a small part of actual network configuration. > I is still unclear to me how you do those tests? AFAIK, ifreload does > not have a --dry-run option. Even when it has such option, it would > need access to the local node? (to see what interfaces exists, ...). > > So if you really need/want to test before apply, we could add and API > call for that: > > POST /api2/json/nodes//test_network_changes > > We can then add a TEST button to the GUI, or call those this test API > on all nodes before we apply changes. > > > and then write directly the conf. (no need vnet.new tmp file). > > I think network configuration is really complex, and we should avoid > to do anything automatically. > I would prefer and "APPLY" button, so that I have full control over > when network changes happen. > Maybe an extra "TEST" button would be also helpful. Probably helpful but as you said, network configuration can be really complex (good luck finding my tinc bridges in the interfaces file - let alone developing a test). Why not have a static (host specific) part that never ever gets touched by pve. Usually all thats needed to get the node into the cluster. Additional parts can be managed via the cluster - selectable on which nodes (including all) to apply. If you select/mark a node, try to apply, if that fails, fall back to your static basic configuration above and show errors so the admin can fix, ideally with the node still in the cluster. So you can test on single test-nodes if you need or apply on all if you're sure about your changes. We don't try to prevent shooting our feet at other places either (like firewalling). Best, Tom ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC container] mountpoints: create parent dirs with correct owner
it seems like you're working in an area of code that could also be
relevant for my (small) problem from:
https://pve.proxmox.com/pipermail/pve-devel/2017-September/028814.html
https://pve.proxmox.com/pipermail/pve-devel/2017-October/029004.html
or maybe this could also be a problem if your mps are owned by user not
root in an unprivileged container.
i didn't really dig into your patch, just a hint.
Regards,
Tom
Am Mittwoch, den 24.07.2019, 13:37 +0200 schrieb Fabian Grünbichler:
> otherwise unprivileged containers might end up with directories that
> they cannot modify since they are owned by the user root in the host
> namespace, instead of root inside the container.
>
> note: the problematic behaviour is only exhibited when an
> intermediate
> directory needs to be created, e.g. a mountpoint /test/mp gets
> mounted,
> and /test does not yet exist.
>
> Signed-off-by: Fabian Grünbichler
> ---
> Notes:
> requires fchownat support in PVE::Tools - see other patch and
> bump
> build-depends + depends accordingly after applying!
>
> I am not sure whether this is 100% correct w.r.t. error edge
> cases, since we
> potentially die after mkdirat without calling fchownat. it is for
> sure better
> than the status quo though ;)
>
> thank you Dietmar for noticing the buggy behaviour!
>
> src/PVE/LXC.pm| 27 ---
> src/PVE/VZDump/LXC.pm | 7 +--
> src/lxc-pve-prestart-hook | 4 +++-
> 3 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index 2252685..0a1d95a 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1177,13 +1177,15 @@ sub mount_all {
> my $volid_list = PVE::LXC::Config->get_vm_volumes($conf);
> PVE::Storage::activate_volumes($storage_cfg, $volid_list);
>
> +my (undef, $rootuid, $rootgid) = parse_id_maps($conf);
> +
> eval {
> PVE::LXC::Config->foreach_mountpoint($conf, sub {
> my ($ms, $mountpoint) = @_;
>
> $mountpoint->{ro} = 0 if $ignore_ro;
>
> - mountpoint_mount($mountpoint, $rootdir, $storage_cfg);
> + mountpoint_mount($mountpoint, $rootdir, $storage_cfg,
> undef, $rootuid, $rootgid);
> });
> };
> if (my $err = $@) {
> @@ -1258,8 +1260,8 @@ sub run_with_loopdev {
> # * the 2nd deepest directory (parent of the above)
> # * directory name of the last directory
> # So that the path $2/$3 should lead to $1 afterwards.
> -sub walk_tree_nofollow($$$) {
> -my ($start, $subdir, $mkdir) = @_;
> +sub walk_tree_nofollow($$$;$$) {
> +my ($start, $subdir, $mkdir, $rootuid, $rootgid) = @_;
>
> # splitdir() returns '' for empty components including the
> leading /
> my @comps = grep { length($_)>0 } File::Spec->splitdir($subdir);
> @@ -1288,6 +1290,9 @@ sub walk_tree_nofollow($$$) {
>
> $next = PVE::Tools::openat(fileno($fd), $component,
> O_NOFOLLOW | O_DIRECTORY);
> die "failed to create path: $dir: $!\n" if !$next;
> +
> + PVE::Tools::fchownat(fileno($next), '', $rootuid, $rootgid,
> PVE::Tools::AT_EMPTY_PATH)
> + if defined($rootuid) && defined($rootgid);
> }
>
> close $second if defined($last_component);
> @@ -1381,17 +1386,17 @@ sub bindmount {
> # from $rootdir and $mount and walk the path from $rootdir to the
> final
> # directory to check for symlinks.
> sub __mount_prepare_rootdir {
> -my ($rootdir, $mount) = @_;
> +my ($rootdir, $mount, $rootuid, $rootgid) = @_;
> $rootdir =~ s!/+!/!g;
> $rootdir =~ s!/+$!!;
> my $mount_path = "$rootdir/$mount";
> -my ($mpfd, $parentfd, $last_dir) = walk_tree_nofollow($rootdir,
> $mount, 1);
> +my ($mpfd, $parentfd, $last_dir) = walk_tree_nofollow($rootdir,
> $mount, 1, $rootuid, $rootgid);
> return ($rootdir, $mount_path, $mpfd, $parentfd, $last_dir);
> }
>
> # use $rootdir = undef to just return the corresponding mount path
> sub mountpoint_mount {
> -my ($mountpoint, $rootdir, $storage_cfg, $snapname) = @_;
> +my ($mountpoint, $rootdir, $storage_cfg, $snapname, $rootuid,
> $rootgid) = @_;
>
> my $volid = $mountpoint->{volume};
> my $mount = $mountpoint->{mp};
> @@ -1408,7 +1413,7 @@ sub mountpoint_mount {
>
> if (defined($rootdir)) {
> ($rootdir, $mount_path, $mpfd, $parentfd, $last_dir) =
> - __mount_prepare_rootdir($rootdir, $mount);
> + __mount_prepare_rootdir($rootdir, $mount, $rootuid,
> $rootgid);
> }
>
> my ($storage, $volname) = PVE::Storage::parse_volume_id($volid,
> 1);
> @@ -2007,7 +2012,7 @@ sub run_unshared {
> }
>
> my $copy_volume = sub {
> -my ($src_volid, $src, $dst_volid, $dest, $storage_cfg,
> $snapname, $bwlimit) = @_;
> +my ($src_volid, $src, $dst_volid, $dest, $storage_cfg,
> $snapname, $bwlimit, $rootuid, $rootgid) = @_;
>
> my $src_mp = { volume => $src_volid, mp => '/', ro => 1 };
> $src_mp->{type}
