Comment 33 for bug 376484

(From update of attachment 289348)
After reading this patch, it appears to me that it behaves as follows:

> static SECStatus
> cert_TestHostName(char * cn, const char * hn)
> {

>+ char *wildcard = PORT_Strchr(cn, '*');
>+ char *firstdot = PORT_Strchr(cn, '.');
>+ char *seconddot = firstdot ? PORT_Strchr(firstdot+1, '.') : NULL;
>+
>+ /* for a cn pattern to be considered valid, the wildcard character...
>+ - may only occur in an FQDN with at least 3 components
>+ - may occur in the first component only, and must be its last character
>+ - may occur only once
>+ - may be preceded by additional characters
>+ */
>+ if (wildcard && firstdot && seconddot && *(seconddot+1)
>+ && firstdot == wildcard + 1
>+ && seconddot - firstdot > 1
>+ && PORT_Strrchr(cn, '*') == wildcard
>+ && PORT_Strncasecmp(cn, hn, wildcard - cn) == 0
>+ && PORT_Strcasecmp(firstdot, PORT_Strchr(hn, '.')) == 0)
>+ /* valid wildcard pattern match */

It implements these rules for the pattern in "cn":
- One and only one asterisk.
- No dots to the left of the asterisk, but other characters are allowed
to precede the asterisk.
- The asterisk must be followed immediately by the first dot.
- At least two dots, and second dot must not be last character in string.
(Note that this does not eliminate the possibility of a trailing dot.)
- At least one character between first dot and second dot.

If cn contains characters other than dot before the asterisk, then it
will compare those characters against an equal number of leading
characters from hn, and any non-match will mean a pattern match failure.
Otherwise, it will compare the strings beginning with the first dot in
each of cn and hn.

This patch relies on the fact that, presently, PORT_Strcasecmp is
defined as PL_strcasecmp, and unlike POSIX strcasecmp, PL_strcasecmp
is defined to NOT crash if either input string arguement is NULL.

Since the PORT_ functions exist to allow implementors to choose
different implemenations for their platforms, and PORT_Strcasecmp
could be defined as some other implementation, I suggest that the
patch should call PL_strcasecmp directly, to ensure that it gets
an implementation that has the desired property.

I'd prefer if PORT_Strchr(hn, '.') was called separately, and its
result stored in a pointer, and used only if it is non-NULL.

>+ return SECSuccess;
>+ else if (PORT_Strcasecmp(hn, cn) == 0)
>+ /* no valid wildcard pattern, but literal match */
>+ return SECSuccess;
>+ else {

Eliminate the elses immediately after returns. They serve no purpose.

>+ PORT_SetError(SSL_ERROR_BAD_CERT_DOMAIN);
>+ return SECFailure;
>+ }
> }

Unindent the final block and remove the braces from it.

Did you mean to disallow trailing dots from the patterns?
The present rules allow
  *.a.b.
but do not allow
  *.a.
Is that intentional?

Assuming it is, then I suspect this patch does what Kaspar intended for it
to do. So the remaining question is: are any of NSS's customers (teams that
develop products that use NSS) going to have a fit over this change?
I will ask about this in various newsgroups and mailing lists.

In the mean time, Kaspar, please address those review comments.
Oh, and put an asterisk in every line in the block comment, so they
form a column.