On 09/04/2014 10:33 AM, Alexey Klyukin wrote:
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.

Hmm. Perhaps we should use X509_NAME_get_index_by_NID + X509_NAME_get_entry instead of X509_NAME_get_text_by_NID. You could then pass the ASN1_STRING object to the certificate_name_entry_validate_match() function, and have it do the ASN1_STRING_length() and ASN1_STRING_data() calls too.

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.

I think we should:

1. Check if there's a common name, and if so, print that
2. Check if there is exactly one SAN, and if so, print that
3. Just print an error without mentioning names.

There's a lot of value in printing the name if possible, so I'd really like to keep that. But I agree that printing all the names if there are several would get complicated and the error message could become very long. Yeah, the error message might need to be different for cases 1 and 2. Or maybe phrase it "server certificate's name \"%s\" does not match host name \"%s\"", which would be reasonable for both 1. and 2.

- Heikki

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

Reply via email to