Re: [Bug-wget] --no-check-cert does not avoid cert warning

[Giuseppe Scrivano]

Hi Karl,

Karl Berry <address@hidden> writes:

>     { wget -d xxx 2>&1 1>&3 | grep -v Saving 1>&2; } 3>&1
>
> This changes the exit value, so that's no good.  Sure, with even more
> complexity the exit status could be preserved too, but IMHO wrapping
> wget in layers of shell mechanisms to work around a warning is crazy.
>
> Giuseppe - please just do the right thing and provide a way to shut off
> the warning.  Especially since it's simple to do.  -k

the right thing to do is to listen to the Tim's opinion, since he is one
of the maintainers and he does a lot of work here.  I would not push any
change if he doesn't agree with it.

Tim agreed so I've changed the patch to add --check-certificate=quiet.

How does the new version look?

Thanks,
Giuseppe

diff --git a/doc/wget.texi b/doc/wget.texi
index c647e33..9cc2bb2 100644
--- a/doc/wget.texi
+++ b/doc/wget.texi
@@ -1725,6 +1725,9 @@ this option to bypass the verification and proceed with 
the download.
 site's authenticity, or if you really don't care about the validity of
 its certificate.}  It is almost always a bad idea not to check the
 certificates when transmitting confidential or important data.
+If you are really sure of what you are doing, you can specify
+--check-certificate=quiet to ask wget to not print any warning about
+invalid certificates, in most cases this is the wrong thing to do.
 
 @cindex SSL certificate
 @item address@hidden
diff --git a/src/gnutls.c b/src/gnutls.c
index d1444fe..d39371f 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -692,6 +692,10 @@ ssl_check_certificate (int fd, const char *host)
   const char *severity = opt.check_cert ? _("ERROR") : _("WARNING");
   bool success = true;
 
+  /* The user explicitly said to not check for the certificate.  */
+  if (opt.check_cert == CHECK_CERT_QUIET)
+    return success;
+
   err = gnutls_certificate_verify_peers2 (ctx->session, &status);
   if (err < 0)
     {
@@ -766,5 +770,5 @@ ssl_check_certificate (int fd, const char *host)
     }
 
  out:
-  return opt.check_cert ? success : true;
+  return opt.check_cert == CHECK_CERT_ON ? success : true;
 }
diff --git a/src/init.c b/src/init.c
index 67c94b9..87fbc9b 100644
--- a/src/init.c
+++ b/src/init.c
@@ -115,6 +115,7 @@ CMD_DECLARE (cmd_spec_secure_protocol);
 CMD_DECLARE (cmd_spec_timeout);
 CMD_DECLARE (cmd_spec_useragent);
 CMD_DECLARE (cmd_spec_verbose);
+CMD_DECLARE (cmd_check_cert);
 
 /* List of recognized commands, each consisting of name, place and
    function.  When adding a new command, simply add it to the list,
@@ -152,7 +153,7 @@ static const struct {
   { "cadirectory",      &opt.ca_directory,      cmd_directory },
   { "certificate",      &opt.cert_file,         cmd_file },
   { "certificatetype",  &opt.cert_type,         cmd_cert_type },
-  { "checkcertificate", &opt.check_cert,        cmd_boolean },
+  { "checkcertificate", &opt.check_cert,        cmd_check_cert },
 #endif
   { "chooseconfig",     &opt.choose_config,     cmd_file },
   { "connecttimeout",   &opt.connect_timeout,   cmd_time },
@@ -415,7 +416,7 @@ defaults (void)
   opt.retr_symlinks = true;
 
 #ifdef HAVE_SSL
-  opt.check_cert = true;
+  opt.check_cert = CHECK_CERT_ON;
   opt.ftps_resume_ssl = true;
   opt.ftps_fallback_to_ftp = false;
   opt.ftps_implicit = false;
@@ -955,6 +956,18 @@ static bool simple_atof (const char *, const char *, 
double *);
                      && (p)[3] == '\0')
 
 
+static int
+cmd_boolean_internal (const char *com, const char *val, void *place)
+{
+  if (CMP2 (val, 'o', 'n') || CMP3 (val, 'y', 'e', 's') || CMP1 (val, '1'))
+    /* "on", "yes" and "1" mean true. */
+    return 1;
+  else if (CMP3 (val, 'o', 'f', 'f') || CMP2 (val, 'n', 'o') || CMP1 (val, 
'0'))
+    /* "off", "no" and "0" mean false. */
+    return 0;
+  return -1;
+}
+
 /* Store the boolean value from VAL to PLACE.  COM is ignored,
    except for error messages.  */
 static bool
@@ -962,24 +975,62 @@ cmd_boolean (const char *com, const char *val, void 
*place)
 {
   bool value;
 
-  if (CMP2 (val, 'o', 'n') || CMP3 (val, 'y', 'e', 's') || CMP1 (val, '1'))
-    /* "on", "yes" and "1" mean true. */
-    value = true;
-  else if (CMP3 (val, 'o', 'f', 'f') || CMP2 (val, 'n', 'o') || CMP1 (val, 
'0'))
-    /* "off", "no" and "0" mean false. */
-    value = false;
-  else
+  switch (cmd_boolean_internal (com, val, place))
     {
-      fprintf (stderr,
-               _("%s: %s: Invalid boolean %s; use `on' or `off'.\n"),
-               exec_name, com, quote (val));
-      return false;
-    }
+    case 0:
+      value = false;
+      break;
 
+    case 1:
+      value = true;
+      break;
+
+    default:
+      {
+        fprintf (stderr,
+                 _("%s: %s: Invalid boolean %s; use `on' or `off'.\n"),
+                 exec_name, com, quote (val));
+        return false;
+      }
+    }
   *(bool *) place = value;
   return true;
 }
 
+/* Store the check_cert value from VAL to PLACE.  COM is ignored,
+   except for error messages.  */
+static bool
+cmd_check_cert (const char *com, const char *val, void *place)
+{
+  int value;
+
+  switch (cmd_boolean_internal (com, val, place))
+    {
+    case 0:
+      value = CHECK_CERT_OFF;
+      break;
+
+    case 1:
+      value = CHECK_CERT_ON;
+      break;
+
+    default:
+      {
+        if (!c_strcasecmp (val, "quiet"))
+          value = CHECK_CERT_QUIET;
+        else
+          {
+            fprintf (stderr,
+                     _("%s: %s: Invalid %s; use `on', `off' or `quiet'.\n"),
+                     exec_name, com, quote (val));
+            return false;
+          }
+      }
+    }
+  *(int *) place = value;
+  return true;
+}
+
 /* Set the non-negative integer value from VAL to PLACE.  With
    incorrect specification, the number remains unchanged.  */
 static bool
diff --git a/src/openssl.c b/src/openssl.c
index 4876048..6701c0d 100644
--- a/src/openssl.c
+++ b/src/openssl.c
@@ -682,6 +682,10 @@ ssl_check_certificate (int fd, const char *host)
   SSL *conn = ctx->conn;
   assert (conn != NULL);
 
+  /* The user explicitly said to not check for the certificate.  */
+  if (opt.check_cert == CHECK_CERT_QUIET)
+    return success;
+
   cert = SSL_get_peer_certificate (conn);
   if (!cert)
     {
@@ -880,13 +884,12 @@ ssl_check_certificate (int fd, const char *host)
   X509_free (cert);
 
  no_cert:
-  if (opt.check_cert && !success)
+  if (opt.check_cert == CHECK_CERT_ON && !success)
     logprintf (LOG_NOTQUIET, _("\
 To connect to %s insecurely, use `--no-check-certificate'.\n"),
                quotearg_style (escape_quoting_style, host));
 
-  /* Allow --no-check-cert to disable certificate checking. */
-  return opt.check_cert ? success : true;
+  return opt.check_cert == CHECK_CERT_ON ? success : true;
 }
 
 /*
diff --git a/src/options.h b/src/options.h
index dad08c1..e378fd7 100644
--- a/src/options.h
+++ b/src/options.h
@@ -29,6 +29,13 @@ Corresponding Source for a non-source form of such a 
combination
 shall include the source code for the parts of OpenSSL used as well
 as that of the covered work.  */
 
+enum CHECK_CERT_MODES
+{
+  CHECK_CERT_OFF,
+  CHECK_CERT_ON,
+  CHECK_CERT_QUIET,
+};
+
 struct options
 {
   int verbose;                  /* Are we verbose?  (First set to -1,
@@ -215,7 +222,7 @@ struct options
     secure_protocol_tlsv1_2,
     secure_protocol_pfs
   } secure_protocol;            /* type of secure protocol to use. */
-  bool check_cert;              /* whether to validate the server's cert */
+  int check_cert;               /* whether to validate the server's cert */
   char *cert_file;              /* external client certificate to use. */
   char *private_key;            /* private key file (if not internal). */
   enum keyfile_type {