Add Cancel-Lock support in innd, new docancels parameter

Implement Cancel-Lock in innd, based on RFC 8315 and libcanlock.

Add new docancels parameter in inn.conf to define which types of cancels innd
should honour.  Deprecate "innd -C" in favour of this new parameter.

see #47

Author: Julien-Elie

doc/pod/inn.conf.pod

@@ -202,6 +202,54 @@ value if you set it, since IPv6 addresses contain colons.
 
 This parameter has no effect when systemd socket activation is used.
 
+=item I<docancels>
+
+This parameter is intended for sites concerned about abuse of cancels, or that
+wish to enforce a mechanism to authenticate cancels.  Unless rejected by the
+use of a filter hook, B<innd> always accepts and propagates cancel articles
+and supersede requests.  However, actually processing such articles on the
+local news server depends on this parameter which can take the following
+values:
+
+=over 4
+
+=item C<require-auth>
+
+Only articles originally protected by the Cancel-Lock authentication
+mechanism can be withdrawn by a valid authenticated cancel article or a valid
+authenticated supersede request.  Withdrawals of articles not originally
+protected by Cancel-Lock will not be executed.
+
+This is the default value if B<innd> knows how to authenticate cancels
+(that is to say if INN was built with Cancel-Lock support).  Otherwise, the
+behaviour will be the same as C<none>.
+
+=item C<auth>
+
+Withdrawals of articles not originally protected by the Cancel-Lock
+authentication mechanism will always be executed.  However, if the original
+article is protected, only a valid authenticated cancel article or a valid
+authenticated supersede request will permit withdrawing it.  (If INN was not
+built with Cancel-Lock support, such protected articles won't be withdrawn.)
+
+=item C<none>
+
+Neither cancel articles nor supersede requests will be processed; no articles
+will be withdrawn.
+
+This is the default value if B<innd> does not know how to authenticate cancels
+(that is to say if INN was not built with Cancel-Lock support) as it has no
+means to ensure that these withdrawal requests are legitimate.
+
+=item C<all>
+
+B<innd> will process all cancel articles and supersede requests, even if
+unauthenticated, forged or with bad authentication.  You should be sure of
+what you are doing if you choose that value as any article can be withdrawn
+(even by someone who is not the author of the article).
+
+=back
+
 =item I<dontrejectfiltered>
 
 Normally innd(8) rejects incoming articles when directed to do so by
@@ -362,8 +410,7 @@ conceal their identity and provided no security".  This check is
 therefore not done as it is extremely easy to spoof.
 
 In order not to actually process any cancel or supersedes messages,
-you can start B<innd> with the B<-C> flag, or add this flag to the
-I<innflags> parameter.
+you can set I<docancels> to C<none>.
 
 =item I<verifygroups>
 

doc/pod/innd.pod

@@ -4,7 +4,7 @@ innd - InterNetNews daemon
 
 =head1 SYNOPSIS
 
-B<innd> [B<-aCdfNrsSu>] [B<-4> I<address>] [B<-6> I<address>] [B<-c> I<days>]
+B<innd> [B<-adfNrsSu>] [B<-4> I<address>] [B<-6> I<address>] [B<-c> I<days>]
 [B<-H> I<count>] [B<-i> I<count>] [B<-l> I<size>] [B<-m> I<mode>]
 [B<-n> I<flag>] [B<-o> I<count>] [B<-P> I<port>] [B<-t> I<timeout>]
 [B<-T> I<count>] [B<-X> I<seconds>]
@@ -114,13 +114,6 @@ specified by the C</remember/> line in expire.ctl(5).  You'll have to
 wait that number of days before being able to inject again an article
 with the same previously rejected Message-ID.
 
-=item B<-C>
-
-This flag tells B<innd> to accept and propagate but not actually process
-cancel or supersedes messages.  This is intended for sites concerned about
-abuse of cancels, or that wish to use another cancel mechanism with
-stronger authentication.
-
 =item B<-d>, B<-f>
 
 B<innd> normally puts itself into the background, points its standard

doc/pod/news.pod

@@ -25,6 +25,20 @@ used for the new Cancel-Lock functionality.
 
 =item *
 
+The B<-C> flag given to B<innd> to disable the execution of cancels has been
+deprecated and is no longer taken into account (an error message will be
+present in your logs if B<innd> is started with it).  Instead, a new parameter
+has been added in F<inn.conf> to tune the types of cancels B<innd> should
+process.  If I<docancels> is set to C<require-auth>, which is the default
+if INN has Cancel-Lock support, only articles originally protected by the
+Cancel-Lock authentication mechanism can be withdrawn by a valid authenticated
+cancel article or a valid authenticated supersede request.  Withdrawals of
+articles not originally protected by Cancel-Lock will not be executed.
+See inn.conf(5) for more details about the different values of the new
+I<docancels> parameter, and make sure to parameter it according to your needs.
+
+=item *
+
 B<filechan> is no longer shipped with INN; it was just a simple version of
 B<buffchan>.  All calls to C<filechan> will be changed to C<buffchan -u>
 (for its unbuffered mode) in F<newsfeeds> by B<innupgrade>.  If you have
@@ -85,10 +99,18 @@ and makehistory(8) man pages.
 
 =item *
 
-Julien Elie has implemented Cancel-Lock support in B<nnrpd>, based on S<RFC
-8315> and libcanlock.  A new F<inn-secrets.conf> configuration file has been
-added in I<pathetc> wherein you can set the secrets to use for Cancel-Lock.
-See the inn-secrets.conf(5) man page for more details.
+Julien Elie has implemented Cancel-Lock support in B<innd> and B<nnrpd>,
+based on S<RFC 8315> and libcanlock.  A new F<inn-secrets.conf> configuration
+file has been added in I<pathetc> wherein you can set the secrets to use for
+Cancel-Lock.  See the inn-secrets.conf(5) man page for more details.
+
+=item *
+
+A new I<docancels> parameter has been added in F<inn.conf> to define which
+types of cancels B<innd> should process.  The B<-C> flag given to B<innd>
+is deprecated in favour of that new parameter (you'll see in your logs the
+message C<innd -C flag has been deprecated and has no effect; use docancels in
+inn.conf> in case you're passing that flag to B<innd>).
 
 =item *
 

include/inn/innconf.h

@@ -36,6 +36,7 @@ struct innconf {
     unsigned long artcutoff;    /* Max accepted article age */
     char *bindaddress;          /* Which interface IP to bind to */
     char *bindaddress6;         /* Which interface IPv6 to bind to */
+    char *docancels;            /* Which cancels to process */
     bool dontrejectfiltered;    /* Don't reject filtered article? */
     unsigned long hiscachesize; /* Size of the history cache in kB */
     bool ignorenewsgroups;      /* Propagate cmsgs by affected group? */

include/inn/libinn.h

@@ -89,11 +89,19 @@ extern void xsignal_enable_masking(void);
 extern void xsignal_forked(void);
 
 
-/* Headers. */
+/*
+**  HEADERS
+**
+**  Do not test HAVE_CANLOCK.  This relieves customers of /usr/include/inn from
+**  the need to guess whether INN was built with Cancel-Lock support in order
+**  to get a header that matches the installed libraries.
+*/
 extern bool gen_cancel_lock(const char *msgid, const char *username,
                             char **canbuff);
 extern bool gen_cancel_key(const char *hdrcontrol, const char *hdrsupersedes,
                            const char *username, char **canbuff);
+extern bool verify_cancel_key(const char *c_key_header,
+                              const char *c_lock_header);
 extern char *GenerateMessageID(char *domain);
 extern void InitializeMessageIDcclass(void);
 extern bool IsValidMessageID(const char *string, bool stripspaces,
@@ -103,6 +111,7 @@ extern bool IsValidHeaderBody(const char *string);
 extern bool IsValidHeaderField(const char *string);
 extern const char *skip_cfws(const char *p);
 extern const char *skip_fws(const char *p);
+extern char *spaced_words_without_cfws(const char *p);
 extern void HeaderCleanFrom(char *from);
 extern struct _DDHANDLE *DDstart(FILE *FromServer, FILE *ToServer);
 extern void DDcheck(struct _DDHANDLE *h, char *group);

innd/Makefile

@@ -32,7 +32,7 @@ clean clobber distclean maintclean:
 ##  Compilation rules.
 
 INNDLIBS 	= $(LIBSTORAGE) $(LIBHIST) $(LIBINN) $(STORAGE_LIBS) \
-		  $(SYSTEMD_LIBS) \
+		  $(SYSTEMD_LIBS) $(CANLOCK_LDFLAGS) $(CANLOCK_LIBS) \
 		  $(PERL_LIBS) $(PYTHON_LIBS) $(REGEX_LIBS) $(LIBS)
 
 perl.o:		perl.c   ; $(CC) $(CFLAGS) $(PERL_CPPFLAGS) -c perl.c

innd/art.c

@@ -1238,11 +1238,17 @@ void
 ARTcancel(const ARTDATA *data, const char *MessageID, const bool Trusted)
 {
     char buff[SMBUF + 16];
+    const char *start, *end;
+    ARTHANDLE *art;
     TOKEN token;
+#if defined(HAVE_CANLOCK)
+    char *canlockhdr;
+    const HDRCONTENT *hc = data->HdrContent;
+#endif
     bool r;
 
     TMRstart(TMR_ARTCNCL);
-    if (!DoCancels && !Trusted) {
+    if (strcasecmp(innconf->docancels, "none") == 0 && !Trusted) {
         TMRstop(TMR_ARTCNCL);
         return;
     }
@@ -1254,10 +1260,76 @@ ARTcancel(const ARTDATA *data, const char *MessageID, const bool Trusted)
         return;
     }
 
+    /* No additional checks if cancels should all be executed. */
+    if (strcasecmp(innconf->docancels, "all") != 0 && !Trusted) {
+        /* Retrieve the Cancel-Lock header field of the article to be
+         * cancelled.  If the original article is not found or has no
+         * Cancel-Lock header field, we cannot authenticate the cancel.
+         *
+         * It also means that we do not honour cancel requests for articles not
+         * yet received, even if they do not have a Cancel-Lock header field
+         * (but we don't know yet). */
+        if (!HISlookup(History, MessageID, NULL, NULL, NULL, &token)) {
+            TMRstop(TMR_ARTCNCL);
+            return;
+        }
+        if ((art = SMretrieve(token, RETR_HEAD)) == NULL) {
+            TMRstop(TMR_ARTCNCL);
+            return;
+        }
+        start = wire_findheader(art->data, art->len, "Cancel-Lock", true);
+
+        if (start == NULL
+            && strcasecmp(innconf->docancels, "require-auth") == 0) {
+            SMfreearticle(art);
+            TMRstop(TMR_ARTCNCL);
+            return;
+        } else if (start != NULL) {
+            /* canlockhdr will store a copy of the Cancel-Lock header field
+             * body of the original article to be cancelled.
+             * end points to the final "\n" of the header field body. */
+            end = wire_endheader(start, art->data + art->len - 1);
+
+            if (end == NULL
+                && strcasecmp(innconf->docancels, "require-auth") == 0) {
+                SMfreearticle(art);
+                TMRstop(TMR_ARTCNCL);
+                return;
+            } else if (end != NULL) {
+                r = false;
+#if defined(HAVE_CANLOCK)
+                if (HDR(HDR__CANCEL_KEY) != NULL) {
+                    canlockhdr = xstrndup(start, end - start);
+
+                    /* Is the Cancel-Key header field legitimate? */
+                    r = verify_cancel_key(HDR(HDR__CANCEL_KEY), canlockhdr);
+
+                    free(canlockhdr);
+                }
+#endif
+                SMfreearticle(art);
+
+                if (r == false) {
+                    TMRstop(TMR_ARTCNCL);
+                    return;
+                }
+            } else {
+                /* docancels is "auth", and there is no Cancel-Lock. */
+                SMfreearticle(art);
+            }
+        } else {
+            /* docancels is "auth", and there is no Cancel-Lock. */
+            SMfreearticle(art);
+        }
+    }
+
     if (!HIScheck(History, MessageID)) {
         /* Article hasn't arrived here, so write a fake entry using
          * most of the information from the cancel message. */
-        if (innconf->verifycancels && !Trusted) {
+        if (innconf->verifycancels
+            && (strcasecmp(innconf->docancels, "require-auth") == 0
+                || strcasecmp(innconf->docancels, "auth") == 0)
+            && !Trusted) {
             TMRstop(TMR_ARTCNCL);
             return;
         }
@@ -2673,7 +2745,7 @@ ARTpost(CHANNEL *cp)
         if (IsControl) {
             ARTcontrol(data, HDR(HDR__CONTROL), cp);
         }
-        if (DoCancels && HDR_FOUND(HDR__SUPERSEDES)) {
+        if (HDR_FOUND(HDR__SUPERSEDES)) {
             if (IsValidMessageID(HDR(HDR__SUPERSEDES), true, laxmid))
                 ARTcancel(data, HDR(HDR__SUPERSEDES), false);
         }

innd/innd.c

@@ -22,7 +22,6 @@ bool Debug = false;
 bool NNRPTracing = false;
 bool StreamingOff = false; /* default is we can stream */
 bool Tracing = false;
-bool DoCancels = true;
 char LogName[] = "SERVER";
 int ErrorCount = IO_ERROR_COUNT;
 OPERATINGMODE Mode = OMrunning;
@@ -147,7 +146,7 @@ const ARTHEADER ARTheaders[] = {
     ARTHEADERINIT("X-Canceled-By",                HTstd),
     /* #define HDR__XCANCELEDBY                             35 */
     ARTHEADERINIT("Cancel-Key",                   HTstd),
-    /* #define HDR__CANCELKEY                               36 */
+    /* #define HDR__CANCEL_KEY                              36 */
     ARTHEADERINIT("User-Agent",                   HTstd),
     /* #define HDR__USER_AGENT                              37 */
     ARTHEADERINIT("X-Original-Message-ID",        HTstd),
@@ -460,7 +459,8 @@ main(int ac, char *av[])
             innconf->artcutoff = strtoul(optarg, NULL, 10);
             break;
         case 'C':
-            DoCancels = false;
+            syslog(LOG_ERR, "innd -C flag has been deprecated and has no "
+                            "effect; use docancels in inn.conf");
             break;
         case 'd':
             Debug = true;

innd/innd.h

@@ -184,7 +184,7 @@ typedef struct _HDRCONTENT {
 #define HDR__XNEWSPOSTER                  33
 #define HDR__XCANCELLEDBY                 34
 #define HDR__XCANCELEDBY                  35
-#define HDR__CANCELKEY                    36
+#define HDR__CANCEL_KEY                   36
 #define HDR__USER_AGENT                   37
 #define HDR__X_ORIGINAL_MESSAGE_ID        38
 #define HDR__CANCEL_LOCK                  39
@@ -643,7 +643,6 @@ extern const ARTHEADER ARTheaders[MAX_ARTHEADER];
 extern bool BufferedLogs;
 EXTERN bool AnyIncoming;
 extern bool Debug;
-extern bool DoCancels;
 extern bool laxmid;
 EXTERN bool ICDneedsetup;
 EXTERN bool NeedHeaders;

lib/canlock.c

@@ -329,4 +329,39 @@ gen_cancel_key(const char *hdrcontrol, const char *hdrsupersedes,
 
     return gencankey;
 }
+
+
+/*  Verify that a Cancel-Key header field body contains an element matching one
+**  of those present in a Cancel-Lock header field body.
+**  This function expects pointers to Cancel-Key and Cancel-Lock header field
+**  bodies as arguments.
+**
+**  Returns true if at least one c-key element matches.
+**  Otherwise, false is returned, that is to say the cancel or supersede
+**  request cannot be authenticated.
+*/
+bool
+verify_cancel_key(const char *c_key_header, const char *c_lock_header)
+{
+    char *c_key_list, *c_lock_list;
+    int status = -1;
+
+    if (c_key_header == NULL || c_lock_header == NULL)
+        return false;
+
+    /* Rewrite header field bodies as space-separated lists of elements. */
+    c_key_list = spaced_words_without_cfws(c_key_header);
+    c_lock_list = spaced_words_without_cfws(c_lock_header);
+
+    if (c_key_list != NULL && c_lock_list != NULL) {
+        /* Now call the function that actually does the check.
+         * The status will be 0 if a match is found. */
+        status = cl_verify_multi(NULL, c_key_list, c_lock_list);
+    }
+
+    free(c_key_list);
+    free(c_lock_list);
+
+    return (status == 0);
+}
 #endif /* HAVE_CANLOCK */