[PATCH 0/2] nntp: cleanup and support CAPABILITIES

[PATCH 0/2] nntp: cleanup and support CAPABILITIES

[Eric Wong]

I haven't noticed people using STARTTLS on public-inbox.org,
but I do notice CAPABILITIES being issued.  Maybe clients
want to check it before trying STARTTLS?

This will also be required to advertise COMPRESS in the future.

Eric Wong (2):
  nntp: remove DISABLED hash checks
  nntp: add support for CAPABILITIES command

 lib/PublicInbox/NNTP.pm | 26 ++++++++++++++++++++------
 t/nntpd-tls.t           | 18 ++++++++++++++++++
 t/nntpd.t               |  5 +++++
 3 files changed, 43 insertions(+), 6 deletions(-)

-- 
EW

[PATCH 1/2] nntp: remove DISABLED hash checks

[Eric Wong]

Before I figured out the long_response API, I figured there'd
be expensive, process-monopolizing commands which admins might
want to disable.  Nearly 4 years later, we've never needed it
and running a server without commands such as OVER/XOVER is
unimaginable.
---
 lib/PublicInbox/NNTP.pm | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index 26bc679f..57a67a50 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -32,9 +32,6 @@ my $OVERVIEW_FMT = join(":\r\n", @OVERVIEW, qw(Bytes Lines)) . ":\r\n";
 my $LIST_HEADERS = join("\r\n", @OVERVIEW,
 			qw(:bytes :lines Xref To Cc)) . "\r\n";
 
-# disable commands with easy DoS potential:
-my %DISABLED; # = map { $_ => 1 } qw(xover list_overview_fmt newnews xhdr);
-
 my $EXPMAP; # fd -> [ idle_time, $self ]
 my $expt;
 our $EXPTIME = 180; # 3 minutes
@@ -105,10 +102,9 @@ sub process_line ($$) {
 	my ($self, $l) = @_;
 	my ($req, @args) = split(/[ \t]/, $l);
 	return 1 unless defined($req); # skip blank line
-	$req = lc($req);
 	$req = eval {
 		no strict 'refs';
-		$req = $DISABLED{$req} ? undef : *{'cmd_'.$req}{CODE};
+		*{'cmd_'.lc($req)}{CODE};
 	};
 	return res($self, '500 command not recognized') unless $req;
 	return res($self, r501) unless args_ok($req, scalar @args);
@@ -187,7 +183,6 @@ sub cmd_list ($;$$) {
 		my $arg = shift @args;
 		$arg =~ tr/A-Z./a-z_/;
 		$arg = "list_$arg";
-		return r501 if $DISABLED{$arg};
 
 		$arg = eval {
 			no strict 'refs';
-- 
EW

[PATCH 2/2] nntp: add support for CAPABILITIES command

[Eric Wong]

Some clients may rely on this for STARTTLS support.
---
 lib/PublicInbox/NNTP.pm | 19 +++++++++++++++++++
 t/nntpd-tls.t           | 18 ++++++++++++++++++
 t/nntpd.t               |  5 +++++
 3 files changed, 42 insertions(+)

diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index 57a67a50..d106e315 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -31,6 +31,14 @@ my @OVERVIEW = qw(Subject From Date Message-ID References Xref);
 my $OVERVIEW_FMT = join(":\r\n", @OVERVIEW, qw(Bytes Lines)) . ":\r\n";
 my $LIST_HEADERS = join("\r\n", @OVERVIEW,
 			qw(:bytes :lines Xref To Cc)) . "\r\n";
+my $CAPABILITIES = <<"";
+101 Capability list:\r
+VERSION 2\r
+READER\r
+NEWNEWS\r
+LIST ACTIVE ACTIVE.TIMES NEWSGROUPS OVERVIEW.FMT\r
+HDR\r
+OVER\r
 
 my $EXPMAP; # fd -> [ idle_time, $self ]
 my $expt;
@@ -121,6 +129,17 @@ sub process_line ($$) {
 	res($self, $res);
 }
 
+# The keyword argument is not used (rfc3977 5.2.2)
+sub cmd_capabilities ($;$) {
+	my ($self, undef) = @_;
+	my $res = $CAPABILITIES;
+	if (ref($self->{sock}) ne 'IO::Socket::SSL' &&
+			$self->{nntpd}->{accept_tls}) {
+		$res .= "STARTTLS\r\n";
+	}
+	$res .= '.';
+}
+
 sub cmd_mode ($$) {
 	my ($self, $arg) = @_;
 	$arg = uc $arg;
diff --git a/t/nntpd-tls.t b/t/nntpd-tls.t
index 82b63f3e..4cf53daa 100644
--- a/t/nntpd-tls.t
+++ b/t/nntpd-tls.t
@@ -128,6 +128,8 @@ for my $args (
 	my $c = Net::NNTP->new($nntps_addr, %o, SSL => 1);
 	my $list = $c->list;
 	is_deeply($list, $expect, 'NNTPS LIST works');
+	unlike(get_capa($c), qr/\bSTARTTLS\r\n/,
+		'STARTTLS not advertised for NNTPS');
 	is($c->command('QUIT')->response(), Net::Cmd::CMD_OK(), 'QUIT works');
 	is(0, sysread($c, my $buf, 1), 'got EOF after QUIT');
 
@@ -139,6 +141,8 @@ for my $args (
 	is($c->code, 382, 'got 382 for STARTTLS');
 	$list = $c->list;
 	is_deeply($list, $expect, 'LIST works after STARTTLS');
+	unlike(get_capa($c), qr/\bSTARTTLS\r\n/,
+		'STARTTLS not advertised after STARTTLS');
 
 	# Net::NNTP won't let us do dumb things, but we need to test
 	# dumb things, so use Net::Cmd directly:
@@ -149,6 +153,7 @@ for my $args (
 	# STARTTLS with bad hostname
 	$o{SSL_hostname} = $o{SSL_verifycn_name} = 'server.invalid';
 	$c = Net::NNTP->new($starttls_addr, %o);
+	like(get_capa($c), qr/\bSTARTTLS\r\n/, 'STARTTLS advertised');
 	$list = $c->list;
 	is_deeply($list, $expect, 'plain LIST works again');
 	ok(!$c->starttls, 'STARTTLS fails with bad hostname');
@@ -217,4 +222,17 @@ for my $args (
 	}
 }
 done_testing();
+
+sub get_capa {
+	my ($sock) = @_;
+	syswrite($sock, "CAPABILITIES\r\n");
+	my $capa = '';
+	do {
+		my $r = sysread($sock, $capa, 8192, length($capa));
+		die "unexpected: $!" unless defined($r);
+		die 'unexpected EOF' if $r == 0;
+	} until $capa =~ /\.\r\n\z/;
+	$capa;
+}
+
 1;
diff --git a/t/nntpd.t b/t/nntpd.t
index 0e59de07..1c5ae8d7 100644
--- a/t/nntpd.t
+++ b/t/nntpd.t
@@ -143,6 +143,11 @@ EOF
 		'got greeting');
 	$s->autoflush(1);
 
+	syswrite($s, "CAPABILITIES\r\n");
+	$buf = read_til_dot($s);
+	like($buf, qr/\r\nVERSION 2\r\n/s, 'CAPABILITIES works');
+	unlike($buf, qr/STARTTLS/s, 'STARTTLS not advertised');
+
 	syswrite($s, "NEWGROUPS 19990424 000000 GMT\r\n");
 	$buf = read_til_dot($s);
 	like($buf, qr/\A231 list of /, 'newgroups OK');
-- 
EW