[pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Dominik Csapak]

adds an option/api call to request an quarantine link for an
email whose domain is in the relay domains

for now, we do not expose that option to the ui, but this can easily be
added if wanted

NOTES on security:

this adds a world reachable api call, that can potentially send e-mails
to users that belong to a relay domain

this is ok, since anybody can already send e-mails to the users
via normal smtp, and since the content of the e-mail cannot be
controlled, the only thing a potential attacker can do is a dos attack
(which can always be done via resource exhaustion, e.g. send a lot of mail)

we could add more checks to make it more secure, but not so convenient:
* add an option for a admin-settable shared secret that users must enter
  (makes it harder for the user to self-service, since the user has to
  know the secret)
* only allow it from 'trusted networks' (this makes probably no sense)
* add an option to allow it from a specific subnet (similar to above,
  but seperate from mail flow, which could make sense, but is also
  not as convenient)

for now all text is hardcoded, templates could be used later on
(if users want that)

also i am open for alternate wordings for all texts, i basically chose
what came to mind first...

changes from v1:
* move config to 'spamquar' section
* show button also on admin interface

pmg-api:

Dominik Csapak (3):
  refactor domain_regex to Utils
  add 'quarantinelink' to spamquar config
  api2/quarantine: add global sendlink api call

 src/PMG/API2/Quarantine.pm  | 87 +++++++++++++++++++++++++++++++++++++
 src/PMG/CLI/pmgqm.pm        | 29 +------------
 src/PMG/Config.pm           |  6 +++
 src/PMG/HTTPServer.pm       |  1 +
 src/PMG/Service/pmgproxy.pm |  4 ++
 src/PMG/Utils.pm            | 26 +++++++++++
 6 files changed, 126 insertions(+), 27 deletions(-)

pmg-gui:

Dominik Csapak (1):
  add 'Request Quarantine Link' Button to LoginView

 js/LoginView.js   | 31 +++++++++++++++++++++++++++++++
 pmg-index.html.tt |  3 ++-
 2 files changed, 33 insertions(+), 1 deletion(-)

-- 
2.20.1

[pmg-devel] [PATCH pmg-api v2 1/3] refactor domain_regex to Utils

[Dominik Csapak]

we will need this somewhere else later

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PMG/CLI/pmgqm.pm | 29 ++---------------------------
 src/PMG/Utils.pm     | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/src/PMG/CLI/pmgqm.pm b/src/PMG/CLI/pmgqm.pm
index 937269f..39253db 100755
--- a/src/PMG/CLI/pmgqm.pm
+++ b/src/PMG/CLI/pmgqm.pm
@@ -33,31 +33,6 @@ sub setup_environment {
     PMG::RESTEnvironment->setup_default_cli_env();
 }
 
-sub domain_regex {
-    my ($domains) = @_;
-
-    my @ra;
-    foreach my $d (@$domains) {
-	# skip domains with non-DNS name characters
-	next if $d =~ m/[^A-Za-z0-9\-\.]/;
-	if ($d =~ m/^\.(.*)$/) {
-	    my $dom = $1;
-	    $dom =~ s/\./\\\./g;
-	    push @ra, $dom;
-	    push @ra, "\.\*\\.$dom";
-	} else {
-	    $d =~ s/\./\\\./g;
-	    push @ra, $d;
-	}
-    }
-
-    my $re = join ('|', @ra);
-
-    my $regex = qr/\@($re)$/i;
-
-    return $regex;
-}
-
 sub get_item_data {
     my ($data, $ref) = @_;
 
@@ -145,7 +120,7 @@ __PACKAGE__->register_method ({
 	my $dbh = PMG::DBTools::open_ruledb();
 
 	my $domains = PVE::INotify::read_file('domains');
-	my $domainregex = domain_regex([keys %$domains]);
+	my $domainregex = PMG::Utils::domain_regex([keys %$domains]);
 
 	my $sth = $dbh->prepare(
 	    "SELECT pmail, AVG(spamlevel) as spamlevel, count(*)  FROM CMailStore, CMSReceivers " .
@@ -278,7 +253,7 @@ __PACKAGE__->register_method ({
 	}
 
 	my $domains = PVE::INotify::read_file('domains');
-	my $domainregex = domain_regex([keys %$domains]);
+	my $domainregex = PMG::Utils::domain_regex([keys %$domains]);
 
 	my $template = "spamreport-${reportstyle}.tt";
 	my $found = 0;
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index d0654e1..a04d41b 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -1417,4 +1417,30 @@ sub reload_smtp_filter {
     return kill (10, $pid); # send SIGUSR1
 }
 
+sub domain_regex {
+    my ($domains) = @_;
+
+    my @ra;
+    foreach my $d (@$domains) {
+	# skip domains with non-DNS name characters
+	next if $d =~ m/[^A-Za-z0-9\-\.]/;
+	if ($d =~ m/^\.(.*)$/) {
+	    my $dom = $1;
+	    $dom =~ s/\./\\\./g;
+	    push @ra, $dom;
+	    push @ra, "\.\*\\.$dom";
+	} else {
+	    $d =~ s/\./\\\./g;
+	    push @ra, $d;
+	}
+    }
+
+    my $re = join ('|', @ra);
+
+    my $regex = qr/\@($re)$/i;
+
+    return $regex;
+}
+
+
 1;
-- 
2.20.1

[pmg-devel] [PATCH pmg-api v2 2/3] add 'quarantinelink' to spamquar config

[Dominik Csapak]

to enable the 'Request Quarantine Link' button and api call

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PMG/Config.pm           | 6 ++++++
 src/PMG/Service/pmgproxy.pm | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
index cd69c9c..155990b 100755
--- a/src/PMG/Config.pm
+++ b/src/PMG/Config.pm
@@ -289,6 +289,11 @@ sub properties {
 	    description => "Text for 'From' header in daily spam report mails.",
 	    type => 'string',
 	},
+	quarantinelink => {
+	    description => "Enables user self-service for Quarantine Links. Caution: this is accessible without authentication",
+	    type => 'boolean',
+	    default => 0,
+	},
     };
 }
 
@@ -303,6 +308,7 @@ sub options {
 	allowhrefs => { optional => 1 },
 	port => { optional => 1 },
 	protocol => { optional => 1 },
+	quarantinelink => { optional => 1 },
     };
 }
 
diff --git a/src/PMG/Service/pmgproxy.pm b/src/PMG/Service/pmgproxy.pm
index ea58b50..cec2754 100755
--- a/src/PMG/Service/pmgproxy.pm
+++ b/src/PMG/Service/pmgproxy.pm
@@ -21,6 +21,7 @@ use PVE::APIServer::Utils;
 
 use PMG::HTTPServer;
 use PMG::API2;
+use PMG::Config;
 
 use Template;
 
@@ -227,6 +228,8 @@ sub get_index {
 	$version = $1;
     };
 
+    my $cfg = PMG::Config->new();
+    my $quarantinelink = $cfg->get('spamquar', 'quarantinelink');
 
     $username = '' if !$username;
 
@@ -242,6 +245,7 @@ sub get_index {
 	debug => $args->{debug} || $server->{debug},
 	version => $version,
 	wtversion => $wtversion,
+	quarantinelink => $quarantinelink,
     };
 
     my $template_name;
-- 
2.20.1

[pmg-devel] [PATCH pmg-api v2 3/3] api2/quarantine: add global sendlink api call

[Dominik Csapak]

this api call takes an email, checks it against the relay domains,
and prepares a custom quarantinelink for that email  and sends it there

this has to happen unauthenticated, since the idea is that the user
want to access the quarantine but has no current ticket (and no
old spam report with a ticket)

to prevent abuse, we let the api call take 3 seconds, even if
it would fail due to an invalid e-mail address, so that an
potential attacker cannot probe for e-mails/relay domains.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PMG/API2/Quarantine.pm | 87 ++++++++++++++++++++++++++++++++++++++
 src/PMG/HTTPServer.pm      |  1 +
 2 files changed, 88 insertions(+)

diff --git a/src/PMG/API2/Quarantine.pm b/src/PMG/API2/Quarantine.pm
index 73fb0ec..cfedc84 100644
--- a/src/PMG/API2/Quarantine.pm
+++ b/src/PMG/API2/Quarantine.pm
@@ -8,6 +8,9 @@ use Data::Dumper;
 use Encode;
 use File::Path;
 use IO::File;
+use MIME::Entity;
+use URI::Escape;
+use Time::HiRes qw(usleep gettimeofday tv_interval);
 
 use Mail::Header;
 use Mail::SpamAssassin;
@@ -195,6 +198,7 @@ __PACKAGE__->register_method ({
 	    { name => 'attachment' },
 	    { name => 'listattachments' },
 	    { name => 'download' },
+	    { name => 'sendlink' },
 	];
 
 	return $result;
@@ -1239,4 +1243,87 @@ __PACKAGE__->register_method ({
 	return undef;
     }});
 
+__PACKAGE__->register_method ({
+    name =>'sendlink',
+    path => 'sendlink',
+    method => 'POST',
+    description => "Send Quarantine link to given e-mail.",
+    permissions => { user => 'world' },
+    protected => 1,
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    mail => get_standard_option('pmg-email-address'),
+	},
+    },
+    returns => { type => "null" },
+    code => sub {
+	my ($param) = @_;
+
+	my $cfg = PMG::Config->new();
+
+	my $hostname = PVE::INotify::nodename();
+
+	my $is_enabled = $cfg->get('spamquar', 'quarantinelink');
+	if (!$is_enabled) {
+	    die "This feature is not enabled\n";
+	}
+
+	my $fqdn = $cfg->get('spamquar', 'hostname') //
+	    PVE::Tools::get_fqdn($hostname);
+
+	my $port = $cfg->get('spamquar', 'port') // 8006;
+
+	my $protocol = $cfg->get('spamquar', 'protocol') // 'https';
+
+	my $protocol_fqdn_port = "$protocol://$fqdn";
+	if (($protocol eq 'https' && $port != 443) ||
+	    ($protocol eq 'http' && $port != 80)) {
+	    $protocol_fqdn_port .= ":$port";
+	}
+
+	my $mailfrom = $cfg->get ('spamquar', 'mailfrom') //
+	    "Proxmox Mail Gateway <postmaster>";
+
+	my $domains = PVE::INotify::read_file('domains');
+	my $domainregex = PMG::Utils::domain_regex([keys %$domains]);
+
+	my $receiver = $param->{mail};
+
+	my $starttime = [gettimeofday];
+	my $delay = 0;
+
+	if ($receiver !~ $domainregex) {
+	    # make all calls to this take the same duration
+	    $delay = 3 - tv_interval($starttime);
+	    $delay = 0 if $delay < 0;
+	    sleep $delay;
+	    return undef; # silently ignore invalid mails
+	}
+
+	my $ticket = PMG::Ticket::assemble_quarantine_ticket($receiver);
+	my $esc_ticket = uri_escape($ticket);
+	my $link = "$protocol_fqdn_port/quarantine?ticket=${esc_ticket}";
+
+	my $text = "Here is your Link for the Spam Quarantine on $fqdn:\n\n$link\n";
+
+	my $top = MIME::Entity->build(
+	    Type    => "text/plain",
+	    To      => $receiver,
+	    From    => $mailfrom,
+	    Subject => "Proxmox Mail Gateway - Quarantine Link",
+	    Data    => $text,
+	);
+
+	# we use an empty envelope sender (we dont want to receive NDRs)
+	PMG::Utils::reinject_mail ($top, '', [$receiver], undef, $fqdn);
+
+	# make all calls to this take the same duration
+	$delay = 3 - tv_interval($starttime);
+	$delay = 0 if $delay < 0;
+	sleep $delay;
+
+	return undef;
+    }});
+
 1;
diff --git a/src/PMG/HTTPServer.pm b/src/PMG/HTTPServer.pm
index eb48b5f..3dc9655 100755
--- a/src/PMG/HTTPServer.pm
+++ b/src/PMG/HTTPServer.pm
@@ -58,6 +58,7 @@ sub auth_handler {
 
     # explicitly allow some calls without auth
     if (($rel_uri eq '/access/domains' && $method eq 'GET') ||
+	($rel_uri eq '/quarantine/sendlink' && ($method eq 'GET' || $method eq 'POST')) ||
 	($rel_uri eq '/access/ticket' && ($method eq 'GET' || $method eq 'POST'))) {
 	$require_auth = 0;
     }
-- 
2.20.1

[pmg-devel] [PATCH pmg-gui v2 1/1] add 'Request Quarantine Link' Button to LoginView

[Dominik Csapak]

if the template has 'quarantinelink' enabled, we
show a button 'Request Quarantine Link' on the quarantine login ui

there a user can enter their e-mail and request a link to the quarantine

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 js/LoginView.js   | 31 +++++++++++++++++++++++++++++++
 pmg-index.html.tt |  3 ++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/js/LoginView.js b/js/LoginView.js
index 8e610aa..5b95234 100644
--- a/js/LoginView.js
+++ b/js/LoginView.js
@@ -10,6 +10,8 @@ Ext.define('PMG.LoginView', {
 
 	    let realmfield = me.lookup('realmfield');
 
+	    me.lookup('quarantineButton').setVisible(!!Proxmox.QuarantineLink);
+
 	    if (view.targetview !== 'quarantineview') {
 		return;
 	    }
@@ -65,6 +67,28 @@ Ext.define('PMG.LoginView', {
 	    }
 	},
 
+	openQuarantineLinkWindow: function() {
+	    let me = this;
+	    me.lookup('loginwindow').setVisible(false);
+	    Ext.create('Proxmox.window.Edit', {
+		title: gettext('Request Quarantine Link'),
+		url: '/quarantine/sendlink',
+		method: 'POST',
+		items: [
+		    {
+			xtype: 'proxmoxtextfield',
+			name: 'mail',
+			fieldLabel: gettext('Your E-Mail'),
+		    },
+		],
+		listeners: {
+		    destroy: function() {
+			me.lookup('loginwindow').show(true);
+		    },
+		},
+	    }).show();
+	},
+
 	control: {
 	    'field[name=lang]': {
 		change: function(f, value) {
@@ -76,6 +100,9 @@ Ext.define('PMG.LoginView', {
 		    window.location.reload();
 		},
 	    },
+	    'button[reference=quarantineButton]': {
+		click: 'openQuarantineLinkWindow',
+	    },
 	    'button[reference=loginButton]': {
 		click: 'submitForm',
 	    },
@@ -172,6 +199,10 @@ Ext.define('PMG.LoginView', {
                         },
 		    ],
 		    buttons: [
+			{
+			    text: gettext('Request Quarantine Link'),
+			    reference: 'quarantineButton',
+			},
 			{
 			    text: gettext('Login'),
 			    reference: 'loginButton',
diff --git a/pmg-index.html.tt b/pmg-index.html.tt
index 4faf0cf..4a29ba2 100644
--- a/pmg-index.html.tt
+++ b/pmg-index.html.tt
@@ -30,7 +30,8 @@
         Setup: { auth_cookie_name: 'PMGAuthCookie' },
         NodeName: '[% nodename %]',
         UserName: '[% username %]',
-        CSRFPreventionToken: '[% token %]'
+        CSRFPreventionToken: '[% token %]',
+        QuarantineLink: [% IF quarantinelink %] true [% ELSE %] false [% END %],
       };
     </script>
     <script type="text/javascript" src="/proxmoxlib.js?ver=[% wtversion %]"></script>
-- 
2.20.1

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Thomas Lamprecht]

On 17.11.20 15:57, Dominik Csapak wrote:
> adds an option/api call to request an quarantine link for an
> email whose domain is in the relay domains
> 
> for now, we do not expose that option to the ui, but this can easily be
> added if wanted
> 
> NOTES on security:
> 
> this adds a world reachable api call, that can potentially send e-mails
> to users that belong to a relay domain
> 
> this is ok, since anybody can already send e-mails to the users
> via normal smtp, and since the content of the e-mail cannot be
> controlled, the only thing a potential attacker can do is a dos attack
> (which can always be done via resource exhaustion, e.g. send a lot of mail)

But, isn't the difference that here the server does it for me, no
greylisting or similar involved? Also possible lower payload required
vs. doing the SMTP myself.


> 
> we could add more checks to make it more secure, but not so convenient:

why not rate limit it to three per day or so? not convenience reducing,
we would need to safe the usage count somewhere though.

> * add an option for a admin-settable shared secret that users must enter
>   (makes it harder for the user to self-service, since the user has to
>   know the secret)
> * only allow it from 'trusted networks' (this makes probably no sense)
> * add an option to allow it from a specific subnet (similar to above,
>   but seperate from mail flow, which could make sense, but is also
>   not as convenient)
> 
> for now all text is hardcoded, templates could be used later on
> (if users want that)
> 
> also i am open for alternate wordings for all texts, i basically chose
> what came to mind first...
> 
> changes from v1:
> * move config to 'spamquar' section
> * show button also on admin interface
> 
> pmg-api:
> 
> Dominik Csapak (3):
>   refactor domain_regex to Utils
>   add 'quarantinelink' to spamquar config
>   api2/quarantine: add global sendlink api call
> 
>  src/PMG/API2/Quarantine.pm  | 87 +++++++++++++++++++++++++++++++++++++
>  src/PMG/CLI/pmgqm.pm        | 29 +------------
>  src/PMG/Config.pm           |  6 +++
>  src/PMG/HTTPServer.pm       |  1 +
>  src/PMG/Service/pmgproxy.pm |  4 ++
>  src/PMG/Utils.pm            | 26 +++++++++++
>  6 files changed, 126 insertions(+), 27 deletions(-)
> 
> pmg-gui:
> 
> Dominik Csapak (1):
>   add 'Request Quarantine Link' Button to LoginView
> 
>  js/LoginView.js   | 31 +++++++++++++++++++++++++++++++
>  pmg-index.html.tt |  3 ++-
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Dominik Csapak]

On 11/17/20 4:29 PM, Thomas Lamprecht wrote:
> On 17.11.20 15:57, Dominik Csapak wrote:
>> adds an option/api call to request an quarantine link for an
>> email whose domain is in the relay domains
>>
>> for now, we do not expose that option to the ui, but this can easily be
>> added if wanted
>>
>> NOTES on security:
>>
>> this adds a world reachable api call, that can potentially send e-mails
>> to users that belong to a relay domain
>>
>> this is ok, since anybody can already send e-mails to the users
>> via normal smtp, and since the content of the e-mail cannot be
>> controlled, the only thing a potential attacker can do is a dos attack
>> (which can always be done via resource exhaustion, e.g. send a lot of mail)
> 
> But, isn't the difference that here the server does it for me, no
> greylisting or similar involved? Also possible lower payload required
> vs. doing the SMTP myself.

sure, but it is basically the same as a 'forgot password' link on any 
website

also i am not sure about the cost of an tls+http call vs plain smtp...
(i guess that this difference will not stop an attacker...)

in general you can always dos a system, given enough network bandwidth...

> 
> 
>>
>> we could add more checks to make it more secure, but not so convenient:
> 
> why not rate limit it to three per day or so? not convenience reducing,
> we would need to safe the usage count somewhere though.

i thought of this, but would take a little more time to develop ;)
if wanted, i can of course implement something like this, though
i am not sure where we would want to save that info, and how much
time i'd need

> 
>> * add an option for a admin-settable shared secret that users must enter
>>    (makes it harder for the user to self-service, since the user has to
>>    know the secret)
>> * only allow it from 'trusted networks' (this makes probably no sense)
>> * add an option to allow it from a specific subnet (similar to above,
>>    but seperate from mail flow, which could make sense, but is also
>>    not as convenient)
>>
>> for now all text is hardcoded, templates could be used later on
>> (if users want that)
>>
>> also i am open for alternate wordings for all texts, i basically chose
>> what came to mind first...
>>
>> changes from v1:
>> * move config to 'spamquar' section
>> * show button also on admin interface
>>
>> pmg-api:
>>
>> Dominik Csapak (3):
>>    refactor domain_regex to Utils
>>    add 'quarantinelink' to spamquar config
>>    api2/quarantine: add global sendlink api call
>>
>>   src/PMG/API2/Quarantine.pm  | 87 +++++++++++++++++++++++++++++++++++++
>>   src/PMG/CLI/pmgqm.pm        | 29 +------------
>>   src/PMG/Config.pm           |  6 +++
>>   src/PMG/HTTPServer.pm       |  1 +
>>   src/PMG/Service/pmgproxy.pm |  4 ++
>>   src/PMG/Utils.pm            | 26 +++++++++++
>>   6 files changed, 126 insertions(+), 27 deletions(-)
>>
>> pmg-gui:
>>
>> Dominik Csapak (1):
>>    add 'Request Quarantine Link' Button to LoginView
>>
>>   js/LoginView.js   | 31 +++++++++++++++++++++++++++++++
>>   pmg-index.html.tt |  3 ++-
>>   2 files changed, 33 insertions(+), 1 deletion(-)
>>
> 
> 

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Stoiko Ivanov]

Thanks for the v2.
moving the option to spamquar should also render quite nicely in our docs!

quickly ran it again - and it works as expected
the series LGTM:
Reviewed-By: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-By: Stoiko Ivanov <s.ivanov@proxmox.com>


On Tue, 17 Nov 2020 15:57:39 +0100
Dominik Csapak <d.csapak@proxmox.com> wrote:

> adds an option/api call to request an quarantine link for an
> email whose domain is in the relay domains
> 
> for now, we do not expose that option to the ui, but this can easily be
> added if wanted
> 
> NOTES on security:
> 
> this adds a world reachable api call, that can potentially send e-mails
> to users that belong to a relay domain
> 
> this is ok, since anybody can already send e-mails to the users
> via normal smtp, and since the content of the e-mail cannot be
> controlled, the only thing a potential attacker can do is a dos attack
> (which can always be done via resource exhaustion, e.g. send a lot of mail)
> 
> we could add more checks to make it more secure, but not so convenient:
> * add an option for a admin-settable shared secret that users must enter
>   (makes it harder for the user to self-service, since the user has to
>   know the secret)
> * only allow it from 'trusted networks' (this makes probably no sense)
> * add an option to allow it from a specific subnet (similar to above,
>   but seperate from mail flow, which could make sense, but is also
>   not as convenient)
> 
> for now all text is hardcoded, templates could be used later on
> (if users want that)
> 
> also i am open for alternate wordings for all texts, i basically chose
> what came to mind first...
> 
> changes from v1:
> * move config to 'spamquar' section
> * show button also on admin interface
> 
> pmg-api:
> 
> Dominik Csapak (3):
>   refactor domain_regex to Utils
>   add 'quarantinelink' to spamquar config
>   api2/quarantine: add global sendlink api call
> 
>  src/PMG/API2/Quarantine.pm  | 87 +++++++++++++++++++++++++++++++++++++
>  src/PMG/CLI/pmgqm.pm        | 29 +------------
>  src/PMG/Config.pm           |  6 +++
>  src/PMG/HTTPServer.pm       |  1 +
>  src/PMG/Service/pmgproxy.pm |  4 ++
>  src/PMG/Utils.pm            | 26 +++++++++++
>  6 files changed, 126 insertions(+), 27 deletions(-)
> 
> pmg-gui:
> 
> Dominik Csapak (1):
>   add 'Request Quarantine Link' Button to LoginView
> 
>  js/LoginView.js   | 31 +++++++++++++++++++++++++++++++
>  pmg-index.html.tt |  3 ++-
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Thomas Lamprecht]

On 17.11.20 16:53, Dominik Csapak wrote:
> On 11/17/20 4:29 PM, Thomas Lamprecht wrote:
>> On 17.11.20 15:57, Dominik Csapak wrote:
>>> adds an option/api call to request an quarantine link for an
>>> email whose domain is in the relay domains
>>>
>>> for now, we do not expose that option to the ui, but this can easily be
>>> added if wanted
>>>
>>> NOTES on security:
>>>
>>> this adds a world reachable api call, that can potentially send e-mails
>>> to users that belong to a relay domain
>>>
>>> this is ok, since anybody can already send e-mails to the users
>>> via normal smtp, and since the content of the e-mail cannot be
>>> controlled, the only thing a potential attacker can do is a dos attack
>>> (which can always be done via resource exhaustion, e.g. send a lot of mail)
>>
>> But, isn't the difference that here the server does it for me, no
>> greylisting or similar involved? Also possible lower payload required
>> vs. doing the SMTP myself.
> 
> sure, but it is basically the same as a 'forgot password' link on any website
> 

those often have captchas, though, at least if you retry a few times.

> also i am not sure about the cost of an tls+http call vs plain smtp...
> (i guess that this difference will not stop an attacker...)
> 
> in general you can always dos a system, given enough network bandwidth...

but misusing the PMG, a project to protect for mail spam, among other things,
to allow producing mail spam which gets relayed to the users behind a network
is something completely different - normally you cannot send anything to them
if they do not open a connection to you, at least for most state full firewall
setups.

Not saying this is outright bad, just that it cannot brushed off with "I can
produce network traffic otherwise" as the real target here can be something
where this may not be true without this feature.

>>
>>
>>>
>>> we could add more checks to make it more secure, but not so convenient:
>>
>> why not rate limit it to three per day or so? not convenience reducing,
>> we would need to safe the usage count somewhere though.
> 
> i thought of this, but would take a little more time to develop ;)
> if wanted, i can of course implement something like this, though
> i am not sure where we would want to save that info, and how much
> time i'd need

as long as this gets logged somewhere, even just HTTP access log (if relevant
params are in the URL itself) then an admin could setup fail2ban and we wouldn't
need to handle this ourself.

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Dietmar Maurer]

IMHO this is too dangerous.

This needs at least some kind of captcha ...

> On 11/17/2020 3:57 PM Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> adds an option/api call to request an quarantine link for an
> email whose domain is in the relay domains
> 
> for now, we do not expose that option to the ui, but this can easily be
> added if wanted
> 
> NOTES on security:
> 
> this adds a world reachable api call, that can potentially send e-mails
> to users that belong to a relay domain
> 
> this is ok, since anybody can already send e-mails to the users
> via normal smtp, and since the content of the e-mail cannot be
> controlled, the only thing a potential attacker can do is a dos attack
> (which can always be done via resource exhaustion, e.g. send a lot of mail)
> 
> we could add more checks to make it more secure, but not so convenient:
> * add an option for a admin-settable shared secret that users must enter
>   (makes it harder for the user to self-service, since the user has to
>   know the secret)
> * only allow it from 'trusted networks' (this makes probably no sense)
> * add an option to allow it from a specific subnet (similar to above,
>   but seperate from mail flow, which could make sense, but is also
>   not as convenient)
> 
> for now all text is hardcoded, templates could be used later on
> (if users want that)
> 
> also i am open for alternate wordings for all texts, i basically chose
> what came to mind first...
> 
> changes from v1:
> * move config to 'spamquar' section
> * show button also on admin interface
> 
> pmg-api:
> 
> Dominik Csapak (3):
>   refactor domain_regex to Utils
>   add 'quarantinelink' to spamquar config
>   api2/quarantine: add global sendlink api call
> 
>  src/PMG/API2/Quarantine.pm  | 87 +++++++++++++++++++++++++++++++++++++
>  src/PMG/CLI/pmgqm.pm        | 29 +------------
>  src/PMG/Config.pm           |  6 +++
>  src/PMG/HTTPServer.pm       |  1 +
>  src/PMG/Service/pmgproxy.pm |  4 ++
>  src/PMG/Utils.pm            | 26 +++++++++++
>  6 files changed, 126 insertions(+), 27 deletions(-)
> 
> pmg-gui:
> 
> Dominik Csapak (1):
>   add 'Request Quarantine Link' Button to LoginView
> 
>  js/LoginView.js   | 31 +++++++++++++++++++++++++++++++
>  pmg-index.html.tt |  3 ++-
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Dietmar Maurer]

> On 11/17/2020 5:27 PM Dietmar Maurer <dietmar@proxmox.com> wrote:
> 
>  
> IMHO this is too dangerous.
> 
> This needs at least some kind of captcha ...

i.e. This would allow direct DOS attacks to the internal mail server.

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Thomas Lamprecht]

On 17.11.20 17:38, Dietmar Maurer wrote:
> 
>> On 11/17/2020 5:27 PM Dietmar Maurer <dietmar@proxmox.com> wrote:
>>
>>  
>> IMHO this is too dangerous.
>>
>> This needs at least some kind of captcha ...
> 
> i.e. This would allow direct DOS attacks to the internal mail server.
> 

I found this captcha solution, relatively sophisticated but not a PITA for the
(human) user, Friendly Captcha[0] used by some official European Union websites.

It uses Proof of Work[2] (i.e. crypto puzzel ones device needs to solve by
computation), the specific library used is "Friendly PoW"[1].

If we go for a captcha I'd like something like this (could be rebuild), as
it avoids the issues with picture texts (easily solved by computers, bad
accessibility for humans) and similar captchas.


[0]: https://github.com/friendlycaptcha/friendly-challenge
[1]: https://github.com/friendlycaptcha/friendly-pow
[2]: https://de.wikipedia.org/wiki/Proof_of_Work

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Dominik Csapak]

On 11/18/20 8:44 AM, Thomas Lamprecht wrote:
> On 17.11.20 17:38, Dietmar Maurer wrote:
>>
>>> On 11/17/2020 5:27 PM Dietmar Maurer <dietmar@proxmox.com> wrote:
>>>
>>>   
>>> IMHO this is too dangerous.
>>>
>>> This needs at least some kind of captcha ...
>>
>> i.e. This would allow direct DOS attacks to the internal mail server.
>>
> 
> I found this captcha solution, relatively sophisticated but not a PITA for the
> (human) user, Friendly Captcha[0] used by some official European Union websites.
> 
> It uses Proof of Work[2] (i.e. crypto puzzel ones device needs to solve by
> computation), the specific library used is "Friendly PoW"[1].
> 
> If we go for a captcha I'd like something like this (could be rebuild), as
> it avoids the issues with picture texts (easily solved by computers, bad
> accessibility for humans) and similar captchas.
> 
> 
> [0]: https://github.com/friendlycaptcha/friendly-challenge
> [1]: https://github.com/friendlycaptcha/friendly-pow
> [2]: https://de.wikipedia.org/wiki/Proof_of_Work
> 

i'd rather go with a rate limited approach
e.g. a file with a
mail -> last click time
mapping
and refuse if the last click time is not older than 5min ?
and only 1 per 5 seconds overall?

a captcha would be much harder to implement (more dependencies,
backend as well as dependent frontend code and in this example
it seems the code is only available for js/ts), though
if we find a simple solution, i am not against it

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Thomas Lamprecht]

On 18.11.20 08:56, Dominik Csapak wrote:
> On 11/18/20 8:44 AM, Thomas Lamprecht wrote:
>> On 17.11.20 17:38, Dietmar Maurer wrote:
>>>
>>>> On 11/17/2020 5:27 PM Dietmar Maurer <dietmar@proxmox.com> wrote:
>>>>
>>>>   IMHO this is too dangerous.
>>>>
>>>> This needs at least some kind of captcha ...
>>>
>>> i.e. This would allow direct DOS attacks to the internal mail server.
>>>
>>
>> I found this captcha solution, relatively sophisticated but not a PITA for the
>> (human) user, Friendly Captcha[0] used by some official European Union websites.
>>
>> It uses Proof of Work[2] (i.e. crypto puzzel ones device needs to solve by
>> computation), the specific library used is "Friendly PoW"[1].
>>
>> If we go for a captcha I'd like something like this (could be rebuild), as
>> it avoids the issues with picture texts (easily solved by computers, bad
>> accessibility for humans) and similar captchas.
>>
>>
>> [0]: https://github.com/friendlycaptcha/friendly-challenge
>> [1]: https://github.com/friendlycaptcha/friendly-pow
>> [2]: https://de.wikipedia.org/wiki/Proof_of_Work
>>
> 
> i'd rather go with a rate limited approach
> e.g. a file with a
> mail -> last click time
> mapping
> and refuse if the last click time is not older than 5min ?
> and only 1 per 5 seconds overall?

or an hour or day? what do you need the mail such often??
The ticket doesn't even expires that fast..

> 
> a captcha would be much harder to implement (more dependencies,
> backend as well as dependent frontend code and in this example
> it seems the code is only available for js/ts), though
> if we find a simple solution, i am not against it
> 

but also more efficient, as the client actually needs to put in
work. JS is no problem, the backend would be nice in rust or
so - maybe in the future when someone gets around some day...

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Dominik Csapak]

On 11/18/20 9:01 AM, Thomas Lamprecht wrote:
> On 18.11.20 08:56, Dominik Csapak wrote:
>> On 11/18/20 8:44 AM, Thomas Lamprecht wrote:
>>> On 17.11.20 17:38, Dietmar Maurer wrote:
>>>>
>>>>> On 11/17/2020 5:27 PM Dietmar Maurer <dietmar@proxmox.com> wrote:
>>>>>
>>>>>    IMHO this is too dangerous.
>>>>>
>>>>> This needs at least some kind of captcha ...
>>>>
>>>> i.e. This would allow direct DOS attacks to the internal mail server.
>>>>
>>>
>>> I found this captcha solution, relatively sophisticated but not a PITA for the
>>> (human) user, Friendly Captcha[0] used by some official European Union websites.
>>>
>>> It uses Proof of Work[2] (i.e. crypto puzzel ones device needs to solve by
>>> computation), the specific library used is "Friendly PoW"[1].
>>>
>>> If we go for a captcha I'd like something like this (could be rebuild), as
>>> it avoids the issues with picture texts (easily solved by computers, bad
>>> accessibility for humans) and similar captchas.
>>>
>>>
>>> [0]: https://github.com/friendlycaptcha/friendly-challenge
>>> [1]: https://github.com/friendlycaptcha/friendly-pow
>>> [2]: https://de.wikipedia.org/wiki/Proof_of_Work
>>>
>>
>> i'd rather go with a rate limited approach
>> e.g. a file with a
>> mail -> last click time
>> mapping
>> and refuse if the last click time is not older than 5min ?
>> and only 1 per 5 seconds overall?
> 
> or an hour or day? what do you need the mail such often??
> The ticket doesn't even expires that fast..

sure, that was just an example (an hour would be ok)
it should not be that far apart that if a user
accidentally deletes the mail he cannot request
a new one in reasonable time frame

the overall timeout would have to be much shorter so that
legitimate users do not block each other, but long
enough to discourage an attacker.. (thus my 5 seconds,
it is short enough that the api call can block without
much hassle for the user, but long enough so that
an attacker can not dos the internal mail server)

> 
>>
>> a captcha would be much harder to implement (more dependencies,
>> backend as well as dependent frontend code and in this example
>> it seems the code is only available for js/ts), though
>> if we find a simple solution, i am not against it
>>
> 
> but also more efficient, as the client actually needs to put in
> work. JS is no problem, the backend would be nice in rust or
> so - maybe in the future when someone gets around some day...
> 

yes js is no problem, but the much bigger "problem" is that for a
captcha you generally have to save a state for each client
(else it would be easy to precalculate or reuse the work)
which is probably much bigger than my 'last click time'
approach from above

Re: [pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Stoiko Ivanov]

Thanks for implementing this - many of our community will be happy!

Regarding the complete series:
applied the patch and tested it on my setup - works as advertised:
Reviewed-By: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-By: Stoiko Ivanov <s.ivanov@proxmox.com>

Have a few small nits/suggestions (will comment on the individual patches)
but those can be addressed later.



On Tue, 17 Nov 2020 09:05:09 +0100
Dominik Csapak <d.csapak@proxmox.com> wrote:

> adds an option/api call to request an quarantine link for an
> email whose domain is in the relay domains
> 
> for now, we do not expose that option to the ui, but this can easily be
> added if wanted
> 
> NOTES on security:
> 
> this adds a world reachable api call, that can potentially send e-mails
> to users that belong to a relay domain
> 
> this is ok, since anybody can already send e-mails to the users
> via normal smtp, and since the content of the e-mail cannot be
> controlled, the only thing a potential attacker can do is a dos attack
> (which can always be done via resource exhaustion, e.g. send a lot of mail)
> 
> we could add more checks to make it more secure, but not so convenient:
> * add an option for a admin-settable shared secret that users must enter
>   (makes it harder for the user to self-service, since the user has to
>   know the secret)
> * only allow it from 'trusted networks' (this makes probably no sense)
> * add an option to allow it from a specific subnet (similar to above,
>   but seperate from mail flow, which could make sense, but is also
>   not as convenient)
> 
> for now all text is hardcoded, templates could be used later on
> (if users want that)
> 
> also i am open for alternate wordings for all texts, i basically chose
> what came to mind first...
> 
> pmg-api:
> 
> Dominik Csapak (3):
>   refactor domain_regex to Utils
>   add 'quarantinelink' to admin config
>   api2/quarantine: add global sendlink api call
> 
>  src/PMG/API2/Quarantine.pm  | 87 +++++++++++++++++++++++++++++++++++++
>  src/PMG/CLI/pmgqm.pm        | 29 +------------
>  src/PMG/Config.pm           |  6 +++
>  src/PMG/HTTPServer.pm       |  1 +
>  src/PMG/Service/pmgproxy.pm |  4 ++
>  src/PMG/Utils.pm            | 26 +++++++++++
>  6 files changed, 126 insertions(+), 27 deletions(-)
> 
> pmg-gui:
> 
> Dominik Csapak (1):
>   add 'Request Quarantine Link' Button to LoginView
> 
>  js/LoginView.js   | 33 +++++++++++++++++++++++++++++++++
>  pmg-index.html.tt |  3 ++-
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 

[pmg-devel] [PATCH pmg-api/gui] add quarantine self service button

[Dominik Csapak]

adds an option/api call to request an quarantine link for an
email whose domain is in the relay domains

for now, we do not expose that option to the ui, but this can easily be
added if wanted

NOTES on security:

this adds a world reachable api call, that can potentially send e-mails
to users that belong to a relay domain

this is ok, since anybody can already send e-mails to the users
via normal smtp, and since the content of the e-mail cannot be
controlled, the only thing a potential attacker can do is a dos attack
(which can always be done via resource exhaustion, e.g. send a lot of mail)

we could add more checks to make it more secure, but not so convenient:
* add an option for a admin-settable shared secret that users must enter
  (makes it harder for the user to self-service, since the user has to
  know the secret)
* only allow it from 'trusted networks' (this makes probably no sense)
* add an option to allow it from a specific subnet (similar to above,
  but seperate from mail flow, which could make sense, but is also
  not as convenient)

for now all text is hardcoded, templates could be used later on
(if users want that)

also i am open for alternate wordings for all texts, i basically chose
what came to mind first...

pmg-api:

Dominik Csapak (3):
  refactor domain_regex to Utils
  add 'quarantinelink' to admin config
  api2/quarantine: add global sendlink api call

 src/PMG/API2/Quarantine.pm  | 87 +++++++++++++++++++++++++++++++++++++
 src/PMG/CLI/pmgqm.pm        | 29 +------------
 src/PMG/Config.pm           |  6 +++
 src/PMG/HTTPServer.pm       |  1 +
 src/PMG/Service/pmgproxy.pm |  4 ++
 src/PMG/Utils.pm            | 26 +++++++++++
 6 files changed, 126 insertions(+), 27 deletions(-)

pmg-gui:

Dominik Csapak (1):
  add 'Request Quarantine Link' Button to LoginView

 js/LoginView.js   | 33 +++++++++++++++++++++++++++++++++
 pmg-index.html.tt |  3 ++-
 2 files changed, 35 insertions(+), 1 deletion(-)

-- 
2.20.1