INN commit: branches/2.6 (5 files)

INN Commit rra at isc.org
Thu Dec 10 20:25:34 UTC 2015


    Date: Thursday, December 10, 2015 @ 12:25:34
  Author: iulius
Revision: 9966

nnrpd:  take into account the require_ssl parameter when using STARTTLS

When TLS is negotiated after a successful use of the STARTTLS command,
nnrpd now updates the permissions of the news client according to
the new secure state of his connection (that is to say auth blocks
in readers.conf using the require_ssl parameter are taken into
account).  Previously, only connections on a dedicated port (usually 563)
were taking benefit from that parameter.

Thanks to Steve Crook for the bug report.

Modified:
  branches/2.6/doc/pod/news.pod
  branches/2.6/nnrpd/misc.c
  branches/2.6/nnrpd/nnrpd.c
  branches/2.6/nnrpd/nnrpd.h
  branches/2.6/nnrpd/perm.c

------------------+
 doc/pod/news.pod |   16 ++++++
 nnrpd/misc.c     |   45 +++++++++++------
 nnrpd/nnrpd.c    |    3 -
 nnrpd/nnrpd.h    |    3 -
 nnrpd/perm.c     |  133 +++++++++++++++++++++++++++++++++--------------------
 5 files changed, 133 insertions(+), 67 deletions(-)

Modified: doc/pod/news.pod
===================================================================
--- doc/pod/news.pod	2015-12-08 21:16:19 UTC (rev 9965)
+++ doc/pod/news.pod	2015-12-10 20:25:34 UTC (rev 9966)
@@ -1,3 +1,19 @@
+=head1 Changes in 2.6.1
+
+=over 2
+
+=item *
+
+When TLS is negotiated after a successful use of the STARTTLS command,
+B<nnrpd> now updates the permissions of the news client according to
+the new secure state of his connection (that is to say auth blocks
+in F<readers.conf> using the I<require_ssl> parameter are taken into
+account).  Previously, only connections on a dedicated port (usually 563)
+were taking benefit from that parameter.  Thanks to Steve Crook for
+the bug report.
+
+=back
+
 =head1 Upgrading from 2.5 to 2.6
 
 The following changes require your full attention because a manual

Modified: nnrpd/misc.c
===================================================================
--- nnrpd/misc.c	2015-12-08 21:16:19 UTC (rev 9965)
+++ nnrpd/misc.c	2015-12-10 20:25:34 UTC (rev 9966)
@@ -478,6 +478,34 @@
         return;
     }
 
+    /* Close out any existing article, report group stats.
+     * RFC 4642 requires the reset of any knowledge about the client. */
+    if (GRPcur) {
+        ARTclose();
+        GRPreport();
+        OVctl(OVCACHEFREE, &boolval);
+        free(GRPcur);
+        GRPcur = NULL;
+        if (ARTcount) {
+            syslog(L_NOTICE, "%s exit for STARTTLS articles %ld groups %ld",
+                   Client.host, ARTcount, GRPcount);
+        }
+        GRPcount = 0;
+        PERMgroupmadeinvalid = false;
+    }
+
+    /* We can now assume a secure connection will be negotiated because
+     * nnrpd will exit if STARTTLS fails.
+     * Check the permissions the client will have after having successfully
+     * negotiated a TLS layer.  (There may be TLS-only auth blocks in
+     * readers.conf that match the connection).
+     * In case the client would no longer have access to the server, or an
+     * authentication error happens, the connection aborts after a fatal 400
+     * response code sent by PERMgetpermissions(). */
+    nnrpd_starttls_done = true;
+    PERMgetaccess(false);
+    PERMgetpermissions();
+
     Reply("%d Begin TLS negotiation now\r\n", NNTP_CONT_STARTTLS);
     fflush(stdout);
 
@@ -505,23 +533,6 @@
     }
 #endif /* HAVE_SASL */
 
-    nnrpd_starttls_done = true;
-
-    /* Close out any existing article, report group stats.
-     * RFC 4642 requires the reset of any knowledge about the client. */
-    if (GRPcur) {
-        ARTclose();
-        GRPreport();
-        OVctl(OVCACHEFREE, &boolval);
-        free(GRPcur);
-        GRPcur = NULL;
-        if (ARTcount)
-            syslog(L_NOTICE, "%s exit for STARTTLS articles %ld groups %ld",
-                   Client.host, ARTcount, GRPcount);
-        GRPcount = 0;
-        PERMgroupmadeinvalid = false;
-    }
-
     /* Reset our read buffer so as to prevent plaintext command injection. */
     line_reset(&NNTPline);
 }

Modified: nnrpd/nnrpd.c
===================================================================
--- nnrpd/nnrpd.c	2015-12-08 21:16:19 UTC (rev 9965)
+++ nnrpd/nnrpd.c	2015-12-10 20:25:34 UTC (rev 9966)
@@ -616,7 +616,8 @@
 
     notice("%s (%s) connect - port %u", Client.host, Client.ip, port);
 
-    PERMgetaccess(NNRPACCESS);
+    PERMgetinitialaccess(NNRPACCESS);
+    PERMgetaccess(true);
     PERMgetpermissions();
 }
 

Modified: nnrpd/nnrpd.h
===================================================================
--- nnrpd/nnrpd.h	2015-12-08 21:16:19 UTC (rev 9965)
+++ nnrpd/nnrpd.h	2015-12-10 20:25:34 UTC (rev 9966)
@@ -232,7 +232,8 @@
 extern void		GRPreport(void);
 extern bool		NGgetlist(char ***argvp, char *list);
 extern bool		PERMartok(void);
-extern void		PERMgetaccess(char *nnrpaccess);
+extern void             PERMgetinitialaccess(char *readersconf);
+extern void             PERMgetaccess(bool initialconnection);
 extern void		PERMgetpermissions(void);
 extern void		PERMlogin(char *uname, char *pass, int* code, char *errorstr);
 extern bool		PERMmatch(char **Pats, char **list);

Modified: nnrpd/perm.c
===================================================================
--- nnrpd/perm.c	2015-12-08 21:16:19 UTC (rev 9965)
+++ nnrpd/perm.c	2015-12-10 20:25:34 UTC (rev 9966)
@@ -1402,10 +1402,9 @@
 }
 
 void
-PERMgetaccess(char *nnrpaccess)
+PERMgetinitialaccess(char *readersconf)
 {
     int i;
-    char *uname;
 
     auth_realms	    = NULL;
     access_realms   = NULL;
@@ -1426,14 +1425,18 @@
     PERMaccessconf = NULL;
 
     if (ConfigBit == NULL) {
-	if (PERMMAX % 8 == 0)
-	    ConfigBitsize = PERMMAX/8;
-	else
-	    ConfigBitsize = (PERMMAX - (PERMMAX % 8))/8 + 1;
-	ConfigBit = xcalloc(ConfigBitsize, 1);
+        if (PERMMAX % 8 == 0) {
+            ConfigBitsize = PERMMAX/8;
+        } else {
+            ConfigBitsize = (PERMMAX - (PERMMAX % 8))/8 + 1;
+        }
+        ConfigBit = xcalloc(ConfigBitsize, 1);
     }
-    PERMreadfile(nnrpaccess);
 
+    /* Parse the readers.conf file. */
+    PERMreadfile(readersconf);
+
+    /* Remove unused access groups. */
     strip_accessgroups();
 
     if (auth_realms == NULL) {
@@ -1446,7 +1449,7 @@
 
     /* auth_realms are all expected to match the user.
      * Be careful whether SSL is required, though. */
-    for (i = 0; auth_realms[i]; i++) {
+    for (i = 0; auth_realms[i] != NULL; i++) {
 	if (auth_realms[i]->auth_methods != NULL) {
 	    PERMcanauthenticate = true;
 #ifdef HAVE_OPENSSL
@@ -1461,55 +1464,89 @@
             || auth_realms[i]->dynamic_script != NULL)
             PERMcanpostgreeting = true;
     }
-    uname = 0;
-    while (!uname && i--) {
+}
+
+void
+PERMgetaccess(bool initialconnection)
+{
+    char *uname;
+    int i;
+
+    if (ConfigBit == NULL) {
+        if (PERMMAX % 8 == 0) {
+            ConfigBitsize = PERMMAX/8;
+        } else {
+            ConfigBitsize = (PERMMAX - (PERMMAX % 8))/8 + 1;
+        }
+        ConfigBit = xcalloc(ConfigBitsize, 1);
+    }
+
+    if (auth_realms == NULL) {
+        return;
+    }
+
+    for (i = 0; auth_realms[i] != NULL; i++) {
+            ;
+    }
+
+    uname = NULL;
+    while (uname == NULL && i-- > 0) {
 #ifdef HAVE_OPENSSL
         /* If SSL is required, check that the connection is encrypted. */
-        if ((auth_realms[i]->require_ssl == true) && !nnrpd_starttls_done)
+        if (auth_realms[i]->require_ssl && !nnrpd_starttls_done) {
             continue;
+        }
 #endif
-	if ((uname = ResolveUser(auth_realms[i])) != NULL)
-	    PERMauthorized = true;
-	if (!uname && auth_realms[i]->default_user)
-	    uname = xstrdup(auth_realms[i]->default_user);
+        if ((uname = ResolveUser(auth_realms[i])) != NULL) {
+            PERMauthorized = true;
+        }
+        if (uname == NULL && auth_realms[i]->default_user != NULL) {
+            uname = xstrdup(auth_realms[i]->default_user);
+        }
     }
-    if (uname) {
-	strlcpy(PERMuser, uname, sizeof(PERMuser));
+    if (uname != NULL) {
+        strlcpy(PERMuser, uname, sizeof(PERMuser));
         free(uname);
-	uname = strchr(PERMuser, '@');
-	if (!uname && auth_realms[i]->default_domain) {
-	    /* Append the default domain to the username. */
-	    strlcat(PERMuser, "@", sizeof(PERMuser));
-	    strlcat(PERMuser, auth_realms[i]->default_domain,
+        uname = strchr(PERMuser, '@');
+        if (uname == NULL && auth_realms[i]->default_domain != NULL) {
+            /* Append the default domain to the username. */
+            strlcat(PERMuser, "@", sizeof(PERMuser));
+            strlcat(PERMuser, auth_realms[i]->default_domain,
                     sizeof(PERMuser));
-	}
-	PERMneedauth = false;
-	success_auth = auth_realms[i];
-	syslog(L_TRACE, "%s res %s", Client.host, PERMuser);
-    } else if (!PERMcanauthenticate) {
-	/* Couldn't resolve the user. */
-	syslog(L_NOTICE, "%s no_user", Client.host);
-	Reply("%d Could not get your access name.  Goodbye!\r\n",
-	  NNTP_ERR_ACCESS);
-	ExitWithStats(1, true);
+        }
+        PERMneedauth = false;
+        success_auth = auth_realms[i];
+        syslog(L_TRACE, "%s res %s", Client.host, PERMuser);
+    } else if (initialconnection && !PERMcanauthenticate) {
+        /* Couldn't resolve the user. */
+        syslog(L_NOTICE, "%s no_user", Client.host);
+        Reply("%d Could not get your access name.  Goodbye!\r\n",
+              NNTP_ERR_ACCESS);
+        ExitWithStats(1, true);
     } else {
-	PERMneedauth = true;
+        PERMneedauth = true;
     }
-    /* Check maximum allowed permissions for any host that matches (for
-     * the greeting string). */
-    for (i = 0; access_realms[i]; i++) {
-	if (!PERMcanread)
-	    PERMcanread = (access_realms[i]->read != NULL);
-	if (!PERMcanpost)
-	    PERMcanpost = (access_realms[i]->post != NULL);
-        if (!PERMcanpostgreeting)
-            PERMcanpostgreeting = (access_realms[i]->post != NULL);
+
+    if (initialconnection) {
+        /* Check maximum allowed permissions for any host that matches (for
+         * the greeting string). */
+        for (i = 0; access_realms[i] != NULL; i++) {
+            if (!PERMcanread) {
+                PERMcanread = (access_realms[i]->read != NULL);
+            }
+            if (!PERMcanpost) {
+                PERMcanpost = (access_realms[i]->post != NULL);
+            }
+            if (!PERMcanpostgreeting) {
+                PERMcanpostgreeting = (access_realms[i]->post != NULL);
+            }
+        }
+        if (i == 0) {
+            /* No applicable access groups.  Zeroing all these makes INN
+             * return permission denied to client. */
+            PERMcanread = PERMcanpost = PERMneedauth = false;
+        }
     }
-    if (!i) {
-	/* No applicable access groups.  Zeroing all these makes INN
-	 * return permission denied to client. */
-	PERMcanread = PERMcanpost = PERMneedauth = false;
-    }
 }
 
 void



More information about the inn-committers mailing list