commit 2745813367777f406d917b8a42f309f8c4da9953
Author: Oswald Buddenhagen <o...@users.sf.net>
Date:   Sat Jul 12 20:35:55 2014 +0200

    re-design SSL/TLS configuration
    
    the combinations of the various options made quite a mess. additionally,
    'RequireSSL no' is inherently insecure - "use SSL if available" is plain
    stupid.
    
    the old options are still accepted, but will elicit a warning.

 NEWS           |    4 +
 src/config.c   |    5 +-
 src/config.h   |    5 ++
 src/drv_imap.c |  151 +++++++++++++++++++++++++++++++++---------------
 src/mbsync.1   |   58 ++++++-------------
 src/socket.c   |   10 ++--
 src/socket.h   |   12 ++++-
 7 files changed, 147 insertions(+), 98 deletions(-)

diff --git a/NEWS b/NEWS
index 63f9d78..a4ff5ec 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@
 
 The 'isync' compatibility wrapper is now deprecated.
 
+The SSL/TLS configuration has been re-designed.
+SSL is now explicitly enabled or disabled - "use SSL if available" is gone.
+Notice: Tunnels are assumed to be secure and thus default to no SSL.
+
 [1.1.0]
 
 Support for hierarchical mailboxes in Patterns.
diff --git a/src/config.c b/src/config.c
index cf3b165..2ac3e70 100644
--- a/src/config.c
+++ b/src/config.c
@@ -36,10 +36,7 @@
 
 store_conf_t *stores;
 
-#define ARG_OPTIONAL 0
-#define ARG_REQUIRED 1
-
-static char *
+char *
 get_arg( conffile_t *cfile, int required, int *comment )
 {
        char *ret, *p, *t;
diff --git a/src/config.h b/src/config.h
index b9b4bfe..e3bc72f 100644
--- a/src/config.h
+++ b/src/config.h
@@ -35,6 +35,11 @@ typedef struct conffile {
        char *cmd, *val, *rest;
 } conffile_t;
 
+#define ARG_OPTIONAL 0
+#define ARG_REQUIRED 1
+
+char *get_arg( conffile_t *cfile, int required, int *comment );
+
 int parse_bool( conffile_t *cfile );
 int parse_int( conffile_t *cfile );
 int parse_size( conffile_t *cfile );
diff --git a/src/drv_imap.c b/src/drv_imap.c
index 299aa23..8981bb4 100644
--- a/src/drv_imap.c
+++ b/src/drv_imap.c
@@ -36,6 +36,10 @@
 #include <time.h>
 #include <sys/wait.h>
 
+#ifdef HAVE_LIBSSL
+enum { SSL_None, SSL_STARTTLS, SSL_IMAPS };
+#endif
+
 typedef struct imap_server_conf {
        struct imap_server_conf *next;
        char *name;
@@ -45,9 +49,7 @@ typedef struct imap_server_conf {
        char *pass_cmd;
        int max_in_progress;
 #ifdef HAVE_LIBSSL
-       char use_ssl;
-       char use_imaps;
-       char require_ssl;
+       char ssl_type;
        char require_cram;
 #endif
 } imap_server_conf_t;
@@ -1532,7 +1534,7 @@ imap_open_store_connected( int ok, void *aux )
        if (!ok)
                imap_open_store_bail( ctx );
 #ifdef HAVE_LIBSSL
-       else if (srvc->use_imaps)
+       else if (srvc->ssl_type == SSL_IMAPS)
                socket_start_tls( &ctx->conn, imap_open_store_tlsstarted1 );
 #endif
 }
@@ -1582,26 +1584,21 @@ imap_open_store_authenticate( imap_store_t *ctx )
 
        if (ctx->greeting != GreetingPreauth) {
 #ifdef HAVE_LIBSSL
-               if (!srvc->use_imaps && srvc->use_ssl) {
-                       /* always try to select SSL support if available */
+               if (srvc->ssl_type == SSL_STARTTLS) {
                        if (CAP(STARTTLS)) {
                                imap_exec( ctx, 0, 
imap_open_store_authenticate_p2, "STARTTLS" );
                                return;
                        } else {
-                               if (srvc->require_ssl) {
-                                       error( "IMAP error: SSL support not 
available\n" );
-                                       imap_open_store_bail( ctx );
-                                       return;
-                               } else {
-                                       warn( "IMAP warning: SSL support not 
available\n" );
-                               }
+                               error( "IMAP error: SSL support not 
available\n" );
+                               imap_open_store_bail( ctx );
+                               return;
                        }
                }
 #endif
                imap_open_store_authenticate2( ctx );
        } else {
 #ifdef HAVE_LIBSSL
-               if (!srvc->use_imaps && srvc->require_ssl) {
+               if (srvc->ssl_type == SSL_STARTTLS) {
                        error( "IMAP error: SSL support not available\n" );
                        imap_open_store_bail( ctx );
                        return;
@@ -2237,8 +2234,13 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep 
)
 {
        imap_store_conf_t *store;
        imap_server_conf_t *server, *srv, sserver;
-       const char *type, *name;
+       const char *type, *name, *arg;
        int acc_opt = 0;
+#ifdef HAVE_LIBSSL
+       /* Legacy SSL options */
+       int require_ssl = -1, use_imaps = -1;
+       int use_sslv2 = -1, use_sslv3 = -1, use_tlsv1 = -1, use_tlsv11 = -1, 
use_tlsv12 = -1;
+#endif
 
        if (!strcasecmp( "IMAPAccount", cfg->cmd )) {
                server = nfcalloc( sizeof(*server) );
@@ -2259,32 +2261,30 @@ imap_parse_store( conffile_t *cfg, store_conf_t 
**storep )
                return 0;
 
 #ifdef HAVE_LIBSSL
-       /* this will probably annoy people, but its the best default just in
-        * case people forget to turn it on
-        */
-       server->require_ssl = 1;
-       server->sconf.use_tlsv1 = 1;
+       server->ssl_type = -1;
+       server->sconf.ssl_versions = -1;
 #endif
        server->max_in_progress = INT_MAX;
 
        while (getcline( cfg ) && cfg->cmd) {
                if (!strcasecmp( "Host", cfg->cmd )) {
                        /* The imap[s]: syntax is just a backwards compat hack. 
*/
+                       arg = cfg->val;
 #ifdef HAVE_LIBSSL
-                       if (starts_with( cfg->val, -1, "imaps:", 6 )) {
-                               cfg->val += 6;
-                               server->use_imaps = 1;
-                               server->sconf.use_sslv2 = 1;
-                               server->sconf.use_sslv3 = 1;
+                       if (starts_with( arg, -1, "imaps:", 6 )) {
+                               arg += 6;
+                               server->ssl_type = SSL_IMAPS;
+                               if (server->sconf.ssl_versions == -1)
+                                       server->sconf.ssl_versions = SSLv2 | 
SSLv3 | TLSv1;
                        } else
 #endif
-                       {
-                               if (starts_with( cfg->val, -1, "imap:", 5 ))
-                                       cfg->val += 5;
-                       }
-                       if (starts_with( cfg->val, -1, "//", 2 ))
-                               cfg->val += 2;
-                       server->sconf.host = nfstrdup( cfg->val );
+                       if (starts_with( arg, -1, "imap:", 5 ))
+                               arg += 5;
+                       if (!memcmp( "//", arg, 2 ))
+                               arg += 2;
+                       if (arg != cfg->val)
+                               warn( "%s:%d: Notice: URL notation is 
deprecated; use a plain host name and possibly 'SSLType IMAPS' instead\n", 
cfg->file, cfg->line );
+                       server->sconf.host = nfstrdup( arg );
                }
                else if (!strcasecmp( "User", cfg->cmd ))
                        server->user = nfstrdup( cfg->val );
@@ -2308,20 +2308,51 @@ imap_parse_store( conffile_t *cfg, store_conf_t 
**storep )
                                           cfg->file, cfg->line, 
server->sconf.cert_file );
                                cfg->err = 1;
                        }
+               } else if (!strcasecmp( "SSLType", cfg->cmd )) {
+                       if (!strcasecmp( "None", cfg->val )) {
+                               server->ssl_type = SSL_None;
+                       } else if (!strcasecmp( "STARTTLS", cfg->val )) {
+                               server->ssl_type = SSL_STARTTLS;
+                       } else if (!strcasecmp( "IMAPS", cfg->val )) {
+                               server->ssl_type = SSL_IMAPS;
+                       } else {
+                               error( "%s:%d: Invalid SSL type\n", cfg->file, 
cfg->line );
+                               cfg->err = 1;
+                       }
+               } else if (!strcasecmp( "SSLVersion", cfg->cmd ) ||
+                          !strcasecmp( "SSLVersions", cfg->cmd )) {
+                       server->sconf.ssl_versions = 0;
+                       arg = cfg->val;
+                       do {
+                               if (!strcasecmp( "SSLv2", arg )) {
+                                       server->sconf.ssl_versions |= SSLv2;
+                               } else if (!strcasecmp( "SSLv3", arg )) {
+                                       server->sconf.ssl_versions |= SSLv3;
+                               } else if (!strcasecmp( "TLSv1", arg )) {
+                                       server->sconf.ssl_versions |= TLSv1;
+                               } else if (!strcasecmp( "TLSv1.1", arg )) {
+                                       server->sconf.ssl_versions |= TLSv1_1;
+                               } else if (!strcasecmp( "TLSv1.2", arg )) {
+                                       server->sconf.ssl_versions |= TLSv1_2;
+                               } else {
+                                       error( "%s:%d: Unrecognized SSL 
version\n", cfg->file, cfg->line );
+                                       cfg->err = 1;
+                               }
+                       } while ((arg = get_arg( cfg, ARG_OPTIONAL, 0 )));
                } else if (!strcasecmp( "RequireSSL", cfg->cmd ))
-                       server->require_ssl = parse_bool( cfg );
+                       require_ssl = parse_bool( cfg );
                else if (!strcasecmp( "UseIMAPS", cfg->cmd ))
-                       server->use_imaps = parse_bool( cfg );
+                       use_imaps = parse_bool( cfg );
                else if (!strcasecmp( "UseSSLv2", cfg->cmd ))
-                       server->sconf.use_sslv2 = parse_bool( cfg );
+                       use_sslv2 = parse_bool( cfg );
                else if (!strcasecmp( "UseSSLv3", cfg->cmd ))
-                       server->sconf.use_sslv3 = parse_bool( cfg );
+                       use_sslv3 = parse_bool( cfg );
                else if (!strcasecmp( "UseTLSv1", cfg->cmd ))
-                       server->sconf.use_tlsv1 = parse_bool( cfg );
+                       use_tlsv1 = parse_bool( cfg );
                else if (!strcasecmp( "UseTLSv1.1", cfg->cmd ))
-                       server->sconf.use_tlsv11 = parse_bool( cfg );
+                       use_tlsv11 = parse_bool( cfg );
                else if (!strcasecmp( "UseTLSv1.2", cfg->cmd ))
-                       server->sconf.use_tlsv12 = parse_bool( cfg );
+                       use_tlsv12 = parse_bool( cfg );
                else if (!strcasecmp( "RequireCRAM", cfg->cmd ))
                        server->require_cram = parse_bool( cfg );
 #endif
@@ -2369,19 +2400,45 @@ imap_parse_store( conffile_t *cfg, store_conf_t 
**storep )
                        return 1;
                }
 #ifdef HAVE_LIBSSL
-               server->use_ssl =
-                       server->sconf.use_sslv2 | server->sconf.use_sslv3 |
-                       server->sconf.use_tlsv1 | server->sconf.use_tlsv11 | 
server->sconf.use_tlsv12;
-               if (server->require_ssl && !server->use_ssl) {
-                       error( "%s '%s' requires SSL but no SSL versions 
enabled\n", type, name );
-                       cfg->err = 1;
-                       return 1;
+               if ((use_sslv2 & use_sslv3 & use_tlsv1 & use_tlsv11 & 
use_tlsv12) != -1 || use_imaps >= 0 || require_ssl >= 0) {
+                       if (server->ssl_type >= 0 || server->sconf.ssl_versions 
>= 0) {
+                               error( "%s '%s': The deprecated UseSSL*, 
UseTLS*, UseIMAPS, and RequireSSL options are mutually exlusive with SSLType 
and SSLVersions.\n", type, name );
+                               cfg->err = 1;
+                               return 1;
+                       }
+                       warn( "Notice: %s '%s': UseSSL*, UseTLS*, UseIMAPS, and 
RequireSSL are deprecated. Use SSLType and SSLVersions instead.\n", type, name 
);
+                       server->sconf.ssl_versions =
+                                       (use_sslv2 != 1 ? 0 : SSLv2) |
+                                       (use_sslv3 != 1 ? 0 : SSLv3) |
+                                       (use_tlsv1 == 0 ? 0 : TLSv1) |
+                                       (use_tlsv11 != 1 ? 0 : TLSv1_1) |
+                                       (use_tlsv12 != 1 ? 0 : TLSv1_2);
+                       if (use_imaps == 1) {
+                               server->ssl_type = SSL_IMAPS;
+                       } else if (require_ssl) {
+                               server->ssl_type = SSL_STARTTLS;
+                       } else if (!server->sconf.ssl_versions) {
+                               server->ssl_type = SSL_None;
+                       } else {
+                               warn( "Notice: %s '%s': 'RequireSSL no' is 
being ignored\n", type, name );
+                               server->ssl_type = SSL_STARTTLS;
+                       }
+                       if (server->ssl_type != SSL_None && 
!server->sconf.ssl_versions) {
+                               error( "%s '%s' requires SSL but no SSL 
versions enabled\n", type, name );
+                               cfg->err = 1;
+                               return 1;
+                       }
+               } else {
+                       if (server->sconf.ssl_versions < 0)
+                               server->sconf.ssl_versions = TLSv1; /* Most 
compatible and still reasonably secure. */
+                       if (server->ssl_type < 0)
+                               server->ssl_type = server->sconf.tunnel ? 
SSL_None : SSL_STARTTLS;
                }
 #endif
                if (!server->sconf.port)
                        server->sconf.port =
 #ifdef HAVE_LIBSSL
-                               server->use_imaps ? 993 :
+                               server->ssl_type == SSL_IMAPS ? 993 :
 #endif
                                143;
        }
diff --git a/src/mbsync.1 b/src/mbsync.1
index 2af9178..fe24f8a 100644
--- a/src/mbsync.1
+++ b/src/mbsync.1
@@ -271,11 +271,6 @@ optional.
 Specify a command to run to establish a connection rather than opening a TCP
 socket.  This allows you to run an IMAP session over an SSH tunnel, for
 example.
-.br
-If \fBUseIMAPS\fR is disabled and the tunnel opens a preauthenticated
-connection, \fBRequireSSL\fR also needs to be disabled.
-If the connection is not preauthenticated, but the tunnel is secure,
-disabling \fBRequireSSL\fR and \fBUseTLSv1\fR is recommended.
 ..
 .TP
 \fBRequireCRAM\fR \fIyes\fR|\fIno\fR
@@ -283,18 +278,26 @@ If set to \fIyes\fR, \fBmbsync\fR will abort the 
connection if no CRAM-MD5
 authentication is possible.  (Default: \fIno\fR)
 ..
 .TP
-\fBUseIMAPS\fR \fIyes\fR|\fIno\fR
-If set to \fIyes\fR, the default for \fBPort\fR is changed to 993 and
-\fBmbsync\fR will start SSL negotiation immediately after establishing
-the connection to the server.
+\fBSSLType\fR {\fINone\fR|\fISTARTTLS\fR|\fIIMAPS\fR}
+Select the connection security/encryption method:
+.br
+\fINone\fR - no security.
+This is the default when \fBTunnel\fR is set, as tunnels are usually secure.
+.br
+\fISTARTTLS\fR - security is established via the STARTTLS extension
+after connecting the regular IMAP port 143. Most servers support this,
+so it is the default (unless a tunnel is used).
 .br
-Note that modern servers support SSL on the regular IMAP port 143 via the
-STARTTLS extension, which will be used automatically by default.
+\fIIMAPS\fR - security is established by starting SSL/TLS negotiation
+right after connecting the secure IMAP port 993.
 ..
 .TP
-\fBRequireSSL\fR \fIyes\fR|\fIno\fR
-\fBmbsync\fR will abort the connection if a TLS/SSL session cannot be
-established with the IMAP server.  (Default: \fIyes\fR)
+\fBSSLVersions\fR [\fISSLv2\fR] [\fISSLv3\fR] [\fITLSv1\fR] [\fITLSv1.1\fR] 
[\fITLSv1.2\fR]
+Select the acceptable SSL/TLS versions.
+Use of SSLv2 is strongly discouraged for security reasons, but might be the
+only option on some very old servers.
+Generally, the newest TLS version is recommended, but as this confuses some
+servers, \fBTLSv1\fR is the default.
 ..
 .TP
 \fBCertificateFile\fR \fIpath\fR
@@ -306,33 +309,6 @@ Note that the system's default certificate store is always 
used and should
 not be specified here.
 ..
 .TP
-\fBUseSSLv2\fR \fIyes\fR|\fIno\fR
-Use SSLv2 for communication with the IMAP server over SSL?
-.br
-Note that this option is deprecated for security reasons.
-(Default: \fIno\fR)
-..
-.TP
-\fBUseSSLv3\fR \fIyes\fR|\fIno\fR
-Use SSLv3 for communication with the IMAP server over SSL?
-(Default: \fIno\fR)
-..
-.TP
-\fBUseTLSv1\fR \fIyes\fR|\fIno\fR
-Use TLSv1 for communication with the IMAP server over SSL?
-(Default: \fIyes\fR)
-..
-.TP
-\fBUseTLSv1.1\fR \fIyes\fR|\fIno\fR
-Use TLSv1.1 for communication with the IMAP server over SSL?
-(Default: \fIno\fR)
-..
-.TP
-\fBUseTLSv1.2\fR \fIyes\fR|\fIno\fR
-Use TLSv1.2 for communication with the IMAP server over SSL?
-(Default: \fIno\fR)
-..
-.TP
 \fBPipelineDepth\fR \fIdepth\fR
 Maximum number of IMAP commands which can be simultaneously in flight.
 Setting this to \fI1\fR disables pipelining.
diff --git a/src/socket.c b/src/socket.c
index 4bc2752..c760c0a 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -205,18 +205,18 @@ init_ssl_ctx( const server_conf_t *conf )
 
        mconf->SSLContext = SSL_CTX_new( SSLv23_client_method() );
 
-       if (!conf->use_sslv2)
+       if (!(conf->ssl_versions & SSLv2))
                options |= SSL_OP_NO_SSLv2;
-       if (!conf->use_sslv3)
+       if (!(conf->ssl_versions & SSLv3))
                options |= SSL_OP_NO_SSLv3;
-       if (!conf->use_tlsv1)
+       if (!(conf->ssl_versions & TLSv1))
                options |= SSL_OP_NO_TLSv1;
 #ifdef SSL_OP_NO_TLSv1_1
-       if (!conf->use_tlsv11)
+       if (!(conf->ssl_versions & TLSv1_1))
                options |= SSL_OP_NO_TLSv1_1;
 #endif
 #ifdef SSL_OP_NO_TLSv1_2
-       if (!conf->use_tlsv12)
+       if (!(conf->ssl_versions & TLSv1_2))
                options |= SSL_OP_NO_TLSv1_2;
 #endif
 
diff --git a/src/socket.h b/src/socket.h
index c7fcb78..bfc5ddf 100644
--- a/src/socket.h
+++ b/src/socket.h
@@ -25,16 +25,26 @@
 
 #include "common.h"
 
+#ifdef HAVE_LIBSSL
 typedef struct ssl_st SSL;
 typedef struct ssl_ctx_st SSL_CTX;
 
+enum {
+       SSLv2 = 1,
+       SSLv3 = 2,
+       TLSv1 = 4,
+       TLSv1_1 = 8,
+       TLSv1_2 = 16
+};
+#endif
+
 typedef struct server_conf {
        char *tunnel;
        char *host;
        int port;
 #ifdef HAVE_LIBSSL
        char *cert_file;
-       char use_sslv2, use_sslv3, use_tlsv1, use_tlsv11, use_tlsv12;
+       char ssl_versions;
 
        /* these are actually variables and are leaked at the end */
        char ssl_ctx_valid;

------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://p.sf.net/sfu/Zoho
_______________________________________________
isync-devel mailing list
isync-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/isync-devel

Reply via email to