Improve SSL error messages for common issues

diff --git a/NEWS b/NEWS
index 654eb6e1c79c6383090487652287abff890debaf..223811adc08fd27348cffc3938cda04110d91523 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -54,14 +54,22 @@ removed from a 6.4.0 or newer release.)
 
 fetchmail-6.3.17 (not yet released):
 
-# BUG FIX
-* Plug memory leak when using a "defaults" entry in the run control file.
-
 # REGRESSION FIX
 * Fix string handling in rcfile scanner, which caused fetchmail to misparse a
   run control file in certain circumstances.  Fixes BerliOS bug #14257.
   Patch by Michael Banack.  This fixes a regression introduced before 6.3.0.
 
+# BUG FIXES
+* Plug memory leak when using a "defaults" entry in the run control file.
+* Do not print SSL certificate mismatches unless verbose or --sslcertck is
+  enabled.
+
+# CHANGES
+* Usability: SSL certificate chains are fully printed in -v -v mode, and there
+  are now helpful pointers to --sslcertpath and c_rehash for "unable to get
+  local issuer certificate" and self-signed certificates -- these usually hint
+  to missing root signing CAs in the certs directory.
+
 # DOCUMENTATION
 * Fix table of global option to read "set softbounce" where there used to be a
   2nd copy of "set spambounce".  Patch by Michael Banack, BerliOS Bug #17067.

diff --git a/README.SSL-SERVER b/README.SSL-SERVER
index 8d4e40b00901ee863e12bc4ef63bf8001904c57a..ae833e608814bf78ffe17f3ec1b8bbfb6a6b3e5f 100644 (file)
--- a/README.SSL-SERVER
+++ b/README.SSL-SERVER
@@ -1,6 +1,10 @@
 SSL server requirements
 -----------------------
 
+This document is meant for Internet service providers as a check-list.  If
+fetchmail refers you to it, mail this file to the support address for the
+server that caused the reference.
+
 In order to let any mail client (not just fetchmail) verify server certificates 
 properly, so that users can be sure their connection is not eavesdropped, there 
 are several requirements that need to be fulfilled.

diff --git a/fetchmail.man b/fetchmail.man
index df823f95941e80eaf9f3a34769998a318187ad3f..1c6344cc873358978855c51903da344a22d6c3d3 100644 (file)
--- a/fetchmail.man
+++ b/fetchmail.man
@@ -510,10 +510,10 @@ fetchmail versions.
 .B \-\-sslcertpath <directory>
 (Keyword: sslcertpath)
 .br
-Sets the directory fetchmail uses to look up local certificates. The default
-is your OpenSSL default one. The directory must be hashed as OpenSSL expects
-it - every time you add or modify a certificate in the directory, you need
-to use the \fBc_rehash\fP tool (which comes with OpenSSL in the tools/
+Sets the directory fetchmail uses to look up local certificates. The default is
+your OpenSSL default directory. The directory must be hashed the way OpenSSL
+expects it - every time you add or modify a certificate in the directory, you
+need to use the \fBc_rehash\fP tool (which comes with OpenSSL in the tools/
 subdirectory). Also, after OpenSSL upgrades, you may need to run
 \fBc_rehash\fP; particularly when upgrading from 0.9.X to 1.0.0.
 .TP

diff --git a/socket.c b/socket.c
index e5c52104e159717320581b8a56b2b6505560cb66..2ef709619cf9d2ba752e06916a2b6e6557581d3d 100644 (file)
--- a/socket.c
+++ b/socket.c
@@ -569,7 +569,9 @@ static      int _check_fp;
 static char *_check_digest;
 static         char *_server_label;
 static int _depth0ck;
+static int _firstrun;
 static int _prev_err;
+static int _verify_ok;
 
 SSL *SSLGetContext( int sock )
 {
@@ -586,6 +588,7 @@ SSL *SSLGetContext( int sock )
    errors, and perform additional validation (e.g. CN matches) */
 static int SSL_verify_callback( int ok_return, X509_STORE_CTX *ctx, int strict )
 {
+#define SSLverbose (((outlevel) >= O_DEBUG) || ((outlevel) >= O_VERBOSE && (depth) == 0)) 
        char buf[257];
        X509 *x509_cert;
        int err, depth, i;
@@ -603,10 +606,21 @@ static int SSL_verify_callback( int ok_return, X509_STORE_CTX *ctx, int strict )
        subj = X509_get_subject_name(x509_cert);
        issuer = X509_get_issuer_name(x509_cert);
 
-       if (depth == 0 && !_depth0ck) {
-               _depth0ck = 1;
+       if (outlevel >= O_VERBOSE) {
+               if (depth == 0 && SSLverbose)
+                       report(stderr, GT_("Server certificate:\n"));
+               else {
+                       if (_firstrun) {
+                               _firstrun = 0;
+                               if (SSLverbose)
+                                       report(stdout, GT_("Certificate chain, from root to peer, starting at depth %d:\n"), depth);
+                       } else {
+                               if (SSLverbose)
+                                       report(stdout, GT_("Certificate at depth %d:\n"), depth);
+                       }
+               }
 
-               if (outlevel >= O_VERBOSE) {
+               if (SSLverbose) {
                        if ((i = X509_NAME_get_text_by_NID(issuer, NID_organizationName, buf, sizeof(buf))) != -1) {
                                report(stdout, GT_("Issuer Organization: %s\n"), (tt = sdump(buf, i)));
                                xfree(tt);
@@ -622,23 +636,33 @@ static int SSL_verify_callback( int ok_return, X509_STORE_CTX *ctx, int strict )
                        } else
                                report(stdout, GT_("Unknown Issuer CommonName\n"));
                }
+       }
+
+       if ((i = X509_NAME_get_text_by_NID(subj, NID_commonName, buf, sizeof(buf))) != -1) {
+               if (SSLverbose) {
+                       report(stdout, GT_("Subject CommonName: %s\n"), (tt = sdump(buf, i)));
+                       xfree(tt);
+               }
+               if ((size_t)i >= sizeof(buf) - 1) {
+                       /* Possible truncation. In this case, this is a DNS name, so this
+                        * is really bad. We do not tolerate this even in the non-strict case. */
+                       report(stderr, GT_("Bad certificate: Subject CommonName too long!\n"));
+                       return (0);
+               }
+               if ((size_t)i > strlen(buf)) {
+                       /* Name contains embedded NUL characters, so we complain. This is likely
+                        * a certificate spoofing attack. */
+                       report(stderr, GT_("Bad certificate: Subject CommonName contains NUL, aborting!\n"));
+                       return 0;
+               }
+       }
+
+       if (depth == 0) { /* peer certificate */
+               if (!_depth0ck) {
+                       _depth0ck = 1;
+               }
+
                if ((i = X509_NAME_get_text_by_NID(subj, NID_commonName, buf, sizeof(buf))) != -1) {
-                       if (outlevel >= O_VERBOSE) {
-                               report(stdout, GT_("Server CommonName: %s\n"), (tt = sdump(buf, i)));
-                               xfree(tt);
-                       }
-                       if ((size_t)i >= sizeof(buf) - 1) {
-                               /* Possible truncation. In this case, this is a DNS name, so this
-                                * is really bad. We do not tolerate this even in the non-strict case. */
-                               report(stderr, GT_("Bad certificate: Subject CommonName too long!\n"));
-                               return (0);
-                       }
-                       if ((size_t)i > strlen(buf)) {
-                               /* Name contains embedded NUL characters, so we complain. This is likely
-                                * a certificate spoofing attack. */
-                               report(stderr, GT_("Bad certificate: Subject CommonName contains NUL, aborting!\n"));
-                               return 0;
-                       }
                        if (_ssl_server_cname != NULL) {
                                char *p1 = buf;
                                char *p2 = _ssl_server_cname;
@@ -690,12 +714,13 @@ static int SSL_verify_callback( int ok_return, X509_STORE_CTX *ctx, int strict )
                                        matched = 1;
                                }
                                if (!matched) {
-                                       report(stderr,
-                                           GT_("Server CommonName mismatch: %s != %s\n"),
-                                           (tt = sdump(buf, i)), _ssl_server_cname );
-                                       xfree(tt);
-                                       if (ok_return && strict)
-                                               return (0);
+                                       if (strict || SSLverbose) {
+                                               report(stderr,
+                                                               GT_("Server CommonName mismatch: %s != %s\n"),
+                                                               (tt = sdump(buf, i)), _ssl_server_cname );
+                                               xfree(tt);
+                                       }
+                                       ok_return = 0;
                                }
                        } else if (ok_return) {
                                report(stderr, GT_("Server name not set, could not verify certificate!\n"));
@@ -750,13 +775,30 @@ static int SSL_verify_callback( int ok_return, X509_STORE_CTX *ctx, int strict )
 
        if (err != X509_V_OK && err != _prev_err && !(_check_fp != 0 && _check_digest && !strict)) {
                _prev_err = err;
-               report(stderr, GT_("Server certificate verification error: %s\n"), X509_verify_cert_error_string(err));
-               /* We gave the error code, but maybe we can add some more details for debugging */
+                                       
+                report(stderr, GT_("Server certificate verification error: %s\n"), X509_verify_cert_error_string(err));
+                /* We gave the error code, but maybe we can add some more details for debugging */
+
                switch (err) {
                case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT:
                        X509_NAME_oneline(issuer, buf, sizeof(buf));
                        buf[sizeof(buf) - 1] = '\0';
                        report(stderr, GT_("unknown issuer (first %d characters): %s\n"), (int)(sizeof(buf)-1), buf);
+                       report(stderr, GT_("This error usually happens when the server provides an incomplete certificate "
+                                               "chain, which is nothing fetchmail could do anything about.  For details, "
+                                               "please see the README.SSL-SERVER document that comes with fetchmail.\n"));
+                       break;
+               case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT:
+               case X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY:
+               case X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN:
+                       X509_NAME_oneline(subj, buf, sizeof(buf));
+                       buf[sizeof(buf) - 1] = '\0';
+                       report(stderr, GT_("This means that the root signing certificate (issued for %s) is not in the "
+                                               "directory of trusted CA certificates, or that c_rehash needs to be run "
+                                               "on that directory. For details, please "
+                                               "see the documentation of --sslcertpath in the manual page.\n"), buf);
+                       break;
+               default:
                        break;
                }
        }
@@ -764,6 +806,7 @@ static int SSL_verify_callback( int ok_return, X509_STORE_CTX *ctx, int strict )
         * If not in strict checking mode (--sslcertck), override this
         * and pretend that verification had succeeded.
         */
+       _verify_ok &= ok_return;
        if (!strict)
                ok_return = 1;
        return (ok_return);
@@ -898,6 +941,8 @@ int SSLOpen(int sock, char *mycert, char *mykey, const char *myproto, int certck
        _check_fp = 1;
        _check_digest = fingerprint;
        _depth0ck = 0;
+       _firstrun = 1;
+       _verify_ok = 1;
        _prev_err = -1;
 
        if( mycert || mykey ) {
@@ -948,6 +993,11 @@ int SSLOpen(int sock, char *mycert, char *mykey, const char *myproto, int certck
                }
        }
 
+       if (!certck && (SSL_get_verify_result(_ssl_context[sock]) != X509_V_OK
+|| !_verify_ok)) {
+               report(stderr, GT_("Warning: the connection is insecure, continuing anyways. (Better use --sslcertck!)\n"));
+       }
+
        return(0);
 }
 #endif