* [PATCH 0/6] lei auth-related fixes
@ 2021-04-03 2:24 Eric Wong
2021-04-03 2:24 ` [PATCH 1/6] URInntps: add URI 5.08 release note Eric Wong
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Eric Wong @ 2021-04-03 2:24 UTC (permalink / raw)
To: meta
Auth failure handling should be more complete, but it's still
horrifically slow to test because the dovecot instance I run has
rate-limits for auth failures.
Perhaps public-inbox-imapd should learn to reject certain
username + password combos, since the -imapd instance on
public-inbox.org sees a fair amount of automated traffic
looking to steal info off leaked credentials.
3/6 is me still learning to use APIs I invent :x
Eric Wong (6):
URInntps: add URI 5.08 release note
lei q: ensure wq workers shutdown on IMAP auth failures
lei tag: fix tagging of IMAP inputs
lei_auth: rename {net_merge} to {net_merge_continue}
net_reader: fix read-only "lei convert" auth failures
xt/lei-auth-fail: test more failure cases
lib/PublicInbox/LEI.pm | 24 ++++++++----------------
lib/PublicInbox/LeiAuth.pm | 17 ++++++++++-------
lib/PublicInbox/LeiTag.pm | 6 +++++-
lib/PublicInbox/NetReader.pm | 5 +++--
lib/PublicInbox/URInntps.pm | 1 +
t/lei-import-imap.t | 1 +
xt/lei-auth-fail.t | 11 +++++++----
7 files changed, 35 insertions(+), 30 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/6] URInntps: add URI 5.08 release note
2021-04-03 2:24 [PATCH 0/6] lei auth-related fixes Eric Wong
@ 2021-04-03 2:24 ` Eric Wong
2021-04-03 2:24 ` [PATCH 2/6] lei q: ensure wq workers shutdown on IMAP auth failures Eric Wong
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-04-03 2:24 UTC (permalink / raw)
To: meta
I wanted to say 2031, but that's probably too aggressive a
removal timeline.
---
lib/PublicInbox/URInntps.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/PublicInbox/URInntps.pm b/lib/PublicInbox/URInntps.pm
index 69fe7163..231b247b 100644
--- a/lib/PublicInbox/URInntps.pm
+++ b/lib/PublicInbox/URInntps.pm
@@ -4,6 +4,7 @@
# deal with the lack of URI::nntps in upstream URI.
# nntps is IANA registered, snews is deprecated
# cf. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983419
+# Fixed in URI 5.08, we can drop this by 2035 when LTS distros all have it
package PublicInbox::URInntps;
use strict;
use parent qw(URI::snews);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/6] lei q: ensure wq workers shutdown on IMAP auth failures
2021-04-03 2:24 [PATCH 0/6] lei auth-related fixes Eric Wong
2021-04-03 2:24 ` [PATCH 1/6] URInntps: add URI 5.08 release note Eric Wong
@ 2021-04-03 2:24 ` Eric Wong
2021-04-03 2:24 ` [PATCH 3/6] lei tag: fix tagging of IMAP inputs Eric Wong
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-04-03 2:24 UTC (permalink / raw)
To: meta
Leaving workers running on after auth failures is bad and messy,
cleanup our process management to have consistent worker
teardowns. Improve error reporting, too, instead of letting
Mail::IMAPClient->exists fail due to undef.
---
lib/PublicInbox/LEI.pm | 24 ++++++++----------------
lib/PublicInbox/LeiAuth.pm | 15 +++++++++------
lib/PublicInbox/NetReader.pm | 2 +-
3 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index f9361c68..cdb4b347 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -350,6 +350,11 @@ my %CONFIG_KEYS = (
my @WQ_KEYS = qw(lxs l2m imp mrr cnv p2q tag sol); # internal workers
+sub _drop_wq {
+ my ($self) = @_;
+ for my $wq (grep(defined, delete(@$self{@WQ_KEYS}))) { $wq->DESTROY }
+}
+
# pronounced "exit": x_it(1 << 8) => exit(1); x_it(13) => SIGPIPE
sub x_it ($$) {
my ($self, $code) = @_;
@@ -360,10 +365,7 @@ sub x_it ($$) {
send($s, "x_it $code", MSG_EOR);
} elsif ($self->{oneshot}) {
# don't want to end up using $? from child processes
- for my $f (@WQ_KEYS) {
- my $wq = delete $self->{$f} or next;
- $wq->DESTROY;
- }
+ _drop_wq($self);
# cleanup anything that has tempfiles or open file handles
%PATH2CFG = ();
delete @$self{qw(ovv dedupe sto cfg)};
@@ -392,11 +394,8 @@ sub qerr ($;@) { $_[0]->{opt}->{quiet} or err(shift, @_) }
sub fail_handler ($;$$) {
my ($lei, $code, $io) = @_;
- for my $f (@WQ_KEYS) {
- my $wq = delete $lei->{$f} or next;
- $wq->wq_wait_old(undef, $lei) if $wq->wq_kill_old; # lei-daemon
- }
close($io) if $io; # needed to avoid warnings on SIGPIPE
+ _drop_wq($lei);
x_it($lei, $code // (1 << 8));
}
@@ -983,14 +982,7 @@ sub accept_dispatch { # Listener {post_accept} callback
sub dclose {
my ($self) = @_;
delete $self->{-progress};
- for my $f (@WQ_KEYS) {
- my $wq = delete $self->{$f} or next;
- if ($wq->wq_kill) {
- $wq->wq_close(0, undef, $self);
- } elsif ($wq->wq_kill_old) {
- $wq->wq_wait_old(undef, $self);
- }
- }
+ _drop_wq($self);
close(delete $self->{1}) if $self->{1}; # may reap_compress
$self->close if $self->{-event_init_done}; # PublicInbox::DS::close
}
diff --git a/lib/PublicInbox/LeiAuth.pm b/lib/PublicInbox/LeiAuth.pm
index 927fe550..48deca93 100644
--- a/lib/PublicInbox/LeiAuth.pm
+++ b/lib/PublicInbox/LeiAuth.pm
@@ -13,12 +13,15 @@ sub do_auth_atfork { # used by IPC WQ workers
return if $wq->{-wq_worker_nr} != 0;
my $lei = $wq->{lei};
my $net = $lei->{net};
- my $mics = $net->imap_common_init($lei);
- my $nn = $net->nntp_common_init($lei);
- pkt_do($lei->{pkt_op_p}, 'net_merge', $net) or
- die "pkt_do net_merge: $!";
- $net->{mics_cached} = $mics if $mics;
- $net->{nn_cached} = $nn if $nn;
+ eval {
+ my $mics = $net->imap_common_init($lei);
+ my $nn = $net->nntp_common_init($lei);
+ pkt_do($lei->{pkt_op_p}, 'net_merge', $net) or
+ die "pkt_do net_merge: $!";
+ $net->{mics_cached} = $mics if $mics;
+ $net->{nn_cached} = $nn if $nn;
+ };
+ $lei->fail($@) if $@;
}
sub net_merge_done1 { # bump merge-count in top-level lei-daemon
diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 6a52b479..c269d841 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -267,7 +267,7 @@ sub imap_common_init ($;$) {
$mics->{$sec} //= mic_for($self, "$sec/", $mic_args, $lei);
next unless $self->isa('PublicInbox::NetWriter');
my $dst = $uri->mailbox // next;
- my $mic = $mics->{$sec};
+ my $mic = $mics->{$sec} // die "Unable to continue\n";
next if $mic->exists($dst); # already exists
$mic->create($dst) or die "CREATE $dst failed <$uri>: $@";
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/6] lei tag: fix tagging of IMAP inputs
2021-04-03 2:24 [PATCH 0/6] lei auth-related fixes Eric Wong
2021-04-03 2:24 ` [PATCH 1/6] URInntps: add URI 5.08 release note Eric Wong
2021-04-03 2:24 ` [PATCH 2/6] lei q: ensure wq workers shutdown on IMAP auth failures Eric Wong
@ 2021-04-03 2:24 ` Eric Wong
2021-04-03 2:24 ` [PATCH 4/6] lei_auth: rename {net_merge} to {net_merge_continue} Eric Wong
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-04-03 2:24 UTC (permalink / raw)
To: meta
We need net_merge_all and to lock the number of worker jobs.
Parallel inputs are not supported, yet (is it needed?, I don't
expect this to be used for multiple files very often...).
---
lib/PublicInbox/LeiTag.pm | 6 +++++-
t/lei-import-imap.t | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/PublicInbox/LeiTag.pm b/lib/PublicInbox/LeiTag.pm
index d572a84a..06313139 100644
--- a/lib/PublicInbox/LeiTag.pm
+++ b/lib/PublicInbox/LeiTag.pm
@@ -112,7 +112,8 @@ sub lei_tag { # the "lei tag" method
my $ops = { '' => [ \&tag_done, $lei ] };
$lei->{auth}->op_merge($ops, $self) if $lei->{auth};
$self->{vmd_mod} = $vmd_mod;
- (my $op_c, $ops) = $lei->workers_start($self, 'lei_tag', 1, $ops);
+ my $j = $self->{-wq_nr_workers} = 1; # locked for now
+ (my $op_c, $ops) = $lei->workers_start($self, 'lei_tag', $j, $ops);
$lei->{tag} = $self;
net_merge_complete($self) unless $lei->{auth};
$op_c->op_wait_event($ops);
@@ -175,4 +176,7 @@ sub _complete_tag {
} grep(/$re\Q$cur/, @all);
}
+no warnings 'once'; # the following works even when LeiAuth is lazy-loaded
+*net_merge_all = \&PublicInbox::LeiAuth::net_merge_all;
+
1;
diff --git a/t/lei-import-imap.t b/t/lei-import-imap.t
index fd38037a..7e4d44b9 100644
--- a/t/lei-import-imap.t
+++ b/t/lei-import-imap.t
@@ -23,5 +23,6 @@ test_lei({ tmpdir => $tmpdir }, sub {
my %r;
for (@$out) { $r{ref($_)}++ }
is_deeply(\%r, { 'HASH' => scalar(@$out) }, 'all hashes');
+ lei_ok([qw(tag +kw:seen), "imap://$host_port/t.v2.0"], undef, undef);
});
done_testing;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/6] lei_auth: rename {net_merge} to {net_merge_continue}
2021-04-03 2:24 [PATCH 0/6] lei auth-related fixes Eric Wong
` (2 preceding siblings ...)
2021-04-03 2:24 ` [PATCH 3/6] lei tag: fix tagging of IMAP inputs Eric Wong
@ 2021-04-03 2:24 ` Eric Wong
2021-04-03 2:24 ` [PATCH 5/6] net_reader: fix read-only "lei convert" auth failures Eric Wong
2021-04-03 2:24 ` [PATCH 6/6] xt/lei-auth-fail: test more failure cases Eric Wong
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-04-03 2:24 UTC (permalink / raw)
To: meta
No reason for the hash key to differ from the subroutine
name, here.
---
lib/PublicInbox/LeiAuth.pm | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/PublicInbox/LeiAuth.pm b/lib/PublicInbox/LeiAuth.pm
index 48deca93..e7b26c5f 100644
--- a/lib/PublicInbox/LeiAuth.pm
+++ b/lib/PublicInbox/LeiAuth.pm
@@ -16,8 +16,8 @@ sub do_auth_atfork { # used by IPC WQ workers
eval {
my $mics = $net->imap_common_init($lei);
my $nn = $net->nntp_common_init($lei);
- pkt_do($lei->{pkt_op_p}, 'net_merge', $net) or
- die "pkt_do net_merge: $!";
+ pkt_do($lei->{pkt_op_p}, 'net_merge_continue', $net) or
+ die "pkt_do net_merge_continue: $!";
$net->{mics_cached} = $mics if $mics;
$net->{nn_cached} = $nn if $nn;
};
@@ -46,7 +46,7 @@ sub net_merge_continue {
sub op_merge { # prepares PktOp->pair ops
my ($self, $ops, $wq) = @_;
- $ops->{net_merge} = [ \&net_merge_continue, $wq ];
+ $ops->{net_merge_continue} = [ \&net_merge_continue, $wq ];
$ops->{net_merge_done1} = [ \&net_merge_done1, $wq ];
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/6] net_reader: fix read-only "lei convert" auth failures
2021-04-03 2:24 [PATCH 0/6] lei auth-related fixes Eric Wong
` (3 preceding siblings ...)
2021-04-03 2:24 ` [PATCH 4/6] lei_auth: rename {net_merge} to {net_merge_continue} Eric Wong
@ 2021-04-03 2:24 ` Eric Wong
2021-04-03 2:24 ` [PATCH 6/6] xt/lei-auth-fail: test more failure cases Eric Wong
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-04-03 2:24 UTC (permalink / raw)
To: meta
"convert" is actually a bit more complicated than "lei import"
since it may need auth for either input or output.
---
lib/PublicInbox/NetReader.pm | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index c269d841..821e5d7f 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -264,10 +264,11 @@ sub imap_common_init ($;$) {
my $mics = {}; # schema://authority => IMAPClient obj
for my $uri (@{$self->{imap_order}}) {
my $sec = uri_section($uri);
- $mics->{$sec} //= mic_for($self, "$sec/", $mic_args, $lei);
+ my $mic = $mics->{$sec} //=
+ mic_for($self, "$sec/", $mic_args, $lei) //
+ die "Unable to continue\n";
next unless $self->isa('PublicInbox::NetWriter');
my $dst = $uri->mailbox // next;
- my $mic = $mics->{$sec} // die "Unable to continue\n";
next if $mic->exists($dst); # already exists
$mic->create($dst) or die "CREATE $dst failed <$uri>: $@";
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 6/6] xt/lei-auth-fail: test more failure cases
2021-04-03 2:24 [PATCH 0/6] lei auth-related fixes Eric Wong
` (4 preceding siblings ...)
2021-04-03 2:24 ` [PATCH 5/6] net_reader: fix read-only "lei convert" auth failures Eric Wong
@ 2021-04-03 2:24 ` Eric Wong
5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-04-03 2:24 UTC (permalink / raw)
To: meta
Because failures are often overlooked, unfortunately.
---
xt/lei-auth-fail.t | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/xt/lei-auth-fail.t b/xt/lei-auth-fail.t
index e352aab3..06cb8533 100644
--- a/xt/lei-auth-fail.t
+++ b/xt/lei-auth-fail.t
@@ -2,17 +2,20 @@
# Copyright (C) 2021 all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
use strict; use v5.10.1; use PublicInbox::TestCommon;
-require_mods(qw(Mail::IMAPClient));
+require_mods(qw(Mail::IMAPClient lei));
# TODO: mock IMAP server which fails at authentication so we don't
# have to make external connections to test this:
my $imap_fail = $ENV{TEST_LEI_IMAP_FAIL_URL} //
'imaps://AzureDiamond:Hunter2@public-inbox.org:994/INBOX';
+my ($ro_home, $cfg_path) = setup_public_inboxes;
test_lei(sub {
- for my $pfx ([qw(convert -o mboxrd:/dev/stdout)], ['import'],
- [qw(tag +L:INBOX)]) {
+ for my $pfx ([qw(q z:0.. --only), "$ro_home/t1", '-o'],
+ [qw(convert -o mboxrd:/dev/stdout)],
+ [qw(convert t/utf8.eml -o), $imap_fail],
+ ['import'], [qw(tag +L:INBOX)]) {
ok(!lei(@$pfx, $imap_fail), "IMAP auth failure on @$pfx");
- like($lei_err, qr!\bE:.*?imaps://.*?!sm, 'error shown');
+ like($lei_err, qr!\bE:.*?imaps?://.*?!sm, 'error shown');
unlike($lei_err, qr!Hunter2!s, 'password not shown');
is($lei_out, '', 'nothing output');
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-03 2:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-03 2:24 [PATCH 0/6] lei auth-related fixes Eric Wong
2021-04-03 2:24 ` [PATCH 1/6] URInntps: add URI 5.08 release note Eric Wong
2021-04-03 2:24 ` [PATCH 2/6] lei q: ensure wq workers shutdown on IMAP auth failures Eric Wong
2021-04-03 2:24 ` [PATCH 3/6] lei tag: fix tagging of IMAP inputs Eric Wong
2021-04-03 2:24 ` [PATCH 4/6] lei_auth: rename {net_merge} to {net_merge_continue} Eric Wong
2021-04-03 2:24 ` [PATCH 5/6] net_reader: fix read-only "lei convert" auth failures Eric Wong
2021-04-03 2:24 ` [PATCH 6/6] xt/lei-auth-fail: test more failure cases Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).