From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 2AA341FF165 for ; Thu, 14 Aug 2025 02:11:19 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1A4C83DF4D; Thu, 14 Aug 2025 02:12:59 +0200 (CEST) Date: Thu, 14 Aug 2025 02:12:55 +0200 From: Stoiko Ivanov To: Maximiliano Sandoval Message-ID: <20250814021255.09d3e9d1@rosa.proxmox.com> In-Reply-To: <20250404131438.328396-4-m.sandoval@proxmox.com> References: <20250404131438.328396-1-m.sandoval@proxmox.com> <20250404131438.328396-4-m.sandoval@proxmox.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1755130342746 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.085 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pmg-devel] [PATCH pmg-api v5 04/11] create new users for the rule db X-BeenThere: pmg-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Mail Gateway development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: pmg-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pmg-devel-bounces@lists.proxmox.com Sender: "pmg-devel" On Fri, 4 Apr 2025 15:14:31 +0200 Maximiliano Sandoval wrote: > These users will be used by the pmg-smtp-filter and pmgpolicy. We add a > helper function to open the rule_db as a given user. > > Signed-off-by: Maximiliano Sandoval > --- > debian/postinst | 8 ++++++++ > src/PMG/DBTools.pm | 26 ++++++++++++++++++++++++-- > src/bin/pmg-smtp-filter | 4 ++-- > src/bin/pmgpolicy | 6 +++--- > 4 files changed, 37 insertions(+), 7 deletions(-) > > diff --git a/debian/postinst b/debian/postinst > index 98444d22..708350ec 100644 > --- a/debian/postinst > +++ b/debian/postinst > @@ -48,6 +48,10 @@ migrate_apt_auth_conf() { > fi > } > > +migrate_pmg_smtp_filter() { > + pmgdb update >/dev/null 2>&1 & > +} > + a) `pmgdb update` gets called below upon upgrade anyways b) the creation of the users happens in upgradedb, which gets called on `pmgdb init` (which also gets called upon upgrade below) - but I see that the naming does invite misunderstandings! > case "$1" in > triggered) > > @@ -67,6 +71,10 @@ case "$1" in > > if test ! -e /proxmox_install_mode ; then > > + if test -n "$2" && dpkg --compare-versions "$2" 'lt' '9.0.0'; then > + migrate_pmg_smtp_filter > + fi > + > pmgconf="/etc/pmg/pmg.conf" > if test -n "$2" && dpkg --compare-versions "$2" 'lt' '8.0.2'; then > # on upgrade add pre 8.0 default values for advfilter, use_awl and use_bayes > diff --git a/src/PMG/DBTools.pm b/src/PMG/DBTools.pm > index 1acc0cb6..7b5181ab 100644 > --- a/src/PMG/DBTools.pm > +++ b/src/PMG/DBTools.pm > @@ -38,7 +38,7 @@ sub cgreylist_merge_sql { > } > > sub open_ruledb { > - my ($database, $host, $port) = @_; > + my ($database, $host, $port, $user) = @_; > > $port //= 5432; > > @@ -74,13 +74,19 @@ sub open_ruledb { > return $rdb; > } else { > my $dsn = "DBI:Pg:dbname=$database;host=/var/run/postgresql;port=$port"; > - my $user = $> == 0 ? 'root' : 'www-data'; > + $user //= $> == 0 ? 'root' : 'www-data'; > my $dbh = DBI->connect($dsn, $user, undef, { PrintError => 0, RaiseError => 1 }); > > return $dbh; > } > } > > +sub open_ruledb_as { > + my ($database, $user) = @_; > + > + open_ruledb($database, undef, undef, $user); could we infer the user from `$ +} > + > sub delete_ruledb { > my ($dbname) = @_; > > @@ -609,6 +615,22 @@ sub upgradedb { > } > } > > + foreach my $user ('pmgpolicy', 'pmg-smtp-filter') { > + eval { > + my $silent_opts = { outfunc => sub {}, errfunc => sub {} }; > + postgres_admin_cmd('createuser', $silent_opts, '-D', $user); > + > + $dbh->begin_work; > + $dbh->do("GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA public TO \"$user\""); > + $dbh->do("GRANT SELECT, INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA public TO \"$user\""); this grants quite many rights to the users - which is probably ok (as the services currently have them as well and both need to write in some tables (statistics for both, greylisting for pmgpolicy at least (from memory)) but please do mention why you think it's ok in the commit-message/a comment - else it's hard to reason why this was ok a few years from now. > + $dbh->commit; this line here - or the use of `do`, or the `createuser` call failing after the user is created once and this line causes the following warning in the journal upon booting: ``` Aug 14 00:41:15 pmg-on-trixietest systemd[1]: Starting pmgsync.service - Sync Proxmox Configuration... Aug 14 00:41:15 pmg-on-trixietest pmgdb[246]: Analyzing/Upgrading existing Databases...rollback ineffective with AutoCommit enabled at /usr/share/perl5/PMG/DBTools.pm line 666. Aug 14 00:41:15 pmg-on-trixietest pmgdb[246]: rollback ineffective with AutoCommit enabled at /usr/share/perl5/PMG/DBTools.pm line 666. Aug 14 00:41:15 pmg-on-trixietest pmgdb[246]: done ``` > + > + }; > + if (my $err = $@) { > + $dbh->rollback; > + } > + } > + > foreach my $table (keys %$tables) { > eval { $dbh->do("ANALYZE $table"); }; > warn $@ if $@; > diff --git a/src/bin/pmg-smtp-filter b/src/bin/pmg-smtp-filter > index fcaaffc5..e95e6458 100755 > --- a/src/bin/pmg-smtp-filter > +++ b/src/bin/pmg-smtp-filter > @@ -387,7 +387,7 @@ sub load_config { > PMG::MailQueue::create_spooldirs($self->{cinfo}->{local}->{cid}); > > eval { > - my $dbh = PMG::DBTools::open_ruledb ($database); > + my $dbh = PMG::DBTools::open_ruledb_as($database, 'pmg-smtp-filter'); > $self->{ruledb} = PMG::RuleDB->new ($dbh); > > # load rulecache > @@ -538,7 +538,7 @@ sub run_dequeue { > > my $cinfo = PVE::INotify::read_file("cluster.conf"); > > - my $dbh = eval { PMG::DBTools::open_ruledb($database) }; > + my $dbh = eval { PMG::DBTools::open_ruledb_as($database, 'pmg-smtp-filter') }; > if ($err = $@) { > $self->log (0, "ERROR: $err"); > return; > diff --git a/src/bin/pmgpolicy b/src/bin/pmgpolicy > index 3f976ff7..92fb6f89 100755 > --- a/src/bin/pmgpolicy > +++ b/src/bin/pmgpolicy > @@ -142,7 +142,7 @@ sub run_dequeue { > my $dbh; > > eval { > - $dbh = PMG::DBTools::open_ruledb($database); > + $dbh = PMG::DBTools::open_ruledb_as($database, 'pmgpolicy'); > }; > my $err = $@; > > @@ -343,7 +343,7 @@ sub load_config { > my $dbh; > > eval { > - $dbh = PMG::DBTools::open_ruledb($database); > + $dbh = PMG::DBTools::open_ruledb_as($database, 'pmgpolicy'); > $self->{ruledb} = PMG::RuleDB->new($dbh); > $self->{rulecache} = PMG::RuleCache->new($self->{ruledb}); > }; > @@ -523,7 +523,7 @@ sub greylist_value { > $self->log(0, 'Database connection broken - trying to reconnect'); > my $dbh; > eval { > - $dbh = PMG::DBTools::open_ruledb($database); > + $dbh = PMG::DBTools::open_ruledb_as($database, 'pmgpolicy'); > }; > my $err = $@; > if ($err) { _______________________________________________ pmg-devel mailing list pmg-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel