[pmg-devel] [PATCH WIP api/gui] Extend rule system

[pmg-devel] [PATCH WIP api/gui] Extend rule system

[Leo Nunner]

This is a WIP series to rework the PMG rule system by adding the
capabiliy to negate and group objects inside rules. Negated objects will
only match if their conditions are not met.

Match groups allow objects of the same type to be grouped together. A
match group evaluates like one big object, only evaluating as a match if
all objects that are contained inside the group match. In contrast to
previous versions, it is now possible to have multiple match groups for
each object type. They can also be named to make their purpose clearer
at first glance.

Match groups can be visualized as follows:

Rule
    - 'Who' Objects
	- Match group
	    Object 1
	    Object 2
	    Object 3
	- Object 4

The 'Who' objects will only match if either (Object 1 AND Object 2 AND
Object 3) evaluates to true, or if Object 4 evaluates to true. This
means that match groups are connected via logical OR to all other
top-level objects inside the rule, while all objects contained inside
the group are connected via logical AND.

Since this series is still WIP and has been changed since the last
iteration, some things are still missing:
    - The matching logic for negation needs proper implementation
	The last iteration of this series had some fundamental errors in
	how negation was handled for some object types. Especially for
	'What' objects it can be rather hard to define what negation
	entails.
    - The matching logic for match groups
	Since we now have multiple match groups for each object type,
	the matching logic needs to be adapted accordingly. This could
	be achieved by first going through all objects which do not have
	their MatchGroup field set, and then going through all match
	groups associated with that rule (the facilities for which are
	already there).
    - UI Tweaks
	The UI for match groups still needs some tweaks, such as edit
	functionality for match groups (which also still needs to be
	implemented in the API), reloading the tree after adding a
	group, and maybe changing some icons (negation comes to mind).

pmg-api:

Leo Nunner (11):
  negation: add field to database
  negation: parse negation value into objects
  negation: expand/implement API endpoints
  negation: implement matching logic
  match groups: update database schema
  match groups: add functions for database access
  match groups: parse field into objects
  match groups: add API endpoints for create/delete
  match groups: list match groups in rule API
  match groups: update existing object API endpoints
  match groups: implement matching logic

 src/PMG/API2/ObjectGroupHelpers.pm     |  10 +-
 src/PMG/API2/RuleDB.pm                 |   4 +-
 src/PMG/API2/Rules.pm                  | 202 ++++++++++++++++++++++++-
 src/PMG/DBTools.pm                     |  40 +++++
 src/PMG/RuleCache.pm                   |  61 ++++++--
 src/PMG/RuleDB.pm                      | 131 +++++++++++++++-
 src/PMG/RuleDB/ArchiveFilter.pm        |   6 +-
 src/PMG/RuleDB/ContentTypeFilter.pm    |   6 +-
 src/PMG/RuleDB/MatchArchiveFilename.pm |   4 +-
 src/PMG/RuleDB/MatchField.pm           |   2 +-
 src/PMG/RuleDB/MatchFilename.pm        |   2 +-
 11 files changed, 445 insertions(+), 23 deletions(-)

pmg-gui:

Leo Nunner (2):
  negate objects inside rules
  introduce logical 'and' for rules

 css/ext6-pmg.css |  20 +++
 js/RuleInfo.js   | 433 +++++++++++++++++++++++++++++++++++++++--------
 js/Utils.js      |  14 +-
 3 files changed, 391 insertions(+), 76 deletions(-)

-- 
2.39.2

[pmg-devel] [PATCH WIP api 01/11] negation: add field to database

[Leo Nunner]

Add a 'Negation field to the RuleGroup table.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PMG/DBTools.pm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/PMG/DBTools.pm b/src/PMG/DBTools.pm
index 9e133bc..81d7fb3 100644
--- a/src/PMG/DBTools.pm
+++ b/src/PMG/DBTools.pm
@@ -417,6 +417,7 @@ sub create_ruledb {
           Objectgroup_ID INTEGER NOT NULL,
           Rule_ID INTEGER NOT NULL,
           Grouptype INTEGER NOT NULL,
+          Negate INTEGER NOT NULL DEFAULT 0,
           PRIMARY KEY (Objectgroup_ID, Rule_ID, Grouptype)
         );
 
@@ -579,6 +580,19 @@ sub upgradedb {
 	}
     }
 
+    # Allow negating rule objects
+    if (!database_column_exists($dbh, 'RuleGroup', 'Negate')) {
+	eval {
+	    $dbh->begin_work;
+	    $dbh->do("ALTER TABLE RuleGroup ADD COLUMN Negate INTEGER NOT NULL DEFAULT 0");
+	    $dbh->commit;
+	};
+	if (my $err = $@) {
+	    $dbh->rollback;
+	    die $err;
+	}
+    }
+
     foreach my $table (keys %$tables) {
 	eval { $dbh->do("ANALYZE $table"); };
 	warn $@ if $@;
-- 
2.39.2

[pmg-devel] [PATCH WIP api 02/11] negation: parse negation value into objects

[Leo Nunner]

Expand the parsing code so that the 'negate' field is written into
objects for further handling. Two new functions are introduced: one for
reading a specific object-rule mapping (and its values), the other to
set the 'negate' value for such an object.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PMG/API2/ObjectGroupHelpers.pm |  5 +++-
 src/PMG/RuleCache.pm               |  7 ++++-
 src/PMG/RuleDB.pm                  | 46 +++++++++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/src/PMG/API2/ObjectGroupHelpers.pm b/src/PMG/API2/ObjectGroupHelpers.pm
index 48078fb..c3e6448 100644
--- a/src/PMG/API2/ObjectGroupHelpers.pm
+++ b/src/PMG/API2/ObjectGroupHelpers.pm
@@ -47,7 +47,10 @@ sub format_object_group {
     my $res = [];
     foreach my $og (@$ogroups) {
 	push @$res, {
-	    id => $og->{id}, name => $og->{name}, info => $og->{info}
+	    id => $og->{id},
+	    name => $og->{name},
+	    info => $og->{info},
+	    negate => $og->{negate},
 	};
     }
     return $res;
diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index b8690ea..c5a57f6 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -56,7 +56,7 @@ sub new {
 	    my ($from, $to, $when, $what, $action);
 
 	    my $sth1 = $dbh->prepare(
-		"SELECT Objectgroup_ID, Grouptype FROM RuleGroup " .
+		"SELECT Objectgroup_ID, Grouptype, Negate FROM RuleGroup " .
 		"where RuleGroup.Rule_ID = '$ruleid' " .
 		"ORDER BY Grouptype, Objectgroup_ID");
 
@@ -64,6 +64,7 @@ sub new {
 	    while (my $ref1 = $sth1->fetchrow_hashref()) {
 		my $gtype = $ref1->{grouptype};
 		my $groupid = $ref1->{objectgroup_id};
+		my $negate = $ref1->{negate};
 
 		# emtyp groups differ from non-existent groups!
 
@@ -90,6 +91,10 @@ sub new {
 		    $sha1->add (join (',', $objid, $gtype, $groupid) . "|");
 		    $sha1->add ($obj->{digest}, "|");
 
+		    if ($gtype != 4) { # it doesn't make any sense to negate actions
+			$obj->{negate} = $negate;
+		    }
+
 		    if ($gtype == 0) {      #from
 			push @$from, $obj;
 		    } elsif ($gtype == 1) { # to
diff --git a/src/PMG/RuleDB.pm b/src/PMG/RuleDB.pm
index a6b0b79..98beda3 100644
--- a/src/PMG/RuleDB.pm
+++ b/src/PMG/RuleDB.pm
@@ -107,7 +107,7 @@ sub load_groups {
 
     my $sth = $self->{dbh}->prepare(
 	"SELECT RuleGroup.Grouptype, Objectgroup.ID, " .
-	"Objectgroup.Name, Objectgroup.Info " .
+	"Objectgroup.Name, Objectgroup.Info, Negate " .
 	"FROM Rulegroup, Objectgroup " .
 	"WHERE Rulegroup.Rule_ID = ? and " .
 	"Rulegroup.Objectgroup_ID = Objectgroup.ID " .
@@ -123,6 +123,10 @@ sub load_groups {
 	my $og = PMG::RuleDB::Group->new($ref->{name}, $ref->{info});
 	$og->{id} = $ref->{id};
 
+	if ($ref->{'grouptype'} != 4) { # this doesn't make any sense for actions
+	    $og->{negate} = $ref->{negate};
+	}
+
 	if ($ref->{'grouptype'} == 0) {      #from
 	    push @$from, $og;
 	} elsif ($ref->{'grouptype'} == 1) { # to
@@ -753,6 +757,46 @@ sub rule_remove_group {
     return 1;
 }
 
+
+sub rule_get_group_settings {
+    my ($self, $ruleid, $groupid, $gtype_str) = @_;
+
+    my $gtype = $grouptype_hash->{$gtype_str} //
+	die "unknown group type '$gtype_str'\n";
+
+    defined($ruleid) || die "undefined rule id: ERROR";
+    defined($groupid) || die "undefined group id: ERROR";
+    defined($gtype) || die "undefined group type: ERROR";
+
+    my $sth = $self->{dbh}->prepare("SELECT * FROM RuleGroup WHERE " .
+		     "Objectgroup_ID = ? and Rule_ID = ? and Grouptype = ?");
+
+    $sth->execute($groupid, $ruleid, $gtype);
+
+    my $ref = $sth->fetchrow_hashref();
+    die "rule does not exist\n" if !defined($ref);
+
+    return $ref;
+}
+
+sub rule_set_group_setting_negate {
+    my ($self, $value, $ruleid, $groupid, $gtype_str) = @_;
+
+    my $gtype = $grouptype_hash->{$gtype_str} //
+	die "unknown group type '$gtype_str'\n";
+
+    defined($ruleid) || die "undefined rule id: ERROR";
+    defined($groupid) || die "undefined group id: ERROR";
+    defined($gtype) || die "undefined group type: ERROR";
+
+    my $sth = $self->{dbh}->prepare("UPDATE RuleGroup SET Negate = ? " .
+		     "WHERE Objectgroup_ID = ? and Rule_ID = ? and Grouptype = ?");
+
+    $sth->execute($value, $groupid, $ruleid, $gtype);
+
+    return 1;
+}
+
 sub load_rule {
     my ($self, $id) = @_;
 
-- 
2.39.2

[pmg-devel] [PATCH WIP api 03/11] negation: expand/implement API endpoints

[Leo Nunner]

The existing 'add' enpoint was expanded with a 'negate' parameter, so
that new objects can be added in an already negated state. New API
endpoints were added to get and update the specific group-rule relation
settings (which is only 'negate' for now). These endpoints get added to
all ogroups except 'actions', since negating actions doesn't make much
sense.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PMG/API2/Rules.pm | 96 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/src/PMG/API2/Rules.pm b/src/PMG/API2/Rules.pm
index 4f8c10b..ad3f6c7 100644
--- a/src/PMG/API2/Rules.pm
+++ b/src/PMG/API2/Rules.pm
@@ -261,6 +261,11 @@ my $register_rule_group_api = sub {
 		    description => "Groups ID.",
 		    type => 'integer',
 		},
+		negate => {
+		    description => "Negate group.",
+		    type => 'boolean',
+		    optional => 1,
+		},
 	    },
 	},
 	returns => { type => 'null' },
@@ -273,6 +278,10 @@ my $register_rule_group_api = sub {
 
 	    $rdb->rule_add_group($param->{id}, $param->{ogroup}, $name);
 
+	    if (defined($param->{negate})) {
+		$rdb->rule_set_group_setting_negate($param->{negate}, $param->{id}, $param->{ogroup}, $name);
+	    }
+
 	    PMG::DBTools::reload_ruledb();
 
 	    return undef;
@@ -314,6 +323,93 @@ my $register_rule_group_api = sub {
 	    return undef;
 	}});
 
+    if($name ne 'action') {
+	__PACKAGE__->register_method ({
+		name => "get_${name}_group_settings",
+		path => "$name/{ogroup}",
+		method => 'GET',
+		description => "Get '$name' group settings for rule.",
+		proxyto => 'master',
+		protected => 1,
+		permissions => { check => [ 'admin' ] },
+		parameters => {
+		    additionalProperties => 0,
+		    properties => {
+			id => {
+			    description => "Rule ID.",
+			    type => 'integer',
+			},
+			ogroup => {
+			    description => "Groups ID.",
+			    type => 'integer',
+			},
+		    },
+		},
+		returns => {
+		    type => 'array',
+		    items => {
+			type => "object",
+			properties => {
+			    negate => {
+				description=>  "Negate group.",
+				type => 'boolean',
+			    },
+			}
+		    }
+		},
+		code => sub {
+		    my ($param) = @_;
+
+		    my $rdb = PMG::RuleDB->new();
+
+		    my $settings = $rdb->rule_get_group_settings($param->{id}, $param->{ogroup}, $name);
+
+		    my $ret = {
+			negate => $settings->{negate},
+		    };
+
+		    return $ret;
+		}});
+
+	__PACKAGE__->register_method ({
+		name => "set_${name}_group_settings",
+		path => "$name/{ogroup}",
+		method => 'PUT',
+		description => "Update '$name' group settings for rule.",
+		proxyto => 'master',
+		protected => 1,
+		permissions => { check => [ 'admin' ] },
+		parameters => {
+		    additionalProperties => 0,
+		    properties => {
+			id => {
+			    description => "Rule ID.",
+			    type => 'integer',
+			},
+			ogroup => {
+			    description => "Groups ID.",
+			    type => 'integer',
+			},
+			negate => {
+			    description => "Negate group.",
+			    type => 'boolean',
+			    optional => 1,
+			},
+		    },
+		},
+		returns => { type => 'null' },
+		code => sub {
+		    my ($param) = @_;
+
+		    my $rdb = PMG::RuleDB->new();
+
+		    if(defined($param->{negate})) {
+			$rdb->rule_set_group_setting_negate($param->{negate}, $param->{id}, $param->{ogroup}, $name);
+		    }
+
+		    return;
+		}});
+    }
 };
 
 $register_rule_group_api->('from');
-- 
2.39.2

[pmg-devel] [PATCH WIP api 04/11] negation: implement matching logic

[Leo Nunner]

Straightforward for most objects, where the matching result gets XOR'd
with the specific negation setting. For 'what' objects, more changes
were necessary: the different types of what matches all needed to be
expanded inside their respective parse_entity functions. For some of
these, the result might be _strange_ (which is more due to the concept
of inverting what objects itself), but seems to work as intended.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PMG/RuleCache.pm                   | 8 ++++----
 src/PMG/RuleDB/ArchiveFilter.pm        | 6 ++++--
 src/PMG/RuleDB/ContentTypeFilter.pm    | 6 ++++--
 src/PMG/RuleDB/MatchArchiveFilename.pm | 4 ++--
 src/PMG/RuleDB/MatchField.pm           | 2 +-
 src/PMG/RuleDB/MatchFilename.pm        | 2 +-
 6 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index c5a57f6..04e35da 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -252,7 +252,7 @@ sub from_match {
     }
 
     foreach my $obj (@$from) {
-	return 1 if $obj->who_match($addr, $ip, $ldap);
+	return 1 if ($obj->who_match($addr, $ip, $ldap) xor $obj->{negate});
     }
 
     return 0;
@@ -266,7 +266,7 @@ sub to_match {
     return 1 if !defined ($to);
 
     foreach my $obj (@$to) {
-	return 1 if $obj->who_match($addr, undef, $ldap);
+	return 1 if ($obj->who_match($addr, undef, $ldap) xor $obj->{negate});
     }
 
     return 0;
@@ -280,7 +280,7 @@ sub when_match {
     return 1 if !defined ($when);
 
     foreach my $obj (@$when) {
-	return 1 if $obj->when_match($time);
+	return 1 if ($obj->when_match($time) xor $obj->{negate});
     }
 
     return 0;
@@ -325,7 +325,7 @@ sub what_match {
     foreach my $obj (@$what) {
 	if ($obj->can ("what_match_targets")) {
 	    my $target_info;
-	    if ($target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) {
+	    if (($target_info = $obj->what_match_targets($queue, $element, $msginfo, $dbh)) xor $obj->{negate}) {
 		foreach my $k (keys %$target_info) {
 		    my $cmarks = $target_info->{$k}->{marks}; # make a copy
 		    $res->{$k} = $target_info->{$k};
diff --git a/src/PMG/RuleDB/ArchiveFilter.pm b/src/PMG/RuleDB/ArchiveFilter.pm
index 6d91556..61f6c50 100644
--- a/src/PMG/RuleDB/ArchiveFilter.pm
+++ b/src/PMG/RuleDB/ArchiveFilter.pm
@@ -60,10 +60,12 @@ sub parse_entity {
 	my $glob_ct = $entity->{PMX_glob_ct};
 
 	if ($header_ct && $header_ct =~ m|$self->{field_value}|) {
-	    push @$res, $id;
+	    push @$res, $id if !$self->{negate};
 	} elsif ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
-	    push @$res, $id;
+	    push @$res, $id if !$self->{negate};
 	} elsif ($glob_ct && $glob_ct =~ m|$self->{field_value}|) {
+	    push @$res, $id if !$self->{negate};
+	} elsif ($self->{negate}) {
 	    push @$res, $id;
 	} else {
 	    # match inside archives 
diff --git a/src/PMG/RuleDB/ContentTypeFilter.pm b/src/PMG/RuleDB/ContentTypeFilter.pm
index 76fc1ce..eb35292 100644
--- a/src/PMG/RuleDB/ContentTypeFilter.pm
+++ b/src/PMG/RuleDB/ContentTypeFilter.pm
@@ -72,10 +72,12 @@ sub parse_entity {
 	my $glob_ct = $entity->{PMX_glob_ct};
 
 	if ($header_ct && $header_ct =~ m|$self->{field_value}|) {
-	    push @$res, $id;
+	    push @$res, $id if !$self->{negate};
 	} elsif ($magic_ct && $magic_ct =~ m|$self->{field_value}|) {
-	    push @$res, $id;
+	    push @$res, $id if !$self->{negate};
 	} elsif ($glob_ct && $glob_ct =~ m|$self->{field_value}|) {
+	    push @$res, $id if !$self->{negate};
+	} elsif ($self->{negate}) {
 	    push @$res, $id;
 	}
     }
diff --git a/src/PMG/RuleDB/MatchArchiveFilename.pm b/src/PMG/RuleDB/MatchArchiveFilename.pm
index 2ef3543..8abd592 100644
--- a/src/PMG/RuleDB/MatchArchiveFilename.pm
+++ b/src/PMG/RuleDB/MatchArchiveFilename.pm
@@ -29,12 +29,12 @@ sub parse_entity {
 	chomp $id;
 
 	my $fn = PMG::Utils::extract_filename($entity->head);
-	if (defined($fn) && $fn =~ m|^$self->{fname}$|i) {
+	if (defined($fn) && ($fn =~ m|^$self->{fname}$|i xor $self->{negate})) {
 	    push @$res, $id;
 	} elsif (my $filenames = $entity->{PMX_filenames}) {
 	    # Match inside archives
 	    for my $fn (keys %$filenames) {
-		if ($fn =~ m|^$self->{fname}$|i) {
+		if ($fn =~ m|^$self->{fname}$|i xor $self->{negate}) {
 		    push @$res, $id;
 		    last;
 		}
diff --git a/src/PMG/RuleDB/MatchField.pm b/src/PMG/RuleDB/MatchField.pm
index 177a283..0748d77 100644
--- a/src/PMG/RuleDB/MatchField.pm
+++ b/src/PMG/RuleDB/MatchField.pm
@@ -118,7 +118,7 @@ sub parse_entity {
 	    $decvalue = PMG::Utils::try_decode_utf8($decvalue);
 
 	    eval {
-		if ($decvalue =~ m|$self->{field_value}|i) {
+		if (($decvalue =~ m|$self->{field_value}|i) xor $self->{negate}) {
 		    push @$res, $id;
 		}
 	    };
diff --git a/src/PMG/RuleDB/MatchFilename.pm b/src/PMG/RuleDB/MatchFilename.pm
index c9cdbe0..0665efc 100644
--- a/src/PMG/RuleDB/MatchFilename.pm
+++ b/src/PMG/RuleDB/MatchFilename.pm
@@ -91,7 +91,7 @@ sub parse_entity {
 	chomp $id;
 
 	if (my $value = PMG::Utils::extract_filename($entity->head)) {
-	    if ($value =~ m|^$self->{fname}$|i) {
+	    if (($value =~ m|^$self->{fname}$|i) xor $self->{negate}) {
 		push @$res, $id;
 	    }
 	}
-- 
2.39.2

[pmg-devel] [PATCH WIP api 05/11] match groups: update database schema

[Leo Nunner]

Adds a 'MatchGroup' table which contains all the data relevant for a
match group:
    - ID: numerical ID of the group
    - Rule ID: of the rule it belongs to
    - Class: which object type it belongs to

The 'RuleGroup' table - which maps objects to a specific rule - now also
has a 'MatchGroup' field to store the ID of the corresponding match
group, should it be part of one.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PMG/DBTools.pm | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/src/PMG/DBTools.pm b/src/PMG/DBTools.pm
index 81d7fb3..1461dae 100644
--- a/src/PMG/DBTools.pm
+++ b/src/PMG/DBTools.pm
@@ -295,6 +295,15 @@ my $userprefs_ctablecmd =  <<__EOD;
 
 __EOD
 
+my $matchgroup_ctablecmd =  <<__EOD;
+    CREATE TABLE MatchGroup
+    (ID SERIAL UNIQUE,
+     RID INTEGER NOT NULL,
+     Name VARCHAR(255),
+     Class VARCHAR(10) NOT NULL,
+     PRIMARY KEY (ID));
+__EOD
+
 sub cond_create_dbtable {
     my ($dbh, $name, $ctablecmd) = @_;
 
@@ -418,6 +427,7 @@ sub create_ruledb {
           Rule_ID INTEGER NOT NULL,
           Grouptype INTEGER NOT NULL,
           Negate INTEGER NOT NULL DEFAULT 0,
+          MatchGroup INTEGER NOT NULL DEFAULT 0,
           PRIMARY KEY (Objectgroup_ID, Rule_ID, Grouptype)
         );
 
@@ -440,6 +450,8 @@ sub create_ruledb {
         $userprefs_ctablecmd;
 
         $virusinfo_stat_ctablecmd;
+
+        $matchgroup_ctablecmd;
 EOD
     );
 
@@ -495,6 +507,7 @@ sub upgradedb {
 	'CStatistic', $cstatistic_ctablecmd,
 	'ClusterInfo', $clusterinfo_ctablecmd,
 	'VirusInfo', $virusinfo_stat_ctablecmd,
+	'MatchGroup', $matchgroup_ctablecmd,
     };
 
     foreach my $table (keys %$tables) {
@@ -593,6 +606,19 @@ sub upgradedb {
 	}
     }
 
+    # Allow logical AND for rule objects
+    if (!database_column_exists($dbh, 'RuleGroup', 'MatchGroup')) {
+	eval {
+	    $dbh->begin_work;
+	    $dbh->do("ALTER TABLE RuleGroup ADD COLUMN MatchGroup INTEGER NOT NULL DEFAULT 0");
+	    $dbh->commit;
+	};
+	if (my $err = $@) {
+	    $dbh->rollback;
+	    die $err;
+	}
+    }
+
     foreach my $table (keys %$tables) {
 	eval { $dbh->do("ANALYZE $table"); };
 	warn $@ if $@;
-- 
2.39.2

[pmg-devel] [PATCH WIP api 06/11] match groups: add functions for database access

[Leo Nunner]

Introduce functions to manage match groups in the database. This
includes adding and deleting match groups, as well as the ability to
load all match groups that are associated with one specific rule.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PMG/RuleDB.pm | 82 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/src/PMG/RuleDB.pm b/src/PMG/RuleDB.pm
index 98beda3..b126d0d 100644
--- a/src/PMG/RuleDB.pm
+++ b/src/PMG/RuleDB.pm
@@ -757,6 +757,70 @@ sub rule_remove_group {
     return 1;
 }
 
+sub rule_add_match_group {
+    my ($self, $ruleid, $gtype_str, $name) = @_;
+
+    defined($ruleid) || die "undefined rule id: ERROR";
+
+    $self->{dbh}->do("INSERT INTO MatchGroup " .
+		     "(RID, Class, Name) " .
+		     "VALUES (?, ?, ?)", undef,
+		     $ruleid, $gtype_str, $name);
+
+    return PMG::Utils::lastid($self->{dbh}, 'matchgroup_id_seq');
+}
+
+sub rule_remove_match_group {
+    my ($self, $ruleid, $groupid) = @_;
+
+    defined($ruleid) || die "undefined rule id: ERROR";
+    defined($groupid) || die "undefined group id: ERROR";
+
+    # first, we check whether there are any objects in this group
+    my $sth = $self->{dbh}->prepare("SELECT COUNT (*) FROM RuleGroup ".
+		     "WHERE Rule_ID = ? and MatchGroup = ?");
+
+    $sth->execute($ruleid, $groupid);
+    my ($count) = $sth->fetchrow_array;
+
+    die "Cannot delete non-empty match group" if $count;
+
+    $self->{dbh}->do("DELETE FROM MatchGroup WHERE " .
+		     "ID = ? and RID = ?", undef,
+		     $groupid, $ruleid);
+}
+
+sub load_match_groups {
+    my ($self, $rule) = @_;
+
+    defined($rule->{id}) || die "undefined rule id: ERROR";
+
+    my $sth = $self->{dbh}->prepare(
+	"SELECT ID, Name, Class " .
+	"FROM MatchGroup " .
+	"WHERE RID = ? " .
+	"ORDER BY Class");
+
+    $sth->execute($rule->{id});
+
+    my $matchgroups = [];
+
+    while (my $ref = $sth->fetchrow_hashref()) {
+	my $mg = {
+	    id => $ref->{id},
+	    class => $ref->{class},
+	    name => $ref->{name},
+	};
+
+	push @$matchgroups, $mg;
+    }
+
+    $sth->finish();
+
+    return $matchgroups;
+}
+
+
 
 sub rule_get_group_settings {
     my ($self, $ruleid, $groupid, $gtype_str) = @_;
@@ -797,6 +861,24 @@ sub rule_set_group_setting_negate {
     return 1;
 }
 
+sub rule_set_group_setting_matchgroup {
+    my ($self, $value, $ruleid, $groupid, $gtype_str) = @_;
+
+    my $gtype = $grouptype_hash->{$gtype_str} //
+	die "unknown group type '$gtype_str'\n";
+
+    defined($ruleid) || die "undefined rule id: ERROR";
+    defined($groupid) || die "undefined group id: ERROR";
+    defined($gtype) || die "undefined group type: ERROR";
+
+    my $sth = $self->{dbh}->prepare("UPDATE RuleGroup SET MatchGroup = ? " .
+		     "WHERE Objectgroup_ID = ? and Rule_ID = ? and Grouptype = ?");
+
+    $sth->execute($value, $groupid, $ruleid, $gtype);
+
+    return 1;
+}
+
 sub load_rule {
     my ($self, $id) = @_;
 
-- 
2.39.2

[pmg-devel] [PATCH WIP api 07/11] match groups: parse field into objects

[Leo Nunner]

Parse the MatchGroup value from the database into the 'group' property of
our abstraction objects. We are skipping this step for 'action' objects.
A function was added to update the MatchGroup value for a specific
object-rule relation.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PMG/API2/ObjectGroupHelpers.pm | 1 +
 src/PMG/RuleCache.pm               | 6 ++++--
 src/PMG/RuleDB.pm                  | 5 +++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/PMG/API2/ObjectGroupHelpers.pm b/src/PMG/API2/ObjectGroupHelpers.pm
index c3e6448..79e0d70 100644
--- a/src/PMG/API2/ObjectGroupHelpers.pm
+++ b/src/PMG/API2/ObjectGroupHelpers.pm
@@ -51,6 +51,7 @@ sub format_object_group {
 	    name => $og->{name},
 	    info => $og->{info},
 	    negate => $og->{negate},
+	    group => $og->{group},
 	};
     }
     return $res;
diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index 04e35da..115bf1a 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -56,7 +56,7 @@ sub new {
 	    my ($from, $to, $when, $what, $action);
 
 	    my $sth1 = $dbh->prepare(
-		"SELECT Objectgroup_ID, Grouptype, Negate FROM RuleGroup " .
+		"SELECT Objectgroup_ID, Grouptype, Negate, MatchGroup FROM RuleGroup " .
 		"where RuleGroup.Rule_ID = '$ruleid' " .
 		"ORDER BY Grouptype, Objectgroup_ID");
 
@@ -65,6 +65,7 @@ sub new {
 		my $gtype = $ref1->{grouptype};
 		my $groupid = $ref1->{objectgroup_id};
 		my $negate = $ref1->{negate};
+		my $group = $ref1->{group};
 
 		# emtyp groups differ from non-existent groups!
 
@@ -91,8 +92,9 @@ sub new {
 		    $sha1->add (join (',', $objid, $gtype, $groupid) . "|");
 		    $sha1->add ($obj->{digest}, "|");
 
-		    if ($gtype != 4) { # it doesn't make any sense to negate actions
+		    if ($gtype != 4) { # it doesn't make any sense to group or negate actions
 			$obj->{negate} = $negate;
+			$obj->{group} = $group;
 		    }
 
 		    if ($gtype == 0) {      #from
diff --git a/src/PMG/RuleDB.pm b/src/PMG/RuleDB.pm
index b126d0d..f12f9a0 100644
--- a/src/PMG/RuleDB.pm
+++ b/src/PMG/RuleDB.pm
@@ -106,8 +106,8 @@ sub load_groups {
     defined($rule->{id}) || die "undefined rule id: ERROR";
 
     my $sth = $self->{dbh}->prepare(
-	"SELECT RuleGroup.Grouptype, Objectgroup.ID, " .
-	"Objectgroup.Name, Objectgroup.Info, Negate " .
+	"SELECT RuleGroup.Grouptype, RuleGroup.Negate, RuleGroup.MatchGroup, " .
+	"Objectgroup.ID, Objectgroup.Name, Objectgroup.Info " .
 	"FROM Rulegroup, Objectgroup " .
 	"WHERE Rulegroup.Rule_ID = ? and " .
 	"Rulegroup.Objectgroup_ID = Objectgroup.ID " .
@@ -125,6 +125,7 @@ sub load_groups {
 
 	if ($ref->{'grouptype'} != 4) { # this doesn't make any sense for actions
 	    $og->{negate} = $ref->{negate};
+	    $og->{group} = $ref->{matchgroup};
 	}
 
 	if ($ref->{'grouptype'} == 0) {      #from
-- 
2.39.2

[pmg-devel] [PATCH WIP api 08/11] match groups: add API endpoints for create/delete

[Leo Nunner]

Add API endpoints to create and delete match groups associated with a
given rule.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PMG/API2/Rules.pm | 79 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/src/PMG/API2/Rules.pm b/src/PMG/API2/Rules.pm
index ad3f6c7..a048872 100644
--- a/src/PMG/API2/Rules.pm
+++ b/src/PMG/API2/Rules.pm
@@ -201,6 +201,85 @@ __PACKAGE__->register_method ({
 	return undef;
    }});
 
+__PACKAGE__->register_method ({
+	name => "create_matchgroup",
+	path => "matchgroup",
+	method => 'POST',
+	description => "Create a new match group for the specified object group.",
+	proxyto => 'master',
+	protected => 1,
+	permissions => { check => [ 'admin' ] },
+	parameters => {
+	    additionalProperties => 0,
+	    properties => {
+		id => {
+		    description => "Rule ID.",
+		    type => 'integer',
+		},
+		class => {
+		    description => "Match group type.",
+		    type => 'string',
+		},
+		name => {
+		    description => "Match group name.",
+		    type => 'string',
+		},
+	    },
+	},
+	returns => {
+	    description=>  "Match group ID.",
+	    type => 'integer',
+	},
+	code => sub {
+	    my ($param) = @_;
+
+	    my $rdb = PMG::RuleDB->new();
+
+	    my $id = $param->{id};
+	    my $class = $param->{class};
+	    my $name = $param->{name};
+
+	    return $rdb->rule_add_match_group($id, $class, $name);
+	}});
+
+__PACKAGE__->register_method ({
+	name => "delete_matchgroup",
+	path => "matchgroup",
+	method => 'DELETE',
+	description => "Delete a match group.",
+	proxyto => 'master',
+	protected => 1,
+	permissions => { check => [ 'admin' ] },
+	parameters => {
+	    additionalProperties => 0,
+	    properties => {
+		id => {
+		    description => "Rule ID.",
+		    type => 'integer',
+		},
+		group => {
+		    description => "Match group ID.",
+		    type => 'integer',
+		},
+	    },
+	},
+	returns => {
+	    type => 'null',
+	},
+	code => sub {
+	    my ($param) = @_;
+
+	    my $rdb = PMG::RuleDB->new();
+
+	    my $id = $param->{id};
+	    my $group = $param->{group};
+
+	    $rdb->rule_remove_match_group($id, $group);
+
+	    return;
+	}});
+
+
 my $register_rule_group_api = sub {
     my ($name) = @_;
 
-- 
2.39.2

[pmg-devel] [PATCH WIP api 09/11] match groups: list match groups in rule API

[Leo Nunner]

Add a list of all match groups that are associated with a rule when
requesting the rule information via the API.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PMG/API2/ObjectGroupHelpers.pm | 4 +++-
 src/PMG/API2/RuleDB.pm             | 4 +++-
 src/PMG/API2/Rules.pm              | 4 +++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/PMG/API2/ObjectGroupHelpers.pm b/src/PMG/API2/ObjectGroupHelpers.pm
index 79e0d70..5e15d37 100644
--- a/src/PMG/API2/ObjectGroupHelpers.pm
+++ b/src/PMG/API2/ObjectGroupHelpers.pm
@@ -14,7 +14,7 @@ use PMG::DBTools;
 use PMG::RuleDB;
 
 sub format_rule {
-    my ($rule, $from, $to, $when, $what, $action) = @_;
+    my ($rule, $from, $to, $when, $what, $action, $match) = @_;
 
     my $cond_create_group = sub {
 	my ($res, $name, $groupdata) = @_;
@@ -38,6 +38,8 @@ sub format_rule {
     $cond_create_group->($data, 'what', $what);
     $cond_create_group->($data, 'action', $action);
 
+    $data->{match} = $match if $match;
+
     return $data;
 }
 
diff --git a/src/PMG/API2/RuleDB.pm b/src/PMG/API2/RuleDB.pm
index 1fddb32..3942f36 100644
--- a/src/PMG/API2/RuleDB.pm
+++ b/src/PMG/API2/RuleDB.pm
@@ -141,8 +141,10 @@ __PACKAGE__->register_method({
 	    my ($from, $to, $when, $what, $action) =
 		$rdb->load_groups($rule);
 
+	    my $matchgroups = $rdb->load_match_groups($rule);
+
 	    my $data = PMG::API2::ObjectGroupHelpers::format_rule(
-		$rule, $from, $to, $when, $what, $action);
+		$rule, $from, $to, $when, $what, $action, $matchgroups);
 
 	    push @$res, $data;
 	}
diff --git a/src/PMG/API2/Rules.pm b/src/PMG/API2/Rules.pm
index a048872..91e91ed 100644
--- a/src/PMG/API2/Rules.pm
+++ b/src/PMG/API2/Rules.pm
@@ -130,8 +130,10 @@ __PACKAGE__->register_method ({
 	my ($from, $to, $when, $what, $action) =
 	    $rdb->load_groups($rule);
 
+	my $matchgroups = $rdb->load_match_groups($rule);
+
 	my $data = PMG::API2::ObjectGroupHelpers::format_rule(
-	    $rule, $from, $to, $when, $what, $action);
+	    $rule, $from, $to, $when, $what, $action, $matchgroups);
 
 	return $data;
    }});
-- 
2.39.2

[pmg-devel] [PATCH WIP api 10/11] match groups: update existing object API endpoints

[Leo Nunner]

Adds a 'group' parameter to all relevant endpoints, which enables
associating objects inside rules with given match groups.
---
 src/PMG/API2/Rules.pm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/PMG/API2/Rules.pm b/src/PMG/API2/Rules.pm
index 91e91ed..ba86ede 100644
--- a/src/PMG/API2/Rules.pm
+++ b/src/PMG/API2/Rules.pm
@@ -347,6 +347,11 @@ my $register_rule_group_api = sub {
 		    type => 'boolean',
 		    optional => 1,
 		},
+		'group' => {
+		    description => "Add to match group.",
+		    type => 'integer',
+		    optional => 1,
+		},
 	    },
 	},
 	returns => { type => 'null' },
@@ -363,6 +368,10 @@ my $register_rule_group_api = sub {
 		$rdb->rule_set_group_setting_negate($param->{negate}, $param->{id}, $param->{ogroup}, $name);
 	    }
 
+	    if (defined($param->{group})) {
+		$rdb->rule_set_group_setting_matchgroup($param->{group}, $param->{id}, $param->{ogroup}, $name);
+	    }
+
 	    PMG::DBTools::reload_ruledb();
 
 	    return undef;
@@ -435,6 +444,10 @@ my $register_rule_group_api = sub {
 				description=>  "Negate group.",
 				type => 'boolean',
 			    },
+			    'group' => {
+				description=>  "Match group ID.",
+				type => 'integer',
+			    }
 			}
 		    }
 		},
@@ -447,6 +460,7 @@ my $register_rule_group_api = sub {
 
 		    my $ret = {
 			negate => $settings->{negate},
+			'group' => $settings->{matchgroup},
 		    };
 
 		    return $ret;
@@ -476,6 +490,11 @@ my $register_rule_group_api = sub {
 			    type => 'boolean',
 			    optional => 1,
 			},
+			'group' => {
+			    description => "Add to match group.",
+			    type => 'integer',
+			    optional => 1,
+			},
 		    },
 		},
 		returns => { type => 'null' },
@@ -488,6 +507,10 @@ my $register_rule_group_api = sub {
 			$rdb->rule_set_group_setting_negate($param->{negate}, $param->{id}, $param->{ogroup}, $name);
 		    }
 
+		    if(defined($param->{group})) {
+			$rdb->rule_set_group_setting_matchgroup($param->{group}, $param->{id}, $param->{ogroup}, $name);
+		    }
+
 		    return;
 		}});
     }
-- 
2.39.2

[pmg-devel] [PATCH WIP api 11/11] match groups: implement matching logic

[Leo Nunner]

Implements the logic behind match groups; A match group holds
several objects inside it, and only evalutes to true if ALL the
contained objects evaluate to true. Match groups are connected via
logical 'or' to all other objects inside the rule:

- Rule
    - Match Group
	- Object 1
	- Object 2
    - Object 3

This rule will match if either (Object 1 && Object 2), or if Object 3
evaluate to true.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 src/PMG/RuleCache.pm | 50 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/src/PMG/RuleCache.pm b/src/PMG/RuleCache.pm
index 115bf1a..9e30460 100644
--- a/src/PMG/RuleCache.pm
+++ b/src/PMG/RuleCache.pm
@@ -253,11 +253,19 @@ sub from_match {
 	$ip = $1;
     }
 
+    # don't return true if there are no elements marked with and!
+    my $and = grep { $_->{and} } @$from;
     foreach my $obj (@$from) {
-	return 1 if ($obj->who_match($addr, $ip, $ldap) xor $obj->{negate});
+	my $match = ($obj->who_match($addr, $ip, $ldap) xor $obj->{negate});
+
+	if ($obj->{and}) {
+	    $and &&= $match;
+	} else {
+	    return 1 if $match;
+	}
     }
 
-    return 0;
+    return $and;
 }
 
 sub to_match {
@@ -267,11 +275,18 @@ sub to_match {
 
     return 1 if !defined ($to);
 
+    my $and = grep { $_->{and} } @$to;
     foreach my $obj (@$to) {
-	return 1 if ($obj->who_match($addr, undef, $ldap) xor $obj->{negate});
+	my $match = ($obj->who_match($addr, undef, $ldap) xor $obj->{negate});
+
+	if ($obj->{and}) {
+	    $and &&= $match;
+	} else {
+	    return 1 if $match;
+	}
     }
 
-    return 0;
+    return $and;
 }
 
 sub when_match {
@@ -281,11 +296,18 @@ sub when_match {
 
     return 1 if !defined ($when);
 
+    my $and = grep { $_->{and} } @$when;
     foreach my $obj (@$when) {
-	return 1 if ($obj->when_match($time) xor $obj->{negate});
+	my $match = ($obj->when_match($time) xor $obj->{negate});
+
+	if ($obj->{and}) {
+	    $and &&= $match;
+	} else {
+	    return 1 if $match;
+	}
     }
 
-    return 0;
+    return $and;
 }
 
 sub what_match {
@@ -311,14 +333,28 @@ sub what_match {
 
     my $marks;
 
+    # First, we loop through all objects which are not 'Spam Filter'
+    my $group_valid = 1;
+    my @group_matches = ();
     foreach my $obj (@$what) {
 	if (!$obj->can('what_match_targets')) {
 	    if (my $match = $obj->what_match($queue, $element, $msginfo, $dbh)) {
-		push @$marks, @$match;
+		if ($obj->{and}) {
+		    push @group_matches, $match;
+		} else {
+		    push @$marks, @$match;
+		}
+	    } elsif ($obj->{and}) {
+		$group_valid = 0;
 	    }
 	}
     }
 
+    # If the 'Match All' group matches, push everything into the list
+    if ($group_valid) {
+	push(@$marks, @group_matches);
+    }
+
     foreach my $target (@{$msginfo->{targets}}) {
 	$res->{$target}->{marks} = $marks;
 	$res->{marks} = $marks;
-- 
2.39.2

[pmg-devel] [PATCH WIP gui 1/2] negate objects inside rules

[Leo Nunner]

This patch exposes the new 'negate' parameter through the GUI. All
objects (except for actions) now have a small icon next to them in the
rule overview, and clicking it will toggle the respective negation
setting. Negated objects are marked by a small 'NOT' before the object
name.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 js/RuleInfo.js | 45 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/js/RuleInfo.js b/js/RuleInfo.js
index 8f39695..4e3bad7 100644
--- a/js/RuleInfo.js
+++ b/js/RuleInfo.js
@@ -58,6 +58,22 @@ Ext.define('PMG.RuleInfo', {
 	    );
 	},
 
+	updateNegateObjectGroup: function(rec) {
+	    var me = this;
+	    Proxmox.Utils.API2Request({
+		url: me.getViewModel().get('baseurl') + '/' + rec.data.oclass + '/'+ rec.data.typeid,
+		method: 'PUT',
+		params: { negate: rec.data.negate ? 0 : 1 },
+		waitMsgTarget: me.getView(),
+		callback: function() {
+		    me.reload();
+		},
+		failure: function(response, opts) {
+		    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+		},
+	    });
+	},
+
 	addObjectGroup: function(type, record) {
 	    var me = this;
 	    var baseurl = me.getViewModel().get('baseurl');
@@ -112,7 +128,7 @@ Ext.define('PMG.RuleInfo', {
 		    });
 		    store.load();
 		    Ext.Array.each(ruledata[oc], function(og) {
-			data.push({ oclass: oc, name: og.name, typeid: og.id });
+			data.push({ oclass: oc, name: og.name, typeid: og.id, negate: og.negate });
 		    });
 		});
 
@@ -125,6 +141,11 @@ Ext.define('PMG.RuleInfo', {
 	    me.removeObjectGroup(record);
 	},
 
+	negateIconClick: function(gridView, rowindex, colindex, button, event, record) {
+	    var me = this;
+	    me.updateNegateObjectGroup(record);
+	},
+
 	removeDrop: function(gridView, data, overModel) {
 	    var me = this;
 	    var record = data.records[0]; // only one
@@ -162,7 +183,7 @@ Ext.define('PMG.RuleInfo', {
 
 	stores: {
 	    objects: {
-		fields: ['oclass', 'name', 'typeid'],
+		fields: ['oclass', 'name', 'typeid', 'negate'],
 		groupField: 'oclass',
 		sorters: 'name',
 	    },
@@ -293,8 +314,28 @@ Ext.define('PMG.RuleInfo', {
 		{
 		    header: gettext('Name'),
 		    dataIndex: 'name',
+		    renderer: function(value, data, record) {
+			return record.data.negate ? '<span style="color:gray">' + gettext("NOT") + ' </span>' + value : value;
+		    },
 		    flex: 1,
 		},
+		{
+		    text: '',
+		    xtype: 'actioncolumn',
+		    align: 'center',
+		    width: 40,
+		    items: [
+			{
+			    getClass: function(v, m, { data }) {
+				if (data.oclass === 'action') return '';
+				return 'fa fa-fw fa-refresh';
+			    },
+			    isActionDisabled: (v, r, c, i, { data }) => data.oclass === 'action',
+			    tooltip: gettext('Negate'),
+			    handler: 'negateIconClick',
+			},
+		    ],
+		},
 		{
 		    text: '',
 		    xtype: 'actioncolumn',
-- 
2.39.2

[pmg-devel] [PATCH WIP gui 2/2] introduce logical 'and' for rules

[Leo Nunner]

Rework the rule info view, which is now displayed as a tree structure.
The first level of objects gets connected via logical 'OR'. The match
group subfolders are for objects that get connected via logical 'AND',
meaning that all objects inside a match group need to match. Match
groups are connected via logical 'OR' to all other top-level objects
inside the rule.

New match groups can be added via the newly introduced 'Add Match Group'
Button, which brings up a menu where one can enter the name and object
type of the new match group. Match groups can only be deleted if they
contain no other objects.

The drag and drop behaviour is the same as before, with the addition of
more specific actions: objects can be dragged into/out of the match
group folders, and can also be dragged directly from the 'Available
Objects' table into their respective match groups.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
 css/ext6-pmg.css |  20 +++
 js/RuleInfo.js   | 418 +++++++++++++++++++++++++++++++++++++----------
 js/Utils.js      |  14 +-
 3 files changed, 363 insertions(+), 89 deletions(-)

diff --git a/css/ext6-pmg.css b/css/ext6-pmg.css
index 2f999f4..ac66f0f 100644
--- a/css/ext6-pmg.css
+++ b/css/ext6-pmg.css
@@ -96,6 +96,12 @@
     content: "\f115";
 }
 
+.x-tree-icon-custom {
+    line-height: 1.6em;
+    color: #555;
+    margin-right: 5px;
+}
+
 /* loading in task list */
 .x-grid-row-loading {
     background: no-repeat center center;
@@ -134,6 +140,20 @@ div.inline-block {
     cursor: pointer;
 }
 
+.grabbable {
+    cursor: move; /* fallback if grab cursor is unsupported */
+    cursor: grab;
+    cursor: -moz-grab;
+    cursor: -webkit-grab;
+}
+
+ /* (Optional) Apply a "closed-hand" cursor during drag operation. */
+.grabbable:active {
+    cursor: grabbing;
+    cursor: -moz-grabbing;
+    cursor: -webkit-grabbing;
+}
+
 .x-grid-filters-filtered-column{
     font-style: italic;
     font-weight: bold;
diff --git a/js/RuleInfo.js b/js/RuleInfo.js
index 4e3bad7..445bbac 100644
--- a/js/RuleInfo.js
+++ b/js/RuleInfo.js
@@ -1,3 +1,37 @@
+Ext.define('PMG.window.MatchGroupEdit', {
+    extend: 'Proxmox.window.Edit',
+
+    width: 300,
+    subject: gettext('Match Group'),
+    isCreate: true,
+    method: 'POST',
+
+    initComponent: function() {
+	var me = this;
+	me.callParent();
+    },
+
+    items: [
+	{
+	    xtype: 'proxmoxtextfield',
+	    name: 'name',
+	    fieldLabel: gettext('Name'),
+	},
+	{
+	    xtype: 'proxmoxKVComboBox',
+	    fieldLabel: gettext('Object type') + ':',
+	    labelWidth: 150,
+	    name: 'class',
+	    comboItems: [
+		['from', 'From'],
+		['to', 'To'],
+		['when', 'When'],
+		['what', 'What'],
+	    ],
+	},
+    ],
+});
+
 Ext.define('PMG.RuleInfo', {
     extend: 'Ext.panel.Panel',
     xtype: 'pmgRuleInfo',
@@ -33,6 +67,144 @@ Ext.define('PMG.RuleInfo', {
 	    });
 	},
 
+	renderUsedObjects: function(v) {
+	    let me = this;
+
+	    me.usedDragZone = new Ext.dd.DragZone(v.getEl(), {
+		getDragData: function(e) {
+		    var sourceEl = e.getTarget(".x-grid-item", 10);
+
+		    if (sourceEl) {
+			let record = v.getView().getRecord(sourceEl);
+
+			let ddel = document.createElement('div');
+			ddel.innerHTML = record.data.name;
+			ddel.id = Ext.id();
+
+			return {
+			    ddel: ddel,
+			    repairXY: Ext.fly(sourceEl).getXY(),
+			    record: record,
+			};
+		    }
+
+		    return null;
+		},
+
+		getRepairXY: function() {
+		    return this.dragData.repairXY;
+		},
+
+		onBeforeDrag: function(source, e) {
+		    return source.record.data.leaf;
+		},
+	    });
+
+	    me.usedDropZone = new Ext.dd.DropZone(v.getEl(), {
+		getTargetFromEvent: function(e) {
+		    var sourceEl = e.getTarget(".x-grid-item", 10);
+		    return sourceEl ? v.getView().getRecord(sourceEl) : null;
+		},
+
+		onNodeOver: function(target, dd, e, source) {
+		    if (source.add) return Ext.dd.DropZone.prototype.dropAllowed;
+		    if (source.record.data.oclass === 'action') return Ext.dd.DropZone.prototype.dropNotAllowed;
+
+		    if (source.record.data.oclass === target.data.oclass &&
+			target.data.leaf === false) {
+			return Ext.dd.DropZone.prototype.dropAllowed;
+		    } else {
+			return Ext.dd.DropZone.prototype.dropNotAllowed;
+		    }
+		},
+
+		onNodeDrop: function(target, dd, e, source) {
+		    if (source.record.data.oclass === 'action') return false;
+
+		    // First, do we want to add a new object?
+		    if (source.add) {
+			let group = target.data.name !== PMG.Utils.oclass_text[source.type]
+			    ? target.data.typeid : 0;
+			me.addObjectGroup(source.type, source.record, group);
+			return true;
+		    }
+
+		    // Otherwise, update an existing one
+		    if (source.record.data.oclass !== target.data.oclass ||
+			target.data.leaf) {
+			return false;
+		    }
+
+		    let record = source.record;
+
+		    if (target.data.name !== PMG.Utils.oclass_text[record.data.oclass]) {
+			if (record.data.group) return false;
+
+			me.updateRecordMatchAll(record, target.data.typeid);
+		    } else {
+			me.updateRecordMatchAll(record, 0);
+		    }
+
+		    return true;
+		},
+	    });
+	},
+
+	renderAvailObjects: function(v) {
+	    let me = this;
+
+	    me.availDragZone = new Ext.dd.DragZone(v.getEl(), {
+		getDragData: function(e) {
+		    var sourceEl = e.getTarget(".x-grid-item", 10);
+
+		    if (sourceEl) {
+			let tab = v.getActiveTab();
+			let record = tab.getView().getRecord(sourceEl);
+
+			let ddel = document.createElement('div');
+			ddel.innerHTML = record.data.name;
+			ddel.id = Ext.id();
+
+			return {
+			    ddel: ddel,
+			    repairXY: Ext.fly(sourceEl).getXY(),
+			    record: record,
+			    add: true,
+			    type: tab.type,
+			};
+		    }
+
+		    return null;
+		},
+
+		getRepairXY: function() {
+		    return this.dragData.repairXY;
+		},
+	    });
+
+	    me.availDropZone = new Ext.dd.DropZone(v.getEl(), {
+		getTargetFromEvent: function(e) {
+		    var sourceEl = e.getTarget(".x-grid-item", 10);
+		    let tab = v.getActiveTab();
+		    return sourceEl ? tab.getView().getRecord(sourceEl) : null;
+		},
+
+		onNodeOver: function(target, dd, e, source) {
+		    if (source.add) {
+			return Ext.dd.DropZone.prototype.dropNotAllowed;
+		    } else {
+			return Ext.dd.DropZone.prototype.dropAllowed;
+		    }
+		},
+
+		onNodeDrop: function(target, dd, e, source) {
+		    if (source.add) return false;
+		    me.removeObjectGroup(source.record);
+		    return true;
+		},
+	    });
+	},
+
 	removeObjectGroup: function(rec) {
 	    var me = this;
 	    Ext.Msg.confirm(
@@ -58,6 +230,31 @@ Ext.define('PMG.RuleInfo', {
 	    );
 	},
 
+	removeMatchGroup: function(rec) {
+	    var me = this;
+	    Ext.Msg.confirm(
+		gettext('Confirm'),
+		Ext.String.format(
+		    gettext('Are you sure you want to remove match group {0}'),
+		    "'" + rec.data.name + "'"),
+		function(button) {
+		    if (button === 'yes') {
+			Proxmox.Utils.API2Request({
+			    url: me.getViewModel().get('baseurl') + '/matchgroup?group=' + rec.data.typeid,
+			    method: 'DELETE',
+			    waitMsgTarget: me.getView(),
+			    callback: function() {
+				me.reload();
+			    },
+			    failure: function(response, opts) {
+				Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+			    },
+			});
+		    }
+		},
+	    );
+	},
+
 	updateNegateObjectGroup: function(rec) {
 	    var me = this;
 	    Proxmox.Utils.API2Request({
@@ -74,14 +271,34 @@ Ext.define('PMG.RuleInfo', {
 	    });
 	},
 
-	addObjectGroup: function(type, record) {
+	updateRecordMatchAll: function(rec, val) {
+	    var me = this;
+	    Proxmox.Utils.API2Request({
+		url: me.getViewModel().get('baseurl') + '/' + rec.data.oclass + '/'+ rec.data.typeid,
+		method: 'PUT',
+		params: { group: val },
+		waitMsgTarget: me.getView(),
+		callback: function() {
+		    me.reload();
+		},
+		failure: function(response, opts) {
+		    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+		},
+	    });
+	},
+
+	addObjectGroup: function(type, record, group, negate) {
 	    var me = this;
 	    var baseurl = me.getViewModel().get('baseurl');
 	    var url = baseurl + '/' + type;
 	    var id = type === 'action'?record.data.ogroup:record.data.id;
 	    Proxmox.Utils.API2Request({
 		url: url,
-		params: { ogroup: id },
+		params: {
+		    ogroup: id,
+		    group: group,
+		    negate: negate ? 1: 0,
+		},
 		method: 'POST',
 		waitMsgTarget: me.getView(),
 		callback: function() {
@@ -104,7 +321,23 @@ Ext.define('PMG.RuleInfo', {
 	    } else {
 		viewmodel.set('selectedRule', ruledata);
 
-		var data = [];
+		var root = { expanded: true, children: [] };
+
+		var matchgroups = { 'from': [], 'to': [], 'when': [], 'what': [], 'action': [] };
+
+		// first we create all match groups
+		Ext.Array.each(ruledata.match, function(og) {
+		    var entry = {
+			oclass: og.class,
+			name: og.name,
+			typeid: og.id,
+			leaf: false,
+			iconCls: 'fa fa-folder',
+			children: [],
+		    };
+		    matchgroups[og.class].push(entry);
+		});
+
 		Ext.Array.each(['from', 'to', 'when', 'what', 'action'], function(oc) {
 		    var store = viewmodel.get(oc + 'objects');
 		    if (ruledata[oc] === undefined || store === undefined) { return; }
@@ -127,18 +360,60 @@ Ext.define('PMG.RuleInfo', {
 			},
 		    });
 		    store.load();
+
+		    var child = {
+			name: PMG.Utils.oclass_text[oc],
+			oclass: oc,
+			iconCls: PMG.Utils.oclass_icon[oc],
+			children: [],
+			leaf: false,
+			expanded: true,
+		    };
+
 		    Ext.Array.each(ruledata[oc], function(og) {
-			data.push({ oclass: oc, name: og.name, typeid: og.id, negate: og.negate });
+			var entry = {
+			    oclass: oc,
+			    name: og.name,
+			    typeid: og.id,
+			    negate: og.negate,
+			    leaf: true,
+			    cls: 'grabbable',
+			    iconCls: PMG.Utils.oclass_icon[oc],
+			};
+
+			if (og.group) {
+			    var group_list = matchgroups[oc].find(x => x.typeid === og.group);
+			    if (group_list) {
+				group_list.children.push(entry);
+			    }
+			} else {
+			    child.children.push(entry);
+			}
 		    });
+
+		    // if (oc !== "action") child.children.push(group_list);
+
+		    Ext.Array.each(matchgroups[oc], function(og) {
+			og.expanded = og.children.length !== 0;
+			child.children.push(og);
+		    });
+
+		    child.expanded = child.children.length;
+
+		    root.children.push(child);
 		});
 
-		viewmodel.get('objects').setData(data);
+		viewmodel.get('objects').setRoot(root);
 	    }
 	},
 
 	removeIconClick: function(gridView, rowindex, colindex, button, event, record) {
 	    var me = this;
-	    me.removeObjectGroup(record);
+	    if (record.data.leaf) {
+		me.removeObjectGroup(record);
+	    } else {
+		me.removeMatchGroup(record);
+	    }
 	},
 
 	negateIconClick: function(gridView, rowindex, colindex, button, event, record) {
@@ -146,33 +421,21 @@ Ext.define('PMG.RuleInfo', {
 	    me.updateNegateObjectGroup(record);
 	},
 
-	removeDrop: function(gridView, data, overModel) {
-	    var me = this;
-	    var record = data.records[0]; // only one
-	    me.removeObjectGroup(record);
-	    return true;
-	},
-
 	addIconClick: function(gridView, rowindex, colindex, button, event, record) {
 	    var me = this;
-	    me.addObjectGroup(gridView.panel.type, record);
+	    me.addObjectGroup(gridView.panel.type, record, false, false);
 	    return true;
 	},
 
-	addDrop: function(gridView, data, overModel) {
+	addMatchGroupClick: function(gridView, rowindex, colindex, button, event, record) {
 	    var me = this;
-	    var record = data.records[0]; // only one
-	    me.addObjectGroup(data.view.panel.type, record);
-	    return true;
-	},
-
-	control: {
-	    'grid[reference=usedobjects]': {
-		drop: 'addDrop',
-	    },
-	    'tabpanel[reference=availobjects] > grid': {
-		drop: 'removeDrop',
-	    },
+	    var baseurl = me.getViewModel().get('baseurl');
+	    var win = Ext.create('PMG.window.MatchGroupEdit', {
+		url: baseurl + '/matchgroup',
+	    });
+	    win.show();
+	    // FIXME: reload not working right now
+	    win.on('destroy', me.reload);
 	},
     },
 
@@ -183,9 +446,10 @@ Ext.define('PMG.RuleInfo', {
 
 	stores: {
 	    objects: {
-		fields: ['oclass', 'name', 'typeid', 'negate'],
-		groupField: 'oclass',
+		fields: ['oclass', 'name', 'typeid', 'negate', 'group'],
 		sorters: 'name',
+		folderSort: true,
+		type: 'tree',
 	    },
 
 	    actionobjects: {
@@ -273,38 +537,14 @@ Ext.define('PMG.RuleInfo', {
 	    ],
 	},
 	{
-	    xtype: 'grid',
+	    xtype: 'treepanel',
 	    reference: 'usedobjects',
 	    hidden: true,
+	    rootVisible: false,
 	    emptyText: gettext('No Objects'),
-	    features: [{
-		id: 'group',
-		ftype: 'grouping',
-		enableGroupingMenu: false,
-		collapsible: false,
-		groupHeaderTpl: [
-		    '{[PMG.Utils.format_oclass(values.name)]}',
-		],
-	    }],
 
 	    title: gettext('Used Objects'),
 
-	    viewConfig: {
-		plugins: {
-		    ptype: 'gridviewdragdrop',
-		    copy: true,
-		    dragGroup: 'usedobjects',
-		    dropGroup: 'unusedobjects',
-
-		    // do not show default grid dragdrop behaviour
-		    dropZone: {
-			indicatorHtml: '',
-			indicatorCls: '',
-			handleNodeDrop: Ext.emptyFn,
-		    },
-		},
-	    },
-
 	    columns: [
 		{
 		    header: gettext('Type'),
@@ -314,6 +554,7 @@ Ext.define('PMG.RuleInfo', {
 		{
 		    header: gettext('Name'),
 		    dataIndex: 'name',
+		    xtype: 'treecolumn',
 		    renderer: function(value, data, record) {
 			return record.data.negate ? '<span style="color:gray">' + gettext("NOT") + ' </span>' + value : value;
 		    },
@@ -323,27 +564,32 @@ Ext.define('PMG.RuleInfo', {
 		    text: '',
 		    xtype: 'actioncolumn',
 		    align: 'center',
-		    width: 40,
+		    width: 80,
 		    items: [
 			{
 			    getClass: function(v, m, { data }) {
 				if (data.oclass === 'action') return '';
-				return 'fa fa-fw fa-refresh';
+				if (data.leaf) return 'fa fa-fw fa-refresh';
+				if (data.parentId !== 'root') return 'fa fa-fw fa-pencil-square-o';
+				return '';
 			    },
-			    isActionDisabled: (v, r, c, i, { data }) => data.oclass === 'action',
+			    isActionDisabled: (v, r, c, i, { data }) => !data.leaf || data.oclass === 'action',
 			    tooltip: gettext('Negate'),
 			    handler: 'negateIconClick',
 			},
-		    ],
-		},
-		{
-		    text: '',
-		    xtype: 'actioncolumn',
-		    align: 'center',
-		    width: 40,
-		    items: [
 			{
-			    iconCls: 'fa fa-fw fa-minus-circle',
+			    getClass: function(v, m, { data }) {
+				if (data.oclass === 'action') return '';
+				if (data.leaf) return 'fa fa-fw fa-minus-circle';
+				if (data.parentId !== 'root') return 'fa fa-fw fa-trash';
+				return '';
+			    },
+			    isActionDisabled: function(v, r, c, i, { data }) {
+				if (!data.leaf && data.parentId !== 'root') {
+				    return data.children.length !== 0;
+				}
+				return false;
+			    },
 			    tooltip: gettext('Remove'),
 			    handler: 'removeIconClick',
 			},
@@ -355,6 +601,24 @@ Ext.define('PMG.RuleInfo', {
 		store: '{objects}',
 		hidden: '{!selectedRule}',
 	    },
+
+	    listeners: {
+		render: "renderUsedObjects",
+	    },
+	},
+	{
+	    xtype: 'container',
+	    hidden: true,
+	    bind: {
+		hidden: '{!selectedRule}',
+	    },
+	    items: [
+		{
+		    xtype: 'button',
+		    text: gettext('Add Match Group'),
+		    handler: 'addMatchGroupClick',
+		},
+	    ],
 	},
 	{
 	    xtype: 'tabpanel',
@@ -367,23 +631,10 @@ Ext.define('PMG.RuleInfo', {
 	    defaults: {
 		xtype: 'grid',
 		emptyText: gettext('No Objects'),
-		viewConfig: {
-		    plugins: {
-			ptype: 'gridviewdragdrop',
-			dragGroup: 'unusedobjects',
-			dropGroup: 'usedobjects',
-
-			// do not show default grid dragdrop behaviour
-			dropZone: {
-			    indicatorHtml: '',
-			    indicatorCls: '',
-			    handleNodeDrop: Ext.emptyFn,
-			},
-		    },
-		},
 		columns: [
 		    {
 			header: gettext('Name'),
+			innerCls: 'grabbable',
 			dataIndex: 'name',
 			flex: 1,
 		    },
@@ -391,7 +642,7 @@ Ext.define('PMG.RuleInfo', {
 			text: '',
 			xtype: 'actioncolumn',
 			align: 'center',
-			width: 40,
+			width: 80,
 			items: [
 			    {
 				iconCls: 'fa fa-fw fa-plus-circle',
@@ -444,6 +695,9 @@ Ext.define('PMG.RuleInfo', {
 		    },
 		},
 	    ],
+	    listeners: {
+		render: "renderAvailObjects",
+	    },
 	},
     ],
 });
diff --git a/js/Utils.js b/js/Utils.js
index 7fa154e..2495f70 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -61,12 +61,12 @@ Ext.define('PMG.Utils', {
     },
 
     oclass_icon: {
-	who: '<span class="fa fa-fw fa-user-circle"></span> ',
-	what: '<span class="fa fa-fw fa-cube"></span> ',
-	when: '<span class="fa fa-fw fa-clock-o"></span> ',
-	action: '<span class="fa fa-fw fa-flag"></span> ',
-	from: '<span class="fa fa-fw fa-user-circle"></span> ',
-	to: '<span class="fa fa-fw fa-user-circle"></span> ',
+	who: 'fa fa-fw fa-user-circle',
+	what: 'fa fa-fw fa-cube',
+	when: 'fa fa-fw fa-clock-o',
+	action: 'fa fa-fw fa-flag',
+	from: 'fa fa-fw fa-user-circle',
+	to: 'fa fa-fw fa-user-circle',
     },
 
     mail_status_map: {
@@ -105,7 +105,7 @@ Ext.define('PMG.Utils', {
     },
 
     format_oclass: function(oclass) {
-	var icon = PMG.Utils.oclass_icon[oclass] || '';
+	var icon = '<span class="' + PMG.Utils.oclass_icon[oclass] + '"></span>' || '';
 	var text = PMG.Utils.oclass_text[oclass] || oclass;
 	return icon + text;
     },
-- 
2.39.2