[pmg-devel] [PATCH pmg-api/package-rebuilds v3] improve fetchmail handling in PMG

[pmg-devel] [PATCH pmg-api/package-rebuilds v3] improve fetchmail handling in PMG

[Stoiko Ivanov]

supersedes: https://lore.proxmox.com/pmg-devel/DD2PXG51NFKF.2D5G45FY3K66F@proxmox.com/T/#t

v2->v3:
* incoroporated parts of Max' feedback - thanks for the effort and looking
  at the details!
* The package-rebuild patches are needed for fetchmail (or rather our
  integration to work - so they fix a currently broken behavior)
* For pmg-api I amended the comment in the templated and added a patch,
  that restarts fetchmail on relevant config-changes.

Added Max' R-b/T-b tags only to patch 1 for pmg-api, and patch 2 for
package-rebuilds as those remain unchanged (save for a typo in the
commit message of package-rebuilds)

package-rebuilds:
Stoiko Ivanov (2):
  fetchmail: d/rules: do not skip dh_installinit
  fetchmail: improve shipped service file

 pkgs/fetchmail/fetchmail-6.4.39/debian/fetchmail.service | 3 ++-
 pkgs/fetchmail/fetchmail-6.4.39/debian/rules             | 3 +--
 2 files changed, 3 insertions(+), 3 deletions(-)


pmg-api:
Stoiko Ivanov (3):
  fix #6798: fetchmail: adapt to changed sslproto semantics
  templates: fetchmail: add comment where users can add manual accounts
  api: fetchmail: restart fetchmail on config changes

 src/PMG/API2/Fetchmail.pm    |  3 +++
 src/PMG/Fetchmail.pm         | 13 ++++++++++++-
 src/templates/fetchmailrc.tt |  3 +++
 3 files changed, 18 insertions(+), 1 deletion(-)

-- 
2.47.3



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH package-rebuilds v3 1/2] fetchmail: d/rules: do not skip dh_installinit

[Stoiko Ivanov]

while removing the init-script and override in d/rules, I skipped
dropping dh_installinit from the list of PHONY targets.

this resulted in /etc/default/fetchmail not being shipped by our
version of fetchmail, thus breaking adding accounts in the PMG GUI
(which rewrites /etc/default/fetchmail, and looks for START_DAEMON as
marker).

debugging this took me quite a bit longer than I wanted - so maybe the
short explanation might help a future reader:
* the issue was not due to changes in debhelper(7), which I initially
  thought, nor was it a bug in debhelper.
* `dh_installinit` does not expect a 'd/fetchmail.init' file in order
  to copy 'd/fetchmail.default'.
These things can quite well be checked by running
`dh binary --no-act -v` - it does list `dh_installinit` right before
`dh_tmpfiles`.

The core issue is that `.PHONY:` is a make-target and all of it's
prerequisites are phony targets[0]. this makes
`override_dh_installinit` materialize as target, which is empty,
because there are no commands in its recipe.
having an empty `override_dh_*` target is the way to tell
`dh(1)` to skip a step.

[0]https://www.gnu.org/software/make/manual/html_node/Special-Targets.html

Fixes:  90b2ccf ("fetchmail: replace sysv init script with systemd units")
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 pkgs/fetchmail/fetchmail-6.4.39/debian/rules | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/pkgs/fetchmail/fetchmail-6.4.39/debian/rules b/pkgs/fetchmail/fetchmail-6.4.39/debian/rules
index f0241e5..a56bde5 100755
--- a/pkgs/fetchmail/fetchmail-6.4.39/debian/rules
+++ b/pkgs/fetchmail/fetchmail-6.4.39/debian/rules
@@ -80,5 +80,4 @@ override_dh_auto_test:
 	dh $@
 
 .PHONY: override_dh_auto_configure override_dh_auto_install \
-	override_dh_installinit	override_dh_installsystemduser \
-	override_dh_auto_test
+	override_dh_installsystemduser override_dh_auto_test
-- 
2.47.3



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH package-rebuilds v3 2/2] fetchmail: improve shipped service file

[Stoiko Ivanov]

fetchmail exits with exit-code 3 if:
'The user authentication step failed...' (see fetchmail(1)).
This also includes the case if there are no accounts configured for
fetching, e.g. if all accounts are configured with 'skip' instead of
'poll'. In PMG you get this when temporary disabling all configured
accounts in the GUI.

So we simply should not consider an exit of 3 as failure.
Additionally adapt the Restart value to 'on-failure' (else systemd
tries restarting 5 times and gives up)
see systemd.service(5).

This behavior was chosen over configuring a larger time for
RestartSec= and increasing the number of restarts, as we should
restart fetchmail actively in pmg-api when its config changes.

Tested-by: Max R. Carrara <m.carrara@proxmox.com>
Reviewed-by: Max R. Carrara <m.carrara@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 pkgs/fetchmail/fetchmail-6.4.39/debian/fetchmail.service | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pkgs/fetchmail/fetchmail-6.4.39/debian/fetchmail.service b/pkgs/fetchmail/fetchmail-6.4.39/debian/fetchmail.service
index a6e3168..b7260ac 100644
--- a/pkgs/fetchmail/fetchmail-6.4.39/debian/fetchmail.service
+++ b/pkgs/fetchmail/fetchmail-6.4.39/debian/fetchmail.service
@@ -21,7 +21,8 @@ User=fetchmail
 Type=exec
 # sort $OPTIONS after "-daemon 300" to allow overwriting the interval using $OPTIONS
 ExecStart=/usr/bin/fetchmail --daemon 300 $OPTIONS --nodetach -f /etc/fetchmailrc --pidfile /run/fetchmail/fetchmail.pid
-Restart=always
+SuccessExitStatus=3
+Restart=on-failure
 
 [Install]
 WantedBy=multi-user.target
-- 
2.47.3



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api v3 1/3] fix #6798: fetchmail: adapt to changed sslproto semantics

[Stoiko Ivanov]

fetchmail defaults to verifying certificates since version 6.4.0
see fetchmail(1)
- sslproto defaults to auto instead of ''
- when sslproto is not '' then implicit/opportunistic TLS (StartTLS)
  is tried over the plain-text port
- this results in the current config parsing and writing to always
  try a TLS-connection if the server offers starttls

additionally sslcertck (only accept trusted certificates) defaults to
true since 6.4.0

The combination of these two things has as a consequence, that
unsetting 'use SSL' will fail for servers which have a self-signed
certificate installed (I expect many to still do so).

This patch simply fixes the 'use SSL' flag to disable all TLS
(explicit and opportunistic) and thus keep the expectations of users.

I did consider changing this to:
* either add a checkbox to ignore an invalid certificate (which feels
  quite wrong).
* allow users to provide a fingerprint instead (not considered
  further as fetchmail (in trixie) uses MD5 fingerprints, and this
  seems a step back).
* keep things as they currently are and document that users need to
  add the self-signed certificate to the system-trust-store
  (/usr/local/share/ca-certificates)

Since we ship versions with the semantic change since PMG 6.x (buster
shipped 6.4.0~beta43[0]) I don't think many users who use fetchmail
ran into this in the past few years - and most ISPs/mail providers
have valid certificates nowadays. So the potential for regression
should not be too large.

We could consider deprecating plain-text IMAP/POP in a future version,
but I'd announce the deprecation with 9.0 to give it some visibility.

[0] https://manpages.debian.org/buster/fetchmail/fetchmail.1.en.html

Tested-by: Max R. Carrara <m.carrara@proxmox.com>
Reviewed-by: Max R. Carrara <m.carrara@proxmox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Fetchmail.pm | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/PMG/Fetchmail.pm b/src/PMG/Fetchmail.pm
index 3a647420..c35e03d8 100644
--- a/src/PMG/Fetchmail.pm
+++ b/src/PMG/Fetchmail.pm
@@ -143,6 +143,11 @@ sub read_fetchmail_conf {
 
         my $finalize_item = sub {
             my ($item) = @_;
+
+            if ($item->{ssl} && !$item->{ssl_proto}) {
+                die "conflicting SSL settings for $item->{id}\n" if $item->{enabled};
+            }
+
             $cfg->{ $item->{id} } = $item;
         };
 
@@ -174,6 +179,8 @@ sub read_fetchmail_conf {
                 $item->{port} = $get_token_argument->();
             } elsif ($token eq 'interval') {
                 $item->{interval} = $get_token_argument->();
+            } elsif ($token eq 'sslproto') {
+                $item->{sslproto} = $get_token_argument->();
             } elsif (
                 $token eq 'ssl'
                 || $token eq 'keep'
@@ -210,7 +217,11 @@ sub write_fetchmail_conf {
         }
         $set_fetchmail_defaults->($item);
         my $options = ['dropdelivered'];
-        push @$options, 'ssl' if $item->{ssl};
+        if ($item->{ssl}) {
+            push @$options, 'ssl';
+        } else {
+            push @$options, ('sslproto', '\'\'');
+        }
         push @$options, 'keep' if $item->{keep};
         $item->{options} = join(' ', @$options);
         $data->{$id} = $item;
-- 
2.47.3



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api v3 2/3] templates: fetchmail: add comment where users can add manual accounts

[Stoiko Ivanov]

our fetchmail module uses /etc/fetchmailrc (symlinked to
/etc/pmg/fetchmailrc) as authoritative source for fetchmail accounts.

This means that if users need to make adaptations to fetchmail options
it breaks the handling of fetchmail in the API and GUI.

based on feedback from #6798 I think providing a hint where users
can add accounts with manual overrides, while keeping the API/GUI
working for all other accounts should help.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/templates/fetchmailrc.tt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/templates/fetchmailrc.tt b/src/templates/fetchmailrc.tt
index 76e591ca..d9886b59 100644
--- a/src/templates/fetchmailrc.tt
+++ b/src/templates/fetchmailrc.tt
@@ -9,6 +9,9 @@ defaults:
 
 smtphost [% ipconfig.int_ip %]/[% pmg.mail.ext_port %]
 
+# add manually configured accounts below and before 'proxmox settings'
+
+
 # proxmox settings (Do not delete this marker!!)
 [% FOREACH item IN fetchmail_users.list('values') %]
 [% IF item.enable %]poll[% ELSE %]skip[% END -%]
-- 
2.47.3



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api v3 3/3] api: fetchmail: restart fetchmail on config changes

[Stoiko Ivanov]

Based on feedback from Max (big Thanks!) in:
https://lore.proxmox.com/pmg-devel/DD2PXG51NFKF.2D5G45FY3K66F@proxmox.com/T/#t

The handling of fetchmail config can probably still be improved, but
restarting the daemon, when something changes (it was the easiest way
I found to get fetchmail to re-read its config) should improve UX
quite a bit. IIRC currently changing the config still requires
restarting fetchmail on the commandline (or rebooting).

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/API2/Fetchmail.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/PMG/API2/Fetchmail.pm b/src/PMG/API2/Fetchmail.pm
index d90c016e..d94b5f52 100644
--- a/src/PMG/API2/Fetchmail.pm
+++ b/src/PMG/API2/Fetchmail.pm
@@ -203,6 +203,7 @@ __PACKAGE__->register_method({
             $fmcfg->{$id} = $entry;
 
             PVE::INotify::write_file('fetchmailrc', $fmcfg);
+            PMG::Utils::service_cmd('fetchmail', 'restart')
         };
 
         PMG::Config::lock_config($code, "update fechtmail configuration failed");
@@ -242,6 +243,7 @@ __PACKAGE__->register_method({
             }
 
             PVE::INotify::write_file('fetchmailrc', $fmcfg);
+            PMG::Utils::service_cmd('fetchmail', 'restart')
         };
 
         PMG::Config::lock_config($code, "update fechtmail configuration failed");
@@ -280,6 +282,7 @@ __PACKAGE__->register_method({
             delete $fmcfg->{$id};
 
             PVE::INotify::write_file('fetchmailrc', $fmcfg);
+            PMG::Utils::service_cmd('fetchmail', 'restart')
         };
 
         PMG::Config::lock_config($code, "delete fechtmail configuration failed");
-- 
2.47.3



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] applied: [PATCH package-rebuilds v3 1/2] fetchmail: d/rules: do not skip dh_installinit

[Thomas Lamprecht]

On Fri, 26 Sep 2025 21:27:08 +0200, Stoiko Ivanov wrote:
> while removing the init-script and override in d/rules, I skipped
> dropping dh_installinit from the list of PHONY targets.
> 
> this resulted in /etc/default/fetchmail not being shipped by our
> version of fetchmail, thus breaking adding accounts in the PMG GUI
> (which rewrites /etc/default/fetchmail, and looks for START_DAEMON as
> marker).
> 
> [...]

Applied, thanks!

[1/2] fetchmail: d/rules: do not skip dh_installinit
      commit: 33880eb406a700e2a186d293c2226a950e045489


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] applied: [PATCH package-rebuilds v3 2/2] fetchmail: improve shipped service file

[Thomas Lamprecht]

On Fri, 26 Sep 2025 21:27:09 +0200, Stoiko Ivanov wrote:
> fetchmail exits with exit-code 3 if:
> 'The user authentication step failed...' (see fetchmail(1)).
> This also includes the case if there are no accounts configured for
> fetching, e.g. if all accounts are configured with 'skip' instead of
> 'poll'. In PMG you get this when temporary disabling all configured
> accounts in the GUI.
> 
> [...]

Applied, thanks!

[2/2] fetchmail: improve shipped service file
      commit: 1bda2a94387bb8d472ae077d282aaf495238e7df


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] applied: [PATCH pmg-api v3 1/3] fix #6798: fetchmail: adapt to changed sslproto semantics

[Thomas Lamprecht]

On Fri, 26 Sep 2025 21:27:10 +0200, Stoiko Ivanov wrote:
> fetchmail defaults to verifying certificates since version 6.4.0
> see fetchmail(1)
> - sslproto defaults to auto instead of ''
> - when sslproto is not '' then implicit/opportunistic TLS (StartTLS)
>   is tried over the plain-text port
> - this results in the current config parsing and writing to always
>   try a TLS-connection if the server offers starttls
> 
> [...]

Applied, thanks!

[1/3] fix #6798: fetchmail: adapt to changed sslproto semantics
      commit: d10204f3896f88ec459bdfb842f6fa0d8d45df56


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] applied: [PATCH pmg-api v3 2/3] templates: fetchmail: add comment where users can add manual accounts

[Thomas Lamprecht]

On Fri, 26 Sep 2025 21:27:11 +0200, Stoiko Ivanov wrote:
> our fetchmail module uses /etc/fetchmailrc (symlinked to
> /etc/pmg/fetchmailrc) as authoritative source for fetchmail accounts.
> 
> This means that if users need to make adaptations to fetchmail options
> it breaks the handling of fetchmail in the API and GUI.
> 
> based on feedback from #6798 I think providing a hint where users
> can add accounts with manual overrides, while keeping the API/GUI
> working for all other accounts should help.
> 
> [...]

Applied, thanks!

[2/3] templates: fetchmail: add comment where users can add manual accounts
      commit: dd3aa49d13568990e363e8e9dc4d1b06d1fb4a9e


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] applied: [PATCH pmg-api v3 3/3] api: fetchmail: restart fetchmail on config changes

[Thomas Lamprecht]

On Fri, 26 Sep 2025 21:27:12 +0200, Stoiko Ivanov wrote:
> Based on feedback from Max (big Thanks!) in:
> https://lore.proxmox.com/pmg-devel/DD2PXG51NFKF.2D5G45FY3K66F@proxmox.com/T/#t
> 
> The handling of fetchmail config can probably still be improved, but
> restarting the daemon, when something changes (it was the easiest way
> I found to get fetchmail to re-read its config) should improve UX
> quite a bit. IIRC currently changing the config still requires
> restarting fetchmail on the commandline (or rebooting).
> 
> [...]

Applied, thanks!

[3/3] api: fetchmail: restart fetchmail on config changes
      commit: 32087c64cc439ffd8549e577bddd6c1554b93038


_______________________________________________
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/package-rebuilds v3] improve fetchmail handling in PMG

[Max R. Carrara]

On Fri Sep 26, 2025 at 9:27 PM CEST, Stoiko Ivanov wrote:
> supersedes: https://lore.proxmox.com/pmg-devel/DD2PXG51NFKF.2D5G45FY3K66F@proxmox.com/T/#t
>
> v2->v3:
> * incoroporated parts of Max' feedback - thanks for the effort and looking
>   at the details!
> * The package-rebuild patches are needed for fetchmail (or rather our
>   integration to work - so they fix a currently broken behavior)
> * For pmg-api I amended the comment in the templated and added a patch,
>   that restarts fetchmail on relevant config-changes.
>
> Added Max' R-b/T-b tags only to patch 1 for pmg-api, and patch 2 for
> package-rebuilds as those remain unchanged (save for a typo in the
> commit message of package-rebuilds)
>
> [...]

Gave this series another spin on my two-node PMG test cluster.

The tl;dr is: Everything appears to work as advertised. Nice job!

package-rebuilds (Patches #1 and #2):

- Even though those two patches were applied already, I've also applied
  them locally, then built and installed the `fetchmail` package

- On the non-master node, the `/etc/default/fetchmail` file
  didn't exist yet; upon installing the package, it appeared again

- On the master node, the `/etc/default/fetchmail` file existed already
  because I created it manually before; upon installing the package, it
  wasn't changed and remained the same (as expected)

pmg-api (Patches #3-5):

- Patch #3 remains unchanged, but for completeness's sake, I also tested
  it again
  - `fetchmail` was able to connect to our mail server with my
    credentials + SSL enabled; works as advertised

- The comment in patch #4 now looks nicer, thanks for incorporating that
  little bit of feedback!

- Patch #5 also works as advertised
  - Created a bunch of dummy accounts via the GUI
  - `fetchmail.service` continued to work as expected, despite the
    restarts
  - I wasn't able to trip `fetchmail.service` up through the GUI, so I
    think that users that create / write / delete a lot of fetchmail
    accounts should also be fine here

To elaborate more on patch #5, since I didn't get fetchmail to trip up,
I think it's perfectly fine to simply restart the service like that.
However, for future reference, if frequent restarts become a problem for
some reason, we can add a call to `sleep` in the `fetchmail.service` unit.

In particular, this would have to be done in `ExecStart=`, as any other
options (that I've tested) cause `systemctl restart` to hang. The
startup delay could be added in a crude way like this:

ExecStart=/usr/bin/bash -c "/usr/bin/sleep 60 && /usr/bin/fetchmail --daemon 300 $OPTIONS --nodetach -f /etc/fetchmailrc --pidfile /run/fetchmail/fetchmail.pid"

During frequent restarts, systemd will terminate the `sleep 60`
(gracefully) if fetchmail itself wasn't yet running; then, the delay
starts again.

Note that I'm not sure if `$OPTIONS` is substituted correctly here; I
only tested the bare minimum there. The best solution—should it actually
be necessary in the future—would probably be to supply a wrapper script
that contains the call to sleep and also passes `$OPTIONS` along to
fetchmail.

Anyway, with all of that aside: LGTM!

Consider:

Tested-by: Max R. Carrara <m.carrara@proxmox.com>
Reviewed-by: Max R. Carrara <m.carrara@proxmox.com>


_______________________________________________
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/package-rebuilds v3] improve fetchmail handling in PMG

[Max R. Carrara]

On Mon Sep 29, 2025 at 3:26 PM CEST, Max R. Carrara wrote:
> On Fri Sep 26, 2025 at 9:27 PM CEST, Stoiko Ivanov wrote:
> > supersedes: https://lore.proxmox.com/pmg-devel/DD2PXG51NFKF.2D5G45FY3K66F@proxmox.com/T/#t
> >
> > v2->v3:
> > * incoroporated parts of Max' feedback - thanks for the effort and looking
> >   at the details!
> > * The package-rebuild patches are needed for fetchmail (or rather our
> >   integration to work - so they fix a currently broken behavior)
> > * For pmg-api I amended the comment in the templated and added a patch,
> >   that restarts fetchmail on relevant config-changes.
> >
> > Added Max' R-b/T-b tags only to patch 1 for pmg-api, and patch 2 for
> > package-rebuilds as those remain unchanged (save for a typo in the
> > commit message of package-rebuilds)
> >
> > [...]
>
> Gave this series another spin on my two-node PMG test cluster.
>
> The tl;dr is: Everything appears to work as advertised. Nice job!
>

Ah nevermind, didn't see it was applied in the meantime. Nevertheless,
the series works fine for me! :)


_______________________________________________
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/package-rebuilds v3] improve fetchmail handling in PMG

[Thomas Lamprecht]

Am 29.09.25 um 15:28 schrieb Max R. Carrara:
>> Gave this series another spin on my two-node PMG test cluster.
>>
>> The tl;dr is: Everything appears to work as advertised. Nice job!
>>
> Ah nevermind, didn't see it was applied in the meantime. Nevertheless,
> the series works fine for me! 🙂
> 

Yeah I pulled it a bit before your mail here, but with the links to
the lore archive being in the commits you're test feedback is at
least referenced and indirectly recorded that way.


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel