[PATCH 12/34] v2writable: inject new Message-IDs on true duplicates

["Eric Wong (Contractor, The Linux Foundation)" <e@80x24.org>]

Since we'll need to support multiple Message-IDs anyways,
inject a new one if we hit a duplicate (or don't get one at
all).

Try to use a deterministic Message-Id for consistency, but give
up determinism and use a random Message-Id if an "attacker"
wants to prevent their message from being archived.
---
 MANIFEST                      |  1 +
 lib/PublicInbox/V2Writable.pm | 88 +++++++++++++++++++++++++++++++++----------
 t/v2writable.t                | 84 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 154 insertions(+), 19 deletions(-)
 create mode 100644 t/v2writable.t

diff --git a/MANIFEST b/MANIFEST
index 1aaf8ff..7366aa0 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -177,5 +177,6 @@ t/spawn.t
 t/thread-all.t
 t/thread-cycle.t
 t/utf8.mbox
+t/v2writable.t
 t/view.t
 t/watch_maildir.t
diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 57cb7d3..6d73827 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -11,8 +11,8 @@ use PublicInbox::SearchIdxSkeleton;
 use PublicInbox::MIME;
 use PublicInbox::Git;
 use PublicInbox::Import;
-use PublicInbox::MID qw(mid_clean mid_mime);
-use PublicInbox::ContentId qw(content_id);
+use PublicInbox::MID qw(mids);
+use PublicInbox::ContentId qw(content_id content_digest);
 use PublicInbox::Inbox;
 
 # an estimate of the post-packed size to the raw uncompressed size
@@ -62,21 +62,8 @@ sub add {
 	# leaking FDs to it...
 	$self->idx_init;
 
-	my $mid = mid_clean(mid_mime($mime));
-	my $num = $self->{skel}->{mm}->mid_insert($mid);
-	if (!defined($num)) { # mid is already known
-		$self->done; # ensure all subprocesses are done writing
-
-		my $existing = $self->lookup_content($mime);
-		warn "<$mid> resent\n" if $existing;
-		return if $existing; # easy, don't store duplicates
-
-		# reuse NNTP article number?
-		warn "<$mid> reused for mismatched content\n";
-		$self->idx_init;
-		$num = $self->{skel}->{mm}->num_for($mid);
-	}
-
+	my $num = num_for($self, $mime);
+	defined $num or return; # duplicate
 	my $im = $self->importer;
 	my $cmt = $im->add($mime);
 	$cmt = $im->get_mark($cmt);
@@ -95,6 +82,70 @@ sub add {
 	$mime;
 }
 
+sub num_for {
+	my ($self, $mime) = @_;
+	my $mids = mids($mime->header_obj);
+	if (@$mids) {
+		my $mid = $mids->[0];
+		my $num = $self->{skel}->{mm}->mid_insert($mid);
+		return $num if defined($num); # common case
+
+		# crap, Message-ID is already known, hope somebody just resent:
+		$self->done; # write barrier, clears $self->{skel}
+		foreach my $m (@$mids) {
+			# read-only lookup now safe to do after above barrier
+			my $existing = $self->lookup_content($mime, $m);
+			if ($existing) {
+				warn "<$m> resent\n";
+				return; # easy, don't store duplicates
+			}
+		}
+
+		# very unlikely:
+		warn "<$mid> reused for mismatched content\n";
+		$self->idx_init;
+
+		# try the rest of the mids
+		foreach my $i (1..$#$mids) {
+			my $m = $mids->[$i];
+			$num = $self->{skel}->{mm}->mid_insert($m);
+			if (defined $num) {
+				warn "alternative <$m> for <$mid> found\n";
+				return $num;
+			}
+		}
+	}
+	# none of the existing Message-IDs are good, generate a new one:
+	num_for_harder($self, $mime);
+}
+
+sub num_for_harder {
+	my ($self, $mime) = @_;
+
+	my $hdr = $mime->header_obj;
+	my $dig = content_digest($mime);
+	my $mid = $dig->clone->hexdigest . '@localhost';
+	my $num = $self->{skel}->{mm}->mid_insert($mid);
+	unless (defined $num) {
+		# it's hard to spoof the last Received: header
+		my @recvd = $hdr->header_raw('Received');
+		$dig->add("Received: $_") foreach (@recvd);
+		$mid = $dig->clone->hexdigest . '@localhost';
+		$num = $self->{skel}->{mm}->mid_insert($mid);
+
+		# fall back to a random Message-ID and give up determinism:
+		until (defined($num)) {
+			$dig->add(rand);
+			$mid = $dig->clone->hexdigest . '@localhost';
+			warn "using random Message-ID <$mid> as fallback\n";
+			$num = $self->{skel}->{mm}->mid_insert($mid);
+		}
+	}
+	my @cur = $hdr->header_raw('Message-Id');
+	$hdr->header_set('Message-Id', @cur, "<$mid>");
+	$num;
+}
+
 sub idx_part {
 	my ($self, $part) = @_;
 	$self->{idx_parts}->[$part];
@@ -268,13 +319,12 @@ sub import_init {
 }
 
 sub lookup_content {
-	my ($self, $mime) = @_;
+	my ($self, $mime, $mid) = @_;
 	my $ibx = $self->{-inbox};
 
 	my $srch = $ibx->search;
 	my $cid = content_id($mime);
 	my $found;
-	my $mid = mid_mime($mime);
 	$srch->each_smsg_by_mid($mid, sub {
 		my ($smsg) = @_;
 		$smsg->load_expand;
diff --git a/t/v2writable.t b/t/v2writable.t
new file mode 100644
index 0000000..bc2437a
--- /dev/null
+++ b/t/v2writable.t
@@ -0,0 +1,84 @@
+# Copyright (C) 2018 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use strict;
+use warnings;
+use Test::More;
+use PublicInbox::MIME;
+use PublicInbox::ContentId qw(content_digest);
+use File::Temp qw/tempdir/;
+foreach my $mod (qw(DBD::SQLite Search::Xapian)) {
+	eval "require $mod";
+	plan skip_all => "$mod missing for nntpd.t" if $@;
+}
+use_ok 'PublicInbox::V2Writable';
+my $mainrepo = tempdir('pi-v2writable-XXXXXX', TMPDIR => 1, CLEANUP => 1);
+my $ibx = {
+	mainrepo => $mainrepo,
+	name => 'test-v2writable',
+	version => 2,
+	-primary_address => 'test@example.com',
+};
+$ibx = PublicInbox::Inbox->new($ibx);
+my $mime = PublicInbox::MIME->create(
+	header => [
+		From => 'a@example.com',
+		To => 'test@example.com',
+		Subject => 'this is a subject',
+		'Message-ID' => '<a-mid@b>',
+		Date => 'Fri, 02 Oct 1993 00:00:00 +0000',
+	],
+	body => "hello world\n",
+);
+
+my $im = PublicInbox::V2Writable->new($ibx, 1);
+ok($im->add($mime), 'ordinary message added');
+{
+	my @warn;
+	local $SIG{__WARN__} = sub { push @warn, @_ };
+	is(undef, $im->add($mime), 'obvious duplicate rejected');
+	like(join(' ', @warn), qr/resent/, 'warned about resent message');
+
+	@warn = ();
+	$mime->header_set('Message-Id', '<a-mid@b>', '<c@d>');
+	ok($im->add($mime), 'secondary MID used');
+	like(join(' ', @warn), qr/mismatched/, 'warned about mismatch');
+	like(join(' ', @warn), qr/alternative/, 'warned about alternative');
+	is_deeply([ '<a-mid@b>', '<c@d>' ],
+		[ $mime->header_obj->header_raw('Message-Id') ],
+		'no new Message-Id added');
+
+	@warn = ();
+	$mime->header_set('Message-Id', '<a-mid@b>');
+	$mime->body_set('different');
+	ok($im->add($mime), 'reused mid ok');
+	like(join(' ', @warn), qr/reused/, 'warned about reused MID');
+	my @mids = $mime->header_obj->header_raw('Message-Id');
+	is($mids[0], '<a-mid@b>', 'original mid not changed');
+	like($mids[1], qr/\A<\w+\@localhost>\z/, 'new MID added');
+	is(scalar(@mids), 2, 'only one new MID added');
+
+	@warn = ();
+	$mime->header_set('Message-Id', '<a-mid@b>');
+	$mime->body_set('this one needs a random mid');
+	my $gen = content_digest($mime)->hexdigest . '@localhost';
+	my $fake = PublicInbox::MIME->new($mime->as_string);
+	$fake->header_set('Message-Id', $gen);
+	ok($im->add($fake), 'fake added easily');
+	is_deeply(\@warn, [], 'no warnings from a faker');
+	ok($im->add($mime), 'random MID made');
+	like(join(' ', @warn), qr/using random/, 'warned about using random');
+	@mids = $mime->header_obj->header_raw('Message-Id');
+	is($mids[0], '<a-mid@b>', 'original mid not changed');
+	like($mids[1], qr/\A<\w+\@localhost>\z/, 'new MID added');
+	is(scalar(@mids), 2, 'only one new MID added');
+
+	@warn = ();
+	$mime->header_set('Message-Id');
+	ok($im->add($mime), 'random MID made for MID free message');
+	@mids = $mime->header_obj->header_raw('Message-Id');
+	like($mids[0], qr/\A<\w+\@localhost>\z/, 'mid was generated');
+	is(scalar(@mids), 1, 'new generated');
+}
+
+$im->done;
+done_testing();
-- 
EW