INN commit: trunk (4 files)

INN Commit rra at isc.org
Fri Aug 14 19:45:50 UTC 2009


    Date: Friday, August 14, 2009 @ 12:45:49
  Author: iulius
Revision: 8564

A patch from Christopher Biedl to alter ARTcancelverify
to check whether at least one group in the cancel message
can be found in the article to be cancelled.

The check for matching Sender: and From: headers is useless
and removed.

close #38

Modified:
  trunk/doc/pod/inn.conf.pod
  trunk/doc/pod/innd.pod
  trunk/doc/pod/news.pod
  trunk/innd/art.c

----------------------+
 doc/pod/inn.conf.pod |   16 +++++++-----
 doc/pod/innd.pod     |    2 -
 doc/pod/news.pod     |   14 ++++++++++
 innd/art.c           |   64 +++++++++++++++++++++++++++----------------------
 4 files changed, 61 insertions(+), 35 deletions(-)

Modified: doc/pod/inn.conf.pod
===================================================================
--- doc/pod/inn.conf.pod	2009-08-14 18:45:27 UTC (rev 8563)
+++ doc/pod/inn.conf.pod	2009-08-14 19:45:49 UTC (rev 8564)
@@ -308,13 +308,17 @@
 =item I<verifycancels>
 
 Set this to true to enable a simplistic check on all cancel messages,
-attempting to verify (by simple header comparison) that the cancel message
-is from the same person as the original post.  This can't be done if the
-cancel arrives before the article does, and is extremely easy to spoof.
-While this check may once have served a purpose, it's now essentially
-security via obscurity, commonly avoided by abusers, and probably not
-useful.  This is a boolean value, and the default is false.
+attempting to verify (by simple header comparison) that at least one newsgroup
+in the cancel message can be found in the article to be cancelled.  This
+check can't be done if the cancel arrives before the article does.  This
+is a boolean value, and the default is false.
 
+Note that RFC 5537 (USEPRO) mentions that "cancel control messages are
+not required to contain From: and Sender: header fields matching the
+target message.  This requirement only encouraged cancel issuers to
+conceal their identity and provided no security".  This check is
+therefore not done as it is extremely easy to spoof.
+
 =item I<verifygroups>
 
 Set this to true to reject incoming articles which contain an unknown

Modified: doc/pod/innd.pod
===================================================================
--- doc/pod/innd.pod	2009-08-14 18:45:27 UTC (rev 8563)
+++ doc/pod/innd.pod	2009-08-14 19:45:49 UTC (rev 8564)
@@ -427,7 +427,6 @@
 reasons for rejection generated by B<innd> include:
 
     "%s" header too long
-    "%s" wants to cancel <%s> by "%s"
     Article exceeds local limit of %s bytes
     Article posted in the future -- "%s"
     Bad "%s" header
@@ -439,6 +438,7 @@
     Missing %s header
     No body
     No colon-space in "%s" header
+    No matching newsgroups in cancel <%s>
     No space
     Space before colon in "%s" header
     Too old -- "%s"

Modified: doc/pod/news.pod
===================================================================
--- doc/pod/news.pod	2009-08-14 18:45:27 UTC (rev 8563)
+++ doc/pod/news.pod	2009-08-14 19:45:49 UTC (rev 8564)
@@ -38,6 +38,20 @@
 
 =item *
 
+The check on cancel messages when I<verifycancels> is set to true
+in F<inn.conf> has been changed to verify that at least one newsgroup
+in the cancel message can be found in the article to be cancelled.
+This new feature is from Christopher Biedl.
+
+The previous behaviour was to check whether the cancel message is
+from the same person as the original post, which is extremely easy
+to spoof; besides, RFC 5537 (USEPRO) mentions that "cancel control
+messages are not required to contain From: and Sender: header fields
+matching the target message.  This requirement only encouraged cancel
+issuers to conceal their identity and provided no security".
+
+=item *
+
 A new B<-L> flag has been added by Jonathan Kamens to B<makehistory>
 so as to specify a load average limit.  If the system load average exceeds
 the specified limit, B<makehistory> sleeps until it goes below the limit.

Modified: innd/art.c
===================================================================
--- innd/art.c	2009-08-14 18:45:27 UTC (rev 8563)
+++ innd/art.c	2009-08-14 19:45:49 UTC (rev 8564)
@@ -1197,17 +1197,18 @@
 }
 
 /*
-**  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 HDRCONTENT *hc = data->HdrContent;
   const char	*p;
   char		*q, *q1;
-  const char	*local, *poster;
+  char          **gp;
+  const char	*local;
   char		buff[SMBUF];
   ARTHANDLE	*art;
   bool		r;
@@ -1216,13 +1217,13 @@
     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')
@@ -1232,30 +1233,37 @@
     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. */
-  if (HDR_FOUND(HDR__SENDER))
-    poster = HDR(HDR__SENDER);
-  else
-    poster = HDR(HDR__FROM);
-  q1 = xstrdup(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);
+  /* Replace separator ',' by '\0'. */
+  for (q1 = q; *q1; q1++) {
+    if (NG_ISSEP(*q1)) {
+      *q1 = '\0';
+    }
   }
-  else {
-    r = true;
+
+  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;
 }
 




More information about the inn-committers mailing list