INN commit: trunk (7 files)

INN Commit rra at isc.org
Sat Oct 24 20:24:37 UTC 2015


    Date: Saturday, October 24, 2015 @ 13:24:36
  Author: iulius
Revision: 9956

nnrpd:  take into account the require_ssl parameter when using SASL

When an encryption layer is negotiated after a successful authentication
using a SASL mechanism which negotiates an encrypted layer, 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 TLS connections either on a dedicated port (usually 563) or after
a successful use of STARTTLS were taking benefit from that parameter.

Modified:
  trunk/doc/pod/news.pod
  trunk/doc/pod/readers.conf.pod
  trunk/nnrpd/commands.c
  trunk/nnrpd/misc.c
  trunk/nnrpd/nnrpd.c
  trunk/nnrpd/perm.c
  trunk/nnrpd/sasl.c

--------------------------+
 doc/pod/news.pod         |   15 +++++-----
 doc/pod/readers.conf.pod |   42 +++++++++++++++-------------
 nnrpd/commands.c         |    8 ++---
 nnrpd/misc.c             |   10 +++---
 nnrpd/nnrpd.c            |   20 +++++++------
 nnrpd/perm.c             |   47 ++++++++++++++++---------------
 nnrpd/sasl.c             |   67 ++++++++++++++++++++++++++-------------------
 7 files changed, 116 insertions(+), 93 deletions(-)

Modified: doc/pod/news.pod
===================================================================
--- doc/pod/news.pod	2015-10-24 19:32:35 UTC (rev 9955)
+++ doc/pod/news.pod	2015-10-24 20:24:36 UTC (rev 9956)
@@ -4,13 +4,14 @@
 
 =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.
+When an encryption layer is negotiated after a successful use of the
+STARTTLS command, or after a successful authentication using a SASL
+mechanism which negotiates an encrypted layer, 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.
 
 =item *
 

Modified: doc/pod/readers.conf.pod
===================================================================
--- doc/pod/readers.conf.pod	2015-10-24 19:32:35 UTC (rev 9955)
+++ doc/pod/readers.conf.pod	2015-10-24 20:24:36 UTC (rev 9956)
@@ -88,10 +88,12 @@
 address in a netblock; for example, "10.10.10.0/24" will match any IP
 address between 10.10.10.0 and 10.10.10.255 inclusive.
 
-If compiled against the TLS/SSL libraries, an auth group with the I<require_ssl>
-parameter set to true only applies if the incoming connection is using
-TLS, either from the beginning if the B<-S> flag was passed to B<nnrpd> or
-after a successful use of STARTTLS.
+If compiled against the TLS/SSL or SASL libraries, an auth group with
+the I<require_ssl> parameter set to true only applies if the incoming
+connection is using an encryption layer, either from the beginning if
+the B<-S> flag was passed to B<nnrpd>, or after a successful use of
+STARTTLS, or after a successful authentication using a SASL mechanism
+which negotiates an encrypted layer.
 
 For any connection from a host that matches that wildmat expression or
 netblock, each <res-program> (multiple res: lines may be present in a
@@ -344,12 +346,14 @@
 
 =item B<require_ssl:>
 
-If set to true, an incoming connection only matches this auth group if
-it is encrypted using TLS/SSL, either from the beginning if the B<-S>
-flag was passed to B<nnrpd> or after a successful use of STARTTLS.
-This parameter is only valid if INN is compiled with TLS/SSL support (by
-default if the OpenSSL SSL and crypto libraries are found at configure
-time, otherwise see the B<--with-openssl> flag passed to configure).
+If set to true, an incoming connection only matches this auth group if it
+is encrypted, either from the beginning if the B<-S> flag was passed to
+B<nnrpd>, or after a successful use of STARTTLS, or after a successful
+authentication using a SASL mechanism which negotiates an encrypted
+layer.  This parameter is only valid if INN is compiled with TLS/SSL
+or SASL support (by default if the OpenSSL SSL and crypto libraries or
+the Cyrus SASL library are found at configure time, otherwise see the
+B<--with-openssl> and B<--with-sasl> flags passed to configure).
 
 =item B<perl_access:>
 
@@ -855,20 +859,20 @@
 Authentication using the AUTHINFO USER/PASS commands passes unencrypted
 over the network.  Extreme caution should therefore be used especially
 with system passwords (e.g. C<auth: ckpasswd -s>).  Passwords can be
-protected by using NNTP over TLS/SSL or through ssh tunnels, and this usage
-can be enforced by a well-considered server configuration that only
-permits certain auth groups to be applied in certain cases.  Here are
-some ideas:
+protected by using NNTP over TLS/SSL or through ssh tunnels, and this
+usage can be enforced by a well-considered server configuration that
+only permits certain auth groups to be applied in certain cases.  One can
+also authenticate using a strong SASL mechanism.  Here are some ideas:
 
 =over 4
 
 =item *
 
-To restrict connections on the standard NNTP port (119) to use TLS for
-some (or all) of the auth groups to match, use the I<require_ssl>
-parameter.  Note that a client can use STARTTLS to negotiate an
-encrypted connection.  A secure layer can also be negotiated during
-authentication via AUTHINFO SASL.
+To restrict connections on the standard NNTP port (119) to use an
+encryption layer for some (or all) of the auth groups to match, use
+the I<require_ssl> parameter.  Note that a client can use STARTTLS
+to negotiate an encrypted TLS connection.  A secure layer can also be
+negotiated during authentication via AUTHINFO SASL.
 
 =item *
 

Modified: nnrpd/commands.c
===================================================================
--- nnrpd/commands.c	2015-10-24 19:32:35 UTC (rev 9955)
+++ nnrpd/commands.c	2015-10-24 20:24:36 UTC (rev 9956)
@@ -14,8 +14,8 @@
 #include "inn/version.h"
 #include "tls.h"
 
-#ifdef HAVE_OPENSSL
-extern bool nnrpd_starttls_done;
+#if defined(HAVE_OPENSSL)
+extern bool encryption_layer_on;
 #endif /* HAVE_OPENSSL */
 
 typedef struct {
@@ -326,7 +326,7 @@
 #ifdef HAVE_OPENSSL
             /* Check whether STARTTLS must be used before trying to authenticate. */
             if (PERMcanauthenticate && !PERMcanauthenticatewithoutSSL
-                && !nnrpd_starttls_done) {
+                && !encryption_layer_on) {
                 Reply("%d Encryption required\r\n", NNTP_FAIL_PRIVACY_NEEDED);
                 return;
             }
@@ -355,7 +355,7 @@
 #ifdef HAVE_OPENSSL
         /* Check whether STARTTLS must be used before trying to authenticate. */
         if (PERMcanauthenticate && !PERMcanauthenticatewithoutSSL
-            && !nnrpd_starttls_done) {
+            && !encryption_layer_on) {
              Reply("%d Encryption required\r\n", NNTP_FAIL_PRIVACY_NEEDED);
              return;
         }

Modified: nnrpd/misc.c
===================================================================
--- nnrpd/misc.c	2015-10-24 19:32:35 UTC (rev 9955)
+++ nnrpd/misc.c	2015-10-24 20:24:36 UTC (rev 9956)
@@ -18,12 +18,12 @@
 /* Outside the ifdef so that make depend works even ifndef HAVE_OPENSSL. */
 #include "inn/ov.h"
 
-#ifdef HAVE_OPENSSL
+#if defined(HAVE_OPENSSL)
 extern SSL *tls_conn;
 extern int tls_cipher_usebits;
 extern char *tls_peer_CN;
-extern bool nnrpd_starttls_done;
-#endif 
+extern bool encryption_layer_on;
+#endif /* HAVE_OPENSSL */ 
 
 
 /*
@@ -459,7 +459,7 @@
     int result;
     bool boolval;
 
-    if (nnrpd_starttls_done) {
+    if (encryption_layer_on) {
         Reply("%d Already using an active TLS layer\r\n", NNTP_ERR_ACCESS);
         return;
     }
@@ -502,7 +502,7 @@
      * 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;
+    encryption_layer_on = true;
     PERMgetaccess(false);
     PERMgetpermissions();
 

Modified: nnrpd/nnrpd.c
===================================================================
--- nnrpd/nnrpd.c	2015-10-24 19:32:35 UTC (rev 9955)
+++ nnrpd/nnrpd.c	2015-10-24 20:24:36 UTC (rev 9956)
@@ -33,9 +33,11 @@
 
 #include "tls.h"
 
-#ifdef HAVE_OPENSSL
+#if defined(HAVE_OPENSSL)
 extern SSL *tls_conn;
-bool nnrpd_starttls_done = false;
+#endif
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
+bool encryption_layer_on = false;
 #endif 
 
 
@@ -373,7 +375,7 @@
          * in its current state. */
         if (PERMcanauthenticate) {
 #ifdef HAVE_OPENSSL
-            if (PERMcanauthenticatewithoutSSL || nnrpd_starttls_done) {
+            if (PERMcanauthenticatewithoutSSL || encryption_layer_on) {
 #endif
                 /* AUTHINFO USER is advertised only if a TLS layer is active,
                  * if compiled with TLS support. */
@@ -380,7 +382,7 @@
                 Printf(" USER");
 #ifdef HAVE_OPENSSL
             } else {
-#ifdef HAVE_SASL
+# ifdef HAVE_SASL
                 /* Remove unsecure PLAIN, LOGIN and EXTERNAL SASL mechanisms,
                  * if compiled with TLS support and a TLS layer is not active. */
                 if (mechlist != NULL) {
@@ -399,7 +401,7 @@
                         memmove(p, p+9, strlen(p)-8);
                     }
                 }
-#endif /* HAVE_SASL */
+# endif /* HAVE_SASL */
             }
 #endif /* HAVE_OPENSSL */
 #ifdef HAVE_SASL
@@ -446,7 +448,7 @@
 
 #ifdef HAVE_OPENSSL
     /* A TLS layer is not active and the client is not already authenticated. */
-    if (!nnrpd_starttls_done
+    if (!encryption_layer_on
         && (!PERMauthorized || PERMneedauth || PERMcanauthenticate)) {
         Printf("STARTTLS\r\n");
     }
@@ -970,7 +972,7 @@
 	    Tracing = true;
 	    break;
 #ifdef HAVE_OPENSSL
-	case 'S':			/* Force SSL negotiation. */
+	case 'S':			/* Force the negotiation of an encryption layer. */
 	    initialSSL = true;
 	    break;
 #endif /* HAVE_OPENSSL */
@@ -1199,10 +1201,10 @@
     if (initialSSL) {
         tls_init();
         if (tls_start_servertls(0, 1) == -1) {
-            Reply("%d SSL connection failed\r\n", NNTP_FAIL_TERMINATING);
+            Reply("%d Encrypted TLS connection failed\r\n", NNTP_FAIL_TERMINATING);
             ExitWithStats(1, false);
         }
-        nnrpd_starttls_done = true;
+        encryption_layer_on = true;
     }
 #endif /* HAVE_OPENSSL */
 

Modified: nnrpd/perm.c
===================================================================
--- nnrpd/perm.c	2015-10-24 19:32:35 UTC (rev 9955)
+++ nnrpd/perm.c	2015-10-24 20:24:36 UTC (rev 9956)
@@ -21,9 +21,9 @@
 # include <sys/select.h>
 #endif
 
-#ifdef HAVE_OPENSSL
-extern bool nnrpd_starttls_done;
-#endif /* HAVE_OPENSSL */
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
+extern bool encryption_layer_on;
+#endif /* HAVE_OPENSSL || HAVE_SASL */
 
 /* Data types. */
 typedef struct _CONFCHAIN {
@@ -42,7 +42,7 @@
 typedef struct _AUTHGROUP {
     char *name;
     char *key;
-#ifdef HAVE_OPENSSL
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
     int require_ssl;
 #endif
     char *hosts;
@@ -161,7 +161,7 @@
 #define PERMperl_access         59
 #define PERMpython_access       60
 #define PERMpython_dynamic      61
-#ifdef HAVE_OPENSSL
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
 #define PERMrequire_ssl         62
 #define PERMMAX                 63
 #else
@@ -251,7 +251,7 @@
     { PERMperl_access,          (char *) "perl_access:"         },
     { PERMpython_access,        (char *) "python_access:"       },
     { PERMpython_dynamic,       (char *) "python_dynamic:"      },
-#ifdef HAVE_OPENSSL
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
     { PERMrequire_ssl,          (char *) "require_ssl:"         },
 #endif
     { 0,                        (char *) NULL                   }
@@ -351,7 +351,7 @@
     else
 	ret->hosts = 0;
 
-#ifdef HAVE_OPENSSL
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
     ret->require_ssl = orig->require_ssl;
 #endif
 
@@ -453,7 +453,7 @@
 static void
 SetDefaultAuth(AUTHGROUP *curauth UNUSED)
 {
-#ifdef HAVE_OPENSSL
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
         curauth->require_ssl = false;
 #endif
 }
@@ -673,7 +673,7 @@
 	curauth->key = xstrdup(tok->name);
 	SET_CONFIG(PERMkey);
 	break;
-#ifdef HAVE_OPENSSL
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
       case PERMrequire_ssl:
         if (boolval != -1)
             curauth->require_ssl = boolval;
@@ -1266,7 +1266,7 @@
 
 		/* Stuff that belongs to an auth group. */
 	      case PERMhost:
-#ifdef HAVE_OPENSSL
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
               case PERMrequire_ssl:
 #endif
 	      case PERMauthprog:
@@ -1448,12 +1448,12 @@
     }
 
     /* auth_realms are all expected to match the user.
-     * Be careful whether SSL is required, though. */
+     * Be careful whether an encryption layer is required, though. */
     for (i = 0; auth_realms[i] != NULL; i++) {
 	if (auth_realms[i]->auth_methods != NULL) {
 	    PERMcanauthenticate = true;
-#ifdef HAVE_OPENSSL
-            if (auth_realms[i]->require_ssl == false)
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
+            if (!auth_realms[i]->require_ssl)
                 PERMcanauthenticatewithoutSSL = true;
 #endif
         }
@@ -1491,9 +1491,10 @@
 
     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 && !nnrpd_starttls_done) {
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
+        /* If an encryption layer is required, check that the connection
+         * really uses one. */
+        if (auth_realms[i]->require_ssl && !encryption_layer_on) {
             continue;
         }
 #endif
@@ -1990,9 +1991,10 @@
     if (auth->res_methods == NULL)
         return NULL;
 
-#ifdef HAVE_OPENSSL
-    /* If SSL is required, check that the connection is encrypted. */
-    if ((auth->require_ssl == true) && !nnrpd_starttls_done)
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
+    /* If an encryption layer is required, check that the connection
+     * really uses one. */
+    if (auth->require_ssl && !encryption_layer_on)
         return NULL;
 #endif
 
@@ -2040,9 +2042,10 @@
     if (auth->auth_methods == NULL)
         return NULL;
 
-#ifdef HAVE_OPENSSL
-    /* If SSL is required, check that the connection is encrypted. */
-    if ((auth->require_ssl == true) && !nnrpd_starttls_done)
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
+    /* If an encryption layer is required, check that the connection
+     * really uses one. */
+    if (auth->require_ssl && !encryption_layer_on)
         return NULL;
 #endif
 

Modified: nnrpd/sasl.c
===================================================================
--- nnrpd/sasl.c	2015-10-24 19:32:35 UTC (rev 9955)
+++ nnrpd/sasl.c	2015-10-24 20:24:36 UTC (rev 9956)
@@ -13,11 +13,13 @@
 /* Outside the ifdef so that make depend works even ifndef HAVE_OPENSSL. */
 #include "inn/ov.h"
 
-#ifdef HAVE_OPENSSL
+#if defined(HAVE_OPENSSL)
 extern int tls_cipher_usebits;
 extern char *tls_peer_CN;
-extern bool nnrpd_starttls_done;
 #endif /* HAVE_OPENSSL */
+#if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
+extern bool encryption_layer_on;
+#endif /* HAVE_OPENSSL || HAVE_SASL */
 
 #ifdef HAVE_SASL
 sasl_conn_t *sasl_conn = NULL;
@@ -94,7 +96,7 @@
         sasl_setprop(sasl_conn, SASL_SEC_PROPS, &secprops);
 #ifdef HAVE_OPENSSL
         /* Tell SASL about the negotiated TLS layer. */
-        if (nnrpd_starttls_done) {
+        if (encryption_layer_on) {
             if (sasl_setprop(sasl_conn, SASL_SSF_EXTERNAL,
                              (sasl_ssf_t *) &tls_cipher_usebits) != SASL_OK) {
                 syslog(L_NOTICE, "sasl_setprop() failed:  TLS layer for SASL");
@@ -153,10 +155,10 @@
     /* Check whether STARTTLS must be used before trying to authenticate
      * with AUTHINFO SASL PLAIN, LOGIN or EXTERNAL. */
     if (PERMcanauthenticate && !PERMcanauthenticatewithoutSSL
-        && !nnrpd_starttls_done && ((strcasecmp(mech, "PLAIN") == 0
+        && !encryption_layer_on && ((strcasecmp(mech, "PLAIN") == 0
                                      || strcasecmp(mech, "LOGIN") == 0
                                      || strcasecmp(mech, "EXTERNAL") == 0))) {
-        Reply("%d Encryption required\r\n", NNTP_FAIL_PRIVACY_NEEDED);
+        Reply("%d Encryption layer required\r\n", NNTP_FAIL_PRIVACY_NEEDED);
         return;
     }
 #endif
@@ -291,26 +293,19 @@
     }
 
     if (r == SASL_OK) {
-	/* Success. */
-	strlcpy(PERMuser, canon_user, sizeof(PERMuser));
-        PERMgetpermissions();
-	PERMneedauth = false;
-	PERMauthorized = true;
-        PERMcanauthenticate = false;
+        /* Success!
+         * First, save info about the negotiated security layer
+         * for I/O functions. */
+        sasl_ssf = *ssfp;
+        sasl_maxout = (*maxoutp == 0 || *maxoutp > NNTP_MAXLEN_COMMAND) ?
+            NNTP_MAXLEN_COMMAND : *maxoutp;
 
-	syslog(L_NOTICE, "%s user %s", Client.host, PERMuser);
+        if (sasl_ssf > 0) {
+            /* For the forthcoming check of the permissions the client now
+             * has, tell the connection is encrypted, so that TLS-only auth
+             * blocks in readers.conf are properly taken into account. */
+            encryption_layer_on = true;
 
-	if (serveroutlen)
-	    Reply("%d %s\r\n", NNTP_OK_SASL, base64);
-	else
-	    Reply("%d Authentication succeeded\r\n", NNTP_OK_AUTHINFO);
-
-	/* Save info about the negotiated security layer for I/O functions. */
-	sasl_ssf = *ssfp;
-	sasl_maxout =
-	    (*maxoutp == 0 || *maxoutp > NNTP_MAXLEN_COMMAND) ? NNTP_MAXLEN_COMMAND : *maxoutp;
-
-        if (sasl_ssf > 0) {
             /* Close out any existing article, report group stats.
              * RFC 4643 requires the reset of any knowledge about the client. */
             if (GRPcur) {
@@ -320,15 +315,33 @@
                 OVctl(OVCACHEFREE, &boolval);
                 free(GRPcur);
                 GRPcur = NULL;
-                if (ARTcount)
-                    syslog(L_NOTICE, "%s exit for AUTHINFO SASL articles %ld groups %ld",
+                if (ARTcount) {
+                    syslog(L_NOTICE,
+                           "%s exit for AUTHINFO SASL 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);
             }
+        }
 
-            /* Reset our read buffer so as to prevent plaintext command injection. */
-            line_reset(&NNTPline);
+        PERMgetaccess(false);
+        strlcpy(PERMuser, canon_user, sizeof(PERMuser));
+        PERMgetpermissions();
+        PERMneedauth = false;
+        PERMauthorized = true;
+        PERMcanauthenticate = false;
+
+        syslog(L_NOTICE, "%s user %s", Client.host, PERMuser);
+
+        if (serveroutlen) {
+            Reply("%d %s\r\n", NNTP_OK_SASL, base64);
+        } else {
+            Reply("%d Authentication succeeded\r\n", NNTP_OK_AUTHINFO);
         }
     } else {
 	/* Failure. */



More information about the inn-committers mailing list