[Bug 49805] SSL Wildcard support is too lenient (and a bunch of SSL tests are broken under OpenSSL)

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=49805

--- Comment #1 from Will Thompson <will.thompson at collabora.co.uk> 2012-05-15 08:09:57 PDT ---
Review of the first few patches:

+    { "/connector/cert-verification/tls/wildcard/level-mismatch/fail",

Could you add a comment specifying why these tests should fail? I *believe*
that that one fails because:

+        { "weasel-juice.org", PORT_XMPP, "thud.org", REACHABLE, UNREACHABLE },
+        { PLAINTEXT_OK,
+          { "moose at weasel-juice.org", "something", PLAIN, TLS },
+          { NULL, 0, XMPP_V1, STARTTLS, CERT_CHECK_STRICT, TLS_CA_DIR } } },

the certificate is for *.weasel-juice.org, which should not match
weasel-juice.org. But I'm not sure where thud.org comes into it. Ditto the
subsequent tests.

It would be good if what's bad about the BADWILD certificate were written down.

In
http://cgit.collabora.com/git/user/vivek/wocky.git/commit/?h=wildcarded-certificate-check&id=0c8d0c9b1f871d8794a3c6716540a904dfba989a
:

+static inline gboolean
+compare_hostname (const char *host, const char *cert)
+{
+    /* advance to first different character */
+    for (; CASELESS_CHARCMP (*cert, *host); cert++, host++);
+
+    /* were the strings entirely, caselessly equal? */
+    return (strlen (cert) == 0 && strlen (host) == 0);
+}

can be replaced by (g_ascii_strcasecmp (host, cert) == 0);

+  while( *certname++ == '*' && *certname++ == '.' )
+      /* a leading '*.' swallows the next domain word */
+      hostname = index( hostname, '.' );
+
+      if( hostname == NULL )

Coding style: while (...) not while( ... ).

I don't really understand this loop. Given this comment:

+  /* wildcard handling: we only allow leading '*.' wildcards:
+     no *foo.blerg.org - that would be a biiig security hole */

why is it a loop? Is it meant to allow lol.*.co.uk to match lol.google.co.uk?
If not, how about this:

if (g_str_has_prefix (certname, "*."))
  {
    const gchar *certname_tail = certname + 2;
    const gchar *hostname_tail = index (hostname, '.');

    if (hostname_tail == NULL)
      return FALSE;

    hostname_tail++;
    DEBUG ("%s ~ %s", hostname_tail, certname_tail);
    return compare_hostname (hostname_tail, certname_tail);
  }

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.