INN commit: trunk (4 files)

INN Commit Russ_Allbery at isc.org
Sun Sep 21 17:39:56 UTC 2008


    Date: Sunday, September 21, 2008 @ 10:39:56
  Author: iulius
Revision: 8044

* It appears that sasl_decode64() returns SASL_CONTINUE instead of SASL_BADPROT
  when there is a base64-encoding error!  Adapt AUTHINFO SASL for that behaviour.
  Segfaults fixed in the process.
* Convert "=" to an empty string "" when a client answers only that.
* Call sasl_done() when the client exits in order to properly close the SASL
  connection object.
* Add NNTP_ERR_BASE64 for NNTP error code 504.

Modified:
  trunk/include/inn/nntp.h
  trunk/nnrpd/line.c
  trunk/nnrpd/nnrpd.c
  trunk/nnrpd/sasl.c

--------------------+
 include/inn/nntp.h |    1 
 nnrpd/line.c       |    3 -
 nnrpd/nnrpd.c      |    4 +
 nnrpd/sasl.c       |  118 +++++++++++++++++++++++++++++----------------------
 4 files changed, 76 insertions(+), 50 deletions(-)

Modified: include/inn/nntp.h
===================================================================
--- include/inn/nntp.h	2008-09-21 09:52:02 UTC (rev 8043)
+++ include/inn/nntp.h	2008-09-21 17:39:56 UTC (rev 8044)
@@ -81,6 +81,7 @@
     NNTP_ERR_SYNTAX             = 501,
     NNTP_ERR_ACCESS             = 502,
     NNTP_ERR_UNAVAILABLE        = 503,
+    NNTP_ERR_BASE64             = 504,
 
     /* Streaming extension. */
     NNTP_OK_STREAM              = 203,

Modified: nnrpd/line.c
===================================================================
--- nnrpd/line.c	2008-09-21 09:52:02 UTC (rev 8043)
+++ nnrpd/line.c	2008-09-21 17:39:56 UTC (rev 8044)
@@ -116,7 +116,8 @@
 	    int r;
 
 	    if ((r = sasl_decode(sasl_conn, p, n, &out, &outlen)) == SASL_OK) {
-		if (outlen) memcpy(p, out, outlen);
+		if (outlen)
+                    memcpy(p, out, outlen);
 		n = outlen;
 	    } else {
 		sysnotice("sasl_decode() failed: %s; %s",

Modified: nnrpd/nnrpd.c
===================================================================
--- nnrpd/nnrpd.c	2008-09-21 09:52:02 UTC (rev 8043)
+++ nnrpd/nnrpd.c	2008-09-21 17:39:56 UTC (rev 8044)
@@ -36,6 +36,7 @@
 bool nnrpd_starttls_done = false;
 #endif 
 
+
 /*
 **  If we have getloadavg, include the appropriate header file.  Otherwise,
 **  just assume that we always have a load of 0.
@@ -102,6 +103,7 @@
 
 static char	CMDfetchhelp[] = "[message-ID|number]";
 
+
 /*
 **  { command base name, function to call, need authentication,
 **    min args, max args, help string }
@@ -183,6 +185,7 @@
     "nntpwrite",
 };
 
+
 /*
 **  Log a summary status message and exit.
 */
@@ -241,6 +244,7 @@
 	sasl_ssf = 0;
 	sasl_maxout = NNTP_MAXLEN_COMMAND;
     }
+    sasl_done();
 #endif /* HAVE_SASL */
 
      if (DaemonMode) {

Modified: nnrpd/sasl.c
===================================================================
--- nnrpd/sasl.c	2008-09-21 09:52:02 UTC (rev 8043)
+++ nnrpd/sasl.c	2008-09-21 17:39:56 UTC (rev 8044)
@@ -1,8 +1,8 @@
 /*
- * AUTHINFO SASL functionality
- *
- * $Id$
- */
+** AUTHINFO SASL functionality.
+**
+** $Id$
+*/
 
 #include "config.h"
 #include "clibrary.h"
@@ -21,8 +21,8 @@
 int sasl_ssf = 0, sasl_maxout = NNTP_MAXLEN_COMMAND;
 
 sasl_callback_t sasl_callbacks[] = {
-    /* XXX do we want a proxy callback? */
-    /* XXX add a getopt callback? */
+    /* XXX Do we want a proxy callback? */
+    /* XXX Add a getopt callback? */
     { SASL_CB_LIST_END, NULL, NULL }
 };
 
@@ -74,6 +74,7 @@
     const int *maxoutp;
     const void *property;
     int r = SASL_OK;
+    int r1;
 
     if (ac < 3 || ac > 4) {
         /* In fact, ac > 4 here. */
@@ -107,92 +108,109 @@
 #endif
 
     if (ac == 4) {
-	/* initial response */
+	/* Initial response. */
 	clientin = av[3];
 
 	if (strcmp(clientin, "=") == 0) {
-	    /* zero-length initial response */
+	    /* Zero-length initial response. */
 	    clientin = "";
+            clientinlen = 0;
 	} else {
-	    /* decode the response */
-	    r = sasl_decode64(clientin, strlen(clientin),
-			      base64, BASE64_BUF_SIZE, &clientinlen);
-	    clientin = base64;
+	    /* Decode the response.  On error, SASL_CONTINUE should not be
+             * given.  Use SASL_BADPROT instead, in order to indicate
+             * a base64-encoding error. */
+            r1 = sasl_decode64(clientin, strlen(clientin),
+                               base64, BASE64_BUF_SIZE, &clientinlen);
+            clientin = base64;
+            r = (r1 == SASL_CONTINUE ? SASL_BADPROT : r1);
 	}
     }
 
     if (r == SASL_OK) {
-	/* start the exchange */
+	/* Start the exchange. */
 	r = sasl_server_start(sasl_conn, mech, clientin, clientinlen,
 			      &serverout, &serveroutlen);
     }
 
-    while (r == SASL_CONTINUE || (r == SASL_OK && serveroutlen)) {
-	if (serveroutlen) {
-	    /* encode the server challenge */
-	    int r1 = sasl_encode64(serverout, serveroutlen,
-				   base64, BASE64_BUF_SIZE, NULL);
-	    if (r1 != SASL_OK) r = r1;
+    while (r == SASL_CONTINUE || (r == SASL_OK && serveroutlen != 0)) {
+	if (serveroutlen != 0) {
+	    /* Encode the server challenge. */
+            r1 = sasl_encode64(serverout, serveroutlen,
+                               base64, BASE64_BUF_SIZE, NULL);
+            if (r1 != SASL_OK)
+                r = r1;
 	}
 
-	/* check for failure or success */
-	if (r != SASL_CONTINUE) break;
-	    
-	/* send the challenge to the client */
+	/* Check for failure or success. */
+        if (r != SASL_CONTINUE)
+            break;
+
+	/* Send the challenge to the client. */
 	Reply("%d %s\r\n", NNTP_CONT_SASL,
-	      serveroutlen ? base64 : "=");
+	      serveroutlen != 0 ? base64 : "=");
 	fflush(stdout);
 
-	/* get response from the client */
-	r = line_read(&NNTPline, PERMaccessconf->clienttimeout,
+	/* Get the response from the client. */
+	r1 = line_read(&NNTPline, PERMaccessconf->clienttimeout,
 		      &clientin, &clientinlen, NULL);
-	switch (r) {
+        
+        switch (r1) {
 	case RTok:
-	    if (clientinlen <= BASE64_BUF_SIZE) break;
+            if (clientinlen <= BASE64_BUF_SIZE)
+                break;
 	    /* FALLTHROUGH */
 	case RTlong:
-	    warn("%s response too long in authinfo sasl", Client.host);
+	    warn("%s response too long in AUTHINFO SASL", Client.host);
 	    ExitWithStats(1, false);
 	    break;
 	case RTtimeout:
-	    warn("%s timeout in authinfo sasl", Client.host);
+	    warn("%s timeout in AUTHINFO SASL", Client.host);
 	    ExitWithStats(1, false);
 	    break;
 	case RTeof:
-	    warn("%s EOF in authinfo sasl", Client.host);
+	    warn("%s EOF in AUTHINFO SASL", Client.host);
 	    ExitWithStats(1, false);
 	    break;
 	default:
-	    warn("%s internal %d in authinfo sasl", Client.host, r);
+	    warn("%s internal %d in AUTHINFO SASL", Client.host, r);
 	    ExitWithStats(1, false);
 	    break;
 	}
 
-	/* check if client cancelled */
+	/* Check if client cancelled. */
 	if (strcmp(clientin, "*") == 0) {
 	    Reply("%d Client cancelled authentication\r\n", NNTP_FAIL_AUTHINFO_BAD);
 	    return;
 	}
 
-	/* decode the response */
-	r = sasl_decode64(clientin, clientinlen,
-			  base64, BASE64_BUF_SIZE, &clientinlen);
-	clientin = base64;
+        if (strcmp(clientin, "=") == 0) {
+            /* Zero-length answer. */
+            clientin = "";
+            clientinlen = 0;
+        } else {
+            /* Decode the response.  On error, SASL_CONTINUE should not be
+             * given.  Use SASL_BADPROT instead, in order to indicate
+             * a base64-encoding error. */
+            r1 = sasl_decode64(clientin, clientinlen,
+                               base64, BASE64_BUF_SIZE, &clientinlen);
+            clientin = base64;
+            r = (r1 == SASL_CONTINUE ? SASL_BADPROT : r1);
+        }
 
-	/* do the next step */
+        /* Do the next step. */
 	if (r == SASL_OK) {
 	    r = sasl_server_step(sasl_conn, clientin, clientinlen,
 				 &serverout, &serveroutlen);
 	}
     }
 
-    /* fetch the username (authorization id) */
+    /* Fetch the username (authorization ID). */
     if (r == SASL_OK) {
 	r = sasl_getprop(sasl_conn, SASL_USERNAME, &property);
         canon_user = property;
     }
 
-    /* grab info about the negotiated layer */
+    /* Grab info about the negotiated layer. */
     if (r == SASL_OK) {
 	r = sasl_getprop(sasl_conn, SASL_SSF, &property);
         ssfp = property;
@@ -204,7 +222,7 @@
     }
 
     if (r == SASL_OK) {
-	/* success */
+	/* Success. */
 	strlcpy(PERMuser, canon_user, sizeof(PERMuser));
         PERMgetpermissions();
 	PERMneedauth = false;
@@ -218,13 +236,12 @@
 	else
 	    Reply("%d Authentication succeeded\r\n", NNTP_OK_AUTHINFO);
 
-	/* save info about the negotiated security layer for I/O functions */
+	/* 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;
-    }
-    else {
-	/* failure */
+    } else {
+	/* Failure. */
 	int resp_code;
 	const char *errstring = sasl_errstring(r, NULL, NULL);
 
@@ -232,14 +249,17 @@
 
 	switch (r) {
 	case SASL_BADPROT:
+            resp_code = NNTP_ERR_BASE64;
+            break;
+        case SASL_BADPARAM:
+        case SASL_NOTDONE:
 	    resp_code = NNTP_FAIL_AUTHINFO_REJECT;
 	    break;
 	case SASL_NOMECH:
-	case SASL_TOOWEAK:
-	    resp_code = NNTP_ERR_SYNTAX;
-	    break;
+            resp_code = NNTP_ERR_UNAVAILABLE;
+            break;
 	case SASL_ENCRYPT:
-	    resp_code = NNTP_FAIL_AUTHINFO_REJECT;
+	    resp_code = NNTP_FAIL_PRIVACY_NEEDED;
 	    break;
 	default:
 	    resp_code = NNTP_FAIL_AUTHINFO_BAD;



More information about the inn-committers mailing list