Hi,

Right now, "allowplaintext" option disallow using a plain authentication if 
session is not protected by TLS.
However, this setting still allows a client to make MD5 or SHA1 auth without 
session being protected by TLS. This can lead to not data confidentiality when 
using not plain auth.
There are several admins now requesting to force TLS for all sessions, and 
although this can be done using "allowplaintext" and removing all mechs but 
Plain, it would be right to be able to provide another layer of security and 
use TLS+SHA1 or so...

Attached is a patch with a new imapd.conf option:
forcetlsauth: 0 | 1. Default 0
If enabled all authentications require a TLS session negotiated before.

Patch also "hides" AUTH and other authentication commands that are not allowed 
before TLS, in Capabilites commands.
Patched in imapd, pop3d, nntpd, httpd.

This patch does not break cyradm functionality at all, however I attach another 
patch for the cyradm perl part to allow "--cafile" option (got tired of 
certificate validation warnings) and also fixed a minor bug when requesting 
capabilities to server without the callback.

Please, consider committing this to mainstream.

Regards,
Carlos Velasco

diff -ur a/imap/httpd.c b/imap/httpd.c
--- a/imap/httpd.c	2015-07-06 02:40:07.000000000 +0200
+++ b/imap/httpd.c	2015-08-09 11:24:53.246626678 +0200
@@ -602,7 +602,10 @@
 	    }
 	}
     }
-    httpd_tls_required = !avail_auth_schemes;
+    if (config_getswitch(IMAPOPT_FORCETLSAUTH))
+	httpd_tls_required = 1;
+    else
+	httpd_tls_required = !avail_auth_schemes;
 
     proc_register("httpd", httpd_clienthost, NULL, NULL, NULL);
 
diff -ur a/imap/imapd.c b/imap/imapd.c
--- a/imap/imapd.c	2015-07-06 02:40:07.000000000 +0200
+++ b/imap/imapd.c	2015-08-09 00:33:02.157296493 +0200
@@ -2398,9 +2398,11 @@
     }
 
     /* possibly disallow login */
-    if (!imapd_starttls_done && (extprops_ssf < 2) &&
-	!config_getswitch(IMAPOPT_ALLOWPLAINTEXT) &&
-	!is_userid_anonymous(canon_user)) {
+    if ((!imapd_starttls_done && (extprops_ssf < 2) &&
+         !config_getswitch(IMAPOPT_ALLOWPLAINTEXT) &&
+	 !is_userid_anonymous(canon_user)) || 
+        (!imapd_starttls_done &&
+	 config_getswitch(IMAPOPT_FORCETLSAUTH))) {
 	eatline(imapd_in, ' ');
 	prot_printf(imapd_out, "%s NO Login only available under a layer\r\n",
 		    tag);
@@ -2548,6 +2550,14 @@
     int r;
     int failedloginpause;
 
+    if (!imapd_starttls_done &&
+        config_getswitch(IMAPOPT_FORCETLSAUTH)) {
+        eatline(imapd_in, ' ');
+        prot_printf(imapd_out, "%s NO Auth only available under a layer\r\n",
+                    tag);
+        return;
+    }
+
     r = saslserver(imapd_saslconn, authtype, resp, "", "+ ", "",
 		   imapd_in, imapd_out, &sasl_result, NULL);
 
@@ -3039,6 +3049,7 @@
 
     /* add the SASL mechs */
     if ((!imapd_authstate || saslprops.ssf) &&
+        (imapd_starttls_done || !config_getswitch(IMAPOPT_FORCETLSAUTH)) &&
 	sasl_listmech(imapd_saslconn, NULL, 
 		      "AUTH=", " AUTH=",
 		      !imapd_authstate ? " SASL-IR" : "", &sasllist,
diff -ur a/imap/nntpd.c b/imap/nntpd.c
--- a/imap/nntpd.c	2015-07-06 02:40:07.000000000 +0200
+++ b/imap/nntpd.c	2015-08-09 02:33:05.773557382 +0200
@@ -1813,20 +1813,22 @@
     if (tls_enabled() && !nntp_starttls_done && !nntp_authstate)
 	prot_printf(nntp_out, "STARTTLS\r\n");
 
-    /* check for SASL mechs */
-    sasl_listmech(nntp_saslconn, NULL, "SASL ", " ", "\r\n",
-		  &mechlist, NULL, &mechcount);
-
-    /* add the AUTHINFO variants */
-    if (!nntp_authstate) {
-	prot_printf(nntp_out, "AUTHINFO%s%s\r\n",
+    if (nntp_starttls_done || !config_getswitch(IMAPOPT_FORCETLSAUTH)) {
+        /* check for SASL mechs */
+        sasl_listmech(nntp_saslconn, NULL, "SASL ", " ", "\r\n",
+		&mechlist, NULL, &mechcount);
+
+        /* add the AUTHINFO variants */
+        if (!nntp_authstate) {
+		prot_printf(nntp_out, "AUTHINFO%s%s\r\n",
 		    (nntp_starttls_done || (extprops_ssf > 1) ||
 		     config_getswitch(IMAPOPT_ALLOWPLAINTEXT)) ?
 		    " USER" : "", mechcount ? " SASL" : "");
-    }
+        }
 
-    /* add the SASL mechs */
-    if (mechcount) prot_printf(nntp_out, "%s", mechlist);
+        /* add the SASL mechs */
+        if (mechcount) prot_printf(nntp_out, "%s", mechlist);
+    }
 
     /* add the reader capabilities/extensions */
     if ((nntp_capa & MODE_READ) && (nntp_authstate || allowanonymous)) {
@@ -1974,9 +1976,10 @@
 	return;
     }
 
-    /* possibly disallow USER */
-    if (!(nntp_starttls_done || (extprops_ssf > 1) ||
-	  config_getswitch(IMAPOPT_ALLOWPLAINTEXT))) {
+    /* possibly disallow AUTHINFO USER */
+    if ((!(nntp_starttls_done || (extprops_ssf > 1) ||
+	  config_getswitch(IMAPOPT_ALLOWPLAINTEXT))) ||
+        (!nntp_starttls_done && config_getswitch(IMAPOPT_FORCETLSAUTH))) {
 	prot_printf(nntp_out,
 		    "483 AUTHINFO USER command only available under a layer\r\n");
 	return;
@@ -2085,6 +2088,13 @@
     int failedloginpause;
     struct proc_limits limits;
 
+    /* possibly disallow AUTHINFO SASL */
+    if (!nntp_starttls_done && config_getswitch(IMAPOPT_FORCETLSAUTH)) {
+        prot_printf(nntp_out,
+                    "483 AUTHINFO SASL command only available under a layer\r\n");
+        return;
+    }
+
     /* Conceal initial response in telemetry log */
     if (nntp_logfd != -1 && resp) {
 	r = ftruncate(nntp_logfd,
diff -ur a/imap/pop3d.c b/imap/pop3d.c
--- a/imap/pop3d.c	2015-07-06 02:40:07.000000000 +0200
+++ b/imap/pop3d.c	2015-08-09 02:13:49.900700249 +0200
@@ -1349,6 +1349,13 @@
     const void *canon_user;
     int failedloginpause;
 
+    /* possibly disallow APOP */
+    if (!popd_starttls_done && config_getswitch(IMAPOPT_FORCETLSAUTH)) {
+        prot_printf(popd_out,
+                    "-ERR [AUTH] APOP command only available under a layer\r\n");
+        return;
+    }
+
     assert(response != NULL);
 
     if (popd_userid) {
@@ -1418,7 +1425,8 @@
 
     /* possibly disallow USER */
     if (!(kflag || popd_starttls_done || (extprops_ssf > 1) ||
-	  config_getswitch(IMAPOPT_ALLOWPLAINTEXT))) {
+	  config_getswitch(IMAPOPT_ALLOWPLAINTEXT)) ||
+         (!popd_starttls_done && config_getswitch(IMAPOPT_FORCETLSAUTH))) {
 	prot_printf(popd_out,
 		    "-ERR [AUTH] USER command only available under a layer\r\n");
 	return;
@@ -1570,6 +1578,7 @@
 
     /* SASL special case: print SASL, then a list of supported capabilities */
     if ((!popd_authstate || saslprops.ssf) &&
+         (popd_starttls_done || !config_getswitch(IMAPOPT_FORCETLSAUTH)) &&
 	sasl_listmech(popd_saslconn,
 		      NULL, /* should be id string */
 		      "SASL ", " ", "\r\n",
@@ -1594,9 +1603,10 @@
     prot_printf(popd_out, "RESP-CODES\r\n");
     prot_printf(popd_out, "AUTH-RESP-CODE\r\n");
 
-    if (!popd_authstate &&
-	(kflag || popd_starttls_done || (extprops_ssf > 1)
-	 || config_getswitch(IMAPOPT_ALLOWPLAINTEXT))) {
+    if ((!popd_authstate &&
+	 (kflag || popd_starttls_done || (extprops_ssf > 1)
+	  || config_getswitch(IMAPOPT_ALLOWPLAINTEXT))) &&
+        (popd_starttls_done || !config_getswitch(IMAPOPT_FORCETLSAUTH))) {
 	prot_printf(popd_out, "USER\r\n");
     }
 
@@ -1619,6 +1629,13 @@
     const char *canon_user;
     int failedloginpause;
 
+    /* possibly disallow AUTH */
+    if (!popd_starttls_done && config_getswitch(IMAPOPT_FORCETLSAUTH)) {
+        prot_printf(popd_out,
+                    "-ERR [AUTH] AUTH command only available under a layer\r\n");
+        return;
+    }
+
     /* if client didn't specify an argument we give them the list
      *
      * XXX This method of mechanism discovery is an undocumented feature
diff -ur a/lib/imapopts.c b/lib/imapopts.c
--- a/lib/imapopts.c	2015-07-06 03:24:21.000000000 +0200
+++ b/lib/imapopts.c	2015-08-08 23:58:26.098643381 +0200
@@ -64,6 +64,10 @@
     (union config_value)((long) 0),
     (union config_value)((long) 0),
     { { NULL, IMAP_ENUM_ZERO } } },
+  { IMAPOPT_FORCETLSAUTH, "forcetlsauth", 0, OPT_SWITCH,
+    (union config_value)((long) 0),
+    (union config_value)((long) 0),
+    { { NULL, IMAP_ENUM_ZERO } } },
   { IMAPOPT_ALLOWUSERMOVES, "allowusermoves", 0, OPT_SWITCH,
     (union config_value)((long) 0),
     (union config_value)((long) 0),
diff -ur a/lib/imapopts.h b/lib/imapopts.h
--- a/lib/imapopts.h	2015-07-06 03:24:21.000000000 +0200
+++ b/lib/imapopts.h	2015-08-09 00:00:26.852675380 +0200
@@ -26,6 +26,7 @@
   IMAPOPT_ALLOWAPOP,
   IMAPOPT_ALLOWNEWNEWS,
   IMAPOPT_ALLOWPLAINTEXT,
+  IMAPOPT_FORCETLSAUTH,
   IMAPOPT_ALLOWUSERMOVES,
   IMAPOPT_ALTNAMESPACE,
   IMAPOPT_ANNOTATION_DB,
diff -ur a/perl/imap/IMAP/Shell.pm b/perl/imap/IMAP/Shell.pm
--- a/perl/imap/IMAP/Shell.pm	2015-07-06 02:40:07.000000000 +0200
+++ b/perl/imap/IMAP/Shell.pm	2015-08-09 11:43:28.536509727 +0200
@@ -444,7 +444,7 @@
 # programs, as opposed to things expected from within a program.)
 sub shell {
   my ($server, $port, $authz, $auth, $systemrc, $userrc, $dorc, $mech, $pw,
-      $tlskey, $notls) =
+      $tlskey, $cafile, $notls) =
     ('', 143, undef, $ENV{USER} || $ENV{LOGNAME}, '/usr/local/etc/cyradmrc.pl',
      "$ENV{HOME}/.cyradmrc.pl", 1, undef, undef, undef, undef);
   GetOptions('user|u=s' => \$auth,
@@ -457,6 +457,7 @@
 	     'auth|a=s' => \$mech,
 	     'password|w=s' => \$pw,
   	     'tlskey|t:s' => \$tlskey,
+	     'cafile=s' => \$cafile,
   	     'notls' => \$notls,
 	     'help|h' => sub { cyradm_usage(); exit(0); },
 	     'version|v' => sub { cyradm_version(); exit(0); }
@@ -478,7 +479,7 @@
 			  -rock => \$cyradm});
     $cyradm->authenticate(-authz => $authz, -user => $auth,
 			  -mechanism => $mech, -password => $pw,
-			  -tlskey => $tlskey, -notls => $notls)
+			  -tlskey => $tlskey, -notls => $notls, -cafile => $cafile)
       or die "cyradm: cannot authenticate to server with $mech as $auth\n";
   }
   my $fstk = [*STDIN, *STDOUT, *STDERR];
diff -ur a/perl/imap/IMAP.pm b/perl/imap/IMAP.pm
--- a/perl/imap/IMAP.pm	2015-07-06 02:40:07.000000000 +0200
+++ b/perl/imap/IMAP.pm	2015-08-09 11:41:26.384300583 +0200
@@ -287,9 +287,21 @@
 
     # Refetch all relevent capabilities
     ($starttls, $logindisabled, $availmechs) = (0, 0, "");
+    $self->addcallback({-trigger => 'CAPABILITY',
+			-callback => sub {my %a = @_;
+					map {
+						$starttls = 1
+						if /^STARTTLS$/i;
+							$logindisabled = 1
+						if /^LOGINDISABLED$/i;
+							$availmechs .= $_ . ' '
+						if s/^AUTH=//;
+					}
+					split(/ /, $a{-text})}});
     $self->send(undef, undef, 'CAPABILITY');
 
     $opts{-mechanism} = $availmechs if ($opts{-mechanism} eq '');
+    $self->addcallback({-trigger => 'CAPABILITY'});
   }
 
   $opts{-mechanism} ||= 'plain';

Reply via email to