Hello,
for quite some time I've seem rogue cancels against a certain group (say
alt.pets.cats.cute). The usual solution was to filter cancels using
cleanfeed (no_cancel_groups).

However: cleanfeed checks the Newsgroups: header of the cancel message,
this fails if the cancel message has a different Newsgroups: line (e.g.
just Newsgroups: alt.dogs.angry). This actually is done by intention to
avoid such a filtering.

Therefore I was looking for a solution of "How do a check the Newsgroups:
line of the article to be cancelled?". The first idea is to enhance the
cleanfeed cancel detection methods. I could retrive the article to be
cancelled using INN::head and check but this has drawbacks:
- It requires a recent INN/cleanfeed version, something that does not
  apply in a certain installation, upgrade is NOT an option.
- There is still a project to allow a standalone running of filter_innd.pl
  for debugging purposes - providing the INN:: routines adds complexity
  and I'd like to avoid that.

Finally I recalled verifycancel, an old option to protect against forged
cancels, not very useful since it can be cheated easily and is therefore
disabled in the default installation. The attached patch alters
ARTcancelverify to check wheter at least one group in the cancel message
can be found in article to be cancelled.
This would still allow to cancel a Crosspost
"alt.pets.cats.cute,alt.dogs.angry" using "alt.dogs.angry" but this does
not happen that often.

It should be noted that this check only prevents the cancel excution
but not the propagation (unlike filter_innd.pl rejects which causes the
article to be dropped). Therefore combining this patch with a
no_cancel_groups => '^alt\.pets\.cats\.cute$',
in cleanfeed will drop cancels for alt.pets.cats.cute but file the forged
cancels without actually cancelling.

A more elegant solution would allow to specify nocancelgroups (wildmat) in
inn.conf; I didn't dare to do that but I could try in my copious free
time.

An even more elegant solution was to have a set of return codes from the
perl filter and then doing the check in cleanfeed again. This codes should
at least include (accept, reject, reject but do not remembertrash {for
articles from non-sound sites}, accept but do not execute cancel {this
problem}). Perhaps in INN 2.6 or so?


If you think that my patch is not really a win for INN, please fix the
comment for ARTcancelverify anyway since "return the list of filenames"
seems to refer to things back in INN 1.x.

By the way, there seems to be a useless variable "c" in ListHas (again
in innd/art.c).

The patch applies to 2.4.1, another one for 2.2.x is not tested yet.

    Christoph


-- Attached file included as plaintext by Ecartis --

--- art.c.ORIG	Wed Jan  7 23:47:19 2004
+++ art.c	Thu Aug 12 13:45:28 2004
@@ -1137,15 +1137,17 @@
 }
 
 /*
-**  Verify if a cancel message is valid.  If the user posting the cancel
-**  matches the user who posted the article, return the list of filenames
-**  otherwise return NULL.
+**  Verify if a cancel message is valid.  Unless at least one group in the
+**  cancel message's Newsgroups: line can be found in the Newsgroups: line
+**  of the article to be cancelled, the cancel is considered bogus and
+**  false is returned.
 */
 static bool
 ARTcancelverify(const ARTDATA *data, const char *MessageID, TOKEN *token)
 {
   const char	*p;
   char		*q, *q1;
+  char		**gp;
   const char	*local;
   char		buff[SMBUF];
   ARTHANDLE	*art;
@@ -1155,13 +1157,12 @@
     return false;
   if ((art = SMretrieve(*token, RETR_HEAD)) == NULL)
     return false;
-  local = wire_findheader(art->data, art->len, "Sender");
+  /* Copy Newsgroups: from article be to cancelled to q.
+   * Double-terminate q (sentinel) */
+  local = wire_findheader(art->data, art->len, "Newsgroups");
   if (local == NULL) {
-    local = wire_findheader(art->data, art->len, "From");
-    if (local == NULL) {
-      SMfreearticle(art);
-      return false;
-    }
+    SMfreearticle(art);
+    return false;
   }
   for (p = local; p < art->data + art->len; p++) {
     if (*p == '\r' || *p == '\n')
@@ -1171,26 +1172,33 @@
     SMfreearticle(art);
     return false;
   }
-  q = xmalloc(p - local + 1);
+  q = xmalloc(p - local + 2);
   memcpy(q, local, p - local);
   SMfreearticle(art);
   q[p - local] = '\0';
-  HeaderCleanFrom(q);
+  q[p - local + 1] = '\0';
 
-  /* Compare canonical forms. */
-  q1 = xstrdup(data->Poster);
-  HeaderCleanFrom(q1);
-  if (strcmp(q, q1) != 0) {
-    r = false;
-    sprintf(buff, "\"%.50s\" wants to cancel %s by \"%.50s\"",
-      q1, MaxLength(MessageID, MessageID), q);
-    ARTlog(data, ART_REJECT, buff);
-  }
-  else {
-    r = true;
+  /* replace separator , by \0 */
+  for (q1 = q; *q1; q1++)
+    if (NG_ISSEP(*q1))
+      *q1 = '\0';
+
+  r = false;
+  for (gp = data->Newsgroups.List; *gp && !r; gp++) {
+    for (q1 = q; *q1; q1 += strlen(q1) + 1) {
+      if (strcmp(q1, *gp) == 0) {
+        r = true;
+        break;
+      }
+    }
   }
-  free(q1);
   free(q);
+
+  if (!r) {
+    sprintf(buff, "no matching Newsgroups in cancel %s",
+      MaxLength(MessageID, MessageID));
+    ARTlog(data, ART_REJECT, buff);
+  }
   return r;
 }