unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 0/8] IMAP/NNTP client improvements
@ 2023-10-03  6:43 Eric Wong
  2023-10-03  6:43 ` [PATCH 1/8] net_reader: bail out on NNTP SOCKS connection failure Eric Wong
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Eric Wong @ 2023-10-03  6:43 UTC (permalink / raw)
  To: meta

These affect both lei and -watch

Eric Wong (8):
  net_reader: bail out on NNTP SOCKS connection failure
  net_reader: avoid IO::Socket::SSL 2.079..2.081 warning
  config: fix key-only truthy values with urlmatch
  net_reader: support imap.sslVerify + nntp.sslVerify
  lei: workers exit after they tell lei-daemon
  net_reader: process title reflects NNTP article
  xt/lei-onion-convert: test TLS + SOCKS
  net_reader: note glob support in .onion hint

 lib/PublicInbox/Config.pm       | 18 ++++---
 lib/PublicInbox/LEI.pm          |  1 +
 lib/PublicInbox/NetNNTPSocks.pm | 10 ++--
 lib/PublicInbox/NetReader.pm    | 87 +++++++++++++++++++++------------
 t/imapd-tls.t                   | 18 +++++--
 t/nntpd-tls.t                   | 19 +++++--
 xt/lei-onion-convert.t          | 21 ++++++--
 7 files changed, 123 insertions(+), 51 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/8] net_reader: bail out on NNTP SOCKS connection failure
  2023-10-03  6:43 [PATCH 0/8] IMAP/NNTP client improvements Eric Wong
@ 2023-10-03  6:43 ` Eric Wong
  2023-10-03  6:43 ` [PATCH 2/8] net_reader: avoid IO::Socket::SSL 2.079..2.081 warning Eric Wong
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2023-10-03  6:43 UTC (permalink / raw)
  To: meta

It takes some effort to get Net::NNTP and IO::Socket::Socks
to place nice together, but we don't want the setsockopt
call to fail on an undefined value.  So die with an error
that tries to show various possible error sources.

$SOCKS_ERROR is a special variable, so even using `//'
(defined-or) operator isn't enough to squelch warnings
about using it in its uninitialized state.
---
 lib/PublicInbox/NetNNTPSocks.pm | 10 ++++++----
 lib/PublicInbox/NetReader.pm    |  3 +--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/PublicInbox/NetNNTPSocks.pm b/lib/PublicInbox/NetNNTPSocks.pm
index fcd2e580..5b15dd59 100644
--- a/lib/PublicInbox/NetNNTPSocks.pm
+++ b/lib/PublicInbox/NetNNTPSocks.pm
@@ -17,16 +17,18 @@ sub new_socks {
 	local %OPT = map {;
 		defined($opt{$_}) ? ($_ => $opt{$_}) : ()
 	} @SOCKS_KEYS;
-	Net::NNTP->new(%opt); # this calls our new() below:
+	no warnings 'uninitialized'; # needed for $SOCKS_ERROR
+	Net::NNTP->new(%opt) // die "errors: \$!=$! SOCKS=",
+				eval('$IO::Socket::Socks::SOCKS_ERROR // ""'),
+				', SSL=',
+				(eval('IO::Socket::SSL->errstr')  // ''), "\n";
 }
 
 # called by Net::NNTP->new
 sub new {
 	my ($self, %opt) = @_;
 	@OPT{qw(ConnectAddr ConnectPort)} = @opt{qw(PeerAddr PeerPort)};
-	my $ret = $self->SUPER::new(%OPT) or
-		die 'SOCKS error: '.eval('$IO::Socket::Socks::SOCKS_ERROR');
-	$ret;
+	$self->SUPER::new(%OPT);
 }
 
 1;
diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 6802fa72..32e4c20f 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -180,8 +180,7 @@ sub nn_new ($$$) {
 	if (defined $nn_arg->{ProxyAddr}) {
 		require PublicInbox::NetNNTPSocks;
 		$nn_arg->{SocksDebug} = 1 if $nn_arg->{Debug};
-		eval { $nn = PublicInbox::NetNNTPSocks->new_socks(%$nn_arg) };
-		die "E: <$uri> $@\n" if $@;
+		$nn = PublicInbox::NetNNTPSocks->new_socks(%$nn_arg) or return;
 	} else {
 		$nn = Net::NNTP->new(%$nn_arg) or return;
 	}

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/8] net_reader: avoid IO::Socket::SSL 2.079..2.081 warning
  2023-10-03  6:43 [PATCH 0/8] IMAP/NNTP client improvements Eric Wong
  2023-10-03  6:43 ` [PATCH 1/8] net_reader: bail out on NNTP SOCKS connection failure Eric Wong
@ 2023-10-03  6:43 ` Eric Wong
  2023-10-03  6:43 ` [PATCH 3/8] config: fix key-only truthy values with urlmatch Eric Wong
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2023-10-03  6:43 UTC (permalink / raw)
  To: meta

IO::Socket::SSL had an unitialized variable warning from a bad
regexp for a few releases.  This will also prepare us to support
imap.sslverify as git does and possibly other TLS-related
options.
---
 lib/PublicInbox/NetReader.pm | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 32e4c20f..e14b5805 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -40,6 +40,15 @@ EOM
 	die "$val not understood (only socks5h:// is supported)\n";
 }
 
+# gives an arrayref suitable for the Mail::IMAPClient Ssl or Starttls arg
+sub mic_tls_opt ($$) {
+	my ($o, $hostname) = @_;
+	require IO::Socket::SSL;
+	$o = {} if !ref($o);
+	$o->{SSL_hostname} //= $hostname;
+	[ map { ($_, $o->{$_}) } keys %$o ];
+}
+
 sub mic_new ($$$$) {
 	my ($self, $mic_arg, $sec, $uri) = @_;
 	my %mic_arg = (%$mic_arg, Keepalive => 1);
@@ -54,12 +63,20 @@ sub mic_new ($$$$) {
 		$opt{ConnectPort} = delete $mic_arg{Port};
 		my $s = IO::Socket::Socks->new(%opt) or die
 			"E: <$uri> ".eval('$IO::Socket::Socks::SOCKS_ERROR');
-		if ($mic_arg->{Ssl}) { # for imaps://
-			require IO::Socket::SSL;
-			$s = IO::Socket::SSL->start_SSL($s) or die
+		if (my $o = delete $mic_arg{Ssl}) { # for imaps://
+			$o = mic_tls_opt($o, $opt{ConnectAddr});
+			$s = IO::Socket::SSL->start_SSL($s, @$o) or die
 				"E: <$uri> ".(IO::Socket::SSL->errstr // '');
+		} elsif ($o = $mic_arg{Starttls}) {
+			# Mail::IMAPClient will use this:
+			$mic_arg{Starttls} = mic_tls_opt($o, $opt{ConnectAddr});
 		}
 		$mic_arg{Socket} = $s;
+	} elsif ($mic_arg{Ssl} || $mic_arg{Starttls}) {
+		for my $f (qw(Ssl Starttls)) {
+			my $o = $mic_arg{$f} or next;
+			$mic_arg{$f} = mic_tls_opt($o, $mic_arg{Server});
+		}
 	}
 	PublicInbox::IMAPClient->new(%mic_arg);
 }

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/8] config: fix key-only truthy values with urlmatch
  2023-10-03  6:43 [PATCH 0/8] IMAP/NNTP client improvements Eric Wong
  2023-10-03  6:43 ` [PATCH 1/8] net_reader: bail out on NNTP SOCKS connection failure Eric Wong
  2023-10-03  6:43 ` [PATCH 2/8] net_reader: avoid IO::Socket::SSL 2.079..2.081 warning Eric Wong
@ 2023-10-03  6:43 ` Eric Wong
  2023-10-03  6:43 ` [PATCH 4/8] net_reader: support imap.sslVerify + nntp.sslVerify Eric Wong
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2023-10-03  6:43 UTC (permalink / raw)
  To: meta

When using --get-urlmatch, we need a way to distinguish between
between key-only or a `key=val' pair even if the `val' is empty.
In other words, git interprets `-c imap.debug' as true and
`-c imap.debug=' as false, but an untyped --get-urlmatch
invocation has no way to distinguish between them.

So we must specify we want `--bool' (we're avoiding `--type=bool'
since that only appears in git 2.18+)

Fixes: f170d220f876 (lei: fix `-c NAME=VALUE' config support)
---
 lib/PublicInbox/Config.pm    | 18 ++++++++++++------
 lib/PublicInbox/NetReader.pm | 18 +++++-------------
 t/imapd-tls.t                |  4 +++-
 t/nntpd-tls.t                |  4 +++-
 4 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index 9f764c32..15e0872e 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -568,28 +568,34 @@ sub config_cmd {
 }
 
 sub urlmatch {
-	my ($self, $key, $url, $try_git) = @_;
+	my $self = shift;
+	my @bool = $_[0] eq '--bool' ? (shift) : ();
+	my ($key, $url, $try_git) = @_;
 	state $urlmatch_broken; # requires git 1.8.5
 	return if $urlmatch_broken;
 	my (%env, %opt);
 	my $cmd = $self->config_cmd(\%env, \%opt);
-	push @$cmd, qw(-z --includes --get-urlmatch), $key, $url;
+	push @$cmd, @bool, qw(--includes -z --get-urlmatch), $key, $url;
 	my $fh = popen_rd($cmd, \%env, \%opt);
 	local $/ = "\0";
 	my $val = <$fh>;
 	if (!close($fh)) {
 		undef $val;
-		if (($? >> 8) != 1) {
+		if (@bool && ($? >> 8) == 128) { # not boolean
+		} elsif (($? >> 8) != 1) {
 			$urlmatch_broken = 1;
 		} elsif ($try_git) { # n.b. this takes cwd into account
-			$cmd = [qw(git config -z --get-urlmatch), $key, $url];
-			$fh = popen_rd($cmd);
+			$fh = popen_rd([qw(git config), @bool,
+					qw(-z --get-urlmatch), $key, $url]);
 			$val = <$fh>;
 			close($fh) or undef($val);
 		}
 	}
 	$? = 0; # don't influence lei exit status
-	chomp $val if defined $val;
+	if (defined($val)) {
+		chomp $val;
+		$val = git_bool($val) if @bool;
+	}
 	$val;
 }
 
diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index e14b5805..5819f210 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -319,14 +319,6 @@ sub cfg_intvl ($$$) {
 	}
 }
 
-sub cfg_bool ($$$) {
-	my ($cfg, $key, $url) = @_;
-	my $orig = $cfg->urlmatch($key, $url) // return;
-	my $bool = $cfg->git_bool($orig);
-	warn "W: $key=$orig for $url is not boolean\n" unless defined($bool);
-	$bool;
-}
-
 # flesh out common IMAP-specific data structures
 sub imap_common_init ($;$) {
 	my ($self, $lei) = @_;
@@ -344,8 +336,8 @@ sub imap_common_init ($;$) {
 
 		# knobs directly for Mail::IMAPClient->new
 		for my $k (qw(Starttls Debug Compress)) {
-			my $bool = cfg_bool($cfg, "imap.$k", $$uri) // next;
-			$mic_common->{$sec}->{$k} = $bool;
+			my $v = $cfg->urlmatch('--bool', "imap.$k", $$uri);
+			$mic_common->{$sec}->{$k} = $v if defined $v;
 		}
 		my $to = cfg_intvl($cfg, 'imap.timeout', $$uri);
 		$mic_common->{$sec}->{Timeout} = $to if $to;
@@ -398,7 +390,7 @@ sub nntp_common_init ($;$) {
 		my $args = $nn_common->{$sec} //= {};
 
 		# Debug and Timeout are passed to Net::NNTP->new
-		my $v = cfg_bool($cfg, 'nntp.Debug', $$uri);
+		my $v = $cfg->urlmatch(qw(--bool nntp.Debug), $$uri);
 		$args->{Debug} = $v if defined $v;
 		my $to = cfg_intvl($cfg, 'nntp.Timeout', $$uri);
 		$args->{Timeout} = $to if $to;
@@ -407,8 +399,8 @@ sub nntp_common_init ($;$) {
 
 		# Net::NNTP post-connect commands
 		for my $k (qw(starttls compress)) {
-			$v = cfg_bool($cfg, "nntp.$k", $$uri) // next;
-			$self->{cfg_opt}->{$sec}->{$k} = $v;
+			$v = $cfg->urlmatch('--bool', "nntp.$k", $$uri);
+			$self->{cfg_opt}->{$sec}->{$k} = $v if defined $v;
 		}
 
 		# -watch internal option
diff --git a/t/imapd-tls.t b/t/imapd-tls.t
index 44ab350c..673a9436 100644
--- a/t/imapd-tls.t
+++ b/t/imapd-tls.t
@@ -158,8 +158,10 @@ for my $args (
 	test_lei(sub {
 		lei_ok qw(ls-mail-source), "imap://$starttls_addr",
 			\'STARTTLS not used by default';
-		ok(!lei(qw(ls-mail-source -c imap.starttls=true),
+		ok(!lei(qw(ls-mail-source -c imap.starttls),
 			"imap://$starttls_addr"), 'STARTTLS verify fails');
+		unlike $lei_err, qr!W: imap\.starttls= .*? is not boolean!i,
+			'no non-boolean warning';
 	});
 
 	SKIP: {
diff --git a/t/nntpd-tls.t b/t/nntpd-tls.t
index 2a76867a..095aef96 100644
--- a/t/nntpd-tls.t
+++ b/t/nntpd-tls.t
@@ -149,10 +149,12 @@ for my $args (
 	test_lei(sub {
 		lei_ok qw(ls-mail-source), "nntp://$starttls_addr",
 			\'STARTTLS not used by default';
-		ok(!lei(qw(ls-mail-source -c nntp.starttls=true),
+		ok(!lei(qw(ls-mail-source -c nntp.starttls),
 			"nntp://$starttls_addr"), 'STARTTLS verify fails');
 		like $lei_err, qr/STARTTLS requested/,
 			'STARTTLS noted in stderr';
+		unlike $lei_err, qr!W: nntp\.starttls= .*? is not boolean!i,
+			'no non-boolean warning';
 	});
 
 	SKIP: {

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/8] net_reader: support imap.sslVerify + nntp.sslVerify
  2023-10-03  6:43 [PATCH 0/8] IMAP/NNTP client improvements Eric Wong
                   ` (2 preceding siblings ...)
  2023-10-03  6:43 ` [PATCH 3/8] config: fix key-only truthy values with urlmatch Eric Wong
@ 2023-10-03  6:43 ` Eric Wong
  2023-10-03  6:43 ` [PATCH 5/8] lei: workers exit after they tell lei-daemon Eric Wong
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2023-10-03  6:43 UTC (permalink / raw)
  To: meta

These options are useful for testing as well as users stuck on
out-of-date systems, dealing with forgetful sysadmins, broken
cronjobs, and/or are willing to risk MITM attacks.
---
 lib/PublicInbox/NetReader.pm | 28 ++++++++++++++++++++++------
 t/imapd-tls.t                | 14 +++++++++++---
 t/nntpd-tls.t                | 15 ++++++++++++---
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 5819f210..2d6cb0d6 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -49,6 +49,13 @@ sub mic_tls_opt ($$) {
 	[ map { ($_, $o->{$_}) } keys %$o ];
 }
 
+sub set_ssl_verify_mode ($$) {
+	my ($o, $bool) = @_;
+	require IO::Socket::SSL;
+	$o->{SSL_verify_mode} = $bool ? IO::Socket::SSL::SSL_VERIFY_PEER() :
+					IO::Socket::SSL::SSL_VERIFY_NONE();
+}
+
 sub mic_new ($$$$) {
 	my ($self, $mic_arg, $sec, $uri) = @_;
 	my %mic_arg = (%$mic_arg, Keepalive => 1);
@@ -138,7 +145,6 @@ sub mic_for ($$$$) { # mic = Mail::IMAPClient
 		Server => $host,
 		%$common, # may set Starttls, Compress, Debug ....
 	};
-	$mic_arg->{Ssl} = 1 if $uri->scheme eq 'imaps';
 	require PublicInbox::IMAPClient;
 	my $mic = mic_new($self, $mic_arg, $sec, $uri);
 	($mic && $mic->IsConnected) or
@@ -341,6 +347,7 @@ sub imap_common_init ($;$) {
 		}
 		my $to = cfg_intvl($cfg, 'imap.timeout', $$uri);
 		$mic_common->{$sec}->{Timeout} = $to if $to;
+		$mic_common->{$sec}->{Ssl} = 1 if $uri->scheme eq 'imaps';
 
 		# knobs we use ourselves:
 		my $sa = socks_args($cfg->urlmatch('imap.Proxy', $$uri));
@@ -350,11 +357,18 @@ sub imap_common_init ($;$) {
 			$self->{cfg_opt}->{$sec}->{$k} = $to;
 		}
 		my $k = 'imap.fetchBatchSize';
-		my $bs = $cfg->urlmatch($k, $$uri) // next;
-		if ($bs =~ /\A([0-9]+)\z/ && $bs > 0) {
-			$self->{cfg_opt}->{$sec}->{batch_size} = $bs;
-		} else {
-			warn "$k=$bs is not a positive integer\n";
+		if (defined(my $bs = $cfg->urlmatch($k, $$uri))) {
+			($bs =~ /\A([0-9]+)\z/ && $bs > 0) ?
+				($self->{cfg_opt}->{$sec}->{batch_size} = $bs) :
+				warn("$k=$bs is not a positive integer\n");
+		}
+		my $v = $cfg->urlmatch(qw(--bool imap.sslVerify), $$uri);
+		if (defined $v) {
+			my $cur = $mic_common->{$sec} //= {};
+			$cur->{Starttls} //= 1 if !$cur->{Ssl};
+			for my $f (grep { $cur->{$_} } qw(Ssl Starttls)) {
+				set_ssl_verify_mode($cur->{$f} = {}, $v);
+			}
 		}
 	}
 	# make sure we can connect and cache the credentials in memory
@@ -402,6 +416,8 @@ sub nntp_common_init ($;$) {
 			$v = $cfg->urlmatch('--bool', "nntp.$k", $$uri);
 			$self->{cfg_opt}->{$sec}->{$k} = $v if defined $v;
 		}
+		$v = $cfg->urlmatch(qw(--bool nntp.sslVerify), $$uri);
+		set_ssl_verify_mode($args, $v) if defined $v;
 
 		# -watch internal option
 		for my $k (qw(pollInterval)) {
diff --git a/t/imapd-tls.t b/t/imapd-tls.t
index 673a9436..e432ef07 100644
--- a/t/imapd-tls.t
+++ b/t/imapd-tls.t
@@ -1,8 +1,7 @@
 #!perl -w
-# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) 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 v5.12;
 use Socket qw(IPPROTO_TCP SOL_SOCKET);
 use PublicInbox::TestCommon;
 # IO::Poll is part of the standard library, but distros may split it off...
@@ -158,10 +157,19 @@ for my $args (
 	test_lei(sub {
 		lei_ok qw(ls-mail-source), "imap://$starttls_addr",
 			\'STARTTLS not used by default';
+		my $plain_out = $lei_out;
 		ok(!lei(qw(ls-mail-source -c imap.starttls),
 			"imap://$starttls_addr"), 'STARTTLS verify fails');
 		unlike $lei_err, qr!W: imap\.starttls= .*? is not boolean!i,
 			'no non-boolean warning';
+		lei_ok qw(-c imap.starttls -c imap.sslVerify= ls-mail-source),
+			"imap://$starttls_addr",
+			\'disabling imap.sslVerify works w/ STARTTLS';
+		is $lei_out, $plain_out, 'sslVerify=false w/ STARTTLS output';
+		lei_ok qw(ls-mail-source -c imap.sslVerify=false),
+			"imaps://$imaps_addr",
+			\'disabling imap.sslVerify works w/ imaps://';
+		is $lei_out, $plain_out, 'sslVerify=false w/ IMAPS output';
 	});
 
 	SKIP: {
diff --git a/t/nntpd-tls.t b/t/nntpd-tls.t
index 095aef96..21377fc0 100644
--- a/t/nntpd-tls.t
+++ b/t/nntpd-tls.t
@@ -1,8 +1,7 @@
 #!perl -w
-# Copyright (C) 2019-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) 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 v5.12;
 use PublicInbox::TestCommon;
 use Socket qw(SOCK_STREAM IPPROTO_TCP SOL_SOCKET);
 # IO::Poll and Net::NNTP are part of the standard library, but
@@ -149,12 +148,22 @@ for my $args (
 	test_lei(sub {
 		lei_ok qw(ls-mail-source), "nntp://$starttls_addr",
 			\'STARTTLS not used by default';
+		my $plain_out = $lei_out;
 		ok(!lei(qw(ls-mail-source -c nntp.starttls),
 			"nntp://$starttls_addr"), 'STARTTLS verify fails');
 		like $lei_err, qr/STARTTLS requested/,
 			'STARTTLS noted in stderr';
 		unlike $lei_err, qr!W: nntp\.starttls= .*? is not boolean!i,
 			'no non-boolean warning';
+		lei_ok qw(-c nntp.starttls -c nntp.sslVerify= ls-mail-source),
+			"nntp://$starttls_addr",
+			\'disabling nntp.sslVerify works w/ STARTTLS';
+		is $lei_out, $plain_out, 'sslVerify=false w/ STARTTLS output';
+
+		lei_ok qw(ls-mail-source -c nntp.sslVerify=false),
+			"nntps://$nntps_addr",
+			\'disabling nntp.sslVerify works w/ nntps://';
+		is $lei_out, $plain_out, 'sslVerify=false w/ NNTPS output';
 	});
 
 	SKIP: {

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/8] lei: workers exit after they tell lei-daemon
  2023-10-03  6:43 [PATCH 0/8] IMAP/NNTP client improvements Eric Wong
                   ` (3 preceding siblings ...)
  2023-10-03  6:43 ` [PATCH 4/8] net_reader: support imap.sslVerify + nntp.sslVerify Eric Wong
@ 2023-10-03  6:43 ` Eric Wong
  2023-10-03  6:43 ` [PATCH 6/8] net_reader: process title reflects NNTP article Eric Wong
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2023-10-03  6:43 UTC (permalink / raw)
  To: meta

We don't want workers continuing after their stdout has triggered
EPIPE or some other write error.

This fixes xt/lei-onion-convert.t to ensure the quit_waiter_pipe
is fully-closed at daemon teardown during tests.  Using the
`exit' perlop still ensures OnDestroy callbacks will fire.
---
 lib/PublicInbox/LEI.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 817772f7..10c08b90 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -485,6 +485,7 @@ sub x_it ($$) {
 	stop_pager($self);
 	if ($self->{pkt_op_p}) { # worker => lei-daemon
 		$self->{pkt_op_p}->pkt_do('x_it', $code);
+		exit($code >> 8);
 	} elsif ($self->{sock}) { # lei->daemon => lei(1) client
 		send($self->{sock}, "x_it $code", 0);
 	} elsif ($quit == \&CORE::exit) { # an admin (one-shot) command

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/8] net_reader: process title reflects NNTP article
  2023-10-03  6:43 [PATCH 0/8] IMAP/NNTP client improvements Eric Wong
                   ` (4 preceding siblings ...)
  2023-10-03  6:43 ` [PATCH 5/8] lei: workers exit after they tell lei-daemon Eric Wong
@ 2023-10-03  6:43 ` Eric Wong
  2023-10-03  6:43 ` [PATCH 7/8] xt/lei-onion-convert: test TLS + SOCKS Eric Wong
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2023-10-03  6:43 UTC (permalink / raw)
  To: meta

This matches the IMAP behavior with UIDs.  While we're in the
area, cut down on LoC a bit and reduce the scope of the $art
variable.
---
 lib/PublicInbox/NetReader.pm | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 2d6cb0d6..76fd3892 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -788,14 +788,12 @@ sub _nntp_fetch_all ($$$) {
 	$beg = $num_a if defined($num_a) && $num_a > $beg && $num_a <= $end;
 	$end = $num_b if defined($num_b) && $num_b >= $beg && $num_b < $end;
 	$end = $beg if defined($num_a) && !defined($num_b);
-	my ($err, $art, $last_art, $kw); # kw stays undef, no keywords in NNTP
-	unless ($self->{quiet}) {
-		warn "# $uri fetching ARTICLE $beg..$end\n";
-	}
+	my ($err, $last_art, $kw); # kw stays undef, no keywords in NNTP
+	warn "# $uri fetching ARTICLE $beg..$end\n" if !$self->{quiet};
 	my $n = $self->{max_batch};
-	for ($beg..$end) {
+	for my $art ($beg..$end) {
 		last if $self->{quit};
-		$art = $_;
+		local $0 = "#$art $group $sec";
 		if (--$n < 0) {
 			run_commit_cb($self);
 			$itrk->update_last(0, $last_art) if $itrk;

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/8] xt/lei-onion-convert: test TLS + SOCKS
  2023-10-03  6:43 [PATCH 0/8] IMAP/NNTP client improvements Eric Wong
                   ` (5 preceding siblings ...)
  2023-10-03  6:43 ` [PATCH 6/8] net_reader: process title reflects NNTP article Eric Wong
@ 2023-10-03  6:43 ` Eric Wong
  2023-10-03  6:43 ` [PATCH 8/8] net_reader: note glob support in .onion hint Eric Wong
  2023-10-03  7:11 ` "SSL" in option names is weird in 2023 Eric Wong
  8 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2023-10-03  6:43 UTC (permalink / raw)
  To: meta

While .onion URLs don't commonly use TLS, using Tor to access
non-.onion URLs is possible and TLS is advisable in that case.

TLS + SOCKS support is also useful for non-Tor SOCKS proxies
(e.g. "ssh -D"), but 127.0.0.1:9050 (Tor) is probably the most
standardized address.

While we're in the area: switch to v5.12, use autodie, and
ensure all necessary modules are present.
---
 xt/lei-onion-convert.t | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/xt/lei-onion-convert.t b/xt/lei-onion-convert.t
index 6dd17065..d3afbbb9 100644
--- a/xt/lei-onion-convert.t
+++ b/xt/lei-onion-convert.t
@@ -1,10 +1,12 @@
 #!perl -w
-# Copyright (C) 2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict; use v5.10; use PublicInbox::TestCommon;
+use v5.12; use PublicInbox::TestCommon;
 use PublicInbox::MboxReader;
+use autodie qw(pipe close);
 my $test_tor = $ENV{TEST_TOR};
 plan skip_all => "TEST_TOR unset" unless $test_tor;
+require_mods qw(IO::Socket::Socks IO::Socket::SSL Mail::IMAPClient Net::NNTP);
 unless ($test_tor =~ m!\Asocks5h://!i) {
 	my $default = 'socks5h://127.0.0.1:9050';
 	diag "using $default (set TEST_TOR=socks5h://ADDR:PORT to override)";
@@ -19,11 +21,24 @@ my @cnv = qw(lei convert -o mboxrd:/dev/stdout);
 my @proxy_cli = ("--proxy=$test_tor");
 my $proxy_cfg = "proxy=$test_tor";
 test_lei(sub {
+	# ensure TLS + SOCKS works
+	ok !lei(qw(ls-mail-source imaps://mews.public-inbox.org/
+		-c), "imap.$proxy_cfg"),
+		'imaps fails on wrong hostname w/ Tor';
+	ok !lei(qw(ls-mail-source nntps://mews.public-inbox.org/
+		-c), "nntp.$proxy_cfg"),
+		'nntps fails on wrong hostname w/ Tor';
+
+	lei_ok qw(ls-mail-source imaps://news.public-inbox.org/
+		-c), "imap.$proxy_cfg";
+	lei_ok qw(ls-mail-source nntps://news.public-inbox.org/
+		-c), "nntp.$proxy_cfg";
+
 	my $run = {};
 	for my $args ([$nntp_url, @proxy_cli], [$imap_url, @proxy_cli],
 			[ $nntp_url, '-c', "nntp.$proxy_cfg" ],
 			[ $imap_url, '-c', "imap.$proxy_cfg" ]) {
-		pipe(my ($r, $w)) or xbail "pipe: $!";
+		pipe(my $r, my $w);
 		my $cmd = [@cnv, @$args];
 		my $td = start_script($cmd, undef, { 1 => $w, run_mode => 0 });
 		$args->[0] =~ s!\A(.+?://).*!$1...!;

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/8] net_reader: note glob support in .onion hint
  2023-10-03  6:43 [PATCH 0/8] IMAP/NNTP client improvements Eric Wong
                   ` (6 preceding siblings ...)
  2023-10-03  6:43 ` [PATCH 7/8] xt/lei-onion-convert: test TLS + SOCKS Eric Wong
@ 2023-10-03  6:43 ` Eric Wong
  2023-10-03  7:11 ` "SSL" in option names is weird in 2023 Eric Wong
  8 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2023-10-03  6:43 UTC (permalink / raw)
  To: meta

It's only available for git 2.26+ users, but I figure most
distros have it at this point.
---
 lib/PublicInbox/NetReader.pm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 76fd3892..95cf1de8 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -95,6 +95,7 @@ sub onion_hint ($$) {
 	$uri->host =~ /\.onion\z/i or return "\n";
 	my $t = $uri->isa('PublicInbox::URIimap') ? 'imap' : 'nntp';
 	my $url = PublicInbox::Config::squote_maybe(uri_section($uri));
+	my $scheme = $uri->scheme;
 	my $set_cfg = 'lei config';
 	if (!$lei) { # public-inbox-watch
 		my $f = PublicInbox::Config::squote_maybe(
@@ -110,6 +111,10 @@ try configuring a socks5h:// proxy:
 	url=$url
 	$set_cfg $t.$dq\$url$dq.proxy socks5h://127.0.0.1:9050
 
+git 2.26+ users may instead rely on `*' to match all .onion URLs:
+
+	$set_cfg '$t.$scheme://*.onion.proxy' socks5h://127.0.0.1:9050
+
 ...before retrying your current command
 EOM
 }

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* "SSL" in option names is weird in 2023
  2023-10-03  6:43 [PATCH 0/8] IMAP/NNTP client improvements Eric Wong
                   ` (7 preceding siblings ...)
  2023-10-03  6:43 ` [PATCH 8/8] net_reader: note glob support in .onion hint Eric Wong
@ 2023-10-03  7:11 ` Eric Wong
  8 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2023-10-03  7:11 UTC (permalink / raw)
  To: meta

Eric Wong <e@80x24.org> wrote:
>   net_reader: support imap.sslVerify + nntp.sslVerify

It still feels awkward to name options with "SSL" in them since
"Secure Sockets Layer" is long deprecated (in favor of "TLS",
"Transport Layer Security").

But git already has imap.sslVerify, so it's not something I want
to deviate from...  git also has a lot of http.ssl* config
options, too, which we might benefit from having NNTP/IMAP
counterparts of...

OpenSSL, IO::Socket::SSL and Net::SSLeay are still called what
they are; and AFAIK nobody's worked on GnuTLS bindings for Perl,
yet...

So I guess we'll probably end up with a bunch of imap.ssl* and
nntp.ssl* options to set ciphers, certs, keys and whatnot...
Of course I don't trust myself to handle anything involving
SSL/TLS properly :<

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-10-03  7:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-03  6:43 [PATCH 0/8] IMAP/NNTP client improvements Eric Wong
2023-10-03  6:43 ` [PATCH 1/8] net_reader: bail out on NNTP SOCKS connection failure Eric Wong
2023-10-03  6:43 ` [PATCH 2/8] net_reader: avoid IO::Socket::SSL 2.079..2.081 warning Eric Wong
2023-10-03  6:43 ` [PATCH 3/8] config: fix key-only truthy values with urlmatch Eric Wong
2023-10-03  6:43 ` [PATCH 4/8] net_reader: support imap.sslVerify + nntp.sslVerify Eric Wong
2023-10-03  6:43 ` [PATCH 5/8] lei: workers exit after they tell lei-daemon Eric Wong
2023-10-03  6:43 ` [PATCH 6/8] net_reader: process title reflects NNTP article Eric Wong
2023-10-03  6:43 ` [PATCH 7/8] xt/lei-onion-convert: test TLS + SOCKS Eric Wong
2023-10-03  6:43 ` [PATCH 8/8] net_reader: note glob support in .onion hint Eric Wong
2023-10-03  7:11 ` "SSL" in option names is weird in 2023 Eric Wong

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).