On Wed, Sep 3, 2014 at 11:50 AM, Heikki Linnakangas
<hlinnakan...@vmware.com> wrote:

> * It's ugly that the caller does the malloc and memcpy, and the
> certificate_name_entry_validate_match function then modifies its name
> argument. Move the malloc+memcpy inside the function.

For the case of CN the caller has to do the malloc anyway, since
X509_NAME_get_text_by_NID expects the already allocated buffer.
This means that 'movable' malloc + memcpy occurs only once, and while
it would still make sense to move it into the function, it would also
mean we would do an unnecessary malloc for the case of CN.

> * The error message in certificate_name_entry_validate_match says "SSL
> certificate's common name contains embedded null" even though it's also used
> for SANs.

Will fix, thank you.

>> The tricky part is the error
>> message if no match was found: initially, it only listed a single
>> common name, but now tracking all DNS names just for the sake of the
>> error message makes the code more bloated, so I'm wondering if simply
>> stating that there was no match, as implemented in the attached patch,
>> would be good enough?
> Hmm. It would still be nice to say something about the certificate that was
> received. How about:
>   server certificate with common name "%s" does not match host name "%s"

We cannot guarantee at this point that the common name is present in
the certificate. And if it is not, which name should we pick instead?
One way to solve this is to take the first non-NULL name, but it if
there is no common name, but then the wording of the error message
would depend on the availability of CN.
Another is to show all available names, but I do not like collecting
them just for the sake of displaying in the error message.
And last one is to just show the error without mentioning names,
that's what I've chosen to be the most consistent.

Alexey Klyukin

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to