Version 2 patch. Including timsieved.
Also in the patch is some code for Serverinfo switch in timsieved to not
disclose name and/or version info in "IMPLEMENTATION" if Serverinfo is Off or
Min.
Regards,
Carlos Velasco
-------- Original Message --------
Subject: Patch: forcing SSL before auth
From: Carlos Velasco <carlos.vela...@nimastelecom.com>
To: cyrus-devel@lists.andrew.cmu.edu
Date: 9/8/2015 12:18:36
> 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 14:46:11.855933782 +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 14:46:11.881933801 +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 14:46:11.883933803 +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 14:46:11.884933804 +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-09 14:46:11.885933805 +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 14:46:11.885933805 +0200
@@ -26,6 +26,7 @@
IMAPOPT_ALLOWAPOP,
IMAPOPT_ALLOWNEWNEWS,
IMAPOPT_ALLOWPLAINTEXT,
+ IMAPOPT_FORCETLSAUTH,
IMAPOPT_ALLOWUSERMOVES,
IMAPOPT_ALTNAMESPACE,
IMAPOPT_ANNOTATION_DB,
diff -ur a/timsieved/actions.c b/timsieved/actions.c
--- a/timsieved/actions.c 2015-07-06 02:40:07.000000000 +0200
+++ b/timsieved/actions.c 2015-08-09 15:40:44.286364806 +0200
@@ -177,12 +177,23 @@
int mechcount;
/* implementation */
- prot_printf(conn,
+ if (config_serverinfo == IMAP_ENUM_SERVERINFO_ON) {
+ prot_printf(conn,
"\"IMPLEMENTATION\" \"Cyrus timsieved%s %s\"\r\n",
config_mupdate_server ? " (Murder)" : "", cyrus_version());
+ } else if (config_serverinfo == IMAP_ENUM_SERVERINFO_MIN) {
+ prot_printf(conn,
+ "\"IMPLEMENTATION\" \"Cyrus timsieved%s\"\r\n",
+ config_mupdate_server ? " (Murder)" : "");
+ } else {
+ /* IMAP_ENUM_SERVERINFO_OFF */
+ prot_printf(conn,
+ "\"IMPLEMENTATION\" \"ManageSieve\"\r\n");
+ }
/* SASL */
if ((!authenticated || sasl_ssf) &&
+ (starttls_done || !config_getswitch(IMAPOPT_FORCETLSAUTH)) &&
sasl_listmech(saslconn, NULL,
"\"SASL\" \"", " ", "\"\r\n",
&sasllist,
diff -ur a/timsieved/parser.c b/timsieved/parser.c
--- a/timsieved/parser.c 2015-07-06 02:40:07.000000000 +0200
+++ b/timsieved/parser.c 2015-08-09 15:47:55.153764300 +0200
@@ -187,6 +187,10 @@
break;
case AUTHENTICATE:
+ if (!starttls_done && config_getswitch(IMAPOPT_FORCETLSAUTH)) {
+ error_msg = "AUTHENTICATE only available under a layer";
+ goto error;
+ }
if (timlex(NULL, NULL, sieved_in)!=SPACE)
{
error_msg = "SPACE must occur after AUTHENTICATE";
